在 11/7/2014 4:20 AM, Marek Vasut 写道:
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.

Sent out v3 patch set just now. Please review.

Thanks,
Peng.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to