| Message ID | e9cf476ffac794bad7b0860dc89afd62a9ebc812.1727853953.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 67c6150c0c5faeaaf5cf5c1b1cc6501d431d58d1 |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | usb: renesas_usbhs: Deprecate renesas,enable-gpio | expand |
> + renesas,enable-gpios:
Isn't this a good occasion to drop the "renesas"-prefix? Binding docs
are full of plain "enable-gpios".
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
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 --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
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(+)