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

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(+)

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

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

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