diff mbox series

[1/2] dt-bindings: usb: renesas,usbhs: Deprecate renesas,enable-gpio

Message ID e9cf476ffac794bad7b0860dc89afd62a9ebc812.1727853953.git.geert+renesas@glider.be (mailing list archive)
State Accepted
Commit 67c6150c0c5faeaaf5cf5c1b1cc6501d431d58d1
Headers show
Series usb: renesas_usbhs: Deprecate renesas,enable-gpio | expand

Commit Message

Geert Uytterhoeven Oct. 2, 2024, 7:35 a.m. UTC
Commit 2071d0968e564b4b ("Documentation: gpio: guidelines for bindings")
deprecated the "gpio" suffix for GPIO consumers in favor of the "gpios"
suffix.  Update the Renesas HS-USB DT bindings to reflect this.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/usb/renesas,usbhs.yaml | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Wolfram Sang Oct. 2, 2024, 7:56 a.m. UTC | #1
> +  renesas,enable-gpios:

Isn't this a good occasion to drop the "renesas"-prefix? Binding docs
are full of plain "enable-gpios".
Geert Uytterhoeven Oct. 2, 2024, 12:10 p.m. UTC | #2
Hi Wolfram,

CC gpio

On Wed, Oct 2, 2024 at 9:56 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > +  renesas,enable-gpios:
>
> Isn't this a good occasion to drop the "renesas"-prefix? Binding docs
> are full of plain "enable-gpios".

Well, that's of course another option (actually 3 ;-)
Compared to simply switching from "renesas,enable-gpio" to
"renesas,enable-gpios", dropping the vendor prefix requires changes to
the HS-USB driver and/or to gpiolib.  Worse, this would also become
a hard dependency for updating the DTS files.

Option A: Add a call to devm_gpiod_get_optional(dev, "enable", GPIOD_IN)
as a fallback to usbhs_probe().

Option B: Switch usbhs_probe() from "renesas,enable" to "enable"
and add quirks to of_find_gpio_rename():

    #if IS_ENABLED(CONFIG_USB_RENESAS_USBHS)
                   /*
                    * The Renesas HS-USB DT bindings happened before
enable-gpios
                    * was established as a generic property
                    */
                   { "enable",     "renesas,enable-gpio",
"renesas,rza1-usbhs" },
                   { "enable",     "renesas,enable-gpio",
"renesas,rza2-usbhs" },
                   { "enable",     "renesas,enable-gpio",
"renesas,rzg2l-usbhs" },
                   { "enable",     "renesas,enable-gpio",
"renesas,rcar-gen2-usbhs" },
                   { "enable",     "renesas,enable-gpio",
"renesas,rcar-gen3-usbhs" },
     #endif

Option C: Add a generic "strip vendor prefix" fallback to
of_find_gpio():

    const char *stripped;

    if (gpiod_not_found(desc) && con_id &&
        (stripped = strchr(con_id, ',')) && *(++stripped)) {
            for_each_gpio_property_name(propname, stripped) {
                    desc = of_get_named_gpiod_flags(np, propname, idx,
&of_flags);
                    if (!gpiod_not_found(desc))
                            break;
            }
    }

Option B adds a bit too much to my liking.
Option C may be useful for others (e.g. {ti,nxp,maxim},enable-gpio(s)),
but might be considered too dangerous as a general fallback?

Thoughts?
Thanks!

Gr{oetje,eeting}s,

                        Geert
Linus Walleij Oct. 2, 2024, 2:09 p.m. UTC | #3
On Wed, Oct 2, 2024 at 2:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Option B: Switch usbhs_probe() from "renesas,enable" to "enable"
> and add quirks to of_find_gpio_rename():
>
>     #if IS_ENABLED(CONFIG_USB_RENESAS_USBHS)
>                    /*
>                     * The Renesas HS-USB DT bindings happened before
> enable-gpios
>                     * was established as a generic property
>                     */
>                    { "enable",     "renesas,enable-gpio",
> "renesas,rza1-usbhs" },
(...)

I would actually prefer this.

> Option C: Add a generic "strip vendor prefix" fallback to
> of_find_gpio():

I understand the appeal, but the idea is for the quirks to be
very specific (hence they are enabled only if specific
drivers are compiled in) and not start to be helpful. Doing
this would make any vendor,foo start to work and I don't like that
at all: any such mechanism will invariably be abused.

Yours,
Linus Walleij
Rob Herring Oct. 2, 2024, 9:36 p.m. UTC | #4
On Wed, Oct 02, 2024 at 04:09:28PM +0200, Linus Walleij wrote:
> On Wed, Oct 2, 2024 at 2:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> > Option B: Switch usbhs_probe() from "renesas,enable" to "enable"
> > and add quirks to of_find_gpio_rename():
> >
> >     #if IS_ENABLED(CONFIG_USB_RENESAS_USBHS)
> >                    /*
> >                     * The Renesas HS-USB DT bindings happened before
> > enable-gpios
> >                     * was established as a generic property
> >                     */
> >                    { "enable",     "renesas,enable-gpio",
> > "renesas,rza1-usbhs" },
> (...)
> 
> I would actually prefer this.
> 
> > Option C: Add a generic "strip vendor prefix" fallback to
> > of_find_gpio():
> 
> I understand the appeal, but the idea is for the quirks to be
> very specific (hence they are enabled only if specific
> drivers are compiled in) and not start to be helpful. Doing
> this would make any vendor,foo start to work and I don't like that
> at all: any such mechanism will invariably be abused.

+1

Unless there's a bunch more platforms coming, I'd just stick with this 
patch (or even do nothing).

Rob
Rob Herring Oct. 2, 2024, 9:37 p.m. UTC | #5
On Wed, 02 Oct 2024 09:35:12 +0200, Geert Uytterhoeven wrote:
> Commit 2071d0968e564b4b ("Documentation: gpio: guidelines for bindings")
> deprecated the "gpio" suffix for GPIO consumers in favor of the "gpios"
> suffix.  Update the Renesas HS-USB DT bindings to reflect this.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  Documentation/devicetree/bindings/usb/renesas,usbhs.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml b/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
index c63db3ebd07bd493..b23ef29bf7949ff9 100644
--- a/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
+++ b/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
@@ -76,6 +76,10 @@  properties:
       Integer to use BUSWAIT register.
 
   renesas,enable-gpio:
+    deprecated: true
+    maxItems: 1
+
+  renesas,enable-gpios:
     maxItems: 1
     description: |
       gpio specifier to check GPIO determining if USB function should be