gpio/aspeed-sgpio: enable access to all 80 input & output sgpios

Currently, the IRQ setup for the SGPIO driver enables all interrupts in dual-edge trigger mode. Since the default handler is handle_bad_irq, any state change on input GPIOs will trigger bad IRQ warnings.

This change applies sensible IRQ defaults: single-edge trigger, and all IRQs disabled.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

---
v2:
- update comments to reflect trigger mode change
- tweak wording to reflect disabled state & single-edge trigger mode

---
 drivers/gpio/gpio-aspeed-sgpio.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

drivers/gpio/gpio-aspeed-sgpio.c +3 -6

index a18ca52432e0..64e54f8c30d2 100644
@@ -452,17 +452,15 @@ static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
        irq->parents = &gpio->irq;
        irq->num_parents = 1;
 
-       /* set IRQ settings and Enable Interrupt */
+       /* Apply default IRQ settings */
        for (i = 0; i < ARRAY_SIZE(aspeed_sgpio_banks); i++) {
                bank = &aspeed_sgpio_banks[i];
                /* set falling or level-low irq */
                iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type0));
                /* trigger type is edge */
                iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type1));
-               /* dual edge trigger mode. */
-               iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_type2));
-               /* enable irq */
-               iowrite32(0xffffffff, bank_reg(gpio, bank, reg_irq_enable));
+               /* single edge trigger */
+               iowrite32(0x00000000, bank_reg(gpio, bank, reg_irq_type2));
        }
 
        return 0;
--

2.28.0

Currently, the IRQ setup for the SGPIO driver enables all interrupts in dual-edge trigger mode. Since the default handler is handle_bad_irq, any state change on input GPIOs will trigger bad IRQ warnings.

This change applies sensible IRQ defaults: single-edge trigger, and all IRQs disabled.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

---
v2:
- Fix warnings from kbuild test robot
- Add comment for input/output GPIO numbering

---
 .../devicetree/bindings/gpio/sgpio-aspeed.txt |   5 +-
 drivers/gpio/gpio-aspeed-sgpio.c              | 126 ++++++++++++------
 2 files changed, 87 insertions(+), 44 deletions(-)

Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt +3 -2

index d4d83916c09d..be329ea4794f 100644
@@ -20,8 +20,9 @@ Required properties:
 - gpio-controller : Marks the device node as a GPIO controller
 - interrupts : Interrupt specifier, see interrupt-controller/interrupts.txt
 - interrupt-controller : Mark the GPIO controller as an interrupt-controller
-- ngpios : number of GPIO lines, see gpio.txt
-  (should be multiple of 8, up to 80 pins)
+- ngpios : number of *hardware* GPIO lines, see gpio.txt. This will expose
+  2 software GPIOs per hardware GPIO: one for hardware input, one for hardware
+  output. Up to 80 pins, must be a multiple of 8.
 - clocks : A phandle to the APB clock for SGPM clock division
 - bus-frequency : SGPM CLK frequency
 

drivers/gpio/gpio-aspeed-sgpio.c +84 -43

index 3aa45934d60c..a18ca52432e0 100644
@@ -17,7 +17,17 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 
-#define MAX_NR_SGPIO                   80
+/*
+ * MAX_NR_HW_GPIO represents the number of actual hardware-supported GPIOs (ie,
+ * slots within the clocked serial GPIO data). Since each HW GPIO is both an
+ * input and an output, we provide MAX_NR_HW_GPIO * 2 lines on our gpiochip
+ * device.
+ *
+ * We use SGPIO_OUTPUT_OFFSET to define the split between the inputs and
+ * outputs; the inputs start at line 0, the outputs start at OUTPUT_OFFSET.
+ */
+#define MAX_NR_HW_SGPIO                        80
+#define SGPIO_OUTPUT_OFFSET            MAX_NR_HW_SGPIO
 
 #define ASPEED_SGPIO_CTRL              0x54
 

@@ -30,8 +40,8 @@ struct aspeed_sgpio {
        struct clk *pclk;
        spinlock_t lock;
        void __iomem *base;
-       uint32_t dir_in[3];
        int irq;
+       int n_sgpio;
 };
 
 struct aspeed_sgpio_bank {

@@ -111,31 +121,69 @@ static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
        }
 }
 
