[PATCH 1/2] gpio: aspeed: Make names two characters

[PATCH 1/2] gpio: aspeed: Make names two characters

[Joel Stanley]

The ast2500 has more GPIOs than can be represented with A-Z, so they use
AA and BB. Mak the names strings to accommodate this.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/gpio/gpio-aspeed.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index e48bd1ea82d1..d7198536de13 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -30,39 +30,39 @@ struct aspeed_gpio {
 struct aspeed_gpio_bank {
 	uint16_t	val_regs;
 	uint16_t	irq_regs;
-	const char	names[4];
+	const char	names[4][2];
 };
 
 static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
 	{
 		.val_regs = 0x0000,
 		.irq_regs = 0x0008,
-		.names = { 'A', 'B', 'C', 'D' },
+		.names = { "A", "B", "C", "D" },
 	},
 	{
 		.val_regs = 0x0020,
 		.irq_regs = 0x0028,
-		.names = { 'E', 'F', 'G', 'H' },
+		.names = { "E", "F", "G", "H" },
 	},
 	{
 		.val_regs = 0x0070,
 		.irq_regs = 0x0098,
-		.names = { 'I', 'J', 'K', 'L' },
+		.names = { "I", "J", "K", "L" },
 	},
 	{
 		.val_regs = 0x0078,
 		.irq_regs = 0x00e8,
-		.names = { 'M', 'N', 'O', 'P' },
+		.names = { "M", "N", "O", "P" },
 	},
 	{
 		.val_regs = 0x0080,
 		.irq_regs = 0x0118,
-		.names = { 'Q', 'R', 'S', 'T' },
+		.names = { "Q", "R", "S", "T" },
 	},
 	{
 		.val_regs = 0x0088,
 		.irq_regs = 0x0148,
-		.names = { 'U', 'V', 'W', 'X' },
+		.names = { "U", "V", "W", "X" },
 	},
 	/*
 	 * A bank exists for { 'Y', 'Z', "AA", "AB" }, but is not implemented.
-- 
2.11.0

[PATCH 2/2] gpio: aspeed: Support banks Y, Z, AA and BB

[Joel Stanley]

These banks have extra limitations compared to those currently
implemented. We must detect when these unique banks are being used and
special case their IRQ and input state.

TODO: Do what it says on the label

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/gpio/gpio-aspeed.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index d7198536de13..c4d87a2a489f 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -65,10 +65,14 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
 		.names = { "U", "V", "W", "X" },
 	},
 	/*
-	 * A bank exists for { 'Y', 'Z', "AA", "AB" }, but is not implemented.
 	 * Only half of GPIOs Y support interrupt configuration, and none of Z,
 	 * AA or AB do as they are output only.
 	 */
+	{
+		.val_regs = 0x01E0,
+		.irq_regs = 0x0178,
+		.names = { "Y", "Z", "AA", "AB" },
+	},
 };
 
 #define GPIO_BANK(x)	((x) >> 5)
@@ -129,6 +133,9 @@ static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, int val
 
 	addr = bank_val_reg(gpio, bank, GPIO_DATA);
 
+	/* TODO: detect when we're dealing with GPIOs that cannot be set as
+	 * inputs */
+
 	reg = ioread32(addr);
 	if (val)
 		reg |= GPIO_BIT(offset);
@@ -158,6 +165,9 @@ static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
 	unsigned long flags;
 	u32 reg;
 
+	/* TODO: detect when we're dealing with GPIOs that cannot be set as
+	 * inputs */
+
 	spin_lock_irqsave(&gpio->lock, flags);
 
 	reg = ioread32(bank_val_reg(gpio, bank, GPIO_DIR));
@@ -233,6 +243,9 @@ static void __aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
 	void __iomem *addr;
 	int rc;
 
+	/* TODO: Detect when a bank cannot be used as an IRQ source and return
+	 * error */
+
 	rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit);
 	if (rc)
 		return;
-- 
2.11.0

Re: [PATCH 2/2] gpio: aspeed: Support banks Y, Z, AA and BB

[Xo Wang]

Hi Joel,

