| Message ID | 20190904061245.30770-3-rashmica.g@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] gpio/aspeed: Fix incorrect number of banks | expand |
On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com> wrote: > > The ast2600 has two gpio controllers, one for 3.6V GPIOS and one for 1.8V GPIOS. > > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com> > - for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) { > + banks = (gpio->config->nr_gpios >> 5) + 1; Same comment as per the other patch. > + for (i = 0; i < banks; i++) { > +static const struct aspeed_bank_props ast2600_bank_props[] = { > + /* input output */ > + {5, 0xffffffff, 0x0000ffff}, /* U/V/W/X */ > + {6, 0xffff0000, 0x0fff0000}, /* Y/Z */ Perhaps GENMASK() for all values? > + { }, Comma is not needed here. > +}; > + > +static const struct aspeed_gpio_config ast2600_config = > + /* 208 3.6V GPIOs */ > + { .nr_gpios = 208, .props = ast2600_bank_props, }; Seems curly braces missed their places. > +static const struct aspeed_bank_props ast2600_1_8v_bank_props[] = { > + /* input output */ > + {1, 0x0000000f, 0x0000000f}, /* E */ GENMASK()? > + { }, No comma. > +}; > +static const struct aspeed_gpio_config ast2600_1_8v_config = > + /* 36 1.8V GPIOs */ > + { .nr_gpios = 36, .props = ast2600_1_8v_bank_props, }; Location of the curly braces?
On Wed, 2019-09-04 at 19:30 +0300, Andy Shevchenko wrote: > On Wed, Sep 4, 2019 at 9:14 AM Rashmica Gupta <rashmica.g@gmail.com> > wrote: > > The ast2600 has two gpio controllers, one for 3.6V GPIOS and one > > for 1.8V GPIOS. > > > > Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com> > > - for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) { > > + banks = (gpio->config->nr_gpios >> 5) + 1; > > Same comment as per the other patch. > > > + for (i = 0; i < banks; i++) { > > +static const struct aspeed_bank_props ast2600_bank_props[] = { > > + /* input output */ > > + {5, 0xffffffff, 0x0000ffff}, /* U/V/W/X */ > > + {6, 0xffff0000, 0x0fff0000}, /* Y/Z */ > > Perhaps GENMASK() for all values? Perhaps this and your other comments below would be best addressed in an additional cleanup patch? This patch follows the formatting of the existing code and it's not very clean to differ from that or to change the formatting of the current code in this patch. > > > + { }, > > Comma is not needed here. > > > +}; > > + > > +static const struct aspeed_gpio_config ast2600_config = > > + /* 208 3.6V GPIOs */ > > + { .nr_gpios = 208, .props = ast2600_bank_props, }; > > Seems curly braces missed their places. > > > +static const struct aspeed_bank_props ast2600_1_8v_bank_props[] = > > { > > + /* input output */ > > + {1, 0x0000000f, 0x0000000f}, /* E */ > > GENMASK()? > > > + { }, > > No comma. > > > +}; > > +static const struct aspeed_gpio_config ast2600_1_8v_config = > > + /* 36 1.8V GPIOs */ > > + { .nr_gpios = 36, .props = ast2600_1_8v_bank_props, }; > > Location of the curly braces? >
On Thu, Sep 5, 2019 at 2:34 AM Rashmica Gupta <rashmica.g@gmail.com> wrote:> > On Wed, 2019-09-04 at 19:30 +0300, Andy Shevchenko wrote: > Perhaps this and your other comments below would be best addressed in > an additional cleanup patch? This patch follows the formatting of the > existing code and it's not very clean to differ from that or to change > the formatting of the current code in this patch. OK.
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c index 60ab042c9129..98881c99d0b9 100644 --- a/drivers/gpio/gpio-aspeed.c +++ b/drivers/gpio/gpio-aspeed.c @@ -662,12 +662,14 @@ static void aspeed_gpio_irq_handler(struct irq_desc *desc) struct gpio_chip *gc = irq_desc_get_handler_data(desc); struct irq_chip *ic = irq_desc_get_chip(desc); struct aspeed_gpio *data = gpiochip_get_data(gc); - unsigned int i, p, girq; + unsigned int i, p, girq, banks; unsigned long reg; + struct aspeed_gpio *gpio = gpiochip_get_data(gc); chained_irq_enter(ic, desc); - for (i = 0; i < ARRAY_SIZE(aspeed_gpio_banks); i++) { + banks = (gpio->config->nr_gpios >> 5) + 1; + for (i = 0; i < banks; i++) { const struct aspeed_gpio_bank *bank = &aspeed_gpio_banks[i]; reg = ioread32(bank_reg(data, bank, reg_irq_status)); @@ -1108,9 +1110,32 @@ static const struct aspeed_gpio_config ast2500_config = /* 232 for simplicity, actual number is 228 (4-GPIO hole in GPIOAB) */ { .nr_gpios = 232, .props = ast2500_bank_props, }; +static const struct aspeed_bank_props ast2600_bank_props[] = { + /* input output */ + {5, 0xffffffff, 0x0000ffff}, /* U/V/W/X */ + {6, 0xffff0000, 0x0fff0000}, /* Y/Z */ + { }, +}; + +static const struct aspeed_gpio_config ast2600_config = + /* 208 3.6V GPIOs */ + { .nr_gpios = 208, .props = ast2600_bank_props, }; + +static const struct aspeed_bank_props ast2600_1_8v_bank_props[] = { + /* input output */ + {1, 0x0000000f, 0x0000000f}, /* E */ + { }, +}; + +static const struct aspeed_gpio_config ast2600_1_8v_config = + /* 36 1.8V GPIOs */ + { .nr_gpios = 36, .props = ast2600_1_8v_bank_props, }; + static const struct of_device_id aspeed_gpio_of_table[] = { { .compatible = "aspeed,ast2400-gpio", .data = &ast2400_config, }, { .compatible = "aspeed,ast2500-gpio", .data = &ast2500_config, }, + { .compatible = "aspeed,ast2600-gpio", .data = &ast2600_config, }, + { .compatible = "aspeed,ast2600-1-8v-gpio", .data = &ast2600_1_8v_config,}, {} }; MODULE_DEVICE_TABLE(of, aspeed_gpio_of_table);
The ast2600 has two gpio controllers, one for 3.6V GPIOS and one for 1.8V GPIOS. Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com> --- drivers/gpio/gpio-aspeed.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)