-#define GPIO_BANK(x)    ((x) >> 5)
-#define GPIO_OFFSET(x)  ((x) & 0x1f)
+#define GPIO_BANK(x)    ((x % SGPIO_OUTPUT_OFFSET) >> 5)
+#define GPIO_OFFSET(x)  ((x % SGPIO_OUTPUT_OFFSET) & 0x1f)
 #define GPIO_BIT(x)     BIT(GPIO_OFFSET(x))
 
 static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
 {
-       unsigned int bank = GPIO_BANK(offset);
+       unsigned int bank;
+
+       bank = GPIO_BANK(offset);
 
        WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
        return &aspeed_sgpio_banks[bank];
 }
 
+static int aspeed_sgpio_init_valid_mask(struct gpio_chip *gc,
+               unsigned long *valid_mask, unsigned int ngpios)
+{
+       struct aspeed_sgpio *sgpio = gpiochip_get_data(gc);
+       int n = sgpio->n_sgpio;
+       int c = SGPIO_OUTPUT_OFFSET - n;
+
+       WARN_ON(ngpios < MAX_NR_HW_SGPIO * 2);
+
+       /* input GPIOs in the lower range */
+       bitmap_set(valid_mask, 0, n);
+       bitmap_clear(valid_mask, n, c);
+
+       /* output GPIOS above SGPIO_OUTPUT_OFFSET */
+       bitmap_set(valid_mask, SGPIO_OUTPUT_OFFSET, n);
+       bitmap_clear(valid_mask, SGPIO_OUTPUT_OFFSET + n, c);
+
+       return 0;
+}
+
+static void aspeed_sgpio_irq_init_valid_mask(struct gpio_chip *gc,
+               unsigned long *valid_mask, unsigned int ngpios)
+{
+       struct aspeed_sgpio *sgpio = gpiochip_get_data(gc);
+       int n = sgpio->n_sgpio;
+
+       WARN_ON(ngpios < MAX_NR_HW_SGPIO * 2);
+
+       /* input GPIOs in the lower range */
+       bitmap_set(valid_mask, 0, n);
+       bitmap_clear(valid_mask, n, ngpios - n);
+}
+
+static bool aspeed_sgpio_is_input(unsigned int offset)
+{
+       return offset < SGPIO_OUTPUT_OFFSET;
+}
+
 static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
 {
        struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
        const struct aspeed_sgpio_bank *bank = to_bank(offset);
        unsigned long flags;
        enum aspeed_sgpio_reg reg;
-       bool is_input;
        int rc = 0;
 
        spin_lock_irqsave(&gpio->lock, flags);
 
-       is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
-       reg = is_input ? reg_val : reg_rdata;
+       reg = aspeed_sgpio_is_input(offset) ? reg_val : reg_rdata;
        rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
 
        spin_unlock_irqrestore(&gpio->lock, flags);

@@ -143,22 +191,31 @@ static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
        return rc;
 }
 
-static void sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val)
+static int sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val)
 {
        struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
        const struct aspeed_sgpio_bank *bank = to_bank(offset);
-       void __iomem *addr;
+       void __iomem *addr_r, *addr_w;
        u32 reg = 0;
 
-       addr = bank_reg(gpio, bank, reg_val);
-       reg = ioread32(addr);
+       if (aspeed_sgpio_is_input(offset))
+               return -EINVAL;
+
+       /* Since this is an output, read the cached value from rdata, then
+        * update val. */
+       addr_r = bank_reg(gpio, bank, reg_rdata);
+       addr_w = bank_reg(gpio, bank, reg_val);
+
+       reg = ioread32(addr_r);
 
        if (val)
                reg |= GPIO_BIT(offset);
        else
                reg &= ~GPIO_BIT(offset);
 
-       iowrite32(reg, addr);
+       iowrite32(reg, addr_w);
+
+       return 0;
 }
 
 static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)