On Sun, Dec 11, 2016 at 8:06 PM, Joel Stanley <joel@jms.id.au> wrote:
> These banks have extra limitations compared to those currently
> implemented. We must detect when these unique banks are being used and
> special case their IRQ and input state.
>
> TODO: Do what it says on the label
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  drivers/gpio/gpio-aspeed.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> index d7198536de13..c4d87a2a489f 100644
> --- a/drivers/gpio/gpio-aspeed.c
> +++ b/drivers/gpio/gpio-aspeed.c
> @@ -65,10 +65,14 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
>                 .names = { "U", "V", "W", "X" },
>         },
>         /*
> -        * A bank exists for { 'Y', 'Z', "AA", "AB" }, but is not implemented.
>          * Only half of GPIOs Y support interrupt configuration, and none of Z,
>          * AA or AB do as they are output only.

Is this comment accurate? I don't see anything in the AST2500
datasheet about GPIOs Z, AA, and AB being output only.

>          */
> +       {
> +               .val_regs = 0x01E0,
> +               .irq_regs = 0x0178,
> +               .names = { "Y", "Z", "AA", "AB" },
> +       },
>  };
>
>  #define GPIO_BANK(x)   ((x) >> 5)
> @@ -129,6 +133,9 @@ static void __aspeed_gpio_set(struct gpio_chip *gc, unsigned int offset, int val
>
>         addr = bank_val_reg(gpio, bank, GPIO_DATA);
>
> +       /* TODO: detect when we're dealing with GPIOs that cannot be set as
> +        * inputs */
> +
>         reg = ioread32(addr);
>         if (val)
>                 reg |= GPIO_BIT(offset);
> @@ -158,6 +165,9 @@ static int aspeed_gpio_dir_in(struct gpio_chip *gc, unsigned int offset)
>         unsigned long flags;
>         u32 reg;
>
> +       /* TODO: detect when we're dealing with GPIOs that cannot be set as
> +        * inputs */
> +
>         spin_lock_irqsave(&gpio->lock, flags);
>
>         reg = ioread32(bank_val_reg(gpio, bank, GPIO_DIR));
> @@ -233,6 +243,9 @@ static void __aspeed_gpio_irq_set_mask(struct irq_data *d, bool set)
>         void __iomem *addr;
>         int rc;
>
> +       /* TODO: Detect when a bank cannot be used as an IRQ source and return
> +        * error */
> +
>         rc = irqd_to_aspeed_gpio_data(d, &gpio, &bank, &bit);
>         if (rc)
>                 return;
> --
> 2.11.0
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

Do you have more information or documentation about the limitations of
those GPIOs?

thanks
xo

Re: [PATCH 2/2] gpio: aspeed: Support banks Y, Z, AA and BB

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

Hi Xo,

On Mon, 2017-01-09 at 16:24 -0800, Xo Wang wrote:
> Hi Joel,
> 
> > On Sun, Dec 11, 2016 at 8:06 PM, Joel Stanley <joel@jms.id.au> wrote:
> > These banks have extra limitations compared to those currently
> > implemented. We must detect when these unique banks are being used and
> > special case their IRQ and input state.
> >
> > TODO: Do what it says on the label
> >
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >  drivers/gpio/gpio-aspeed.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
> > index d7198536de13..c4d87a2a489f 100644
> > --- a/drivers/gpio/gpio-aspeed.c
> > +++ b/drivers/gpio/gpio-aspeed.c
> > @@ -65,10 +65,14 @@ static const struct aspeed_gpio_bank aspeed_gpio_banks[] = {
> >                 .names = { "U", "V", "W", "X" },
> >         },
> >         /*
> > -        * A bank exists for { 'Y', 'Z', "AA", "AB" }, but is not implemented.
> >          * Only half of GPIOs Y support interrupt configuration, and none of Z,
> >          * AA or AB do as they are output only.
> 
> Is this comment accurate? I don't see anything in the AST2500
> datasheet about GPIOs Z, AA, and AB being output only.

These banks are output-only on the AST2400. The comment is not accurate
for the 2500 as far as I can see. Again this was a patch that needed
some improvement, we just haven't got around to cleaning it up.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]