On 11/8/2014 7:33 PM, Marek Vasut wrote:
On Saturday, November 08, 2014 at 05:07:21 AM, Peng Fan wrote:
在 11/7/2014 8:17 PM, Marek Vasut 写道:
On Friday, November 07, 2014 at 12:45:51 PM, Peng Fan wrote:
在 11/7/2014 7:09 PM, Marek Vasut 写道:
On Friday, November 07, 2014 at 12:03:30 PM, Peng Fan wrote:
[...]
@@ -160,7 +174,7 @@ static int usb_phy_enable(int index, struct
usb_ehci *ehci) val |= (USBPHY_CTRL_ENUTMILEVEL2 |
USBPHY_CTRL_ENUTMILEVEL3);
__raw_writel(val, phy_ctrl);
- return val & USBPHY_CTRL_OTG_ID;
+ return board_usb_phy_mode(index);
This should be called from ehci_hcd_init() right after
usb_phy_enable(). Afterall, the mode detection has nothing to do with
the PHY enabling.
This back to what I did in patch v2. right after usb_phy_enable(),
just paste that piece of code here:
The weak function:
+int __weak board_ehci_usb_mode(int index, enum usb_init_type *type)
+{
+ return 0;
+}
+
type = usb_phy_enable(index, ehci) ? USB_INIT_DEVICE :
USB_INIT_HOST;
+ board_usb_phy_mode(index, &type);
+
The usb_phy_enable() should not return the PHY mode at all though.
It should be the board_usb_phy_mode() which adjusts the PHY type.
The usb_phy_enable() should return just a success/failure return
value.
ok. got it.
What need to do is to let board can modify the `type` like following:
+int board_usb_phy_mode(int port, enum usb_init_type *type)
+{
+ if (port == 1)
+ /* port1 works in HOST Mode */
+ *type = USB_INIT_HOST;
+
+ return 0;
+}
+
This is the way that I did in patch v2. If this is fine, I'll resent
this patch set.
It should really explicitly set it, not modify it, see above.
I have an idea about this patch:
1. usb_phy_enable will not be touched.
2. replace "type = usb_phy_enable(index, ehci) ? USB_INIT_DEVICE :
USB_INIT_HOST;" with "usb_phy_enable(index, ehci)".
3. right after usb_phy_enable, add this line "type =
board_usb_phy_mode(index)" or "type = board_usb_phy_mode((struct usb_phy
*)PHY_ADDRESS)". Here I also think pass phy register definition to board
level code is not fine just as what we talked about passing ehci struct
to board level code in patch v2.
4. in ehci-mx6.c, implement the weak function "int __weak
board_usb_phy_mode(xxx)", and it's return value is the mode, HOST or
DEVICE. If the board code want to implement this function, just return
what the board want.
After all, this patch may looks like this:
In ehci-mx6.c
+int __weak board_usb_phy_mode(int port)
+{
+ void __iomem *phy_reg;
+ void __iomem *phy_ctrl;
+ u32 val;
+
+ phy_reg = (void __iomem *)phy_bases[port];
+ phy_ctrl = (void __iomem *)(phy_reg + USBPHY_CTRL);
+
+ val = __raw_readl(phy_ctrl);
+
+ return val & USBPHY_CTRL_OTG_ID;
+}
+
- type = usb_phy_enable(index, ehci) ? USB_INIT_DEVICE : USB_INIT_HOST;
+ usb_phy_enable(index, ehci);
+ type = board_usb_phy_mode(index);
in board code, which is not in this patch, just list here:
+int board_usb_phy_mode(int port)
+{
+ if (port == 1)
+ return USB_INIT_HOST;
+ else
+ return USB_INIT_DEVICE;
+}
I just want to keep it simple and do not want to touch usb phy register
in board code.
Any ideas?
This seems OKish for all but the part where usb_phy_enable() shouldn't be
touched. The return value of usb_phy_enable() should really be a regular
return code, not the PHY mode.
ok. I'll fix this.
You can also still implement a function to query a PHY for it's mode, so
you don't need to explicitly read the USBPHY_CTRL_OTG_ID in the board
code.
I am not sure whether this following way is fine or not.
+int board_usb_phy_mode(int index)
+ __attribute__((weak, alias("usb_phy_mode")));
__weak board_usb_phy_mode(int index) is fine.
+
in usb_phy_mode, query a PHY for it's mode.
And righter after usb_phy_enable in ehci-mx6.c.
- type = usb_phy_enable(index, ehci) ? USB_INIT_DEVICE :
USB_INIT_HOST;
+ usb_phy_enable(index, ehci);
+ type = usb_phy_mode(index);
usb_phy_enable return 0 but not return val & USBPHY_CTRL_OTG_ID. There
is no status bit for query enabled or not, so just return 0.
In board file:
int board_usb_phy_mode(int port)
{
if (port == 1)
return USB_INIT_HOST;
else
return usb_phy_mode(port);
}
I think this is better way then previous patch, but i did not find where
to put the usb_phy_mode prototype type, since board file will use it.
Looks OK otherwise.
Sent out v4 patch, please review.
Thanks,
Peng.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
