Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
On Monday, November 10, 2014 at 02:01:50 AM, Peng Fan wrote:
[...]
+
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! Will do as time permits. Sorry for possible delays.
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
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
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
On Friday, November 07, 2014 at 02:08:12 AM, Peng Fan wrote:
Include a weak function board_usb_phy_mode.
usb_phy_enable decide whether the controller in device mode or in host mode
by '*phy_ctrl USBPHY_CTRL_OTG_ID'.
There are two usb port on mx6sxsabresd and also mx6slevk.
1. OTG port
2. HOST port
There are connected to SOC USB controller OTG1 core and OTG2 core as
following: OTG1 core board OTG port
OTG2 core board HOST port
This patch has nothing to do with any board, so this part should not
be in the commit message.
However to these two board, no ID pin for the board host port. If only use
'*phy_ctrl USBPHY_CTRL_OTG_ID' to decide the work mode, the 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.
So introduce this weak function to let board level code can decide to work
in host or device mode, if board level code want to implement this
function.
Signed-off-by: Peng Fan peng@freescale.com
Signed-off-by: Ye Li b37...@freescale.com
---
Changes v3:
Take Marek's suggestions, replace 'return val USBPHY_CTRL_OTG_ID' with
this new function like 'return board_usb_phy_mode(index);'
Here board_usb_phy_mode only has one parameter 'index' as board_ehci_power
and board_echi_hcd_init do.
http://lists.denx.de/pipermail/u-boot/2014-November/194183.html; has
detailed discussion.
Changes v2:
Introduce a new weak function to let board have a choice to decide which
mode to work at.
drivers/usb/host/ehci-mx6.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 9ec5a0a..e2a247e 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -113,6 +113,20 @@ static void usb_power_config(int index)
pll_480_ctrl_set);
}
+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;
+}
+
/* Return 0 : host node, 0 : device mode */
static int usb_phy_enable(int index, struct usb_ehci *ehci)
{
@@ -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.
btw. an idea for a separate patch(set) -- the PHY registers should be
converted to struct-based access.
}
/* Base address for this IP block is 0x02184800 */
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
在 11/7/2014 4:25 PM, Marek Vasut 写道:
On Friday, November 07, 2014 at 02:08:12 AM, Peng Fan wrote:
Include a weak function board_usb_phy_mode.
usb_phy_enable decide whether the controller in device mode or in host mode
by '*phy_ctrl USBPHY_CTRL_OTG_ID'.
There are two usb port on mx6sxsabresd and also mx6slevk.
1. OTG port
2. HOST port
There are connected to SOC USB controller OTG1 core and OTG2 core as
following: OTG1 core board OTG port
OTG2 core board HOST port
This patch has nothing to do with any board, so this part should not
be in the commit message.
However to these two board, no ID pin for the board host port. If only use
'*phy_ctrl USBPHY_CTRL_OTG_ID' to decide the work mode, the 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.
So introduce this weak function to let board level code can decide to work
in host or device mode, if board level code want to implement this
function.
Signed-off-by: Peng Fan peng@freescale.com
Signed-off-by: Ye Li b37...@freescale.com
---
Changes v3:
Take Marek's suggestions, replace 'return val USBPHY_CTRL_OTG_ID' with
this new function like 'return board_usb_phy_mode(index);'
Here board_usb_phy_mode only has one parameter 'index' as board_ehci_power
and board_echi_hcd_init do.
http://lists.denx.de/pipermail/u-boot/2014-November/194183.html; has
detailed discussion.
Changes v2:
Introduce a new weak function to let board have a choice to decide which
mode to work at.
drivers/usb/host/ehci-mx6.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 9ec5a0a..e2a247e 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -113,6 +113,20 @@ static void usb_power_config(int index)
pll_480_ctrl_set);
}
+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;
+}
+
/* Return 0 : host node, 0 : device mode */
static int usb_phy_enable(int index, struct usb_ehci *ehci)
{
@@ -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);
+
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.
btw. an idea for a separate patch(set) -- the PHY registers should be
converted to struct-based access.
Yeah, struct based access PHY register should be done. After this board
level usb support patch is finished.
}
/* Base address for this IP block is 0x02184800 */
Regards,
Peng.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
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.
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.
[...]
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
在 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?
[...]
Regards,
Peng.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
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.
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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
在 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)));
+
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.
Regards,
Peng.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
在 11/8/2014 12:07 PM, Peng Fan 写道:
在 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)));
+
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);
This should be 'type = board_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.
Regards,
Peng.
Regards,
Peng.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 1/3] usb:ehci-mx6 add board_usb_phy_mode function
Include a weak function board_usb_phy_mode.
usb_phy_enable decide whether the controller in device mode or in host mode by
'*phy_ctrl USBPHY_CTRL_OTG_ID'.
There are two usb port on mx6sxsabresd and also mx6slevk.
1. OTG port
2. HOST port
There are connected to SOC USB controller OTG1 core and OTG2 core as following:
OTG1 core board OTG port
OTG2 core board HOST port
However to these two board, no ID pin for the board host port. If only use
'*phy_ctrl USBPHY_CTRL_OTG_ID' to decide the work mode, the 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.
So introduce this weak function to let board level code can decide to work
in host or device mode, if board level code want to implement this function.
Signed-off-by: Peng Fan peng@freescale.com
Signed-off-by: Ye Li b37...@freescale.com
---
Changes v3:
Take Marek's suggestions, replace 'return val USBPHY_CTRL_OTG_ID' with
this new function like 'return board_usb_phy_mode(index);'
Here board_usb_phy_mode only has one parameter 'index' as board_ehci_power and
board_echi_hcd_init do.
http://lists.denx.de/pipermail/u-boot/2014-November/194183.html; has detailed
discussion.
Changes v2:
Introduce a new weak function to let board have a choice to decide which mode
to work at.
drivers/usb/host/ehci-mx6.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 9ec5a0a..e2a247e 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -113,6 +113,20 @@ static void usb_power_config(int index)
pll_480_ctrl_set);
}
+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;
+}
+
/* Return 0 : host node, 0 : device mode */
static int usb_phy_enable(int index, struct usb_ehci *ehci)
{
@@ -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);
}
/* Base address for this IP block is 0x02184800 */
--
1.8.4.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
