| Message ID | 2bad662ba61cadc3bbbb10d115f60589c23d9374.1646816742.git.michal.simek@xilinx.com |
|---|---|
| State | Accepted |
| Commit | 142d50fbce7c364a74f5e8204dba491b9f066e6c |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | usb: dwc3: Enable PHY support | expand |
On Wed, Mar 9, 2022 at 5:06 PM Michal Simek <michal.simek@xilinx.com> wrote: > > When usb3-phy label is found, PHY driver is called and serdes line is > initialized. This is preparation for serdes/psgtr driver to configure GT > lines based on description in DT. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > Changes in v3: > - Add cover letter > > Changes in v2: > - Add missing header > > drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
On 09.03.22 10:05, Michal Simek wrote: > When usb3-phy label is found, PHY driver is called and serdes line is > initialized. This is preparation for serdes/psgtr driver to configure GT > lines based on description in DT. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > Changes in v3: > - Add cover letter > > Changes in v2: > - Add missing header > > drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c > index 01bd0ca190e3..2c5205df62cd 100644 > --- a/drivers/usb/dwc3/dwc3-generic.c > +++ b/drivers/usb/dwc3/dwc3-generic.c > @@ -14,6 +14,7 @@ > #include <dm/device-internal.h> > #include <dm/lists.h> > #include <dwc3-uboot.h> > +#include <generic-phy.h> > #include <linux/bitops.h> > #include <linux/delay.h> > #include <linux/usb/ch9.h> > @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) > struct udevice *child = NULL; > int index = 0; > int ret; > + struct phy phy; > + > + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); > + if (!ret) { > + ret = generic_phy_init(&phy); > + if (ret) > + return ret; > + } else if (ret != -ENOENT) { > + debug("could not get phy (err %d)\n", ret); > + return ret; > + } > > glue->regs = dev_read_addr(dev); > > @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice *dev) > if (ret) > return ret; > > + if (phy.dev) { > + ret = generic_phy_power_on(&phy); > + if (ret) > + return ret; > + } > + > ret = device_find_first_child(dev, &child); > if (ret) > return ret; Breaks USB on the iot2050-pg1 (am65x) - this one has NO usb3-phy: ... starting USB... Bus usb@10000: probe failed, error -61 Bus usb@10000: probe failed, error -61 No working controllers found USB is stopped. Please issue 'usb start' first. starting USB... Bus usb@10000: probe failed, error -61 Bus usb@10000: probe failed, error -61 No working controllers found USB is stopped. Please issue 'usb start' first. starting USB... Bus usb@10000: probe failed, error -61 Bus usb@10000: probe failed, error -61 No working controllers found USB is stopped. Please issue 'usb start' first. Is there anything that boards need to consider now? Jan
Hi Jan, On 4/25/22 11:47, Jan Kiszka wrote: > On 09.03.22 10:05, Michal Simek wrote: >> When usb3-phy label is found, PHY driver is called and serdes line is >> initialized. This is preparation for serdes/psgtr driver to configure GT >> lines based on description in DT. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> Changes in v3: >> - Add cover letter >> >> Changes in v2: >> - Add missing header >> >> drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c >> index 01bd0ca190e3..2c5205df62cd 100644 >> --- a/drivers/usb/dwc3/dwc3-generic.c >> +++ b/drivers/usb/dwc3/dwc3-generic.c >> @@ -14,6 +14,7 @@ >> #include <dm/device-internal.h> >> #include <dm/lists.h> >> #include <dwc3-uboot.h> >> +#include <generic-phy.h> >> #include <linux/bitops.h> >> #include <linux/delay.h> >> #include <linux/usb/ch9.h> >> @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) >> struct udevice *child = NULL; >> int index = 0; >> int ret; >> + struct phy phy; >> + >> + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); >> + if (!ret) { >> + ret = generic_phy_init(&phy); >> + if (ret) >> + return ret; >> + } else if (ret != -ENOENT) { >> + debug("could not get phy (err %d)\n", ret); >> + return ret; >> + } >> >> glue->regs = dev_read_addr(dev); >> >> @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice *dev) >> if (ret) >> return ret; >> >> + if (phy.dev) { >> + ret = generic_phy_power_on(&phy); >> + if (ret) >> + return ret; >> + } >> + >> ret = device_find_first_child(dev, &child); >> if (ret) >> return ret; > > Breaks USB on the iot2050-pg1 (am65x) - this one has NO usb3-phy: > > ... > starting USB... > Bus usb@10000: probe failed, error -61 > Bus usb@10000: probe failed, error -61 > No working controllers found > USB is stopped. Please issue 'usb start' first. > starting USB... > Bus usb@10000: probe failed, error -61 > Bus usb@10000: probe failed, error -61 > No working controllers found > USB is stopped. Please issue 'usb start' first. > starting USB... > Bus usb@10000: probe failed, error -61 > Bus usb@10000: probe failed, error -61 > No working controllers found > USB is stopped. Please issue 'usb start' first. > > Is there anything that boards need to consider now? -61 is ENODATA. I have looked at DT and there is no usb3-phy property. That's why generic_phy_get_by_name() can't return 0. Does it return -ENOENT? Maybe it returns ENODATA and it should be also handled in else part. Can you please enable debug and see? Thanks, Michal
On 25.04.22 11:56, Michal Simek wrote: > Hi Jan, > > On 4/25/22 11:47, Jan Kiszka wrote: >> On 09.03.22 10:05, Michal Simek wrote: >>> When usb3-phy label is found, PHY driver is called and serdes line is >>> initialized. This is preparation for serdes/psgtr driver to configure GT >>> lines based on description in DT. >>> >>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>> --- >>> >>> Changes in v3: >>> - Add cover letter >>> >>> Changes in v2: >>> - Add missing header >>> >>> drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/dwc3-generic.c >>> b/drivers/usb/dwc3/dwc3-generic.c >>> index 01bd0ca190e3..2c5205df62cd 100644 >>> --- a/drivers/usb/dwc3/dwc3-generic.c >>> +++ b/drivers/usb/dwc3/dwc3-generic.c >>> @@ -14,6 +14,7 @@ >>> #include <dm/device-internal.h> >>> #include <dm/lists.h> >>> #include <dwc3-uboot.h> >>> +#include <generic-phy.h> >>> #include <linux/bitops.h> >>> #include <linux/delay.h> >>> #include <linux/usb/ch9.h> >>> @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) >>> struct udevice *child = NULL; >>> int index = 0; >>> int ret; >>> + struct phy phy; >>> + >>> + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); >>> + if (!ret) { >>> + ret = generic_phy_init(&phy); >>> + if (ret) >>> + return ret; >>> + } else if (ret != -ENOENT) { >>> + debug("could not get phy (err %d)\n", ret); >>> + return ret; >>> + } >>> glue->regs = dev_read_addr(dev); >>> @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice *dev) >>> if (ret) >>> return ret; >>> + if (phy.dev) { >>> + ret = generic_phy_power_on(&phy); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> ret = device_find_first_child(dev, &child); >>> if (ret) >>> return ret; >> >> Breaks USB on the iot2050-pg1 (am65x) - this one has NO usb3-phy: >> >> ... >> starting USB... >> Bus usb@10000: probe failed, error -61 >> Bus usb@10000: probe failed, error -61 >> No working controllers found >> USB is stopped. Please issue 'usb start' first. >> starting USB... >> Bus usb@10000: probe failed, error -61 >> Bus usb@10000: probe failed, error -61 >> No working controllers found >> USB is stopped. Please issue 'usb start' first. >> starting USB... >> Bus usb@10000: probe failed, error -61 >> Bus usb@10000: probe failed, error -61 >> No working controllers found >> USB is stopped. Please issue 'usb start' first. >> >> Is there anything that boards need to consider now? > > -61 is ENODATA. I have looked at DT and there is no usb3-phy property. > That's why generic_phy_get_by_name() can't return 0. Does it return > -ENOENT? > > Maybe it returns ENODATA and it should be also handled in else part. > > Can you please enable debug and see? > #define DEBUG in the patched file or where? Jan
On 4/25/22 12:05, Jan Kiszka wrote: > On 25.04.22 11:56, Michal Simek wrote: >> Hi Jan, >> >> On 4/25/22 11:47, Jan Kiszka wrote: >>> On 09.03.22 10:05, Michal Simek wrote: >>>> When usb3-phy label is found, PHY driver is called and serdes line is >>>> initialized. This is preparation for serdes/psgtr driver to configure GT >>>> lines based on description in DT. >>>> >>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>> --- >>>> >>>> Changes in v3: >>>> - Add cover letter >>>> >>>> Changes in v2: >>>> - Add missing header >>>> >>>> drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ >>>> 1 file changed, 18 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c >>>> b/drivers/usb/dwc3/dwc3-generic.c >>>> index 01bd0ca190e3..2c5205df62cd 100644 >>>> --- a/drivers/usb/dwc3/dwc3-generic.c >>>> +++ b/drivers/usb/dwc3/dwc3-generic.c >>>> @@ -14,6 +14,7 @@ >>>> #include <dm/device-internal.h> >>>> #include <dm/lists.h> >>>> #include <dwc3-uboot.h> >>>> +#include <generic-phy.h> >>>> #include <linux/bitops.h> >>>> #include <linux/delay.h> >>>> #include <linux/usb/ch9.h> >>>> @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) >>>> struct udevice *child = NULL; >>>> int index = 0; >>>> int ret; >>>> + struct phy phy; >>>> + >>>> + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); >>>> + if (!ret) { >>>> + ret = generic_phy_init(&phy); >>>> + if (ret) >>>> + return ret; >>>> + } else if (ret != -ENOENT) { >>>> + debug("could not get phy (err %d)\n", ret); >>>> + return ret; >>>> + } >>>> glue->regs = dev_read_addr(dev); >>>> @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice *dev) >>>> if (ret) >>>> return ret; >>>> + if (phy.dev) { >>>> + ret = generic_phy_power_on(&phy); >>>> + if (ret) >>>> + return ret; >>>> + } >>>> + >>>> ret = device_find_first_child(dev, &child); >>>> if (ret) >>>> return ret; >>> >>> Breaks USB on the iot2050-pg1 (am65x) - this one has NO usb3-phy: >>> >>> ... >>> starting USB... >>> Bus usb@10000: probe failed, error -61 >>> Bus usb@10000: probe failed, error -61 >>> No working controllers found >>> USB is stopped. Please issue 'usb start' first. >>> starting USB... >>> Bus usb@10000: probe failed, error -61 >>> Bus usb@10000: probe failed, error -61 >>> No working controllers found >>> USB is stopped. Please issue 'usb start' first. >>> starting USB... >>> Bus usb@10000: probe failed, error -61 >>> Bus usb@10000: probe failed, error -61 >>> No working controllers found >>> USB is stopped. Please issue 'usb start' first. >>> >>> Is there anything that boards need to consider now? >> >> -61 is ENODATA. I have looked at DT and there is no usb3-phy property. >> That's why generic_phy_get_by_name() can't return 0. Does it return >> -ENOENT? >> >> Maybe it returns ENODATA and it should be also handled in else part. >> >> Can you please enable debug and see? >> > > #define DEBUG in the patched file or where? yes above of headers in this file is enough. M
On 25.04.22 12:06, Michal Simek wrote: > > > On 4/25/22 12:05, Jan Kiszka wrote: >> On 25.04.22 11:56, Michal Simek wrote: >>> Hi Jan, >>> >>> On 4/25/22 11:47, Jan Kiszka wrote: >>>> On 09.03.22 10:05, Michal Simek wrote: >>>>> When usb3-phy label is found, PHY driver is called and serdes line is >>>>> initialized. This is preparation for serdes/psgtr driver to >>>>> configure GT >>>>> lines based on description in DT. >>>>> >>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>> --- >>>>> >>>>> Changes in v3: >>>>> - Add cover letter >>>>> >>>>> Changes in v2: >>>>> - Add missing header >>>>> >>>>> drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ >>>>> 1 file changed, 18 insertions(+) >>>>> >>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c >>>>> b/drivers/usb/dwc3/dwc3-generic.c >>>>> index 01bd0ca190e3..2c5205df62cd 100644 >>>>> --- a/drivers/usb/dwc3/dwc3-generic.c >>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c >>>>> @@ -14,6 +14,7 @@ >>>>> #include <dm/device-internal.h> >>>>> #include <dm/lists.h> >>>>> #include <dwc3-uboot.h> >>>>> +#include <generic-phy.h> >>>>> #include <linux/bitops.h> >>>>> #include <linux/delay.h> >>>>> #include <linux/usb/ch9.h> >>>>> @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) >>>>> struct udevice *child = NULL; >>>>> int index = 0; >>>>> int ret; >>>>> + struct phy phy; >>>>> + >>>>> + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); >>>>> + if (!ret) { >>>>> + ret = generic_phy_init(&phy); >>>>> + if (ret) >>>>> + return ret; >>>>> + } else if (ret != -ENOENT) { >>>>> + debug("could not get phy (err %d)\n", ret); >>>>> + return ret; >>>>> + } >>>>> glue->regs = dev_read_addr(dev); >>>>> @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice >>>>> *dev) >>>>> if (ret) >>>>> return ret; >>>>> + if (phy.dev) { >>>>> + ret = generic_phy_power_on(&phy); >>>>> + if (ret) >>>>> + return ret; >>>>> + } >>>>> + >>>>> ret = device_find_first_child(dev, &child); >>>>> if (ret) >>>>> return ret; >>>> >>>> Breaks USB on the iot2050-pg1 (am65x) - this one has NO usb3-phy: >>>> >>>> ... >>>> starting USB... >>>> Bus usb@10000: probe failed, error -61 >>>> Bus usb@10000: probe failed, error -61 >>>> No working controllers found >>>> USB is stopped. Please issue 'usb start' first. >>>> starting USB... >>>> Bus usb@10000: probe failed, error -61 >>>> Bus usb@10000: probe failed, error -61 >>>> No working controllers found >>>> USB is stopped. Please issue 'usb start' first. >>>> starting USB... >>>> Bus usb@10000: probe failed, error -61 >>>> Bus usb@10000: probe failed, error -61 >>>> No working controllers found >>>> USB is stopped. Please issue 'usb start' first. >>>> >>>> Is there anything that boards need to consider now? >>> >>> -61 is ENODATA. I have looked at DT and there is no usb3-phy property. >>> That's why generic_phy_get_by_name() can't return 0. Does it return >>> -ENOENT? >>> >>> Maybe it returns ENODATA and it should be also handled in else part. >>> >>> Can you please enable debug and see? >>> >> >> #define DEBUG in the patched file or where? > > yes above of headers in this file is enough. > > M starting USB... Bus usb@10000: could not get phy (err -61) probe failed, error -61 Bus usb@10000: could not get phy (err -61) probe failed, error -61 No working controllers found Jan
On 25.04.22 12:09, Jan Kiszka wrote: > On 25.04.22 12:06, Michal Simek wrote: >> >> >> On 4/25/22 12:05, Jan Kiszka wrote: >>> On 25.04.22 11:56, Michal Simek wrote: >>>> Hi Jan, >>>> >>>> On 4/25/22 11:47, Jan Kiszka wrote: >>>>> On 09.03.22 10:05, Michal Simek wrote: >>>>>> When usb3-phy label is found, PHY driver is called and serdes line is >>>>>> initialized. This is preparation for serdes/psgtr driver to >>>>>> configure GT >>>>>> lines based on description in DT. >>>>>> >>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>>>>> --- >>>>>> >>>>>> Changes in v3: >>>>>> - Add cover letter >>>>>> >>>>>> Changes in v2: >>>>>> - Add missing header >>>>>> >>>>>> drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ >>>>>> 1 file changed, 18 insertions(+) >>>>>> >>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c >>>>>> b/drivers/usb/dwc3/dwc3-generic.c >>>>>> index 01bd0ca190e3..2c5205df62cd 100644 >>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c >>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c >>>>>> @@ -14,6 +14,7 @@ >>>>>> #include <dm/device-internal.h> >>>>>> #include <dm/lists.h> >>>>>> #include <dwc3-uboot.h> >>>>>> +#include <generic-phy.h> >>>>>> #include <linux/bitops.h> >>>>>> #include <linux/delay.h> >>>>>> #include <linux/usb/ch9.h> >>>>>> @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) >>>>>> struct udevice *child = NULL; >>>>>> int index = 0; >>>>>> int ret; >>>>>> + struct phy phy; >>>>>> + >>>>>> + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); >>>>>> + if (!ret) { >>>>>> + ret = generic_phy_init(&phy); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + } else if (ret != -ENOENT) { >>>>>> + debug("could not get phy (err %d)\n", ret); >>>>>> + return ret; >>>>>> + } >>>>>> glue->regs = dev_read_addr(dev); >>>>>> @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice >>>>>> *dev) >>>>>> if (ret) >>>>>> return ret; >>>>>> + if (phy.dev) { >>>>>> + ret = generic_phy_power_on(&phy); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> ret = device_find_first_child(dev, &child); >>>>>> if (ret) >>>>>> return ret; >>>>> >>>>> Breaks USB on the iot2050-pg1 (am65x) - this one has NO usb3-phy: >>>>> >>>>> ... >>>>> starting USB... >>>>> Bus usb@10000: probe failed, error -61 >>>>> Bus usb@10000: probe failed, error -61 >>>>> No working controllers found >>>>> USB is stopped. Please issue 'usb start' first. >>>>> starting USB... >>>>> Bus usb@10000: probe failed, error -61 >>>>> Bus usb@10000: probe failed, error -61 >>>>> No working controllers found >>>>> USB is stopped. Please issue 'usb start' first. >>>>> starting USB... >>>>> Bus usb@10000: probe failed, error -61 >>>>> Bus usb@10000: probe failed, error -61 >>>>> No working controllers found >>>>> USB is stopped. Please issue 'usb start' first. >>>>> >>>>> Is there anything that boards need to consider now? >>>> >>>> -61 is ENODATA. I have looked at DT and there is no usb3-phy property. >>>> That's why generic_phy_get_by_name() can't return 0. Does it return >>>> -ENOENT? >>>> >>>> Maybe it returns ENODATA and it should be also handled in else part. >>>> >>>> Can you please enable debug and see? >>>> >>> >>> #define DEBUG in the patched file or where? >> >> yes above of headers in this file is enough. >> >> M > > starting USB... > Bus usb@10000: could not get phy (err -61) > probe failed, error -61 > Bus usb@10000: could not get phy (err -61) > probe failed, error -61 > No working controllers found > You need the -ENODATA check as well, see e.g. drivers/usb/dwc3/dwc3-meson-g12a.c. Jan
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 01bd0ca190e3..2c5205df62cd 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -14,6 +14,7 @@ #include <dm/device-internal.h> #include <dm/lists.h> #include <dwc3-uboot.h> +#include <generic-phy.h> #include <linux/bitops.h> #include <linux/delay.h> #include <linux/usb/ch9.h> @@ -409,6 +410,17 @@ static int dwc3_glue_probe(struct udevice *dev) struct udevice *child = NULL; int index = 0; int ret; + struct phy phy; + + ret = generic_phy_get_by_name(dev, "usb3-phy", &phy); + if (!ret) { + ret = generic_phy_init(&phy); + if (ret) + return ret; + } else if (ret != -ENOENT) { + debug("could not get phy (err %d)\n", ret); + return ret; + } glue->regs = dev_read_addr(dev); @@ -420,6 +432,12 @@ static int dwc3_glue_probe(struct udevice *dev) if (ret) return ret; + if (phy.dev) { + ret = generic_phy_power_on(&phy); + if (ret) + return ret; + } + ret = device_find_first_child(dev, &child); if (ret) return ret;
When usb3-phy label is found, PHY driver is called and serdes line is initialized. This is preparation for serdes/psgtr driver to configure GT lines based on description in DT. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- Changes in v3: - Add cover letter Changes in v2: - Add missing header drivers/usb/dwc3/dwc3-generic.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)