| Message ID | 1579258447-28135-2-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: host: ehci-platform: add a quirk to avoid stuck | expand |
Hi Shimoda-san, Thanks for your patch! On Fri, Jan 17, 2020 at 11:54 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Since EHCI/OHCI controllers on R-Car Gen3 SoCs are possible to > be getting stuck very rarely after a full/low usb device was > disconnected. To detect/recover from such a situation, the controllers > require a special way which poll the EHCI PORTSC register and changes > the OHCI functional state. Oops... Is this limited to the EHCI/OHCI implementation on R-Car Gen3? Or can it happen on any EHCI/OHCI controller? > --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml > +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml > @@ -63,6 +63,11 @@ properties: > description: > Set this flag to force EHCI reset after resume. > > + needs-polling-to-avoid-stuck: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Set this flag to avoid getting EHCI stuck. > + > companion: > $ref: /schemas/types.yaml#/definitions/phandle > description: If this issue is specific to the EHCI/OHCI implementation on R-Car Gen3, I don't think this is the best solution to handle it. It might be better to add family/SoC-specific compatible values for the EHCI/OHCI controllers in R-Car Gen3 SoCs, cfr. the (undocumented) "ibm,usb-ehci-440epx" and "allwinner,sun4i-a10-ehci" compatible values in the example in the DT bindings file (probably we should have done so from the start, like for all other devices). Then the driver can handle the issue based on the compatible value. Besides, what about DT backwards compatibility? To enable this quirk handling, the DT must be updated first. This is true for solutions based on either a DT property or on a different compatible value. Of course, you can always use soc_device_match()... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert-san, Thank you for your review! > From: Geert Uytterhoeven, Sent: Saturday, January 18, 2020 1:03 AM > > Hi Shimoda-san, > > Thanks for your patch! > > On Fri, Jan 17, 2020 at 11:54 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Since EHCI/OHCI controllers on R-Car Gen3 SoCs are possible to > > be getting stuck very rarely after a full/low usb device was > > disconnected. To detect/recover from such a situation, the controllers > > require a special way which poll the EHCI PORTSC register and changes > > the OHCI functional state. > > Oops... Is this limited to the EHCI/OHCI implementation on R-Car Gen3? > Or can it happen on any EHCI/OHCI controller? This is limited on R-Car Gen3 (and perhaps RZ/A2 and RZ/G2). But, R-Mobile A1 and R-Car Gen1/2 don't have this issue. And I don't know any other EHCI/OHCI controller has this issue. > > --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml > > +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml > > @@ -63,6 +63,11 @@ properties: > > description: > > Set this flag to force EHCI reset after resume. > > > > + needs-polling-to-avoid-stuck: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + Set this flag to avoid getting EHCI stuck. > > + > > companion: > > $ref: /schemas/types.yaml#/definitions/phandle > > description: > > If this issue is specific to the EHCI/OHCI implementation on R-Car Gen3, > I don't think this is the best solution to handle it. > It might be better to add family/SoC-specific compatible values for the > EHCI/OHCI controllers in R-Car Gen3 SoCs, cfr. the (undocumented) > "ibm,usb-ehci-440epx" and "allwinner,sun4i-a10-ehci" compatible values > in the example in the DT bindings file (probably we should have done so > from the start, like for all other devices). > Then the driver can handle the issue based on the compatible value. I understood it. And I'm also think adding family/SoC-specific compatible values are better. > Besides, what about DT backwards compatibility? > To enable this quirk handling, the DT must be updated first. > This is true for solutions based on either a DT property or on a > different compatible value. > Of course, you can always use soc_device_match()... In this case, I should apply this quirk to some SoCs, I think using DT is better than soc_device_match(). Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Geert-san again, > From: Yoshihiro Shimoda, Sent: Monday, January 20, 2020 5:05 PM > > > --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml > > > +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml > > > @@ -63,6 +63,11 @@ properties: > > > description: > > > Set this flag to force EHCI reset after resume. > > > > > > + needs-polling-to-avoid-stuck: > > > + $ref: /schemas/types.yaml#/definitions/flag > > > + description: > > > + Set this flag to avoid getting EHCI stuck. > > > + > > > companion: > > > $ref: /schemas/types.yaml#/definitions/phandle > > > description: > > > > If this issue is specific to the EHCI/OHCI implementation on R-Car Gen3, > > I don't think this is the best solution to handle it. > > It might be better to add family/SoC-specific compatible values for the > > EHCI/OHCI controllers in R-Car Gen3 SoCs, cfr. the (undocumented) > > "ibm,usb-ehci-440epx" and "allwinner,sun4i-a10-ehci" compatible values > > in the example in the DT bindings file (probably we should have done so > > from the start, like for all other devices). > > Then the driver can handle the issue based on the compatible value. > > I understood it. And I'm also think adding family/SoC-specific compatible > values are better. I'm trying to add some undocumented compatible values, but it seems hard to add because: - Some dts[i] files have undocumented compatible strings. # We can find it by using the following command: # $ grep "generic-ehci" `find -name "*.dts*"` | grep "," - I tried to use "oneOf:" and "contains:" combination, but it failed. - This generic-ehci.yaml uses "contains:" for the compatible now. So, even if compatible property has undocumented compatible string, make dtbs_check command succeeded (except node names). - In my opinion, almost all user (excect R-Car SoCs) doesn't needs specific compatible values, so that adding such compatible values causes less usability in the future. So, I suspended adding specific compatible values and I'll use soc_device_match() for this workaround for now... Best regards, Yoshihiro Shimoda
Hi Shimoda-san, On Thu, Jan 23, 2020 at 9:17 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Yoshihiro Shimoda, Sent: Monday, January 20, 2020 5:05 PM > > > > --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml > > > > +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml > > > > @@ -63,6 +63,11 @@ properties: > > > > description: > > > > Set this flag to force EHCI reset after resume. > > > > > > > > + needs-polling-to-avoid-stuck: > > > > + $ref: /schemas/types.yaml#/definitions/flag > > > > + description: > > > > + Set this flag to avoid getting EHCI stuck. > > > > + > > > > companion: > > > > $ref: /schemas/types.yaml#/definitions/phandle > > > > description: > > > > > > If this issue is specific to the EHCI/OHCI implementation on R-Car Gen3, > > > I don't think this is the best solution to handle it. > > > It might be better to add family/SoC-specific compatible values for the > > > EHCI/OHCI controllers in R-Car Gen3 SoCs, cfr. the (undocumented) > > > "ibm,usb-ehci-440epx" and "allwinner,sun4i-a10-ehci" compatible values > > > in the example in the DT bindings file (probably we should have done so > > > from the start, like for all other devices). > > > Then the driver can handle the issue based on the compatible value. > > > > I understood it. And I'm also think adding family/SoC-specific compatible > > values are better. > > I'm trying to add some undocumented compatible values, but it seems hard > to add because: > - Some dts[i] files have undocumented compatible strings. > # We can find it by using the following command: > # $ grep "generic-ehci" `find -name "*.dts*"` | grep "," > > - I tried to use "oneOf:" and "contains:" combination, but it failed. > > - This generic-ehci.yaml uses "contains:" for the compatible now. > So, even if compatible property has undocumented compatible string, > make dtbs_check command succeeded (except node names). Probably you'll have to write a separate DT binding doc file for R-Car Gen3, referring to generic-ehci.yaml using $ref. > - In my opinion, almost all user (excect R-Car SoCs) doesn't needs > specific compatible values, so that adding such compatible values > causes less usability in the future. > > So, I suspended adding specific compatible values and I'll use > soc_device_match() for this workaround for now... Which has the advantage that it will enable the quirk with old DTBs, too ;-) Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, January 23, 2020 5:57 PM <snip> > > I'm trying to add some undocumented compatible values, but it seems hard > > to add because: > > - Some dts[i] files have undocumented compatible strings. > > # We can find it by using the following command: > > # $ grep "generic-ehci" `find -name "*.dts*"` | grep "," > > > > - I tried to use "oneOf:" and "contains:" combination, but it failed. > > > > - This generic-ehci.yaml uses "contains:" for the compatible now. > > So, even if compatible property has undocumented compatible string, > > make dtbs_check command succeeded (except node names). > > Probably you'll have to write a separate DT binding doc file for R-Car Gen3, > referring to generic-ehci.yaml using $ref. I see. > > - In my opinion, almost all user (excect R-Car SoCs) doesn't needs > > specific compatible values, so that adding such compatible values > > causes less usability in the future. > > > > So, I suspended adding specific compatible values and I'll use > > soc_device_match() for this workaround for now... > > Which has the advantage that it will enable the quirk with old DTBs, too ;-) I think so :) Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml index 10edd05..c3d2dae 100644 --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml @@ -63,6 +63,11 @@ properties: description: Set this flag to force EHCI reset after resume. + needs-polling-to-avoid-stuck: + $ref: /schemas/types.yaml#/definitions/flag + description: + Set this flag to avoid getting EHCI stuck. + companion: $ref: /schemas/types.yaml#/definitions/phandle description:
Since EHCI/OHCI controllers on R-Car Gen3 SoCs are possible to be getting stuck very rarely after a full/low usb device was disconnected. To detect/recover from such a situation, the controllers require a special way which poll the EHCI PORTSC register and changes the OHCI functional state. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- Documentation/devicetree/bindings/usb/generic-ehci.yaml | 5 +++++ 1 file changed, 5 insertions(+)