[PATCH] gpio-aspeed: Write value on switch to output

[Joel Stanley]

On Fri, Aug 5, 2016 at 9:53 PM, Andrew Jeffery <andrew at aj.id.au> wrote:
> [0] notes that the Barreleye beep LED turned off during boot despite
> configuring the default-state DT property to "on". The documentation for the
> direction_output() callback in struct gpio_chip doesn't make any mention of
> setting values[1] but the function prototype takes a value as a formal
> parameter. Additionally, other drivers' callback implementations (e.g. the
> gemini GPIO driver) write out the provided value.
>
> Make the Aspeed GPIO driver implementation consistent with the expected
> behaviour.
>
> [0] https://github.com/openbmc/linux/issues/98
> [1] "@direction_output: configures signal "offset" as output, or returns error"

Thanks. I applied the version that you intended to send to my
development tree. I think we want to push this out to dev-4.4 for the
barreleye stable branch too?

Cheers,

Joel

>
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
>  drivers/gpio/gpio-aspeed.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index 2ecaa2b12759..7817b98fe8cd 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -120,23 +120,29 @@ static int aspeed_gpio_get(struct gpio_chip *gc, unsigned int offset)
>                         & GPIO_BIT(offset));
>  }
>
> -static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
> -                           int val)
> +static void __aspeed_gpio_set(void __iomem *addr, unsigned int offset, int val)
>  {
> -       struct aspeed_gpio *gpio = to_aspeed_gpio(gc);
> -       const struct aspeed_gpio_bank *bank = to_bank(offset);
> -       unsigned long flags;
>         u32 reg;
>
> -       spin_lock_irqsave(&gpio->lock, flags);
> -
> -       reg = ioread32(bank_val_reg(gpio, bank, GPIO_DATA));
> +       reg = ioread32(addr);
>         if (val)
>                 reg |= GPIO_BIT(offset);
>         else
>                 reg &= ~GPIO_BIT(offset);
>
> -       iowrite32(reg, bank_val_reg(gpio, bank, GPIO_DATA));
> +       iowrite32(reg, addr);
> +}
> +
> +static void aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset,
> +                           int val)
> +{
> +       struct aspeed_gpio *gpio = to_aspeed_gpio(gc);
> +       const struct aspeed_gpio_bank *bank = to_bank(offset);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&gpio->lock, flags);
> +
> +       __aspeed_gpio_set(bank_val_reg(gpio, bank, GPIO_DATA), offset, val);
>
>         spin_unlock_irqrestore(&gpio->lock, flags);
>  }
> @@ -163,13 +169,16 @@ static int aspeed_gpio_dir_out(struct gpio_chip *gc,
>  {
>         struct aspeed_gpio *gpio = to_aspeed_gpio(gc);
>         const struct aspeed_gpio_bank *bank = to_bank(offset);
> +       void __iomem *addr = bank_val_reg(gpio, bank, GPIO_DIR);
>         unsigned long flags;
>         u32 reg;
>
>         spin_lock_irqsave(&gpio->lock, flags);
>
> -       reg = ioread32(bank_val_reg(gpio, bank, GPIO_DIR));
> -       iowrite32(reg | GPIO_BIT(offset), bank_val_reg(gpio, bank, GPIO_DIR));
> +       reg = ioread32(addr);
> +       iowrite32(reg | GPIO_BIT(offset), addr);
> +
> +       __aspeed_gpio_set(addr, offset, val);
>
>         spin_unlock_irqrestore(&gpio->lock, flags);
>
> --
> 2.9.2.597.g66765a4
>