| Message ID | YzN6A9Y20Ea1LdEz@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC/PATCH] backlight: hx8357: prepare to conversion to gpiod API | expand |
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@gmail.com> > --- > > 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.
On Wed, Sep 28, 2022 at 12:00:51PM +0100, Daniel Thompson wrote: > 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@gmail.com> > > --- > > > > 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). I believe I was able to convince Rob once or twice that keeping compatibility was not worth it (not in general but in a couple of concrete cases), at least while device tree bindings are part of the kernel. Can't find the emails though... I think we should consider several options: 1. DTS/DTB is in firmware. In this case absolutely, we need to keep binary compatibility as we can not expect users to reflash firmware (there might not even be a new firmware). This situation matches what we have with ACPI systems where we are trying to work around issues 2. DTS is shipped with the kernel: 2a. DTS is present in upstream kernel - awesome, we can patch it as needed and have one less thing to worry about. 2b. DTS is not upstream. Vendor did not bother sending it. In this case it is extremely unlikely that an upstream kernel will work on such system out of the box, and updating the kernel is a large engineering task where you pull down new kernel, rework and apply non-upstream patches, rework kernel config (new Kconfig options can be introduced, old options can be renamed, etc). And then spend several weeks stabilizing the system and tracking down regressions (in general, not DTS-related ones) 3. DTS is not in firmware and not in kernel. Are there such systems? So my opinion is that while device trees are part of kernel code and have not been split into a separate project they are a fair game. If the change can be handled in the driver without much effort (something like "wakeup-source" vs "linux,wakeup" vs "linux,keypad-wakeup") - fine, we can just put a tiny quirk in the driver, but if we need something more substantial we need to think long and hard if we should implement a fallback and how much effort there is to maintain/test it so it does not bitrot. Anyway, I hope Rob, Linux and Krzysztof to chime in on this exciting topic once again ;) > > [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. Thanks.
On Wed, Sep 28, 2022 at 11:33:52AM -0700, Dmitry Torokhov wrote: > On Wed, Sep 28, 2022 at 12:00:51PM +0100, Daniel Thompson wrote: > > 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@gmail.com> > > > --- > > > > > > 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). > > I believe I was able to convince Rob once or twice that keeping > compatibility was not worth it (not in general but in a couple of > concrete cases), at least while device tree bindings are part of the > kernel. Can't find the emails though... > > I think we should consider several options: I have to note that these are *non-exclusive* options > 1. DTS/DTB is in firmware. In this case absolutely, we need to keep > binary compatibility as we can not expect users to reflash firmware > (there might not even be a new firmware). This situation matches what we > have with ACPI systems where we are trying to work around issues > > 2. DTS is shipped with the kernel: > 2a. DTS is present in upstream kernel - awesome, we can patch it > as needed and have one less thing to worry about. I don't think the presence of a DT within the kernel can be the basis for any useful reasoning. a. "Better" firmware projects aimed are likely to consume a DT that is shipped with the kernel and pin it (meaning the kernel cannot solve version skew problems by updating it's copies of the DT). I think tow-boot to be a specific example of this. b. The fact there are are consumers of the binding shipped with the kernel isn't sufficient to show that *all* consumers of the binding are shipped with the kernel. On other words I don't think the presence of a DT in the kernel is especially useful to showing that neither #1 nor #3 apply. > 2b. DTS is not upstream. Vendor did not bother sending it. In > this case it is extremely unlikely that an upstream kernel > will work on such system out of the box, and updating the > kernel is a large engineering task where you pull down new > kernel, rework and apply non-upstream patches, rework kernel > config (new Kconfig options can be introduced, old options > can be renamed, etc). And then spend several weeks > stabilizing the system and tracking down regressions (in > general, not DTS-related ones) > > 3. DTS is not in firmware and not in kernel. Are there such systems? DT overlays strike me are an example of this case. I'm particularly thinking of daughterboard/expansion card examples here where the DT overlay could be any several different places: firmware, an add on boards I2C FLASH, daughterboard documentation, blog posts, etc. That is especially relevant to this specific patch since HX8357 is found on several widely available add-on boards. > So my opinion is that while device trees are part of kernel code and > have not been split into a separate project they are a fair game. If the > change can be handled in the driver without much effort (something like > "wakeup-source" vs "linux,wakeup" vs "linux,keypad-wakeup") - fine, we > can just put a tiny quirk in the driver, but if we need something more > substantial we need to think long and hard if we should implement a > fallback and how much effort there is to maintain/test it so it does not > bitrot. To be honest my original thoughts were that for simple renames, a rename quirk table shared by all renames needed to introduce libgpiod would probably have a lower impact than all the "tiny" per-driver quirks (because it could share code across multiple names). > Anyway, I hope Rob, Linux and Krzysztof to chime in on this exciting > topic once again ;) I'm especially interested in a gpiod point of view! I have invested quite a few characters in this thread. That is because, if accepted, adding strings to a quirks table is much less effort for patch submitters than having to demonstrate on a per-patch basis the due diligence that has been undertaken to show that cases #1 and #3 do not apply to the particular rename being sought. Daniel.
On Wed, Sep 28, 2022 at 12:32 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> 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. > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> I think the gods of Open Firmware will try to punish you for such incompatible changes. But I have long since renounced them. > 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. We need to know if i.MX is shipping device trees stored in flash, or if they bundle it with the kernel. In the former case, you have to add quirks, in the latter case this patch is fine. Sascha, what does the Freescale maintainer say? Yours, Linus Walleij
On Tue, Oct 04, 2022 at 11:02:06AM +0200, Linus Walleij wrote: > On Wed, Sep 28, 2022 at 12:32 AM Dmitry Torokhov > <dmitry.torokhov@gmail.com> 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. > > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > I think the gods of Open Firmware will try to punish you for such > incompatible changes. But I have long since renounced them. > > > 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. > > We need to know if i.MX is shipping device trees stored in flash, > or if they bundle it with the kernel. This part is frequently found in add-on boards so it's not purely an i.MX-only question. > In the former case, you have to add quirks, in the latter case this > patch is fine. > > Sascha, what does the Freescale maintainer say? IMHO for not-in-the-soc devices like this the presence of in-kernel DTs isn't enough to make a decision. What is needed is a degree of due-diligence to show that there are no obvious out-of-kernel users. To be honest, I suspect the due-diligence checks will probably yield a green light for this one. Most of the tutorials for the popular HX8357 devices, show how to run python code in userspace that sends raw SPI commands. That sucks but at least it doesn't raise any concerns about bindings maintenance. Daniel.
On Tue, Oct 4, 2022 at 2:54 PM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > We need to know if i.MX is shipping device trees stored in flash, > > or if they bundle it with the kernel. > > This part is frequently found in add-on boards so it's not purely an > i.MX-only question. Oh > IMHO for not-in-the-soc devices like this the presence of in-kernel DTs > isn't enough to make a decision. What is needed is a degree of > due-diligence to show that there are no obvious out-of-kernel users. OK I think this is a good case to use a special quirk in this case. I actually envisioned keeping piling them up, and that they would not be innumerable. Dmitry, could you fix this? Just patch away in gpiolib-of.c. Yours, Linus Walleij
On Tue, Oct 04, 2022 at 09:50:25PM +0200, Linus Walleij wrote: > On Tue, Oct 4, 2022 at 2:54 PM Daniel Thompson > <daniel.thompson@linaro.org> wrote: > > > > We need to know if i.MX is shipping device trees stored in flash, > > > or if they bundle it with the kernel. > > > > This part is frequently found in add-on boards so it's not purely an > > i.MX-only question. > > Oh > > > IMHO for not-in-the-soc devices like this the presence of in-kernel DTs > > isn't enough to make a decision. What is needed is a degree of > > due-diligence to show that there are no obvious out-of-kernel users. > > OK I think this is a good case to use a special quirk in this case. > I actually envisioned keeping piling them up, and that they would > not be innumerable. > > Dmitry, could you fix this? Just patch away in gpiolib-of.c. Sure, I'll add a few quirks. I wonder what is the best way to merge this? I can create a bunch of IBs to be pulled, or I can send quirks to you/Bartosz and once they land send the patches to drivers... Thanks.
On Tue, Oct 4, 2022 at 10:35 PM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > Dmitry, could you fix this? Just patch away in gpiolib-of.c. > > Sure, I'll add a few quirks. I wonder what is the best way to merge > this? I can create a bunch of IBs to be pulled, or I can send quirks to > you/Bartosz and once they land send the patches to drivers... When I did it I was sufficiently convinced that I was the only one patching the quirks in gpiolib-of.c that merge window so I just included it as a hunk in the driver patch. If there will be some more patches to that file I guess some separate patch(es) for gpiolib-of.c is needed, maybe an immutable branch for those if it becomes a lot. Yours, Linus Walleij
On Thu, Oct 06, 2022 at 11:03:15AM +0200, Linus Walleij wrote: > On Tue, Oct 4, 2022 at 10:35 PM Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > > > Dmitry, could you fix this? Just patch away in gpiolib-of.c. > > > > Sure, I'll add a few quirks. I wonder what is the best way to merge > > this? I can create a bunch of IBs to be pulled, or I can send quirks to > > you/Bartosz and once they land send the patches to drivers... > > When I did it I was sufficiently convinced that I was the only one patching > the quirks in gpiolib-of.c that merge window so I just included it as > a hunk in the driver patch. If there will be some more patches to that > file I guess some separate patch(es) for gpiolib-of.c is needed, maybe > an immutable branch for those if it becomes a lot. Are renames likely to be a common quirk on the road to libgpiod conversion? I admit I sort of expected it to be common enough that there would be one rename quirk in the code supported by an alphabetized string table. Such a table would certainly still provoke troublesome merges but ones that are trivially resolved. Daniel.
On Thu, Oct 6, 2022 at 12:05 PM Daniel Thompson <daniel.thompson@linaro.org> wrote: > On Thu, Oct 06, 2022 at 11:03:15AM +0200, Linus Walleij wrote: > > On Tue, Oct 4, 2022 at 10:35 PM Dmitry Torokhov > > <dmitry.torokhov@gmail.com> wrote: > > > > > > Dmitry, could you fix this? Just patch away in gpiolib-of.c. > > > > > > Sure, I'll add a few quirks. I wonder what is the best way to merge > > > this? I can create a bunch of IBs to be pulled, or I can send quirks to > > > you/Bartosz and once they land send the patches to drivers... > > > > When I did it I was sufficiently convinced that I was the only one patching > > the quirks in gpiolib-of.c that merge window so I just included it as > > a hunk in the driver patch. If there will be some more patches to that > > file I guess some separate patch(es) for gpiolib-of.c is needed, maybe > > an immutable branch for those if it becomes a lot. > > Are renames likely to be a common quirk on the road to libgpiod > conversion? > > I admit I sort of expected it to be common enough that there would be > one rename quirk in the code supported by an alphabetized string table. > Such a table would certainly still provoke troublesome merges but ones > that are trivially resolved. Dmitry added a table of sorts, the problems are usually a bit unique for each instance of nonstandard DT GPIO bindings, that's why I mostly solved it with open coding in gpiolib-of.c. Yours, Linus Walleij
On Mon, Oct 10, 2022 at 10:36:00PM +0200, Linus Walleij wrote: > On Thu, Oct 6, 2022 at 12:05 PM Daniel Thompson > <daniel.thompson@linaro.org> wrote: > > On Thu, Oct 06, 2022 at 11:03:15AM +0200, Linus Walleij wrote: > > > On Tue, Oct 4, 2022 at 10:35 PM Dmitry Torokhov > > > <dmitry.torokhov@gmail.com> wrote: > > > > > > > > Dmitry, could you fix this? Just patch away in gpiolib-of.c. > > > > > > > > Sure, I'll add a few quirks. I wonder what is the best way to merge > > > > this? I can create a bunch of IBs to be pulled, or I can send quirks to > > > > you/Bartosz and once they land send the patches to drivers... > > > > > > When I did it I was sufficiently convinced that I was the only one patching > > > the quirks in gpiolib-of.c that merge window so I just included it as > > > a hunk in the driver patch. If there will be some more patches to that > > > file I guess some separate patch(es) for gpiolib-of.c is needed, maybe > > > an immutable branch for those if it becomes a lot. > > > > Are renames likely to be a common quirk on the road to libgpiod > > conversion? > > > > I admit I sort of expected it to be common enough that there would be > > one rename quirk in the code supported by an alphabetized string table. > > Such a table would certainly still provoke troublesome merges but ones > > that are trivially resolved. > > Dmitry added a table of sorts, the problems are usually a bit unique > for each instance of nonstandard DT GPIO bindings, that's why I > mostly solved it with open coding in gpiolib-of.c. OK, so I sent out the patch adding "reset-gpios" -> "gpios-reset" translation quirk to keep compatibility with the legacy names, but we still need to figure out what to do with incorrect line polarity in current DTses in tree. Unlike with names we have no indication if out of tree DTSes specify correct polarity or not, so we can't reasonably come up with workarounds that are guaranteed to work for everyone. I see several options: - the driver could continue ignoring line polarity specified in DTS (and use gpiod_set_value_raw()) and hope that they all/will be wired in the same way? - we could fix up in-kernel DTSes, allow flexibility of connection in new designs and respect polarity specified in out of tree DTSes, but accept that there can be breakages with old DTSes not contributed to the mainline or DTSes that were "cached" from an older kernel release - add another quirk forcing "active low" polarity for the legacy "gpios-reset" name, which will allow us respecting polarity in new "reset-gpios" name. Thanks.
On Wed, Oct 12, 2022 at 01:34:38PM -0700, Dmitry Torokhov wrote: > On Mon, Oct 10, 2022 at 10:36:00PM +0200, Linus Walleij wrote: > > On Thu, Oct 6, 2022 at 12:05 PM Daniel Thompson > > <daniel.thompson@linaro.org> wrote: > > > On Thu, Oct 06, 2022 at 11:03:15AM +0200, Linus Walleij wrote: > > > > On Tue, Oct 4, 2022 at 10:35 PM Dmitry Torokhov > > > > <dmitry.torokhov@gmail.com> wrote: > > > > > > > > > > Dmitry, could you fix this? Just patch away in gpiolib-of.c. > > > > > > > > > > Sure, I'll add a few quirks. I wonder what is the best way to merge > > > > > this? I can create a bunch of IBs to be pulled, or I can send quirks to > > > > > you/Bartosz and once they land send the patches to drivers... > > > > > > > > When I did it I was sufficiently convinced that I was the only one patching > > > > the quirks in gpiolib-of.c that merge window so I just included it as > > > > a hunk in the driver patch. If there will be some more patches to that > > > > file I guess some separate patch(es) for gpiolib-of.c is needed, maybe > > > > an immutable branch for those if it becomes a lot. > > > > > > Are renames likely to be a common quirk on the road to libgpiod > > > conversion? > > > > > > I admit I sort of expected it to be common enough that there would be > > > one rename quirk in the code supported by an alphabetized string table. > > > Such a table would certainly still provoke troublesome merges but ones > > > that are trivially resolved. > > > > Dmitry added a table of sorts, the problems are usually a bit unique > > for each instance of nonstandard DT GPIO bindings, that's why I > > mostly solved it with open coding in gpiolib-of.c. > > OK, so I sent out the patch adding "reset-gpios" -> "gpios-reset" > translation quirk to keep compatibility with the legacy names, but > we still need to figure out what to do with incorrect line polarity > in current DTses in tree. Unlike with names we have no indication > if out of tree DTSes specify correct polarity or not, so we can't > reasonably come up with workarounds that are guaranteed to work for > everyone. I see several options: > > 1 the driver could continue ignoring line polarity specified in DTS > (and use gpiod_set_value_raw()) and hope that they all/will be > wired in the same way? > > 2 we could fix up in-kernel DTSes, allow flexibility of connection > in new designs and respect polarity specified in out of tree DTSes, > but accept that there can be breakages with old DTSes not contributed > to the mainline or DTSes that were "cached" from an older kernel > release > > 3 add another quirk forcing "active low" polarity for the legacy > "gpios-reset" name, which will allow us respecting polarity in > new "reset-gpios" name. I don't think they is any point in having a rename if legacy DTs break anyway! Thus if there is a rename then there should also be an active low quirk applied when the old name is used. Daniel.
diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts index 9ef0d567ea48..ae51a2aa2028 100644 --- a/arch/arm/boot/dts/imx28-cfa10049.dts +++ b/arch/arm/boot/dts/imx28-cfa10049.dts @@ -3,6 +3,7 @@ * Copyright 2012 Free Electrons */ +#include <dt-bindings/gpio/gpio.h> /* * The CFA-10049 is an expansion board for the CFA-10036 module, thus we * need to include the CFA-10036 DTS. @@ -346,8 +347,10 @@ hx8357: hx8357@0 { spi-max-frequency = <100000>; spi-cpol; spi-cpha; - gpios-reset = <&gpio3 30 0>; - im-gpios = <&gpio5 4 0 &gpio5 5 0 &gpio5 6 0>; + reset-gpios = <&gpio3 30 GPIO_ACTIVE_LOW>; + im-gpios = <&gpio5 4 GPIO_ACTIVE_HIGH + &gpio5 5 GPIO_ACTIVE_HIGH + &gpio5 6 GPIO_ACTIVE_HIGH>; }; }; diff --git a/arch/arm/boot/dts/imx28-cfa10055.dts b/arch/arm/boot/dts/imx28-cfa10055.dts index fac5bbda7a93..70e4dc67f7d2 100644 --- a/arch/arm/boot/dts/imx28-cfa10055.dts +++ b/arch/arm/boot/dts/imx28-cfa10055.dts @@ -4,6 +4,7 @@ * Free Electrons */ +#include <dt-bindings/gpio/gpio.h> /* * The CFA-10055 is an expansion board for the CFA-10036 module and * CFA-10037, thus we need to include the CFA-10037 DTS. @@ -148,7 +149,7 @@ hx8357: hx8357@0 { spi-max-frequency = <100000>; spi-cpol; spi-cpha; - gpios-reset = <&gpio3 30 0>; + reset-gpios = <&gpio3 30 GPIO_ACTIVE_LOW>; }; }; diff --git a/arch/arm/boot/dts/imx28-cfa10056.dts b/arch/arm/boot/dts/imx28-cfa10056.dts index c5f3337e8b39..687eaa555a15 100644 --- a/arch/arm/boot/dts/imx28-cfa10056.dts +++ b/arch/arm/boot/dts/imx28-cfa10056.dts @@ -3,6 +3,7 @@ * Copyright 2013 Free Electrons */ +#include <dt-bindings/gpio/gpio.h> /* * The CFA-10055 is an expansion board for the CFA-10036 module and * CFA-10037, thus we need to include the CFA-10037 DTS. @@ -107,7 +108,7 @@ hx8369: hx8369@0 { spi-max-frequency = <100000>; spi-cpol; spi-cpha; - gpios-reset = <&gpio3 30 0>; + reset-gpios = <&gpio3 30 GPIO_ACTIVE_LOW>; }; }; }; 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;
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. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- 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. arch/arm/boot/dts/imx28-cfa10049.dts | 7 +++++-- arch/arm/boot/dts/imx28-cfa10055.dts | 3 ++- arch/arm/boot/dts/imx28-cfa10056.dts | 3 ++- drivers/video/backlight/hx8357.c | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-)