Re: [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports
From: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Date: 2021-01-27 11:38:11
Also in:
lkml
Hello Nikita! On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin wrote:
Replace boolean values used for determining if gpiochip is IRQ capable or not with index.
What's the purpose of this patch? It neither makes the code more effective nor does it make the code more readable IMO. There are also some changes in the code which were not mentioned in the commit message.
quoted hunk ↗ jump to hunk
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> --- drivers/gpio/gpio-ep93xx.c | 47 ++++++++++++++++++++---------------- -- 1 file changed, 25 insertions(+), 22 deletions(-)diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c index 2aee13b5067d..f75a33b79504 100644 --- a/drivers/gpio/gpio-ep93xx.c +++ b/drivers/gpio/gpio-ep93xx.c@@ -44,6 +44,8 @@ struct ep93xx_gpio {struct irq_chip ic[EP93XX_GPIO_IRQ_CHIPS_NUM]; }; +#define EP93XX_GPIO_A_PORT_INDEX 0 +#define EP93XX_GPIO_B_PORT_INDEX 1 /* * F Port index in GPIOCHIP'S array is 5 * but we use index 2 for stored values and offsets@@ -291,38 +293,36 @@ static struct irq_chip ep93xx_gpio_irq_chip = {* gpiolib interface for EP93xx on-chip GPIOs ********************************************************************* ****/ struct ep93xx_gpio_bank { + u8 idx; const char *label; int data; int dir; int base; - bool has_irq; - bool has_hierarchical_irq; unsigned int irq_base; }; -#define EP93XX_GPIO_BANK(_label, _data, _dir, _base, _has_irq, _has_hier, _irq_base) \ +#define EP93XX_GPIO_BANK(_index, _label, _data, _dir, _base, _irq_base) \ { \ + .idx = _index, \ .label = _label, \ .data = _data, \ .dir = _dir, \ .base = _base, \ - .has_irq = _has_irq, \ - .has_hierarchical_irq = _has_hier, \ .irq_base = _irq_base, \ } static struct ep93xx_gpio_bank ep93xx_gpio_banks[] = { /* Bank A has 8 IRQs */ - EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false, EP93XX_GPIO_A_IRQ_BASE), + EP93XX_GPIO_BANK(0, "A", 0x00, 0x10, 0, EP93XX_GPIO_A_IRQ_BASE), /* Bank B has 8 IRQs */ - EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true, false, EP93XX_GPIO_B_IRQ_BASE), - EP93XX_GPIO_BANK("C", 0x08, 0x18, 40, false, false, 0), - EP93XX_GPIO_BANK("D", 0x0c, 0x1c, 24, false, false, 0), - EP93XX_GPIO_BANK("E", 0x20, 0x24, 32, false, false, 0), + EP93XX_GPIO_BANK(1, "B", 0x04, 0x14, 8, EP93XX_GPIO_B_IRQ_BASE), + EP93XX_GPIO_BANK(2, "C", 0x08, 0x18, 40, 0), + EP93XX_GPIO_BANK(3, "D", 0x0c, 0x1c, 24, 0), + EP93XX_GPIO_BANK(4, "E", 0x20, 0x24, 32, 0), /* Bank F has 8 IRQs */ - EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, false, true, EP93XX_GPIO_F_IRQ_BASE), - EP93XX_GPIO_BANK("G", 0x38, 0x3c, 48, false, false, 0), - EP93XX_GPIO_BANK("H", 0x40, 0x44, 56, false, false, 0), + EP93XX_GPIO_BANK(5, "F", 0x30, 0x34, 16, EP93XX_GPIO_F_IRQ_BASE), + EP93XX_GPIO_BANK(6, "G", 0x38, 0x3c, 48, 0), + EP93XX_GPIO_BANK(7, "H", 0x40, 0x44, 56, 0), }; static int ep93xx_gpio_set_config(struct gpio_chip *gc, unsigned offset,@@ -356,10 +356,11 @@ static void ep93xx_init_irq_chips(structep93xx_gpio *epg) } static int ep93xx_gpio_add_ab_irq_chip(struct platform_device *pdev, - struct gpio_irq_chip *girq, + struct gpio_chip *gc, unsigned int irq_base) { struct device *dev = &pdev->dev; + struct gpio_irq_chip *girq = &gc->irq; int ab_parent_irq = platform_get_irq(pdev, 0); girq->parent_handler = ep93xx_gpio_ab_irq_handler;@@ -378,12 +379,13 @@ static int ep93xx_gpio_add_ab_irq_chip(structplatform_device *pdev, } static int ep93xx_gpio_add_f_irq_chip(struct platform_device *pdev, - struct gpio_irq_chip *girq, + struct gpio_chip *gc, unsigned int irq_base) { int gpio_irq; int i; struct device *dev = &pdev->dev; + struct gpio_irq_chip *girq = &gc->irq; /* * FIXME: convert this to use hierarchical IRQ support!@@ -397,10 +399,10 @@ static int ep93xx_gpio_add_f_irq_chip(structplatform_device *pdev, if (!girq->parents) return -ENOMEM; /* Pick resources 1..8 for these IRQs */ - for (i = 0; i < ARRAY_SIZE(girq->parents); i++) { + for (i = 0; i < girq->num_parents; i++) { girq->parents[i] = platform_get_irq(pdev, i + 1); gpio_irq = irq_base + i; - irq_set_chip_data(gpio_irq, &epg->gc[5]); + irq_set_chip_data(gpio_irq, gc); irq_set_chip_and_handler(gpio_irq, &ep93xx_gpio_irq_chip, handle_level_irq);@@ -433,21 +435,22 @@ static int ep93xx_gpio_add_bank(structgpio_chip *gc, gc->base = bank->base; girq = &gc->irq; - if (bank->has_irq || bank->has_hierarchical_irq) { + if (bank->irq_base != 0) { gc->set_config = ep93xx_gpio_set_config; port = ep93xx_gpio_port(epg, gc); girq->chip = &epg->ic[port]; } - if (bank->has_irq) { - err = ep93xx_gpio_add_ab_irq_chip(pdev, girq, bank-quoted
irq_base);+ if (bank->idx == EP93XX_GPIO_A_PORT_INDEX || + bank->idx == EP93XX_GPIO_B_PORT_INDEX) { + err = ep93xx_gpio_add_ab_irq_chip(pdev, gc, bank-quoted
irq_base);if (err) return err; } /* Only bank F has especially funky IRQ handling */ - if (bank->has_hierarchical_irq) { - err = ep93xx_gpio_add_f_irq_chip(pdev, girq, bank-quoted
irq_base);+ if (bank->idx == EP93XX_GPIO_F_PORT_INDEX) { + err = ep93xx_gpio_add_f_irq_chip(pdev, gc, bank-quoted
irq_base);if (err) return err; }
-- Alexander Sverdlin.