On Wed, 2024-10-09 at 02:28 +0000, Billy Tsai wrote:

quoted

quoted

In the 7th generation of the SoC from Aspeed, the control logic
of the
GPIO controller has been updated to support per-pin control. Each
pin now
has its own 32-bit register, allowing for individual control of
the pin's
value, direction, interrupt type, and other settings. The
permission for
coprocessor access is supported by the hardware but hasn't been
implemented in the current patch.

Signed-off-by: Billy Tsai <redacted>
---
 drivers/gpio/gpio-aspeed.c | 147
+++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-

aspeed.c
index 5d583cc9cbc7..208f95fb585e 100644

--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c

@@ -30,6 +30,27 @@

 #include <linux/gpio/consumer.h>
 #include "gpiolib.h"

+/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */
+#define field_get(_mask, _reg)       (((_reg) & (_mask)) >>
(ffs(_mask) - 1))
+#define field_prep(_mask, _val)      (((_val) << (ffs(_mask) -
1)) & (_mask))
+
+#define GPIO_G7_IRQ_STS_BASE 0x100
+#define GPIO_G7_IRQ_STS_OFFSET(x) (GPIO_G7_IRQ_STS_BASE + (x) *
0x4)
+#define GPIO_G7_CTRL_REG_BASE 0x180
+#define GPIO_G7_CTRL_REG_OFFSET(x) (GPIO_G7_CTRL_REG_BASE + (x)
* 0x4)
+#define GPIO_G7_CTRL_OUT_DATA BIT(0)
+#define GPIO_G7_CTRL_DIR BIT(1)
+#define GPIO_G7_CTRL_IRQ_EN BIT(2)
+#define GPIO_G7_CTRL_IRQ_TYPE0 BIT(3)
+#define GPIO_G7_CTRL_IRQ_TYPE1 BIT(4)
+#define GPIO_G7_CTRL_IRQ_TYPE2 BIT(5)
+#define GPIO_G7_CTRL_RST_TOLERANCE BIT(6)
+#define GPIO_G7_CTRL_DEBOUNCE_SEL1 BIT(7)
+#define GPIO_G7_CTRL_DEBOUNCE_SEL2 BIT(8)
+#define GPIO_G7_CTRL_INPUT_MASK BIT(9)
+#define GPIO_G7_CTRL_IRQ_STS BIT(12)
+#define GPIO_G7_CTRL_IN_DATA BIT(13)
+
 struct aspeed_bank_props {
      unsigned int bank;
      u32 input;

@@ -95,6 +116,22 @@ struct aspeed_gpio_bank {

  */

 static const int debounce_timers[4] = { 0x00, 0x50, 0x54, 0x58
};
+static const int g7_debounce_timers[4] = { 0x00, 0x00, 0x04,
0x08 };
+
+/*
+ * The debounce timers array is used to configure the debounce
timer settings.Here’s how it works:
+ * Array Value: Indicates the offset for configuring the
debounce timer.
+ * Array Index: Corresponds to the debounce setting register.
+ * The debounce timers array follows this pattern for
configuring the debounce setting registers:
+ * Array Index 0: No debounce timer is set;
+ *             Array Value is irrelevant (don’t care).
+ * Array Index 1: Debounce setting #2 is set to 1, and debounce
setting #1 is set to 0.
+ *             Array Value: offset for configuring debounce
timer 0 (g4: 0x50, g7: 0x00)
+ * Array Index 2: Debounce setting #2 is set to 0, and debounce
setting #1 is set to 1.
+ *             Array Value: offset for configuring debounce
timer 1 (g4: 0x54, g7: 0x04)
+ * Array Index 3: Debounce setting #2 is set to 1, and debounce
setting #1 is set to 1.
+ *             Array Value: offset for configuring debounce
timer 2 (g4: 0x58, g7: 0x8)
+ */

 static const struct aspeed_gpio_copro_ops *copro_ops;
 static void *copro_data;

@@ -250,6 +287,39 @@ static void __iomem

*aspeed_gpio_g4_bank_reg(struct aspeed_gpio *gpio,
      BUG();
 }

+static u32 aspeed_gpio_g7_reg_mask(const enum aspeed_gpio_reg
reg)
+{
+     switch (reg) {
+     case reg_val:
+             return GPIO_G7_CTRL_OUT_DATA;

quoted

I think a problem is that we want this to be GPIO_G7_CTRL_IN_DATA
for
reads, but GPIO_G7_CTRL_OUT_DATA for writes?

Yes. So in my aspeed_g7_bit_get, I will change the mask to
GPIO_G7_CTRL_IN_DATA.

+static bool aspeed_g7_reg_bit_get(struct aspeed_gpio *gpio, unsigned
int offset,
+                                 const enum aspeed_gpio_reg reg)
+{
+       u32 mask = aspeed_gpio_g7_reg_mask(reg);
+       void __iomem *addr;
+
+       addr = gpio->base + GPIO_G7_CTRL_REG_OFFSET(offset);
+       if (reg == reg_val)
+               mask = GPIO_G7_CTRL_IN_DATA;
+
+       if (mask)
+               return field_get(mask, ioread32(addr));
+       else
+               return 0;
+}
+

Ah, I see that's already what you have. Thanks.

Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>