| Message ID | 20180815201855.29738-1-mail@david-bauer.net |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | gpio: 74x164: add lines-initial-states property | expand |
Hi David, On Wed, Aug 15, 2018 at 10:19 PM David Bauer <mail@david-bauer.net> wrote: > This adds the ability to define the initial state of each output line on > device probe. > > Signed-off-by: David Bauer <mail@david-bauer.net> (...) > Optional properties: > - enable-gpios: GPIO connected to the OE (Output Enable) pin. > +- lines-initial-states: Bitmask that specifies the initial state of > + each line. When a bit is set to zero, the corresponding output line > + is initialized LOW. When a bit is set to one, the corresponding > + output line is initialized HIGH. In case this property is not > + defined, all lines will be initialized as LOW. This sounds like something that should be generic, and not use a bitmask, but offsets. It should work even if the number of GPIOs from the chip is > 32. Is the usecase different from hogs? See Documentation/devicetree/bindings/gpio.txt There has been extensive discussion about supporting initial values with something similar to hogs, but I haven't got anything ACKed by the DT maintainers so it has kind of stalled. I would make sure to both make it generic, get ACK from the DT mainatiners, and make sure to implement it in drivers/gpio/gpiolib-of.c and not locally in drivers. Yours, Linus Walleij
Hi, this email is from Rob's (experimental) review bot. I found a couple of common problems with your patch. Please see below. On Wed, 15 Aug 2018 22:18:54 +0200, David Bauer wrote: > This adds the ability to define the initial state of each output line on > device probe. > > Signed-off-by: David Bauer <mail@david-bauer.net> The preferred subject prefix is "dt-bindings: <binding dir>: ...". > --- > Documentation/devicetree/bindings/gpio/gpio-74x164.txt | 5 +++++ > drivers/gpio/gpio-74x164.c | 3 +++ > 2 files changed, 8 insertions(+) > DT bindings (including binding headers) should be a separate patch. See Documentation/devicetree/bindings/submitting-patches.txt.
Hi Linus, On 8/16/18 10:11 AM, Linus Walleij wrote: > This sounds like something that should be generic, and not use > a bitmask, but offsets. It should work even if the number of > GPIOs from the chip is > 32. > > Is the usecase different from hogs? > See Documentation/devicetree/bindings/gpio.txt Thanks for pointing that out. Indeed for my use-case (Asserting single output to be high on driver probe) hogs are are sufficient solution. Best wishes David
diff --git a/Documentation/devicetree/bindings/gpio/gpio-74x164.txt b/Documentation/devicetree/bindings/gpio/gpio-74x164.txt index 2a97553d8d76..580b18065ad3 100644 --- a/Documentation/devicetree/bindings/gpio/gpio-74x164.txt +++ b/Documentation/devicetree/bindings/gpio/gpio-74x164.txt @@ -14,6 +14,11 @@ Required properties: Optional properties: - enable-gpios: GPIO connected to the OE (Output Enable) pin. +- lines-initial-states: Bitmask that specifies the initial state of + each line. When a bit is set to zero, the corresponding output line + is initialized LOW. When a bit is set to one, the corresponding + output line is initialized HIGH. In case this property is not + defined, all lines will be initialized as LOW. Example: diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c index fb7b620763a2..275310a0a538 100644 --- a/drivers/gpio/gpio-74x164.c +++ b/drivers/gpio/gpio-74x164.c @@ -150,6 +150,9 @@ static int gen_74x164_probe(struct spi_device *spi) chip->gpio_chip.parent = &spi->dev; chip->gpio_chip.owner = THIS_MODULE; + of_property_read_u8_array(spi->dev.of_node, "lines-initial-states", + chip->buffer, chip->registers); + mutex_init(&chip->lock); ret = __gen_74x164_write_config(chip);
This adds the ability to define the initial state of each output line on device probe. Signed-off-by: David Bauer <mail@david-bauer.net> --- Documentation/devicetree/bindings/gpio/gpio-74x164.txt | 5 +++++ drivers/gpio/gpio-74x164.c | 3 +++ 2 files changed, 8 insertions(+)