@@ -175,43 +232,28 @@ static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)
 
 static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
 {
-       struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
-       unsigned long flags;
-
-       spin_lock_irqsave(&gpio->lock, flags);
-       gpio->dir_in[GPIO_BANK(offset)] |= GPIO_BIT(offset);
-       spin_unlock_irqrestore(&gpio->lock, flags);
-
-       return 0;
+       return aspeed_sgpio_is_input(offset) ? 0 : -EINVAL;
 }
 
 static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int val)
 {
        struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
        unsigned long flags;
+       int rc;
 
-       spin_lock_irqsave(&gpio->lock, flags);
-
-       gpio->dir_in[GPIO_BANK(offset)] &= ~GPIO_BIT(offset);
-       sgpio_set_value(gc, offset, val);
+       /* No special action is required for setting the direction; we'll
+        * error-out in sgpio_set_value if this isn't an output GPIO */
 
+       spin_lock_irqsave(&gpio->lock, flags);
+       rc = sgpio_set_value(gc, offset, val);
        spin_unlock_irqrestore(&gpio->lock, flags);
 
-       return 0;
+       return rc;
 }
 
 static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
-       int dir_status;
-       struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
-       unsigned long flags;
-
-       spin_lock_irqsave(&gpio->lock, flags);
-       dir_status = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
-       spin_unlock_irqrestore(&gpio->lock, flags);
-
-       return dir_status;
-
+       return !!aspeed_sgpio_is_input(offset);
 }
 
 static void irqd_to_aspeed_sgpio_data(struct irq_data *d,

@@ -402,6 +444,7 @@ static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
 
        irq = &gpio->chip.irq;
        irq->chip = &aspeed_sgpio_irqchip;
+       irq->init_valid_mask = aspeed_sgpio_irq_init_valid_mask;
        irq->handler = handle_bad_irq;
        irq->default_type = IRQ_TYPE_NONE;
        irq->parent_handler = aspeed_sgpio_irq_handler;

@@ -452,11 +495,12 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
        if (rc < 0) {
                dev_err(&pdev->dev, "Could not read ngpios property\n");
                return -EINVAL;
-       } else if (nr_gpios > MAX_NR_SGPIO) {
+       } else if (nr_gpios > MAX_NR_HW_SGPIO) {
                dev_err(&pdev->dev, "Number of GPIOs exceeds the maximum of %d: %d\n",
-                       MAX_NR_SGPIO, nr_gpios);
+                       MAX_NR_HW_SGPIO, nr_gpios);
                return -EINVAL;
        }
