On Wednesday, November 05, 2014 at 10:18:25 AM, Peng Fan wrote: > 在 11/5/2014 5:03 PM, Marek Vasut 写道: > > On Wednesday, November 05, 2014 at 07:00:32 AM, Peng Fan wrote: > >> 在 11/5/2014 1:33 AM, Marek Vasut 写道: > >>> On Tuesday, November 04, 2014 at 02:29:56 PM, Peng Fan wrote: > >>>> Hi Marek, > >>>> > >>>> 在 11/4/2014 7:01 PM, Marek Vasut 写道: > >>>>> On Tuesday, November 04, 2014 at 11:50:29 AM, Peng Fan wrote: > >>>>>> 在 11/4/2014 6:33 PM, Marek Vasut 写道: > >>>>>>> On Tuesday, November 04, 2014 at 08:50:00 AM, Peng Fan wrote: > >>>>>>>> Include a weak function board_ehci_usb_mode to gives board code > >>>>>>>> a choice. > >>>>>>> > >>>>>>> What choice? > >>>>>>> > >>>>>>>> If the board want the otg port work in host mode but not > >>>>>>>> device mode, this should be handled. > >>>>>>> > >>>>>>> How? > >>>>>>> > >>>>>>> Also, isn't usb_phy_enable() supposed to do exactly this kind of > >>>>>>> selection between device and host mode ? > >>>>>> > >>>>>> In mx6sxsabresd board, there are two usb port, one used for otg, the > >>>>>> other used for host. However they are connected to SOC USB > >>>>>> controller otg1 core and otg2 core respectively. Like following: > >>>>>> > >>>>>> OTG1 CORE <----> board otg port > >>>>>> OTG2 CORE <----> board host port > >>>>>> > >>>>>> However the board do not have ID pin set for board host port. If > >>>>>> just use usb_phy_enable, the board host port will not work, because > >>>>>> "type = usb_phy_enable(index, ehci) ? USB_INIT_DEVICE : > >>>>>> USB_INIT_HOST;" will always set type with USB_INIT_DEVICE. > >>>>>> > >>>>>> Because i did not find way to handle this situation in > >>>>>> board/freescale/mx6sxsabresd/mx6sxsabresd.c, add this function to > >>>>>> let board level code handle handle 'type', if board level code want > >>>>>> to set it's own 'type'. > >>>>> > >>>>> This part in usb_phy_enable() > >>>>> > >>>>> 163 return val & USBPHY_CTRL_OTG_ID; > >>>>> > >>>>> should be replaced by some kind of a board-specific callback then, > >>>>> with default implmentation being the above (reading the phy ctrl > >>>>> register). > >>>> > >>>> How about using the following piece of code? > >>>> in ehci-mx6.c > >>>> > >>>> unsigned int __weak board_usb_phy_mode(int index, unsigned int val) > >>>> { > >>>> > >>>> return val & USBPHY_CTRL_OTG_ID; > >>>> > >>>> } > >>>> > >>>> replace "return val & USBPHY_CTRL_OTG_ID;" using " > >>>> return board_usb_phy_mode(index, val);" > >>>> > >>>> In board file, > >>>> unsigned int board_usb_phy_mode(int index, unsigned int val) > >>> > >>> Why not pass in full struct usb_ehci * instead ? Passing some ad-hoc > >>> $val into the function doesn't seem like a scalable future-proof > >>> solution. [...] > >> > >> Passing struct usb_ehci * to board code seems exports ehci register > >> definition to board layer. > > > > Yeah. > > > >> How about just use > >> "int board_usb_phy_mode(int index)" without using 'val' or 'struct > >> usb_ehci *ehci'. > > > > The board part might need to read the EHCI registers though. How would > > the board part be able to do it if you didn't pass the *ehci in ? > > To imx6, the ID bit is in PHY ctrl reg 'USBPHYx_CTRLn', also the phy > regs definition are not included in "struct usb_ehci". I just think > expose the ehci register to board layer is not fine and > board_usb_phy_mode does not need this. I define this just as > "board_ehci_hcd_init" and "board_ehci_power". Their prototype are > int __weak board_ehci_hcd_init(int port); > int __weak board_ehci_power(int port, int on); > > My implementation is the following: > > replace "return val & USBPHY_CTRL_OTG_ID;" using "return > board_usb_phy_mode(index);" in usb_phy_enable > > In drivers/usb/host/ehci-mx6.c: > 116 int __weak board_usb_phy_mode(int index) > 117 { > 118 void __iomem *phy_reg; > 119 void __iomem *phy_ctrl; > 120 u32 val; > 121 > 122 phy_reg = (void __iomem *)phy_bases[index]; > 123 phy_ctrl = (void __iomem *)(phy_reg + USBPHY_CTRL); > 124 > 125 val = __raw_readl(phy_ctrl); > 126 > 127 return val & USBPHY_CTRL_OTG_ID; > 128 } > > In board/freescale/mx6sxsabresd/mx6sxsabresd.c: > 295 int board_usb_phy_mode(int port) > 296 { > 297 void __iomem *phy_reg; > 298 void __iomem *phy_ctrl; > 299 u32 val; > 300 > 301 switch (port) { > 302 case 0: > 303 phy_reg = (void __iomem *)USB_PHY0_BASE_ADDR; > 304 phy_ctrl = (void __iomem *)(phy_reg + USBPHY_CTRL); > 305 val = __raw_readl(phy_ctrl); > 306 return val & USBPHY_CTRL_OTG_ID; > 307 case 1: > 308 /* Work in HOST mode. */ > 309 return 0; > 310 } > 311 > 312 /* suppress warning msg */ > 313 return 0; > 314 } > > Is this piece of code fine?
These ad-hoc hooks are starting to become absolute horror, but I guess this one (if properly documented) might just work. Let's see what will come out of this approach. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot