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

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Wolfram,

CC gpio

On Wed, Oct 2, 2024 at 9:56 AM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> 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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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