+       gpio->n_sgpio = nr_gpios;
 
        rc = of_property_read_u32(pdev->dev.of_node, "bus-frequency", &sgpio_freq);
        if (rc < 0) {

@@ -497,7 +541,8 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
        spin_lock_init(&gpio->lock);
 
        gpio->chip.parent = &pdev->dev;
-       gpio->chip.ngpio = nr_gpios;
+       gpio->chip.ngpio = MAX_NR_HW_SGPIO * 2;
+       gpio->chip.init_valid_mask = aspeed_sgpio_init_valid_mask;
        gpio->chip.direction_input = aspeed_sgpio_dir_in;
        gpio->chip.direction_output = aspeed_sgpio_dir_out;
        gpio->chip.get_direction = aspeed_sgpio_get_direction;

@@ -509,9 +554,6 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
        gpio->chip.label = dev_name(&pdev->dev);
        gpio->chip.base = -1;
 
-       /* set all SGPIO pins as input (1). */
-       memset(gpio->dir_in, 0xff, sizeof(gpio->dir_in));
-
        aspeed_sgpio_setup_irqs(gpio, pdev);
 
        rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
--

2.28.0

Quoted 12 lines · Jeremy Kerr

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

Quoted 5 lines

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

Reviewed-by Joel Stanley <[email protected]>

Quoted 271 lines

---
v2:
- Fix warnings from kbuild test robot
- Add comment for input/output GPIO numbering

---
 .../devicetree/bindings/gpio/sgpio-aspeed.txt |   5 +-
 drivers/gpio/gpio-aspeed-sgpio.c              | 126 ++++++++++++------
 2 files changed, 87 insertions(+), 44 deletions(-)

Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt +3 -2

index d4d83916c09d..be329ea4794f 100644
@@ -20,8 +20,9 @@ Required properties:
 - gpio-controller : Marks the device node as a GPIO controller
 - interrupts : Interrupt specifier, see interrupt-controller/interrupts.txt
 - interrupt-controller : Mark the GPIO controller as an interrupt-controller
-- ngpios : number of GPIO lines, see gpio.txt
-  (should be multiple of 8, up to 80 pins)
+- ngpios : number of *hardware* GPIO lines, see gpio.txt. This will expose
+  2 software GPIOs per hardware GPIO: one for hardware input, one for hardware
+  output. Up to 80 pins, must be a multiple of 8.
 - clocks : A phandle to the APB clock for SGPM clock division
 - bus-frequency : SGPM CLK frequency
 

drivers/gpio/gpio-aspeed-sgpio.c +84 -43

index 3aa45934d60c..a18ca52432e0 100644
@@ -17,7 +17,17 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 
-#define MAX_NR_SGPIO                   80
+/*
+ * MAX_NR_HW_GPIO represents the number of actual hardware-supported GPIOs (ie,
+ * slots within the clocked serial GPIO data). Since each HW GPIO is both an
+ * input and an output, we provide MAX_NR_HW_GPIO * 2 lines on our gpiochip
+ * device.
+ *
+ * We use SGPIO_OUTPUT_OFFSET to define the split between the inputs and
+ * outputs; the inputs start at line 0, the outputs start at OUTPUT_OFFSET.
+ */
+#define MAX_NR_HW_SGPIO                        80
+#define SGPIO_OUTPUT_OFFSET            MAX_NR_HW_SGPIO
 
 #define ASPEED_SGPIO_CTRL              0x54
 

@@ -30,8 +40,8 @@ struct aspeed_sgpio {
        struct clk *pclk;
        spinlock_t lock;
        void __iomem *base;
-       uint32_t dir_in[3];
        int irq;
+       int n_sgpio;
 };
 
 struct aspeed_sgpio_bank {

@@ -111,31 +121,69 @@ static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
        }
 }
 
-#define GPIO_BANK(x)    ((x) >> 5)
-#define GPIO_OFFSET(x)  ((x) & 0x1f)
+#define GPIO_BANK(x)    ((x % SGPIO_OUTPUT_OFFSET) >> 5)
+#define GPIO_OFFSET(x)  ((x % SGPIO_OUTPUT_OFFSET) & 0x1f)
 #define GPIO_BIT(x)     BIT(GPIO_OFFSET(x))
 
 static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
 {
-       unsigned int bank = GPIO_BANK(offset);
+       unsigned int bank;
+
+       bank = GPIO_BANK(offset);
 
        WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
        return &aspeed_sgpio_banks[bank];
 }
 
+static int aspeed_sgpio_init_valid_mask(struct gpio_chip *gc,
+               unsigned long *valid_mask, unsigned int ngpios)
+{
+       struct aspeed_sgpio *sgpio = gpiochip_get_data(gc);
+       int n = sgpio->n_sgpio;
+       int c = SGPIO_OUTPUT_OFFSET - n;
+
+       WARN_ON(ngpios < MAX_NR_HW_SGPIO * 2);
+
+       /* input GPIOs in the lower range */
+       bitmap_set(valid_mask, 0, n);
+       bitmap_clear(valid_mask, n, c);
+
+       /* output GPIOS above SGPIO_OUTPUT_OFFSET */
+       bitmap_set(valid_mask, SGPIO_OUTPUT_OFFSET, n);
+       bitmap_clear(valid_mask, SGPIO_OUTPUT_OFFSET + n, c);
+
+       return 0;
+}
+
+static void aspeed_sgpio_irq_init_valid_mask(struct gpio_chip *gc,
+               unsigned long *valid_mask, unsigned int ngpios)
+{
+       struct aspeed_sgpio *sgpio = gpiochip_get_data(gc);
+       int n = sgpio->n_sgpio;
+
+       WARN_ON(ngpios < MAX_NR_HW_SGPIO * 2);
+
+       /* input GPIOs in the lower range */
+       bitmap_set(valid_mask, 0, n);
+       bitmap_clear(valid_mask, n, ngpios - n);
+}
+
+static bool aspeed_sgpio_is_input(unsigned int offset)
+{
+       return offset < SGPIO_OUTPUT_OFFSET;
+}
+
 static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
 {
        struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
        const struct aspeed_sgpio_bank *bank = to_bank(offset);
        unsigned long flags;
        enum aspeed_sgpio_reg reg;
-       bool is_input;
        int rc = 0;
 
        spin_lock_irqsave(&gpio->lock, flags);
 
-       is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
-       reg = is_input ? reg_val : reg_rdata;
+       reg = aspeed_sgpio_is_input(offset) ? reg_val : reg_rdata;
        rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
 
        spin_unlock_irqrestore(&gpio->lock, flags);

@@ -143,22 +191,31 @@ static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
        return rc;
 }
 
