On Tue, May 16, 2017 at 5:31 PM, Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> The Arizona devices only maintain the state of output GPIOs whilst the
> CODEC is active, this can cause issues if the CODEC suspends whilst
> something is relying on the state of one of its GPIOs. However, in
> many systems the CODEC GPIOs are used for audio related features
> and thus the state of the GPIOs is unimportant whilst the CODEC is
> suspended. Often keeping the CODEC resumed in such a system would
> incur a power impact that is unacceptable.
>
> Allow the user to select whether a GPIO output should keep the
> CODEC resumed, by adding a flag through the second cell of the GPIO
> specifier in device tree.
>
> Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
This adds the handling locally in the driver for a certain Arizona chip.
And then the next time you have to duplicate the code again and again.
That's not working, put this in the core.
> static int arizona_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
> {
> struct arizona_gpio *arizona_gpio = gpiochip_get_data(chip);
> struct arizona *arizona = arizona_gpio->arizona;
> + int status = arizona_gpio->status[offset];
Don't use a local array for this.
This should be something like gpiochip_get_sleep_persistance(chip, offset)
> + if (change && !(status & GPIO_SLEEP_MAY_LOOSE_VALUE)) {
That us a dt-binfings header define, that is not a Linux constant.
Add FLAG_MAY_LOOSE_VALUE_DURING_SLEEP or something in
drivers/gpio/gpiolib.h, store this inside the struct gpio_desc like all
other flags and retrieve it from there.
Augment the code in gpiolib.c and gpiolib-of.c to parse this and
store it properly in the gpio_desc and use it as described above.
Then it is reusable for others.
> +#ifdef CONFIG_OF_GPIO
> +static int arizona_gpio_of_xlate(struct gpio_chip *chip,
> + const struct of_phandle_args *gpiospec,
> + u32 *flags)
> +{
> + struct arizona_gpio *arizona_gpio = gpiochip_get_data(chip);
> + int offset;
> +
> + offset = of_gpio_simple_xlate(chip, gpiospec, flags);
> + if (offset < 0)
> + return offset;
> +
> + if (flags)
> + arizona_gpio->status[offset] = *flags;
> +
> + return offset;
> +}
> +#endif
No need for this hackery if you move the parsing to the core, so do that.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
