On Tue, Sep 27, 2022 at 03:32:35PM -0700, Dmitry Torokhov wrote:
> Properties describing GPIOs should be named as "<property>-gpios" or
> "<property>-gpio", and that is what gpiod API expects, however the
> driver uses non-standard "gpios-reset" name. Let's adjust this, and also
> note that the reset line is active low as that is also important to
> gpiod API.
No objections to the goal but...
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>
> Another option is to add another quirk into gpiolib-of.c, but we
> may end up with a ton of them once we convert everything away from
> of_get_named_gpio() to gpiod API, so I'd prefer not doing that.
... it is unusual to permit backwards incompatible changes to the DT
bindings[1]: creating "flag days" where hardware stops functioning if
you boot an new kernel with an old DT is a known annoyance to users.
I usually favour quirks tables or similar[2] rather than break legacy
DTs. Very occasionally I accept (believable) arguments that no legacy
DTs actually exist but that can very difficult to verify.
Overall I'd like to solicit views from both GPIO and DT maintainers
before rejecting quirks tables as a way to help smooth these sort of
changes (or links to ML archives if this has already been discussed).
[1] For this particular driver the situation is muddied slightly
because it looks like complex since it looks the bindings for
himax,hx8357 and himax,hx8369 are undocumented (and badly named).
[2] When the property is not parsed by library code mostly we handle
legacy by consuming both new or old names in the parser code.
> diff --git a/drivers/video/backlight/hx8357.c b/drivers/video/backlight/hx8357.c
> index 9b50bc96e00f..41332f48b2df 100644
> --- a/drivers/video/backlight/hx8357.c
> +++ b/drivers/video/backlight/hx8357.c
> @@ -601,7 +601,7 @@ static int hx8357_probe(struct spi_device *spi)
> if (!match || !match->data)
> return -EINVAL;
>
> - lcd->reset = of_get_named_gpio(spi->dev.of_node, "gpios-reset", 0);
> + lcd->reset = of_get_named_gpio(spi->dev.of_node, "reset-gpios", 0);
> if (!gpio_is_valid(lcd->reset)) {
> dev_err(&spi->dev, "Missing dt property: gpios-reset\n");
> return -EINVAL;
Daniel.
