This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC.
Nuvoton NPCM SGPIO module is combine serial to parallel IC (HC595)
and parallel to serial IC (HC165), and use APB3 clock to control it.
This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
NPCM7xx/NPCM8xx have two sgpio module each module can support up
to 64 output pins,and up to 64 input pin, the pin is only for gpi or gpo.
GPIO pins have sequential, First half is gpo and second half is gpi.
Jim Liu (3):
dt-bindings: gpio: add NPCM sgpio driver bindings
arm: dts: nuvoton: npcm: Add sgpio feature
gpio: nuvoton: Add Nuvoton NPCM sgpio driver
.../bindings/gpio/nuvoton,sgpio.yaml | 86 +++
.../dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 24 +
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-npcm-sgpio.c | 649 ++++++++++++++++++
5 files changed, 768 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
create mode 100644 drivers/gpio/gpio-npcm-sgpio.c
--
2.25.1
Add dt-bindings document for the Nuvoton NPCM7xx sgpio driver
Signed-off-by: Jim Liu <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Changes for v7:
- no change
Changes for v6:
- Drop quotes for $ref
- Add and drop '|' for description
- Add space after 'exposed.'
- remove status
Changes for v5:
- remove bus bus-frequency
- modify in/out description
Changes for v4:
- modify in/out property
- modify bus-frequency property
Changes for v3:
- modify description
- modify in/out property name
Changes for v2:
- modify description
---
.../bindings/gpio/nuvoton,sgpio.yaml | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
new file mode 100644
index 000000000000..84e0dbcb066c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/nuvoton,sgpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton SGPIO controller
+
+maintainers:
+ - Jim LIU <[email protected]>
+
+description: |
+ This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC.
+ Nuvoton NPCM7xx SGPIO module is combine serial to parallel IC (HC595)
+ and parallel to serial IC (HC165), and use APB3 clock to control it.
+ This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
+ NPCM7xx/NPCM8xx have two sgpio module each module can support up
+ to 64 output pins,and up to 64 input pin, the pin is only for gpi or gpo.
+ GPIO pins have sequential, First half is gpo and second half is gpi.
+ GPIO pins can be programmed to support the following options
+ - Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+ - ngpios is number of nuvoton,input-ngpios GPIO lines and nuvoton,output-ngpios GPIO lines.
+ nuvoton,input-ngpios GPIO lines is only for gpi.
+ nuvoton,output-ngpios GPIO lines is only for gpo.
+
+properties:
+ compatible:
+ enum:
+ - nuvoton,npcm750-sgpio
+ - nuvoton,npcm845-sgpio
+
+ reg:
+ maxItems: 1
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ nuvoton,input-ngpios:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The numbers of GPIO's exposed. GPIO lines is only for gpi.
+ minimum: 0
+ maximum: 64
+
+ nuvoton,output-ngpios:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The numbers of GPIO's exposed. GPIO lines is only for gpo.
+ minimum: 0
+ maximum: 64
+
+required:
+ - compatible
+ - reg
+ - gpio-controller
+ - '#gpio-cells'
+ - interrupts
+ - nuvoton,input-ngpios
+ - nuvoton,output-ngpios
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ gpio8: gpio@101000 {
+ compatible = "nuvoton,npcm750-sgpio";
+ reg = <0x101000 0x200>;
+ clocks = <&clk NPCM7XX_CLK_APB3>;
+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ nuvoton,input-ngpios = <64>;
+ nuvoton,output-ngpios = <64>;
+ };
--
2.25.1
Add the SGPIO controller to the NPCM7xx devicetree
Signed-off-by: Jim Liu <[email protected]>
---
Changes for v7:
- no change
Changes for v6:
- remove bus-frequency
- check with dtbs_check
Changes for v5:
- remove npcm8xx node
Changes for v4:
- add npcm8xx gpio node
Changes for v3:
- modify node name
- modify in/out property name
Changes for v2:
- modify dts node
---
.../dts/nuvoton/nuvoton-common-npcm7xx.dtsi | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi
index 868454ae6bde..df91517a4842 100644
--- a/arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi
+++ b/arch/arm/boot/dts/nuvoton/nuvoton-common-npcm7xx.dtsi
@@ -372,6 +372,30 @@ &fanin12_pins &fanin13_pins
status = "disabled";
};
+ gpio8: gpio@101000 {
+ compatible = "nuvoton,npcm750-sgpio";
+ reg = <0x101000 0x200>;
+ clocks = <&clk NPCM7XX_CLK_APB3>;
+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ nuvoton,input-ngpios = <64>;
+ nuvoton,output-ngpios = <64>;
+ status = "disabled";
+ };
+
+ gpio9: gpio@102000 {
+ compatible = "nuvoton,npcm750-sgpio";
+ reg = <0x102000 0x200>;
+ clocks = <&clk NPCM7XX_CLK_APB3>;
+ interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ nuvoton,input-ngpios = <64>;
+ nuvoton,output-ngpios = <64>;
+ status = "disabled";
+ };
+
i2c0: i2c@80000 {
reg = <0x80000 0x1000>;
compatible = "nuvoton,npcm750-i2c";
--
2.25.1
Add Nuvoton BMC NPCM7xx/NPCM8xx sgpio driver support.
Nuvoton NPCM SGPIO module is combine serial to parallel IC (HC595)
and parallel to serial IC (HC165), and use APB3 clock to control it.
This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
BMC can use this driver to increase 64 gpi pins and 64 gpo pins to use.
Signed-off-by: Jim Liu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
Changes for v7:
- remove unused variable
- remove return in bank_reg function
- fix warning for const issue
Changes for v6:
- Remove bus-frequency property set
- Use GPIO_GENERIC
- Move iowrite8 out of the branches
- Add error codes in npcm_sgpio_get_direction
- Use used a ternary operator for npcm_sgpio_init_port
- change npcm_clk_cfg u8 to unsigned int
- Fix typo
Changes for v5:
- remove printk
- add descriptive for to_bank
- using "GPL" instead of "GPL v2"
Changes for v4:
- followed reviewer suggestion to modify npcm_sgpio_dir_in
- blank line in npcm_sgpio_dir_out
- use int type for dir in npcm_sgpio_get
Changes for v3:
- remove return in bank_reg function
Changes for v2:
- add prefix
- write the enum values in all capitals
- remove _init in npcm_sgpio_probe
---
drivers/gpio/Kconfig | 8 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-npcm-sgpio.c | 649 +++++++++++++++++++++++++++++++++
3 files changed, 658 insertions(+)
create mode 100644 drivers/gpio/gpio-npcm-sgpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 673bafb8be58..94c621c7618b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -478,6 +478,14 @@ config GPIO_MXS
select GPIO_GENERIC
select GENERIC_IRQ_CHIP
+config GPIO_NPCM_SGPIO
+ bool "Nuvoton SGPIO support"
+ depends on (ARCH_NPCM || COMPILE_TEST) && OF_GPIO
+ select GPIO_GENERIC
+ select GPIOLIB_IRQCHIP
+ help
+ Say Y here to support Nuvoton NPCM7XX/NPCM8XX SGPIO functionality.
+
config GPIO_OCTEON
tristate "Cavium OCTEON GPIO"
depends on CAVIUM_OCTEON_SOC
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index eb73b5d633eb..373aa2943de5 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_GPIO_MT7621) += gpio-mt7621.o
obj-$(CONFIG_GPIO_MVEBU) += gpio-mvebu.o
obj-$(CONFIG_GPIO_MXC) += gpio-mxc.o
obj-$(CONFIG_GPIO_MXS) += gpio-mxs.o
+obj-$(CONFIG_GPIO_NPCM_SGPIO) += gpio-npcm-sgpio.o
obj-$(CONFIG_GPIO_OCTEON) += gpio-octeon.o
obj-$(CONFIG_GPIO_OMAP) += gpio-omap.o
obj-$(CONFIG_GPIO_PALMAS) += gpio-palmas.o
diff --git a/drivers/gpio/gpio-npcm-sgpio.c b/drivers/gpio/gpio-npcm-sgpio.c
new file mode 100644
index 000000000000..ed8066626687
--- /dev/null
+++ b/drivers/gpio/gpio-npcm-sgpio.c
@@ -0,0 +1,649 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton NPCM Serial GPIO Driver
+ *
+ * Copyright (C) 2021 Nuvoton Technologies
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/hashtable.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+
+#define MAX_NR_HW_SGPIO 64
+
+#define NPCM_IOXCFG1 0x2A
+#define NPCM_IOXCFG1_SFT_CLK GENMASK(3, 0)
+#define NPCM_IOXCFG1_SCLK_POL BIT(4)
+#define NPCM_IOXCFG1_LDSH_POL BIT(5)
+
+#define NPCM_IOXCTS 0x28
+#define NPCM_IOXCTS_IOXIF_EN BIT(7)
+#define NPCM_IOXCTS_RD_MODE GENMASK(2, 1)
+#define NPCM_IOXCTS_RD_MODE_PERIODIC BIT(2)
+#define NPCM_IOXCTS_RD_MODE_CONTINUOUS GENMASK(2, 1)
+
+#define NPCM_IOXCFG2 0x2B
+#define NPCM_IXOEVCFG_MASK 0x3
+#define NPCM_IXOEVCFG_BOTH 0x3
+#define NPCM_IXOEVCFG_FALLING 0x2
+#define NPCM_IXOEVCFG_RISING 0x1
+
+#define NPCM_CLK_TH 8000000
+
+#define GPIO_BANK(x) ((x) / 8)
+#define GPIO_BIT(x) ((x) % 8)
+
+/*
+ * Select the frequency of shift clock.
+ * The shift clock is a division of the APB clock.
+ */
+struct npcm_clk_cfg {
+ const unsigned int *SFT_CLK;
+ const unsigned int *CLK_SEL;
+ const unsigned int cfg_opt;
+};
+
+struct npcm_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ struct irq_chip intc;
+ spinlock_t lock; /*protect event config*/
+ void __iomem *base;
+ int irq;
+ u8 nin_sgpio;
+ u8 nout_sgpio;
+ u8 in_port;
+ u8 out_port;
+ u8 int_type[MAX_NR_HW_SGPIO];
+};
+
+struct npcm_sgpio_bank {
+ u8 rdata_reg;
+ u8 wdata_reg;
+ u8 event_config;
+ u8 event_status;
+};
+
+enum npcm_sgpio_reg {
+ READ_DATA,
+ WRITE_DATA,
+ EVENT_CFG,
+ EVENT_STS,
+};
+
+static const struct npcm_sgpio_bank npcm_sgpio_banks[] = {
+ {
+ .wdata_reg = 0x00,
+ .rdata_reg = 0x08,
+ .event_config = 0x10,
+ .event_status = 0x20,
+ },
+ {
+ .wdata_reg = 0x01,
+ .rdata_reg = 0x09,
+ .event_config = 0x12,
+ .event_status = 0x21,
+ },
+ {
+ .wdata_reg = 0x02,
+ .rdata_reg = 0x0a,
+ .event_config = 0x14,
+ .event_status = 0x22,
+ },
+ {
+ .wdata_reg = 0x03,
+ .rdata_reg = 0x0b,
+ .event_config = 0x16,
+ .event_status = 0x23,
+ },
+ {
+ .wdata_reg = 0x04,
+ .rdata_reg = 0x0c,
+ .event_config = 0x18,
+ .event_status = 0x24,
+ },
+ {
+ .wdata_reg = 0x05,
+ .rdata_reg = 0x0d,
+ .event_config = 0x1a,
+ .event_status = 0x25,
+ },
+ {
+ .wdata_reg = 0x06,
+ .rdata_reg = 0x0e,
+ .event_config = 0x1c,
+ .event_status = 0x26,
+ },
+ {
+ .wdata_reg = 0x07,
+ .rdata_reg = 0x0f,
+ .event_config = 0x1e,
+ .event_status = 0x27,
+ },
+
+};
+
+static void __iomem *bank_reg(struct npcm_sgpio *gpio,
+ const struct npcm_sgpio_bank *bank,
+ const enum npcm_sgpio_reg reg)
+{
+ switch (reg) {
+ case READ_DATA:
+ return gpio->base + bank->rdata_reg;
+ case WRITE_DATA:
+ return gpio->base + bank->wdata_reg;
+ case EVENT_CFG:
+ return gpio->base + bank->event_config;
+ case EVENT_STS:
+ return gpio->base + bank->event_status;
+ default:
+ /* actually if code runs to here, it's an error case */
+ WARN(true, "Getting here is an error condition");
+ }
+}
+
+static const struct npcm_sgpio_bank *offset_to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ return &npcm_sgpio_banks[bank];
+}
+
+static void irqd_to_npcm_sgpio_data(struct irq_data *d,
+ struct npcm_sgpio **gpio,
+ const struct npcm_sgpio_bank **bank,
+ u8 *bit, int *offset)
+{
+ struct npcm_sgpio *internal;
+
+ *offset = irqd_to_hwirq(d);
+ internal = irq_data_get_irq_chip_data(d);
+ WARN_ON(!internal);
+
+ *gpio = internal;
+ *offset -= internal->nout_sgpio;
+ *bank = offset_to_bank(*offset);
+ *bit = GPIO_BIT(*offset);
+}
+
+static int npcm_sgpio_init_port(struct npcm_sgpio *gpio)
+{
+ u8 in_port, out_port, set_port, reg;
+
+ in_port = gpio->nin_sgpio / 8;
+ if (gpio->nin_sgpio % 8 > 0)
+ in_port += 1;
+
+ out_port = gpio->nout_sgpio / 8;
+ if (gpio->nout_sgpio % 8 > 0)
+ out_port += 1;
+
+ gpio->in_port = in_port;
+ gpio->out_port = out_port;
+ set_port = ((out_port & 0xf) << 4) | (in_port & 0xf);
+ iowrite8(set_port, gpio->base + NPCM_IOXCFG2);
+
+ reg = ioread8(gpio->base + NPCM_IOXCFG2);
+
+ return reg == set_port ? 0 : -EINVAL;
+
+}
+
+static int npcm_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+ struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+
+ return offset < gpio->nout_sgpio ? -EINVAL : 0;
+
+}
+
+static int npcm_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int val)
+{
+ struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+
+ if (offset < gpio->nout_sgpio) {
+ gc->set(gc, offset, val);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int npcm_sgpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+ struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+
+ if (offset > gpio->chip.ngpio)
+ return -EINVAL;
+
+ if (offset < gpio->nout_sgpio)
+ return GPIO_LINE_DIRECTION_OUT;
+ return GPIO_LINE_DIRECTION_IN;
+}
+
+static void npcm_sgpio_set(struct gpio_chip *gc, unsigned int offset, int val)
+{
+ struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+ const struct npcm_sgpio_bank *bank = offset_to_bank(offset);
+ void __iomem *addr;
+ u8 reg = 0;
+
+ addr = bank_reg(gpio, bank, WRITE_DATA);
+ reg = ioread8(addr);
+
+ if (val)
+ reg |= (val << GPIO_BIT(offset));
+ else
+ reg &= ~(1 << GPIO_BIT(offset));
+
+ iowrite8(reg, addr);
+}
+
+static int npcm_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+ const struct npcm_sgpio_bank *bank;
+ void __iomem *addr;
+ u8 reg;
+ int dir;
+
+ dir = npcm_sgpio_get_direction(gc, offset);
+ if (dir == 0) {
+ bank = offset_to_bank(offset);
+
+ addr = bank_reg(gpio, bank, WRITE_DATA);
+ reg = ioread8(addr);
+ reg = (reg >> GPIO_BIT(offset)) & 0x01;
+ } else {
+ offset -= gpio->nout_sgpio;
+ bank = offset_to_bank(offset);
+
+ addr = bank_reg(gpio, bank, READ_DATA);
+ reg = ioread8(addr);
+ reg = (reg >> GPIO_BIT(offset)) & 0x01;
+ }
+
+ return reg;
+}
+
+static void npcm_sgpio_setup_enable(struct npcm_sgpio *gpio, bool enable)
+{
+ u8 reg = 0;
+
+ reg = ioread8(gpio->base + NPCM_IOXCTS);
+ reg = reg & ~NPCM_IOXCTS_RD_MODE;
+ reg = reg | NPCM_IOXCTS_RD_MODE_PERIODIC;
+
+ if (enable) {
+ reg |= NPCM_IOXCTS_IOXIF_EN;
+ iowrite8(reg, gpio->base + NPCM_IOXCTS);
+ } else {
+ reg &= ~NPCM_IOXCTS_IOXIF_EN;
+ iowrite8(reg, gpio->base + NPCM_IOXCTS);
+ }
+}
+
+static int npcm_sgpio_setup_clk(struct npcm_sgpio *gpio,
+ const struct npcm_clk_cfg *clk_cfg)
+{
+ unsigned long apb_freq;
+ u32 val;
+ u8 tmp;
+ int i;
+
+ apb_freq = clk_get_rate(gpio->pclk);
+ tmp = ioread8(gpio->base + NPCM_IOXCFG1) & ~NPCM_IOXCFG1_SFT_CLK;
+
+ for (i = 0; i < clk_cfg->cfg_opt; i++) {
+ val = apb_freq / clk_cfg->SFT_CLK[i];
+ if ((NPCM_CLK_TH < val) && (i != 0) ) {
+ iowrite8(clk_cfg->CLK_SEL[i-1] | tmp, gpio->base + NPCM_IOXCFG1);
+ return 0;
+ } else if (i == (clk_cfg->cfg_opt-1) && (NPCM_CLK_TH > val)) {
+ iowrite8(clk_cfg->CLK_SEL[i] | tmp, gpio->base + NPCM_IOXCFG1);
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static void npcm_sgpio_irq_init_valid_mask(struct gpio_chip *gc,
+ unsigned long *valid_mask, unsigned int ngpios)
+{
+ struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+ int n = gpio->nin_sgpio;
+
+ /* input GPIOs in the high range */
+ bitmap_set(valid_mask, gpio->nout_sgpio, n);
+ bitmap_clear(valid_mask, 0, gpio->nout_sgpio);
+}
+
+static void npcm_sgpio_irq_set_mask(struct irq_data *d, bool set)
+{
+ const struct npcm_sgpio_bank *bank;
+ struct npcm_sgpio *gpio;
+ unsigned long flags;
+ void __iomem *addr;
+ int offset;
+ u16 reg, type;
+ u8 bit;
+
+ irqd_to_npcm_sgpio_data(d, &gpio, &bank, &bit, &offset);
+ addr = bank_reg(gpio, bank, EVENT_CFG);
+
+ spin_lock_irqsave(&gpio->lock, flags);
+
+ npcm_sgpio_setup_enable(gpio, false);
+
+ reg = ioread16(addr);
+ if (set) {
+ reg &= ~(NPCM_IXOEVCFG_MASK << (bit * 2));
+ } else {
+ type = gpio->int_type[offset];
+ reg |= (type << (bit * 2));
+ }
+
+ iowrite16(reg, addr);
+
+ npcm_sgpio_setup_enable(gpio, true);
+
+ addr = bank_reg(gpio, bank, EVENT_STS);
+ reg = ioread8(addr);
+ reg |= BIT(bit);
+ iowrite8(reg, addr);
+
+ spin_unlock_irqrestore(&gpio->lock, flags);
+}
+
+static void npcm_sgpio_irq_ack(struct irq_data *d)
+{
+ const struct npcm_sgpio_bank *bank;
+ struct npcm_sgpio *gpio;
+ unsigned long flags;
+ void __iomem *status_addr;
+ int offset;
+ u8 bit;
+
+ irqd_to_npcm_sgpio_data(d, &gpio, &bank, &bit, &offset);
+ status_addr = bank_reg(gpio, bank, EVENT_STS);
+ spin_lock_irqsave(&gpio->lock, flags);
+ iowrite8(BIT(bit), status_addr);
+ spin_unlock_irqrestore(&gpio->lock, flags);
+}
+
+static void npcm_sgpio_irq_mask(struct irq_data *d)
+{
+ npcm_sgpio_irq_set_mask(d, true);
+}
+
+static void npcm_sgpio_irq_unmask(struct irq_data *d)
+{
+ npcm_sgpio_irq_set_mask(d, false);
+}
+
+static int npcm_sgpio_set_type(struct irq_data *d, unsigned int type)
+{
+ const struct npcm_sgpio_bank *bank;
+ irq_flow_handler_t handler;
+ struct npcm_sgpio *gpio;
+ unsigned long flags;
+ void __iomem *addr;
+ int offset;
+ u16 reg, val;
+ u8 bit;
+
+ irqd_to_npcm_sgpio_data(d, &gpio, &bank, &bit, &offset);
+
+ switch (type & IRQ_TYPE_SENSE_MASK) {
+ case IRQ_TYPE_EDGE_BOTH:
+ val = NPCM_IXOEVCFG_BOTH;
+ handler = handle_edge_irq;
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ val = NPCM_IXOEVCFG_RISING;
+ handler = handle_edge_irq;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ val = NPCM_IXOEVCFG_FALLING;
+ handler = handle_edge_irq;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ val = NPCM_IXOEVCFG_RISING;
+ handler = handle_level_irq;
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ val = NPCM_IXOEVCFG_FALLING;
+ handler = handle_level_irq;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ gpio->int_type[offset] = val;
+
+ spin_lock_irqsave(&gpio->lock, flags);
+ npcm_sgpio_setup_enable(gpio, false);
+ addr = bank_reg(gpio, bank, EVENT_CFG);
+ reg = ioread16(addr);
+
+ reg |= (val << (bit * 2));
+
+ iowrite16(reg, addr);
+ npcm_sgpio_setup_enable(gpio, true);
+ spin_unlock_irqrestore(&gpio->lock, flags);
+
+ irq_set_handler_locked(d, handler);
+
+ return 0;
+}
+
+static void npcm_sgpio_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 npcm_sgpio *gpio = gpiochip_get_data(gc);
+ unsigned int i, j, girq;
+ unsigned long reg;
+
+ chained_irq_enter(ic, desc);
+
+ for (i = 0; i < ARRAY_SIZE(npcm_sgpio_banks); i++) {
+ const struct npcm_sgpio_bank *bank = &npcm_sgpio_banks[i];
+
+ reg = ioread8(bank_reg(gpio, bank, EVENT_STS));
+ for_each_set_bit(j, ®, 8) {
+ girq = irq_find_mapping(gc->irq.domain, i * 8 + gpio->nout_sgpio + j);
+ generic_handle_irq(girq);
+ }
+ }
+
+ chained_irq_exit(ic, desc);
+}
+
+static const struct irq_chip sgpio_irq_chip = {
+ .name = "sgpio-irq",
+ .irq_ack = npcm_sgpio_irq_ack,
+ .irq_mask = npcm_sgpio_irq_mask,
+ .irq_unmask = npcm_sgpio_irq_unmask,
+ .irq_set_type = npcm_sgpio_set_type,
+ .flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND,
+ GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int npcm_sgpio_setup_irqs(struct npcm_sgpio *gpio,
+ struct platform_device *pdev)
+{
+ int rc, i;
+ struct gpio_irq_chip *irq;
+
+ rc = platform_get_irq(pdev, 0);
+ if (rc < 0)
+ return rc;
+
+ gpio->irq = rc;
+
+ npcm_sgpio_setup_enable(gpio, false);
+
+ /* Disable IRQ and clear Interrupt status registers for all SGPIO Pins. */
+ for (i = 0; i < ARRAY_SIZE(npcm_sgpio_banks); i++) {
+ const struct npcm_sgpio_bank *bank = &npcm_sgpio_banks[i];
+
+ iowrite16(0x0000, bank_reg(gpio, bank, EVENT_CFG));
+ iowrite8(0xff, bank_reg(gpio, bank, EVENT_STS));
+ }
+
+ irq = &gpio->chip.irq;
+ gpio_irq_chip_set_chip(irq, &sgpio_irq_chip);
+ irq->init_valid_mask = npcm_sgpio_irq_init_valid_mask;
+ irq->handler = handle_bad_irq;
+ irq->default_type = IRQ_TYPE_NONE;
+ irq->parent_handler = npcm_sgpio_irq_handler;
+ irq->parent_handler_data = gpio;
+ irq->parents = &gpio->irq;
+ irq->num_parents = 1;
+
+ return 0;
+}
+
+static const unsigned int npcm750_SFT_CLK[] = {
+ 1024, 32, 8, 4, 3, 2,
+};
+
+static const unsigned int npcm750_CLK_SEL[] = {
+ 0x00, 0x05, 0x07, 0x0C, 0x0D, 0x0E,
+};
+
+static const unsigned int npcm845_SFT_CLK[] = {
+ 1024, 32, 16, 8, 4,
+};
+
+static const unsigned int npcm845_CLK_SEL[] = {
+ 0x00, 0x05, 0x06, 0x07, 0x0C,
+};
+
+static const struct npcm_clk_cfg npcm750_sgpio_pdata = {
+ .SFT_CLK = npcm750_SFT_CLK,
+ .CLK_SEL = npcm750_CLK_SEL,
+ .cfg_opt = 6,
+};
+
+static const struct npcm_clk_cfg npcm845_sgpio_pdata = {
+ .SFT_CLK = npcm845_SFT_CLK,
+ .CLK_SEL = npcm845_CLK_SEL,
+ .cfg_opt = 5,
+};
+
+static const struct of_device_id npcm_sgpio_of_table[] = {
+ { .compatible = "nuvoton,npcm750-sgpio", .data = &npcm750_sgpio_pdata, },
+ { .compatible = "nuvoton,npcm845-sgpio", .data = &npcm845_sgpio_pdata, },
+ {}
+};
+
+MODULE_DEVICE_TABLE(of, npcm_sgpio_of_table);
+
+static int npcm_sgpio_probe(struct platform_device *pdev)
+{
+ struct npcm_sgpio *gpio;
+ const struct npcm_clk_cfg *clk_cfg;
+ int rc;
+ u32 nin_gpios, nout_gpios;
+
+ gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
+ if (!gpio)
+ return -ENOMEM;
+
+ gpio->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(gpio->base))
+ return PTR_ERR(gpio->base);
+
+ clk_cfg = device_get_match_data(&pdev->dev);
+ if (!clk_cfg)
+ return -EINVAL;
+
+ rc = device_property_read_u32(&pdev->dev, "nuvoton,input-ngpios", &nin_gpios);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "Could not read ngpios property\n");
+ return -EINVAL;
+ }
+
+ rc = device_property_read_u32(&pdev->dev, "nuvoton,output-ngpios", &nout_gpios);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "Could not read ngpios property\n");
+ return -EINVAL;
+ }
+
+ gpio->nin_sgpio = nin_gpios;
+ gpio->nout_sgpio = nout_gpios;
+ if (gpio->nin_sgpio > MAX_NR_HW_SGPIO || gpio->nout_sgpio > MAX_NR_HW_SGPIO) {
+ dev_err(&pdev->dev, "Number of GPIOs exceeds the maximum of %d: input: %d output: %d\n",
+ MAX_NR_HW_SGPIO, nin_gpios, nout_gpios);
+ return -EINVAL;
+ }
+
+ gpio->pclk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(gpio->pclk)) {
+ dev_err(&pdev->dev, "Could not get pclk\n");
+ return PTR_ERR(gpio->pclk);
+ }
+
+ rc = npcm_sgpio_setup_clk(gpio, clk_cfg);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "Failed to setup clock\n");
+ return -EINVAL;
+ }
+
+ spin_lock_init(&gpio->lock);
+ gpio->chip.parent = &pdev->dev;
+ gpio->chip.ngpio = gpio->nin_sgpio + gpio->nout_sgpio;
+ gpio->chip.direction_input = npcm_sgpio_dir_in;
+ gpio->chip.direction_output = npcm_sgpio_dir_out;
+ gpio->chip.get_direction = npcm_sgpio_get_direction;
+ gpio->chip.request = NULL;
+ gpio->chip.free = NULL;
+ gpio->chip.get = npcm_sgpio_get;
+ gpio->chip.set = npcm_sgpio_set;
+ gpio->chip.set_config = NULL;
+ gpio->chip.label = dev_name(&pdev->dev);
+ gpio->chip.base = -1;
+
+ rc = npcm_sgpio_init_port(gpio);
+ if (rc < 0)
+ return rc;
+
+ rc = npcm_sgpio_setup_irqs(gpio, pdev);
+ if (rc < 0)
+ return rc;
+
+ rc = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
+ if (rc < 0)
+ return rc;
+
+ npcm_sgpio_setup_enable(gpio, true);
+ dev_info(&pdev->dev, "NPCM: SGPIO module is ready\n");
+
+ return 0;
+}
+
+static struct platform_driver npcm_sgpio_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = npcm_sgpio_of_table,
+ },
+ .probe = npcm_sgpio_probe,
+};
+
+module_platform_driver(npcm_sgpio_driver);
+
+MODULE_AUTHOR("Jim Liu <[email protected]>");
+MODULE_AUTHOR("Joseph Liu <[email protected]>");
+MODULE_DESCRIPTION("Nuvoton NPCM Serial GPIO Driver");
+MODULE_LICENSE("GPL v2");
--
2.25.1
On Wed, Nov 01, 2023 at 10:51:10AM +0800, Jim Liu wrote:
> Add Nuvoton BMC NPCM7xx/NPCM8xx sgpio driver support.
> Nuvoton NPCM SGPIO module is combine serial to parallel IC (HC595)
> and parallel to serial IC (HC165), and use APB3 clock to control it.
> This interface has 4 pins (D_out , D_in, S_CLK, LDSH).
> BMC can use this driver to increase 64 gpi pins and 64 gpo pins to use.
GPI
GPO
...
> Reported-by: kernel test robot <[email protected]>
Can't be true. The absence of a new code may not be "reported".
...
> +config GPIO_NPCM_SGPIO
> + bool "Nuvoton SGPIO support"
> + depends on (ARCH_NPCM || COMPILE_TEST) && OF_GPIO
No new driver should use OF_GPIO.
And I do not see where it's being used. Care to elaborate?
> + select GPIO_GENERIC
> + select GPIOLIB_IRQCHIP
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/hashtable.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
...
> +#define NPCM_IXOEVCFG_MASK 0x3
GENMASK()
...
> +#define NPCM_IXOEVCFG_FALLING 0x2
> +#define NPCM_IXOEVCFG_RISING 0x1
BIT()
> +#define NPCM_IXOEVCFG_BOTH 0x3
#define NPCM_IXOEVCFG_BOTH (NPCM_IXOEVCFG_FALLING | NPCM_IXOEVCFG_RISING)
...
> +#define NPCM_CLK_TH 8000000
Hmm... What is the units here? Hz? Can you use HZ_PER_MHZ multiplier?
> +#define GPIO_BANK(x) ((x) / 8)
> +#define GPIO_BIT(x) ((x) % 8)
...
> +struct npcm_clk_cfg {
> + const unsigned int *SFT_CLK;
> + const unsigned int *CLK_SEL;
Why capital?
> + const unsigned int cfg_opt;
> +};
How const for all of them makes any sense here? Just mark the entire struct
with const whenever you are using it.
...
> +struct npcm_sgpio {
> + struct gpio_chip chip;
> + struct clk *pclk;
> + struct irq_chip intc;
> + spinlock_t lock; /*protect event config*/
Missing spaces inside the comment.
> + void __iomem *base;
> + int irq;
> + u8 nin_sgpio;
> + u8 nout_sgpio;
> + u8 in_port;
> + u8 out_port;
> + u8 int_type[MAX_NR_HW_SGPIO];
> +};
...
> +static void __iomem *bank_reg(struct npcm_sgpio *gpio,
> + const struct npcm_sgpio_bank *bank,
> + const enum npcm_sgpio_reg reg)
Something wrong with indentation.
> +{
> + switch (reg) {
> + case READ_DATA:
> + return gpio->base + bank->rdata_reg;
> + case WRITE_DATA:
> + return gpio->base + bank->wdata_reg;
> + case EVENT_CFG:
> + return gpio->base + bank->event_config;
> + case EVENT_STS:
> + return gpio->base + bank->event_status;
> + default:
> + /* actually if code runs to here, it's an error case */
> + WARN(true, "Getting here is an error condition");
You have a device pointer, why not dev_WARN()?
> + }
> +}
...
> +static void irqd_to_npcm_sgpio_data(struct irq_data *d,
> + struct npcm_sgpio **gpio,
> + const struct npcm_sgpio_bank **bank,
> + u8 *bit, int *offset)
offset should be of the proper type.
> +{
> + struct npcm_sgpio *internal;
> +
> + *offset = irqd_to_hwirq(d);
> + internal = irq_data_get_irq_chip_data(d);
> + WARN_ON(!internal);
When does this make sense? I.o.w. when the internal can be NULL?
> +
> + *gpio = internal;
> + *offset -= internal->nout_sgpio;
> + *bank = offset_to_bank(*offset);
> + *bit = GPIO_BIT(*offset);
> +}
...
> +static int npcm_sgpio_init_port(struct npcm_sgpio *gpio)
> +{
> + u8 in_port, out_port, set_port, reg;
> +
> + in_port = gpio->nin_sgpio / 8;
> + if (gpio->nin_sgpio % 8 > 0)
> + in_port += 1;
You created macros and you don't use them?
> + out_port = gpio->nout_sgpio / 8;
> + if (gpio->nout_sgpio % 8 > 0)
> + out_port += 1;
Ditto.
> + gpio->in_port = in_port;
> + gpio->out_port = out_port;
> + set_port = ((out_port & 0xf) << 4) | (in_port & 0xf);
GENMASK() ?
> + iowrite8(set_port, gpio->base + NPCM_IOXCFG2);
> +
> + reg = ioread8(gpio->base + NPCM_IOXCFG2);
> +
> + return reg == set_port ? 0 : -EINVAL;
> +
> +}
...
> +static int npcm_sgpio_dir_out(struct gpio_chip *gc, unsigned int offset, int val)
> +{
> + struct npcm_sgpio *gpio = gpiochip_get_data(gc);
> +
> + if (offset < gpio->nout_sgpio) {
> + gc->set(gc, offset, val);
> + return 0;
> + }
> + return -EINVAL;
When is this true? Same question to all these checks over the code.
> +}
...
> + if (val)
> + reg |= (val << GPIO_BIT(offset));
And if val == 3? See below as well.
> + else
> + reg &= ~(1 << GPIO_BIT(offset));
Why not BIT() macro?
...
> + reg = (reg >> GPIO_BIT(offset)) & 0x01;
reg = !!(reg & GPIO_BIT(...));
...
> + reg = (reg >> GPIO_BIT(offset)) & 0x01;
Ditto.
...
> + .flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND,
Indentation issue...
Check entire code for this.
...
It's a big driver and you have a lot of different kinds of problems.
One of them is RT kernel support. You have an IRQ chip implementation
here, can it be used in RT?
If so, consider different type of lock.
Also use cleanup.h.
...
> +static const struct of_device_id npcm_sgpio_of_table[] = {
> + { .compatible = "nuvoton,npcm750-sgpio", .data = &npcm750_sgpio_pdata, },
> + { .compatible = "nuvoton,npcm845-sgpio", .data = &npcm845_sgpio_pdata, },
> + {}
> +};
> +
Redundant blank line.
> +MODULE_DEVICE_TABLE(of, npcm_sgpio_of_table);
...
> + rc = device_property_read_u32(&pdev->dev, "nuvoton,input-ngpios", &nin_gpios);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "Could not read ngpios property\n");
> + return -EINVAL;
return dev_err_probe();
> + }
...
> + rc = device_property_read_u32(&pdev->dev, "nuvoton,output-ngpios", &nout_gpios);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "Could not read ngpios property\n");
> + return -EINVAL;
Ditto. And so on for the entire ->probe() stage.
> + }
...
> + gpio->pclk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(gpio->pclk)) {
> + dev_err(&pdev->dev, "Could not get pclk\n");
> + return PTR_ERR(gpio->pclk);
Not using dev_err_probe() will spam the log.
> + }
...
> + gpio->chip.request = NULL;
> + gpio->chip.free = NULL;
> + gpio->chip.set_config = NULL;
Redundant assignments.
...
> + dev_info(&pdev->dev, "NPCM: SGPIO module is ready\n");
Useless noisy message.
...
> +
Unneeded blank line.
> +module_platform_driver(npcm_sgpio_driver);
--
With Best Regards,
Andy Shevchenko
Hi Jim,
thanks for your patch!
I saw that Andy already provided some good feedback but couldn't help
but to notice this:
On Wed, Nov 1, 2023 at 3:52 AM Jim Liu <[email protected]> wrote:
> Changes for v6:
> - Remove bus-frequency property set
> - Use GPIO_GENERIC
(...)
> +config GPIO_NPCM_SGPIO
> + bool "Nuvoton SGPIO support"
> + depends on (ARCH_NPCM || COMPILE_TEST) && OF_GPIO
> + select GPIO_GENERIC
You select GPIO_GENERIC but you don't actually use it.
If you were using it, your code would be calling bgpio_init()
and it does not.
Yours,
Linus Walleij