-static void sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val)
+static int sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val)
 {
        struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
        const struct aspeed_sgpio_bank *bank = to_bank(offset);
-       void __iomem *addr;
+       void __iomem *addr_r, *addr_w;
        u32 reg = 0;
 
-       addr = bank_reg(gpio, bank, reg_val);
-       reg = ioread32(addr);
+       if (aspeed_sgpio_is_input(offset))
+               return -EINVAL;
+
+       /* Since this is an output, read the cached value from rdata, then
+        * update val. */
+       addr_r = bank_reg(gpio, bank, reg_rdata);
+       addr_w = bank_reg(gpio, bank, reg_val);
+
+       reg = ioread32(addr_r);
 
        if (val)
                reg |= GPIO_BIT(offset);
        else
                reg &= ~GPIO_BIT(offset);
 
-       iowrite32(reg, addr);
+       iowrite32(reg, addr_w);
+
+       return 0;
 }
 
 static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)

@@ -175,43 +232,28 @@ static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)
 
 static int aspeed_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
 {
-       struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
-       unsigned long flags;
-
-       spin_lock_irqsave(&gpio->lock, flags);
-       gpio->dir_in[GPIO_BANK(offset)] |= GPIO_BIT(offset);
-       spin_unlock_irqrestore(&gpio->lock, flags);
-
-       return 0;
+       return aspeed_sgpio_is_input(offset) ? 0 : -EINVAL;
 }
 
 static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int val)
 {
        struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
        unsigned long flags;
+       int rc;
 
-       spin_lock_irqsave(&gpio->lock, flags);
-
-       gpio->dir_in[GPIO_BANK(offset)] &= ~GPIO_BIT(offset);
-       sgpio_set_value(gc, offset, val);
+       /* No special action is required for setting the direction; we'll
+        * error-out in sgpio_set_value if this isn't an output GPIO */
 
+       spin_lock_irqsave(&gpio->lock, flags);
+       rc = sgpio_set_value(gc, offset, val);
        spin_unlock_irqrestore(&gpio->lock, flags);
 
-       return 0;
+       return rc;
 }
 
 static int aspeed_sgpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
-       int dir_status;
-       struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
-       unsigned long flags;
-
-       spin_lock_irqsave(&gpio->lock, flags);
-       dir_status = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
-       spin_unlock_irqrestore(&gpio->lock, flags);
-
-       return dir_status;
-
+       return !!aspeed_sgpio_is_input(offset);
 }
 
 static void irqd_to_aspeed_sgpio_data(struct irq_data *d,

@@ -402,6 +444,7 @@ static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
 
        irq = &gpio->chip.irq;
        irq->chip = &aspeed_sgpio_irqchip;
+       irq->init_valid_mask = aspeed_sgpio_irq_init_valid_mask;
        irq->handler = handle_bad_irq;
        irq->default_type = IRQ_TYPE_NONE;
        irq->parent_handler = aspeed_sgpio_irq_handler;

@@ -452,11 +495,12 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
        if (rc < 0) {
                dev_err(&pdev->dev, "Could not read ngpios property\n");
                return -EINVAL;
-       } else if (nr_gpios > MAX_NR_SGPIO) {
+       } else if (nr_gpios > MAX_NR_HW_SGPIO) {
                dev_err(&pdev->dev, "Number of GPIOs exceeds the maximum of %d: %d\n",
-                       MAX_NR_SGPIO, nr_gpios);
+                       MAX_NR_HW_SGPIO, nr_gpios);
                return -EINVAL;
        }
+       gpio->n_sgpio = nr_gpios;
 
        rc = of_property_read_u32(pdev->dev.of_node, "bus-frequency", &sgpio_freq);
        if (rc < 0) {

@@ -497,7 +541,8 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
        spin_lock_init(&gpio->lock);
 
        gpio->chip.parent = &pdev->dev;
-       gpio->chip.ngpio = nr_gpios;
+       gpio->chip.ngpio = MAX_NR_HW_SGPIO * 2;
+       gpio->chip.init_valid_mask = aspeed_sgpio_init_valid_mask;
        gpio->chip.direction_input = aspeed_sgpio_dir_in;
        gpio->chip.direction_output = aspeed_sgpio_dir_out;
        gpio->chip.get_direction = aspeed_sgpio_get_direction;

@@ -509,9 +554,6 @@ static int __init aspeed_sgpio_probe(struct platform_device *pdev)
        gpio->chip.label = dev_name(&pdev->dev);
        gpio->chip.base = -1;
 
-       /* set all SGPIO pins as input (1). */
-       memset(gpio->dir_in, 0xff, sizeof(gpio->dir_in));
-
        aspeed_sgpio_setup_irqs(gpio, pdev);
 
        rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
--

2.28.0

Hi GPIO maintainers,

Quoted 17 lines · Joel Stanley

Quoted 15 lines · Jeremy Kerr

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Quoted 4 lines

Quoted 2 lines

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

Reviewed-by Joel Stanley <[email protected]>

This series is good to go in for 5.10:

Quoted 24 lines · Joel Stanley

Hi GPIO maintainers,

Quoted 20 lines · Joel Stanley

Quoted 17 lines · Jeremy Kerr

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

Quoted 4 lines

Quoted 1 line

Reviewed-by Joel Stanley <[email protected]>

This series is good to go in for 5.10:

Hi Joel,

I don't have this in my inbox. Did you copy me on this series?

Quoted 26 lines · Bartosz Golaszewski

Quoted 24 lines · Joel Stanley

Hi GPIO maintainers,

Quoted 20 lines · Joel Stanley

Quoted 17 lines · Jeremy Kerr

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Reviewed-by Joel Stanley <[email protected]>

This series is good to go in for 5.10:

Hi Joel,

I don't have this in my inbox. Did you copy me on this series?

I did not; I am not the author of the patches as you can see.

I notice that Jeremy sent them to the linux-gpio list, but you were not copied. Are you able to grab them from lore, or do you need him to resend them?

Cheers,

Joel

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

Currently, the aspeed-sgpio driver exposes up to 80 GPIO lines, corresponding to the 80 status bits available in hardware. Each of these lines can be configured as either an input or an output.

However, each of these GPIOs is actually an input *and* an output; we actually have 80 inputs plus 80 outputs.

This change expands the maximum number of GPIOs to 160; the lower half of this range are the input-only GPIOs, the upper half are the outputs. We fix the GPIO directions to correspond to this mapping.

This also fixes a bug when setting GPIOs - we were reading from the input register, making it impossible to set more than one output GPIO.

Signed-off-by Jeremy Kerr <[email protected]>

Fixes 7db47faae79b ("gpio: aspeed: Add SGPIO driver")

---
v2:
- Fix warnings from kbuild test robot
- Add comment for input/output GPIO numbering

---
 .../devicetree/bindings/gpio/sgpio-aspeed.txt |   5 +-
 drivers/gpio/gpio-aspeed-sgpio.c              | 126 ++++++++++++------
 2 files changed, 87 insertions(+), 44 deletions(-)