* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
@ 2012-10-18 2:40 Shengzhou Liu
2012-10-18 4:21 ` Marek Vasut
0 siblings, 1 reply; 10+ messages in thread
From: Shengzhou Liu @ 2012-10-18 2:40 UTC (permalink / raw)
To: u-boot
when missing USB PHY clock, u-boot will hang during USB
initialization when issuing "usb start". We should check
USBGP[PHY_CLK_VALID] bit to avoid CPU hanging in this case.
Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
---
against master branch of upstream u-boot tree
drivers/usb/host/ehci-fsl.c | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 7b8f033..68abf11 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -30,6 +30,18 @@
#include "ehci.h"
+/* Check USB PHY clock valid */
+static int usb_phy_clk_valid(struct usb_ehci *ehci)
+{
+ if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
+ (!in_be32(&ehci->prictrl))) {
+ printf("USB PHY clock invalid!\n");
+ return 0;
+ } else {
+ return 1;
+ }
+}
+
/*
* Create the appropriate control structures to manage
* a new EHCI host controller.
@@ -82,18 +94,16 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
udelay(1000); /* delay required for PHY Clk to appear */
#endif
out_le32(&(*hcor)->or_portsc[0], PORT_PTS_UTMI);
+ setbits_be32(&ehci->control, USB_EN);
} else {
-#if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
- clrbits_be32(&ehci->control, UTMI_PHY_EN);
setbits_be32(&ehci->control, PHY_CLK_SEL_ULPI);
+ clrsetbits_be32(&ehci->control, UTMI_PHY_EN, USB_EN);
udelay(1000); /* delay required for PHY Clk to appear */
-#endif
+ if (!usb_phy_clk_valid(ehci))
+ return -EINVAL;
out_le32(&(*hcor)->or_portsc[0], PORT_PTS_ULPI);
}
- /* Enable interface. */
- setbits_be32(&ehci->control, USB_EN);
-
out_be32(&ehci->prictrl, 0x0000000c);
out_be32(&ehci->age_cnt_limit, 0x00000040);
out_be32(&ehci->sictrl, 0x00000001);
--
1.6.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 2:40 [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock Shengzhou Liu
@ 2012-10-18 4:21 ` Marek Vasut
2012-10-18 6:28 ` Liu Shengzhou-B36685
2012-10-19 20:13 ` Anatolij Gustschin
0 siblings, 2 replies; 10+ messages in thread
From: Marek Vasut @ 2012-10-18 4:21 UTC (permalink / raw)
To: u-boot
Dear Shengzhou Liu,
> when missing USB PHY clock, u-boot will hang during USB
> initialization when issuing "usb start". We should check
> USBGP[PHY_CLK_VALID] bit to avoid CPU hanging in this case.
>
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
CCing PPC experts.
> ---
> against master branch of upstream u-boot tree
>
> drivers/usb/host/ehci-fsl.c | 22 ++++++++++++++++------
> 1 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> index 7b8f033..68abf11 100644
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
> @@ -30,6 +30,18 @@
>
> #include "ehci.h"
>
> +/* Check USB PHY clock valid */
> +static int usb_phy_clk_valid(struct usb_ehci *ehci)
> +{
> + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
> + (!in_be32(&ehci->prictrl))) {
(!A && !B) condition can certainly be done without the double negation ;-)
> + printf("USB PHY clock invalid!\n");
debug() ?
> + return 0;
> + } else {
> + return 1;
> + }
> +}
> +
> /*
> * Create the appropriate control structures to manage
> * a new EHCI host controller.
> @@ -82,18 +94,16 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr,
> struct ehci_hcor **hcor) udelay(1000); /* delay required for PHY Clk to
> appear */
> #endif
> out_le32(&(*hcor)->or_portsc[0], PORT_PTS_UTMI);
> + setbits_be32(&ehci->control, USB_EN);
> } else {
> -#if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
> - clrbits_be32(&ehci->control, UTMI_PHY_EN);
> setbits_be32(&ehci->control, PHY_CLK_SEL_ULPI);
> + clrsetbits_be32(&ehci->control, UTMI_PHY_EN, USB_EN);
> udelay(1000); /* delay required for PHY Clk to appear */
> -#endif
> + if (!usb_phy_clk_valid(ehci))
> + return -EINVAL;
> out_le32(&(*hcor)->or_portsc[0], PORT_PTS_ULPI);
> }
>
> - /* Enable interface. */
> - setbits_be32(&ehci->control, USB_EN);
> -
> out_be32(&ehci->prictrl, 0x0000000c);
> out_be32(&ehci->age_cnt_limit, 0x00000040);
> out_be32(&ehci->sictrl, 0x00000001);
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 4:21 ` Marek Vasut
@ 2012-10-18 6:28 ` Liu Shengzhou-B36685
2012-10-18 7:15 ` Marek Vasut
2012-10-19 20:13 ` Anatolij Gustschin
1 sibling, 1 reply; 10+ messages in thread
From: Liu Shengzhou-B36685 @ 2012-10-18 6:28 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> >
> > +/* Check USB PHY clock valid */
> > +static int usb_phy_clk_valid(struct usb_ehci *ehci) {
> > + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
> > + (!in_be32(&ehci->prictrl))) {
>
> (!A && !B) condition can certainly be done without the double negation ;-)
[Shengzhou] Yes, using !(A||B) is also okay:)
>
> > + printf("USB PHY clock invalid!\n");
>
> debug() ?
>
[Shengzhou] No, it's not for debug purpose, it should be printf() as a necessary info.
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 6:28 ` Liu Shengzhou-B36685
@ 2012-10-18 7:15 ` Marek Vasut
2012-10-18 8:54 ` Liu Shengzhou-B36685
0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2012-10-18 7:15 UTC (permalink / raw)
To: u-boot
Dear Liu Shengzhou-B36685,
> > -----Original Message-----
> >
> > > +/* Check USB PHY clock valid */
> > > +static int usb_phy_clk_valid(struct usb_ehci *ehci) {
> > > + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
> > > + (!in_be32(&ehci->prictrl))) {
> >
> > (!A && !B) condition can certainly be done without the double negation
> > ;-)
>
> [Shengzhou] Yes, using !(A||B) is also okay:)
Good, you did your logic homework well. Now go one step further:
if (a || b)
return 1;
printf()
return 0;
How will that work?
>
> > > + printf("USB PHY clock invalid!\n");
> >
> > debug() ?
>
> [Shengzhou] No, it's not for debug purpose, it should be printf() as a
> necessary info.
OK.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 7:15 ` Marek Vasut
@ 2012-10-18 8:54 ` Liu Shengzhou-B36685
2012-10-18 9:04 ` Marek Vasut
0 siblings, 1 reply; 10+ messages in thread
From: Liu Shengzhou-B36685 @ 2012-10-18 8:54 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, October 18, 2012 3:16 PM
> To: Liu Shengzhou-B36685
> Cc: u-boot at lists.denx.de; Stefan Roese; agust at denx.de
> Subject: Re: [PATCH] powerpc/usb: fix bug of CPU hang when missing USB
> PHY clock
>
> Dear Liu Shengzhou-B36685,
>
> > > -----Original Message-----
> > >
> > > > +/* Check USB PHY clock valid */
> > > > +static int usb_phy_clk_valid(struct usb_ehci *ehci) {
> > > > + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
> > > > + (!in_be32(&ehci->prictrl))) {
> > >
> > > (!A && !B) condition can certainly be done without the double negation ;-)
> >
> > [Shengzhou] Yes, using !(A||B) is also okay:)
>
> Good, you did your logic homework well. Now go one step further:
>
> if (a || b)
> return 1;
>
[Shengzhou] No, this doesn't work, b is 0 at initial time, but b is 1 at the second time, a is depend on the register PHY_CLK_VALID bit,
We just want to check it at the first time and then think it is always valid after that, it's using a trick:)
> printf()
> return 0;
>
> How will that work?
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 8:54 ` Liu Shengzhou-B36685
@ 2012-10-18 9:04 ` Marek Vasut
2012-10-19 21:21 ` Andy Fleming
0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2012-10-18 9:04 UTC (permalink / raw)
To: u-boot
Dear Liu Shengzhou-B36685,
> > -----Original Message-----
> > From: Marek Vasut [mailto:marex at denx.de]
> > Sent: Thursday, October 18, 2012 3:16 PM
> > To: Liu Shengzhou-B36685
> > Cc: u-boot at lists.denx.de; Stefan Roese; agust at denx.de
> > Subject: Re: [PATCH] powerpc/usb: fix bug of CPU hang when missing USB
> > PHY clock
> >
> > Dear Liu Shengzhou-B36685,
> >
> > > > -----Original Message-----
> > > >
> > > > > +/* Check USB PHY clock valid */
> > > > > +static int usb_phy_clk_valid(struct usb_ehci *ehci) {
> > > > > + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
> > > > > + (!in_be32(&ehci->prictrl))) {
> > > >
> > > > (!A && !B) condition can certainly be done without the double
> > > > negation ;-)
> > >
> > > [Shengzhou] Yes, using !(A||B) is also okay:)
> >
> > Good, you did your logic homework well. Now go one step further:
> >
> > if (a || b)
> >
> > return 1;
>
> [Shengzhou] No, this doesn't work, b is 0 at initial time, but b is 1 at
> the second time, a is depend on the register PHY_CLK_VALID bit, We just
> want to check it at the first time and then think it is always valid after
> that, it's using a trick:)
Good point, I was just testing you of course ;-)
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 9:04 ` Marek Vasut
@ 2012-10-19 21:21 ` Andy Fleming
2012-10-22 4:51 ` Liu Shengzhou-B36685
0 siblings, 1 reply; 10+ messages in thread
From: Andy Fleming @ 2012-10-19 21:21 UTC (permalink / raw)
To: u-boot
On Thu, Oct 18, 2012 at 4:04 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Liu Shengzhou-B36685,
>> > > >
>> > > > > +/* Check USB PHY clock valid */
>> > > > > +static int usb_phy_clk_valid(struct usb_ehci *ehci) {
>> > > > > + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
>> > > > > + (!in_be32(&ehci->prictrl))) {
>> > > >
>> > > > (!A && !B) condition can certainly be done without the double
>> > > > negation ;-)
>> > >
>> > > [Shengzhou] Yes, using !(A||B) is also okay:)
>> >
>> > Good, you did your logic homework well. Now go one step further:
>> >
>> > if (a || b)
>> >
>> > return 1;
>>
>> [Shengzhou] No, this doesn't work, b is 0 at initial time, but b is 1 at
>> the second time, a is depend on the register PHY_CLK_VALID bit, We just
>> want to check it at the first time and then think it is always valid after
>> that, it's using a trick:)
>
> Good point, I was just testing you of course ;-)
I may just be dim. Why is this a good point? If
in_be32(&ehci->prictrl) is non-zero, then this function will return
'1'. If (in_be32(&ehci->control) & PHY_CLK_VALID) is non-zero, this
function will return 1.
What am I missing?
Andy
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-19 21:21 ` Andy Fleming
@ 2012-10-22 4:51 ` Liu Shengzhou-B36685
0 siblings, 0 replies; 10+ messages in thread
From: Liu Shengzhou-B36685 @ 2012-10-22 4:51 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Andy Fleming [mailto:afleming at gmail.com]
> Sent: Saturday, October 20, 2012 5:22 AM
> To: Marek Vasut
> Cc: Liu Shengzhou-B36685; u-boot at lists.denx.de; Stefan Roese
> Subject: Re: [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when
> missing USB PHY clock
>
> On Thu, Oct 18, 2012 at 4:04 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Liu Shengzhou-B36685,
>
> >> > > >
> >> > > > > +/* Check USB PHY clock valid */ static int
> >> > > > > +usb_phy_clk_valid(struct usb_ehci *ehci) {
> >> > > > > + if ((!(in_be32(&ehci->control) & PHY_CLK_VALID)) &&
> >> > > > > + (!in_be32(&ehci->prictrl))) {
> >> > > >
> >> > > > (!A && !B) condition can certainly be done without the double
> >> > > > negation ;-)
> >> > >
> >> > > [Shengzhou] Yes, using !(A||B) is also okay:)
> >> >
> >> > Good, you did your logic homework well. Now go one step further:
> >> >
> >> > if (a || b)
> >> >
> >> > return 1;
> >>
> >> [Shengzhou] No, this doesn't work, b is 0 at initial time, but b is 1
> >> at the second time, a is depend on the register PHY_CLK_VALID bit, We
> >> just want to check it at the first time and then think it is always
> >> valid after that, it's using a trick:)
> >
> > Good point, I was just testing you of course ;-)
>
>
> I may just be dim. Why is this a good point? If
> in_be32(&ehci->prictrl) is non-zero, then this function will return '1'.
> If (in_be32(&ehci->control) & PHY_CLK_VALID) is non-zero, this function
> will return 1.
>
> What am I missing?
>
> Andy
[Shengzhou] Because the indication of PHY_CLK_VALID is time-sensitive, we just think the value read at the first time is reliable, then PHY_CLK_VALID will be zero after that though actually PHY clock is still valid.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-18 4:21 ` Marek Vasut
2012-10-18 6:28 ` Liu Shengzhou-B36685
@ 2012-10-19 20:13 ` Anatolij Gustschin
2012-10-22 3:47 ` Liu Shengzhou-B36685
1 sibling, 1 reply; 10+ messages in thread
From: Anatolij Gustschin @ 2012-10-19 20:13 UTC (permalink / raw)
To: u-boot
Hi,
On Thu, 18 Oct 2012 06:21:56 +0200
Marek Vasut <marex@denx.de> wrote:
> Dear Shengzhou Liu,
>
> > when missing USB PHY clock, u-boot will hang during USB
> > initialization when issuing "usb start". We should check
> > USBGP[PHY_CLK_VALID] bit to avoid CPU hanging in this case.
> >
> > Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>
> CCing PPC experts.
...
> > @@ -82,18 +94,16 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr,
> > struct ehci_hcor **hcor) udelay(1000); /* delay required for PHY Clk to
> > appear */
> > #endif
> > out_le32(&(*hcor)->or_portsc[0], PORT_PTS_UTMI);
> > + setbits_be32(&ehci->control, USB_EN);
> > } else {
> > -#if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
> > - clrbits_be32(&ehci->control, UTMI_PHY_EN);
> > setbits_be32(&ehci->control, PHY_CLK_SEL_ULPI);
> > + clrsetbits_be32(&ehci->control, UTMI_PHY_EN, USB_EN);
> > udelay(1000); /* delay required for PHY Clk to appear */
> > -#endif
> > + if (!usb_phy_clk_valid(ehci))
> > + return -EINVAL;
> > out_le32(&(*hcor)->or_portsc[0], PORT_PTS_ULPI);
> > }
> >
> > - /* Enable interface. */
> > - setbits_be32(&ehci->control, USB_EN);
you moved the USB interface enabling before the PHY CLK check but the
commit description doesn't mention why it is needed. It would be good
to mention the reason in the commit log.
Thanks,
Anatolij
^ permalink raw reply [flat|nested] 10+ messages in thread* [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock
2012-10-19 20:13 ` Anatolij Gustschin
@ 2012-10-22 3:47 ` Liu Shengzhou-B36685
0 siblings, 0 replies; 10+ messages in thread
From: Liu Shengzhou-B36685 @ 2012-10-22 3:47 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Anatolij Gustschin [mailto:agust at denx.de]
> Sent: Saturday, October 20, 2012 4:14 AM
> To: Liu Shengzhou-B36685
> Cc: Marek Vasut; u-boot at lists.denx.de; Stefan Roese
> Subject: Re: [PATCH] powerpc/usb: fix bug of CPU hang when missing USB
> PHY clock
>
> you moved the USB interface enabling before the PHY CLK check but the
> commit description doesn't mention why it is needed. It would be good to
> mention the reason in the commit log.
>
> Thanks,
> Anatolij
[Shengzhou] Ok, will add it in the commit log, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-10-22 4:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-18 2:40 [U-Boot] [PATCH] powerpc/usb: fix bug of CPU hang when missing USB PHY clock Shengzhou Liu
2012-10-18 4:21 ` Marek Vasut
2012-10-18 6:28 ` Liu Shengzhou-B36685
2012-10-18 7:15 ` Marek Vasut
2012-10-18 8:54 ` Liu Shengzhou-B36685
2012-10-18 9:04 ` Marek Vasut
2012-10-19 21:21 ` Andy Fleming
2012-10-22 4:51 ` Liu Shengzhou-B36685
2012-10-19 20:13 ` Anatolij Gustschin
2012-10-22 3:47 ` Liu Shengzhou-B36685
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox