All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3]sgpio:nuvoton:add support for Nuvoton NPCM SoCs
@ 2022-03-11  6:09 ` jimliu2
  0 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

Add sgpio feature for Nuvoton NPCM SoCs

jimliu2 (3):
  dts: add Nuvoton sgpio feature
  dt-bindings: support Nuvoton sgpio
  gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver

 .../bindings/gpio/nuvoton,sgpio.yaml          |  78 +++
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |  30 +
 drivers/gpio/Kconfig                          |  12 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-npcm-sgpio.c                | 634 ++++++++++++++++++
 5 files changed, 755 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
 create mode 100644 drivers/gpio/gpio-npcm-sgpio.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 0/3]sgpio:nuvoton:add support for Nuvoton NPCM SoCs
@ 2022-03-11  6:09 ` jimliu2
  0 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

Add sgpio feature for Nuvoton NPCM SoCs

jimliu2 (3):
  dts: add Nuvoton sgpio feature
  dt-bindings: support Nuvoton sgpio
  gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver

 .../bindings/gpio/nuvoton,sgpio.yaml          |  78 +++
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |  30 +
 drivers/gpio/Kconfig                          |  12 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-npcm-sgpio.c                | 634 ++++++++++++++++++
 5 files changed, 755 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
 create mode 100644 drivers/gpio/gpio-npcm-sgpio.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 1/3] dts: add Nuvoton sgpio feature
  2022-03-11  6:09 ` jimliu2
@ 2022-03-11  6:09   ` jimliu2
  -1 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

add Nuvoton sgpio feature

Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
---
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
index 3696980a3da1..58f4b463c745 100644
--- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
+++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
@@ -329,6 +329,36 @@
 				status = "disabled";
 			};
 
+			sgpio1: sgpio@101000 {
+				clocks = <&clk NPCM7XX_CLK_APB3>;
+				compatible = "nuvoton,npcm750-sgpio";
+				interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&iox1_pins>;
+				bus-frequency = <16000000>;
+				nin_gpios = <64>;
+				nout_gpios = <64>;
+				reg = <0x101000 0x200>;
+				status = "disabled";
+			};
+
+			sgpio2: sgpio@102000 {
+				clocks = <&clk NPCM7XX_CLK_APB3>;
+				compatible = "nuvoton,npcm750-sgpio";
+				interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&iox2_pins>;
+				bus-frequency = <16000000>;
+				nin_gpios = <64>;
+				nout_gpios = <64>;
+				reg = <0x102000 0x200>;
+				status = "disabled";
+			};
+
 			pwm_fan: pwm-fan-controller@103000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 1/3] dts: add Nuvoton sgpio feature
@ 2022-03-11  6:09   ` jimliu2
  0 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

add Nuvoton sgpio feature

Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
---
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
index 3696980a3da1..58f4b463c745 100644
--- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
+++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
@@ -329,6 +329,36 @@
 				status = "disabled";
 			};
 
+			sgpio1: sgpio@101000 {
+				clocks = <&clk NPCM7XX_CLK_APB3>;
+				compatible = "nuvoton,npcm750-sgpio";
+				interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&iox1_pins>;
+				bus-frequency = <16000000>;
+				nin_gpios = <64>;
+				nout_gpios = <64>;
+				reg = <0x101000 0x200>;
+				status = "disabled";
+			};
+
+			sgpio2: sgpio@102000 {
+				clocks = <&clk NPCM7XX_CLK_APB3>;
+				compatible = "nuvoton,npcm750-sgpio";
+				interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&iox2_pins>;
+				bus-frequency = <16000000>;
+				nin_gpios = <64>;
+				nout_gpios = <64>;
+				reg = <0x102000 0x200>;
+				status = "disabled";
+			};
+
 			pwm_fan: pwm-fan-controller@103000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 2/3] dt-bindings: support Nuvoton sgpio
  2022-03-11  6:09 ` jimliu2
@ 2022-03-11  6:09   ` jimliu2
  -1 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

Add nuvoton sgpio yaml in dt-bindings

Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
---
 .../bindings/gpio/nuvoton,sgpio.yaml          | 78 +++++++++++++++++++
 1 file changed, 78 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..8766e1fa4528
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
@@ -0,0 +1,78 @@
+# 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 <JJLIU0@nuvoton.com>
+
+description:
+  This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC,
+  NPCM7xx/NPCM8xx have two sgpio module each module can support up
+  to 64 output pins,and up to 64 input pin.
+  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)
+  - Directly connected to APB bus and its shift clock is from APB bus clock
+    divided by a programmable value.
+  - ngpios is number of nin_gpios GPIO lines and nout_gpios GPIO lines.
+
+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
+
+  nin_gpios: true
+
+  nout_gpios: true
+
+  bus-frequency: true
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - '#gpio-cells'
+  - interrupts
+  - nin_gpios
+  - nout_gpios
+  - clocks
+  - bus-frequency
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    sgpio1: sgpio@101000 {
+        compatible = "nuvoton,npcm750-sgpio";
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+        reg = <0x101000 0x200>;
+        clocks = <&clk NPCM7XX_CLK_APB3>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&iox1_pins>;
+        nin_gpios = <64>;
+        nout_gpios = <64>;
+        bus-frequency = <16000000>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 2/3] dt-bindings: support Nuvoton sgpio
@ 2022-03-11  6:09   ` jimliu2
  0 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

Add nuvoton sgpio yaml in dt-bindings

Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
---
 .../bindings/gpio/nuvoton,sgpio.yaml          | 78 +++++++++++++++++++
 1 file changed, 78 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..8766e1fa4528
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
@@ -0,0 +1,78 @@
+# 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 <JJLIU0@nuvoton.com>
+
+description:
+  This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC,
+  NPCM7xx/NPCM8xx have two sgpio module each module can support up
+  to 64 output pins,and up to 64 input pin.
+  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)
+  - Directly connected to APB bus and its shift clock is from APB bus clock
+    divided by a programmable value.
+  - ngpios is number of nin_gpios GPIO lines and nout_gpios GPIO lines.
+
+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
+
+  nin_gpios: true
+
+  nout_gpios: true
+
+  bus-frequency: true
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - '#gpio-cells'
+  - interrupts
+  - nin_gpios
+  - nout_gpios
+  - clocks
+  - bus-frequency
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    sgpio1: sgpio@101000 {
+        compatible = "nuvoton,npcm750-sgpio";
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
+        reg = <0x101000 0x200>;
+        clocks = <&clk NPCM7XX_CLK_APB3>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&iox1_pins>;
+        nin_gpios = <64>;
+        nout_gpios = <64>;
+        bus-frequency = <16000000>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 3/3] gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver
  2022-03-11  6:09 ` jimliu2
@ 2022-03-11  6:09   ` jimliu2
  -1 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

Add Nuvoton sgpio driver and add config to Kconfig/Makefile

Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
---
 drivers/gpio/Kconfig           |  12 +
 drivers/gpio/Makefile          |   1 +
 drivers/gpio/gpio-npcm-sgpio.c | 634 +++++++++++++++++++++++++++++++++
 3 files changed, 647 insertions(+)
 create mode 100644 drivers/gpio/gpio-npcm-sgpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1c211b4c63be..f4d97016d184 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1575,6 +1575,18 @@ config GPIO_SODAVILLE
 
 endmenu
 
+config GPIO_NUVOTON_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.
+	  If unsure, say N.
+	  NPCM7XX/NPCM8XX can use same serial GPIO driver.
+	  NPCM7XX/NPCM8XX includes two SGPIO modules, SIOX1 ans SIOX2 are
+	  under BMC control.
+
 menu "SPI GPIO expanders"
 	depends on SPI_MASTER
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index edbaa3cb343c..109c4a5c845b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -106,6 +106,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_NUVOTON_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..e8c13dd51564
--- /dev/null
+++ b/drivers/gpio/gpio-npcm-sgpio.c
@@ -0,0 +1,634 @@
+// 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  IOXCFG1 0x2A
+#define  IOXCFG1_SFT_CLK GENMASK(3, 0)
+#define  IOXCFG1_SCLK_POL BIT(4)
+#define  IOXCFG1_LDSH_POL BIT(5)
+
+#define  IOXCTS 0x28
+#define  IOXCTS_IOXIF_EN BIT(7)
+#define  IOXCTS_RD_MODE GENMASK(2, 1)
+#define  IOXCTS_RD_MODE_PERIODIC BIT(2)
+#define  IOXCTS_RD_MODE_CONTINUOUS GENMASK(2, 1)
+
+#define  IOXCFG2 0x2B
+#define  IXOEVCFG_MASK 0x3
+#define  IXOEVCFG_BOTH 0x3
+#define  IXOEVCFG_FALLING 0x2
+#define  IXOEVCFG_RISING 0x1
+
+#define GPIO_BANK(x)    ((x) / 8)
+#define GPIO_BIT(x)     ((x) % 8)
+
+/*
+ * Slect the freqency of shift clock.
+ * The shift clock is a division of the APB clock.
+ */
+struct npcm_clk_cfg {
+	const int *SFT_CLK;
+	const u8 *CLK_SEL;
+	const u8 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:
+		/* acturally if code runs to here, it's an error case */
+		BUG();
+	}
+}
+
+static const struct npcm_sgpio_bank *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 = to_bank(*offset);
+	*bit = GPIO_BIT(*offset);
+}
+
+static int npcm_sgpio_init_valid_mask(struct gpio_chip *gc,
+				      unsigned long *valid_mask, unsigned int ngpios)
+{
+	struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+	u8 in_port, out_port, set_port;
+
+	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 + IOXCFG2);
+
+	return 0;
+}
+
+static int npcm_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+	struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+
+	if (offset < gpio->nout_sgpio)
+		return -EINVAL;
+
+	return 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->nout_sgpio)
+		return 0;
+	return 1;
+}
+
+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 = 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));
+		iowrite8(reg, addr);
+	} 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 dir, reg;
+
+	dir = npcm_sgpio_get_direction(gc, offset);
+	if (dir == 0) {
+		bank = 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 = 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 + IOXCTS);
+	reg = reg & ~IOXCTS_RD_MODE;
+	reg = reg | IOXCTS_RD_MODE_PERIODIC;
+
+	if (enable) {
+		reg |= IOXCTS_IOXIF_EN;
+		iowrite8(reg, gpio->base + IOXCTS);
+	} else {
+		reg &= ~IOXCTS_IOXIF_EN;
+		iowrite8(reg, gpio->base + IOXCTS);
+	}
+}
+
+static int npcm_sgpio_setup_clk(struct npcm_sgpio *gpio,
+				const struct npcm_clk_cfg *clk_cfg, u32 sgpio_freq)
+{
+	unsigned long apb_freq;
+	u32 sgpio_clk_div;
+	u8 tmp;
+	int i;
+
+	apb_freq = clk_get_rate(gpio->pclk);
+	sgpio_clk_div = (apb_freq / sgpio_freq);
+	if ((apb_freq % sgpio_freq) != 0)
+		sgpio_clk_div += 1;
+
+	tmp = ioread8(gpio->base + IOXCFG1) & ~IOXCFG1_SFT_CLK;
+
+	for (i = 0; i < clk_cfg->cfg_opt; i++) {
+		if (sgpio_clk_div >= clk_cfg->SFT_CLK[i]) {
+			iowrite8(clk_cfg->CLK_SEL[i] | tmp, gpio->base + 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 &= ~(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 = IXOEVCFG_BOTH;
+		handler = handle_edge_irq;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		val = IXOEVCFG_RISING;
+		handler = handle_edge_irq;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		val = IXOEVCFG_FALLING;
+		handler = handle_edge_irq;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		val = IXOEVCFG_RISING;
+		handler = handle_level_irq;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		val = 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, &reg, 8) {
+			girq = irq_find_mapping(gc->irq.domain, i * 8 + gpio->nout_sgpio + j);
+			generic_handle_irq(girq);
+		}
+	}
+
+	chained_irq_exit(ic, desc);
+}
+
+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;
+
+	/* 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));
+	}
+
+	gpio->intc.name = dev_name(&pdev->dev);
+	gpio->intc.irq_ack = npcm_sgpio_irq_ack;
+	gpio->intc.irq_mask = npcm_sgpio_irq_mask;
+	gpio->intc.irq_unmask = npcm_sgpio_irq_unmask;
+	gpio->intc.irq_set_type = npcm_sgpio_set_type;
+
+	irq = &gpio->chip.irq;
+	irq->chip = &gpio->intc;
+	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 int npcm7xx_SFT_CLK[] = {
+		1024, 32, 8, 4, 3, 2,
+};
+
+static const u8 npcm7xx_CLK_SEL[] = {
+		0x00, 0x05, 0x07, 0x0C, 0x0D, 0x0E,
+};
+
+static const int npcm845_SFT_CLK[] = {
+		1024, 32, 16, 8, 4,
+};
+
+static const u8 npcm845_CLK_SEL[] = {
+		0x00, 0x05, 0x06, 0x07, 0x0C,
+};
+
+static const struct npcm_clk_cfg npcm7xx_sgpio_pdata = {
+	.SFT_CLK = npcm7xx_SFT_CLK,
+	.CLK_SEL = npcm7xx_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 = &npcm7xx_sgpio_pdata, },
+	{ .compatible = "nuvoton,npcm845-sgpio", .data = &npcm845_sgpio_pdata, },
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, npcm_sgpio_of_table);
+
+static int __init 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, sgpio_freq;
+
+	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, "nin_gpios", &nin_gpios);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Could not read ngpios property\n");
+		return -EINVAL;
+	}
+	rc = device_property_read_u32(&pdev->dev, "nout_gpios", &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;
+	}
+
+	rc = device_property_read_u32(&pdev->dev, "bus-frequency", &sgpio_freq);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Could not read bus-frequency property\n");
+		return -EINVAL;
+	}
+
+	gpio->pclk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(gpio->pclk)) {
+		dev_err(&pdev->dev, "devm_clk_get failed\n");
+		return PTR_ERR(gpio->pclk);
+	}
+
+	rc = npcm_sgpio_setup_clk(gpio, clk_cfg, sgpio_freq);
+	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.init_valid_mask = npcm_sgpio_init_valid_mask;
+	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_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);
+	return 0;
+}
+
+static struct platform_driver npcm_sgpio_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = npcm_sgpio_of_table,
+	},
+};
+
+module_platform_driver_probe(npcm_sgpio_driver, npcm_sgpio_probe);
+MODULE_AUTHOR("Jim Liu <jjliu0@nuvoton.com>");
+MODULE_AUTHOR("Joseph Liu <kwliu@nuvoton.com>");
+MODULE_DESCRIPTION("Nuvoton NPCM Serial GPIO Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 3/3] gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver
@ 2022-03-11  6:09   ` jimliu2
  0 siblings, 0 replies; 18+ messages in thread
From: jimliu2 @ 2022-03-11  6:09 UTC (permalink / raw)
  To: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, jim.t90615,
	CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

Add Nuvoton sgpio driver and add config to Kconfig/Makefile

Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
---
 drivers/gpio/Kconfig           |  12 +
 drivers/gpio/Makefile          |   1 +
 drivers/gpio/gpio-npcm-sgpio.c | 634 +++++++++++++++++++++++++++++++++
 3 files changed, 647 insertions(+)
 create mode 100644 drivers/gpio/gpio-npcm-sgpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1c211b4c63be..f4d97016d184 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1575,6 +1575,18 @@ config GPIO_SODAVILLE
 
 endmenu
 
+config GPIO_NUVOTON_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.
+	  If unsure, say N.
+	  NPCM7XX/NPCM8XX can use same serial GPIO driver.
+	  NPCM7XX/NPCM8XX includes two SGPIO modules, SIOX1 ans SIOX2 are
+	  under BMC control.
+
 menu "SPI GPIO expanders"
 	depends on SPI_MASTER
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index edbaa3cb343c..109c4a5c845b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -106,6 +106,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_NUVOTON_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..e8c13dd51564
--- /dev/null
+++ b/drivers/gpio/gpio-npcm-sgpio.c
@@ -0,0 +1,634 @@
+// 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  IOXCFG1 0x2A
+#define  IOXCFG1_SFT_CLK GENMASK(3, 0)
+#define  IOXCFG1_SCLK_POL BIT(4)
+#define  IOXCFG1_LDSH_POL BIT(5)
+
+#define  IOXCTS 0x28
+#define  IOXCTS_IOXIF_EN BIT(7)
+#define  IOXCTS_RD_MODE GENMASK(2, 1)
+#define  IOXCTS_RD_MODE_PERIODIC BIT(2)
+#define  IOXCTS_RD_MODE_CONTINUOUS GENMASK(2, 1)
+
+#define  IOXCFG2 0x2B
+#define  IXOEVCFG_MASK 0x3
+#define  IXOEVCFG_BOTH 0x3
+#define  IXOEVCFG_FALLING 0x2
+#define  IXOEVCFG_RISING 0x1
+
+#define GPIO_BANK(x)    ((x) / 8)
+#define GPIO_BIT(x)     ((x) % 8)
+
+/*
+ * Slect the freqency of shift clock.
+ * The shift clock is a division of the APB clock.
+ */
+struct npcm_clk_cfg {
+	const int *SFT_CLK;
+	const u8 *CLK_SEL;
+	const u8 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:
+		/* acturally if code runs to here, it's an error case */
+		BUG();
+	}
+}
+
+static const struct npcm_sgpio_bank *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 = to_bank(*offset);
+	*bit = GPIO_BIT(*offset);
+}
+
+static int npcm_sgpio_init_valid_mask(struct gpio_chip *gc,
+				      unsigned long *valid_mask, unsigned int ngpios)
+{
+	struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+	u8 in_port, out_port, set_port;
+
+	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 + IOXCFG2);
+
+	return 0;
+}
+
+static int npcm_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
+{
+	struct npcm_sgpio *gpio = gpiochip_get_data(gc);
+
+	if (offset < gpio->nout_sgpio)
+		return -EINVAL;
+
+	return 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->nout_sgpio)
+		return 0;
+	return 1;
+}
+
+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 = 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));
+		iowrite8(reg, addr);
+	} 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 dir, reg;
+
+	dir = npcm_sgpio_get_direction(gc, offset);
+	if (dir == 0) {
+		bank = 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 = 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 + IOXCTS);
+	reg = reg & ~IOXCTS_RD_MODE;
+	reg = reg | IOXCTS_RD_MODE_PERIODIC;
+
+	if (enable) {
+		reg |= IOXCTS_IOXIF_EN;
+		iowrite8(reg, gpio->base + IOXCTS);
+	} else {
+		reg &= ~IOXCTS_IOXIF_EN;
+		iowrite8(reg, gpio->base + IOXCTS);
+	}
+}
+
+static int npcm_sgpio_setup_clk(struct npcm_sgpio *gpio,
+				const struct npcm_clk_cfg *clk_cfg, u32 sgpio_freq)
+{
+	unsigned long apb_freq;
+	u32 sgpio_clk_div;
+	u8 tmp;
+	int i;
+
+	apb_freq = clk_get_rate(gpio->pclk);
+	sgpio_clk_div = (apb_freq / sgpio_freq);
+	if ((apb_freq % sgpio_freq) != 0)
+		sgpio_clk_div += 1;
+
+	tmp = ioread8(gpio->base + IOXCFG1) & ~IOXCFG1_SFT_CLK;
+
+	for (i = 0; i < clk_cfg->cfg_opt; i++) {
+		if (sgpio_clk_div >= clk_cfg->SFT_CLK[i]) {
+			iowrite8(clk_cfg->CLK_SEL[i] | tmp, gpio->base + 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 &= ~(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 = IXOEVCFG_BOTH;
+		handler = handle_edge_irq;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		val = IXOEVCFG_RISING;
+		handler = handle_edge_irq;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		val = IXOEVCFG_FALLING;
+		handler = handle_edge_irq;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		val = IXOEVCFG_RISING;
+		handler = handle_level_irq;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		val = 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, &reg, 8) {
+			girq = irq_find_mapping(gc->irq.domain, i * 8 + gpio->nout_sgpio + j);
+			generic_handle_irq(girq);
+		}
+	}
+
+	chained_irq_exit(ic, desc);
+}
+
+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;
+
+	/* 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));
+	}
+
+	gpio->intc.name = dev_name(&pdev->dev);
+	gpio->intc.irq_ack = npcm_sgpio_irq_ack;
+	gpio->intc.irq_mask = npcm_sgpio_irq_mask;
+	gpio->intc.irq_unmask = npcm_sgpio_irq_unmask;
+	gpio->intc.irq_set_type = npcm_sgpio_set_type;
+
+	irq = &gpio->chip.irq;
+	irq->chip = &gpio->intc;
+	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 int npcm7xx_SFT_CLK[] = {
+		1024, 32, 8, 4, 3, 2,
+};
+
+static const u8 npcm7xx_CLK_SEL[] = {
+		0x00, 0x05, 0x07, 0x0C, 0x0D, 0x0E,
+};
+
+static const int npcm845_SFT_CLK[] = {
+		1024, 32, 16, 8, 4,
+};
+
+static const u8 npcm845_CLK_SEL[] = {
+		0x00, 0x05, 0x06, 0x07, 0x0C,
+};
+
+static const struct npcm_clk_cfg npcm7xx_sgpio_pdata = {
+	.SFT_CLK = npcm7xx_SFT_CLK,
+	.CLK_SEL = npcm7xx_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 = &npcm7xx_sgpio_pdata, },
+	{ .compatible = "nuvoton,npcm845-sgpio", .data = &npcm845_sgpio_pdata, },
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, npcm_sgpio_of_table);
+
+static int __init 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, sgpio_freq;
+
+	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, "nin_gpios", &nin_gpios);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Could not read ngpios property\n");
+		return -EINVAL;
+	}
+	rc = device_property_read_u32(&pdev->dev, "nout_gpios", &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;
+	}
+
+	rc = device_property_read_u32(&pdev->dev, "bus-frequency", &sgpio_freq);
+	if (rc < 0) {
+		dev_err(&pdev->dev, "Could not read bus-frequency property\n");
+		return -EINVAL;
+	}
+
+	gpio->pclk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(gpio->pclk)) {
+		dev_err(&pdev->dev, "devm_clk_get failed\n");
+		return PTR_ERR(gpio->pclk);
+	}
+
+	rc = npcm_sgpio_setup_clk(gpio, clk_cfg, sgpio_freq);
+	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.init_valid_mask = npcm_sgpio_init_valid_mask;
+	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_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);
+	return 0;
+}
+
+static struct platform_driver npcm_sgpio_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = npcm_sgpio_of_table,
+	},
+};
+
+module_platform_driver_probe(npcm_sgpio_driver, npcm_sgpio_probe);
+MODULE_AUTHOR("Jim Liu <jjliu0@nuvoton.com>");
+MODULE_AUTHOR("Joseph Liu <kwliu@nuvoton.com>");
+MODULE_DESCRIPTION("Nuvoton NPCM Serial GPIO Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/3] dts: add Nuvoton sgpio feature
  2022-03-11  6:09   ` jimliu2
@ 2022-03-11  9:18     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11  9:18 UTC (permalink / raw)
  To: jimliu2, JJLIU0, KWLIU, linus.walleij, brgl, robh+dt,
	avifishman70, tmaimon77, tali.perry1, venture, yuenn,
	benjaminfair, CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

On 11/03/2022 07:09, jimliu2 wrote:
> add Nuvoton sgpio feature
> 
> Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> ---
>  arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> index 3696980a3da1..58f4b463c745 100644
> --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> @@ -329,6 +329,36 @@
>  				status = "disabled";
>  			};
>  
> +			sgpio1: sgpio@101000 {

Generic node name.

> +				clocks = <&clk NPCM7XX_CLK_APB3>;
> +				compatible = "nuvoton,npcm750-sgpio";
> +				interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&iox1_pins>;
> +				bus-frequency = <16000000>;
> +				nin_gpios = <64>;
> +				nout_gpios = <64>;
> +				reg = <0x101000 0x200>;

In each node first goes compatible, then reg.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/3] dts: add Nuvoton sgpio feature
@ 2022-03-11  9:18     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11  9:18 UTC (permalink / raw)
  To: jimliu2, JJLIU0, KWLIU, linus.walleij, brgl, robh+dt,
	avifishman70, tmaimon77, tali.perry1, venture, yuenn,
	benjaminfair, CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

On 11/03/2022 07:09, jimliu2 wrote:
> add Nuvoton sgpio feature
> 
> Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> ---
>  arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> index 3696980a3da1..58f4b463c745 100644
> --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> @@ -329,6 +329,36 @@
>  				status = "disabled";
>  			};
>  
> +			sgpio1: sgpio@101000 {

Generic node name.

> +				clocks = <&clk NPCM7XX_CLK_APB3>;
> +				compatible = "nuvoton,npcm750-sgpio";
> +				interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&iox1_pins>;
> +				bus-frequency = <16000000>;
> +				nin_gpios = <64>;
> +				nout_gpios = <64>;
> +				reg = <0x101000 0x200>;

In each node first goes compatible, then reg.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/3]sgpio:nuvoton:add support for Nuvoton NPCM SoCs
  2022-03-11  6:09 ` jimliu2
@ 2022-03-11  9:19   ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11  9:19 UTC (permalink / raw)
  To: jimliu2, JJLIU0, KWLIU, linus.walleij, brgl, robh+dt,
	avifishman70, tmaimon77, tali.perry1, venture, yuenn,
	benjaminfair, CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

On 11/03/2022 07:09, jimliu2 wrote:
> Add sgpio feature for Nuvoton NPCM SoCs
> 

1. Explain what is SGPIO.
2. Fix all subjects to match subsystems.
3. Check git log to see what subjects are being used and how they are
formatted.

> jimliu2 (3):
>   dts: add Nuvoton sgpio feature
>   dt-bindings: support Nuvoton sgpio
>   gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver

Space missing after ':'.

> 
>  .../bindings/gpio/nuvoton,sgpio.yaml          |  78 +++
>  arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |  30 +
>  drivers/gpio/Kconfig                          |  12 +
>  drivers/gpio/Makefile                         |   1 +
>  drivers/gpio/gpio-npcm-sgpio.c                | 634 ++++++++++++++++++
>  5 files changed, 755 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
>  create mode 100644 drivers/gpio/gpio-npcm-sgpio.c
> 


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 0/3]sgpio:nuvoton:add support for Nuvoton NPCM SoCs
@ 2022-03-11  9:19   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11  9:19 UTC (permalink / raw)
  To: jimliu2, JJLIU0, KWLIU, linus.walleij, brgl, robh+dt,
	avifishman70, tmaimon77, tali.perry1, venture, yuenn,
	benjaminfair, CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

On 11/03/2022 07:09, jimliu2 wrote:
> Add sgpio feature for Nuvoton NPCM SoCs
> 

1. Explain what is SGPIO.
2. Fix all subjects to match subsystems.
3. Check git log to see what subjects are being used and how they are
formatted.

> jimliu2 (3):
>   dts: add Nuvoton sgpio feature
>   dt-bindings: support Nuvoton sgpio
>   gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver

Space missing after ':'.

> 
>  .../bindings/gpio/nuvoton,sgpio.yaml          |  78 +++
>  arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |  30 +
>  drivers/gpio/Kconfig                          |  12 +
>  drivers/gpio/Makefile                         |   1 +
>  drivers/gpio/gpio-npcm-sgpio.c                | 634 ++++++++++++++++++
>  5 files changed, 755 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
>  create mode 100644 drivers/gpio/gpio-npcm-sgpio.c
> 


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/3] dt-bindings: support Nuvoton sgpio
  2022-03-11  6:09   ` jimliu2
@ 2022-03-11  9:25     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11  9:25 UTC (permalink / raw)
  To: jimliu2, JJLIU0, KWLIU, linus.walleij, brgl, robh+dt,
	avifishman70, tmaimon77, tali.perry1, venture, yuenn,
	benjaminfair, CTCCHIEN
  Cc: linux-gpio, devicetree, linux-kernel, openbmc

On 11/03/2022 07:09, jimliu2 wrote:
> Add nuvoton sgpio yaml in dt-bindings

Missing full stop.

Subject: missing prefix. Check git log history.

> 
> Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> ---
>  .../bindings/gpio/nuvoton,sgpio.yaml          | 78 +++++++++++++++++++
>  1 file changed, 78 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..8766e1fa4528
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> @@ -0,0 +1,78 @@
> +# 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 <JJLIU0@nuvoton.com>
> +
> +description:
> +  This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC,
> +  NPCM7xx/NPCM8xx have two sgpio module each module can support up
> +  to 64 output pins,and up to 64 input pin.
> +  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)
> +  - Directly connected to APB bus and its shift clock is from APB bus clock
> +    divided by a programmable value.
> +  - ngpios is number of nin_gpios GPIO lines and nout_gpios GPIO lines.
> +
> +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
> +
> +  nin_gpios: true
> +
> +  nout_gpios: true

Both do not look like proper property. No description, no vendor prefix,
no type, wrong value (true).

> +
> +  bus-frequency: true

Why a GPIO controller needs this legacy bus-frequency property? Which
bus frequency is it? Internal? APB? If APB, use assigned-clocks.

> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - '#gpio-cells'
> +  - interrupts
> +  - nin_gpios
> +  - nout_gpios
> +  - clocks
> +  - bus-frequency
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    sgpio1: sgpio@101000 {

Generic node name, so "gpio".

> +        compatible = "nuvoton,npcm750-sgpio";
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +        reg = <0x101000 0x200>;

reg goes after compatible.

> +        clocks = <&clk NPCM7XX_CLK_APB3>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&iox1_pins>;
> +        nin_gpios = <64>;
> +        nout_gpios = <64>;
> +        bus-frequency = <16000000>;
> +    };


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/3] dt-bindings: support Nuvoton sgpio
@ 2022-03-11  9:25     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-11  9:25 UTC (permalink / raw)
  To: jimliu2, JJLIU0, KWLIU, linus.walleij, brgl, robh+dt,
	avifishman70, tmaimon77, tali.perry1, venture, yuenn,
	benjaminfair, CTCCHIEN
  Cc: linux-gpio, openbmc, linux-kernel, devicetree

On 11/03/2022 07:09, jimliu2 wrote:
> Add nuvoton sgpio yaml in dt-bindings

Missing full stop.

Subject: missing prefix. Check git log history.

> 
> Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> ---
>  .../bindings/gpio/nuvoton,sgpio.yaml          | 78 +++++++++++++++++++
>  1 file changed, 78 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..8766e1fa4528
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/nuvoton,sgpio.yaml
> @@ -0,0 +1,78 @@
> +# 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 <JJLIU0@nuvoton.com>
> +
> +description:
> +  This SGPIO controller is for NUVOTON NPCM7xx and NPCM8xx SoC,
> +  NPCM7xx/NPCM8xx have two sgpio module each module can support up
> +  to 64 output pins,and up to 64 input pin.
> +  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)
> +  - Directly connected to APB bus and its shift clock is from APB bus clock
> +    divided by a programmable value.
> +  - ngpios is number of nin_gpios GPIO lines and nout_gpios GPIO lines.
> +
> +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
> +
> +  nin_gpios: true
> +
> +  nout_gpios: true

Both do not look like proper property. No description, no vendor prefix,
no type, wrong value (true).

> +
> +  bus-frequency: true

Why a GPIO controller needs this legacy bus-frequency property? Which
bus frequency is it? Internal? APB? If APB, use assigned-clocks.

> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - '#gpio-cells'
> +  - interrupts
> +  - nin_gpios
> +  - nout_gpios
> +  - clocks
> +  - bus-frequency
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    sgpio1: sgpio@101000 {

Generic node name, so "gpio".

> +        compatible = "nuvoton,npcm750-sgpio";
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> +        reg = <0x101000 0x200>;

reg goes after compatible.

> +        clocks = <&clk NPCM7XX_CLK_APB3>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&iox1_pins>;
> +        nin_gpios = <64>;
> +        nout_gpios = <64>;
> +        bus-frequency = <16000000>;
> +    };


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 3/3] gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver
  2022-03-11  6:09   ` jimliu2
@ 2022-04-05 15:00     ` Bartosz Golaszewski
  -1 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2022-04-05 15:00 UTC (permalink / raw)
  To: jimliu2
  Cc: KWLIU, Tomer Maimon, open list:GPIO SUBSYSTEM, Avi Fishman,
	Patrick Venture, Linus Walleij, JJLIU0, CTCCHIEN, Tali Perry,
	devicetree, Rob Herring, OpenBMC Maillist,
	Linux Kernel Mailing List, Benjamin Fair

On Fri, Mar 11, 2022 at 7:17 AM jimliu2 <jim.t90615@gmail.com> wrote:
>
> Add Nuvoton sgpio driver and add config to Kconfig/Makefile
>
> Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> ---
>  drivers/gpio/Kconfig           |  12 +
>  drivers/gpio/Makefile          |   1 +
>  drivers/gpio/gpio-npcm-sgpio.c | 634 +++++++++++++++++++++++++++++++++
>  3 files changed, 647 insertions(+)
>  create mode 100644 drivers/gpio/gpio-npcm-sgpio.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1c211b4c63be..f4d97016d184 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1575,6 +1575,18 @@ config GPIO_SODAVILLE
>
>  endmenu
>
> +config GPIO_NUVOTON_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.
> +         If unsure, say N.
> +         NPCM7XX/NPCM8XX can use same serial GPIO driver.
> +         NPCM7XX/NPCM8XX includes two SGPIO modules, SIOX1 ans SIOX2 are
> +         under BMC control.
> +
>  menu "SPI GPIO expanders"
>         depends on SPI_MASTER
>
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index edbaa3cb343c..109c4a5c845b 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -106,6 +106,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_NUVOTON_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..e8c13dd51564
> --- /dev/null
> +++ b/drivers/gpio/gpio-npcm-sgpio.c
> @@ -0,0 +1,634 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nuvoton NPCM Serial GPIO Driver
> + *
> + * Copyright (C) 2021 Nuvoton Technologies
> + */

Add a newline here.

> +#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  IOXCFG1 0x2A
> +#define  IOXCFG1_SFT_CLK GENMASK(3, 0)
> +#define  IOXCFG1_SCLK_POL BIT(4)
> +#define  IOXCFG1_LDSH_POL BIT(5)
> +
> +#define  IOXCTS 0x28
> +#define  IOXCTS_IOXIF_EN BIT(7)
> +#define  IOXCTS_RD_MODE GENMASK(2, 1)
> +#define  IOXCTS_RD_MODE_PERIODIC BIT(2)
> +#define  IOXCTS_RD_MODE_CONTINUOUS GENMASK(2, 1)
> +
> +#define  IOXCFG2 0x2B
> +#define  IXOEVCFG_MASK 0x3
> +#define  IXOEVCFG_BOTH 0x3
> +#define  IXOEVCFG_FALLING 0x2
> +#define  IXOEVCFG_RISING 0x1
> +
> +#define GPIO_BANK(x)    ((x) / 8)
> +#define GPIO_BIT(x)     ((x) % 8)
> +

Would you mind adding the NPCM_ prefix to the above? So that's it's
clear they come from this driver?

> +/*
> + * Slect the freqency of shift clock.
> + * The shift clock is a division of the APB clock.
> + */
> +struct npcm_clk_cfg {
> +       const int *SFT_CLK;
> +       const u8 *CLK_SEL;
> +       const u8 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;

Don't use tabs like this, please.

> +       u8      wdata_reg;
> +       u8      event_config;
> +       u8      event_status;
> +};
> +
> +enum npcm_sgpio_reg {
> +       read_data,
> +       write_data,
> +       event_cfg,
> +       event_sts,
> +};

For consistency write the enum values in all capitals.

> +
> +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:
> +               /* acturally if code runs to here, it's an error case */
> +               BUG();

But maybe it shouldn't crash the kernel? How about a WARN?

> +       }
> +}
> +
> +static const struct npcm_sgpio_bank *to_bank(unsigned int offset)
> +{
> +       unsigned int bank = GPIO_BANK(offset);
> +
> +       return &npcm_sgpio_banks[bank];
> +}

Usually functions starting with to_ cast structure pointers to their
containing parent structures. Can we make it a simple npcm_get_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 = to_bank(*offset);
> +       *bit = GPIO_BIT(*offset);
> +}
> +
> +static int npcm_sgpio_init_valid_mask(struct gpio_chip *gc,
> +                                     unsigned long *valid_mask, unsigned int ngpios)
> +{
> +       struct npcm_sgpio *gpio = gpiochip_get_data(gc);
> +       u8 in_port, out_port, set_port;
> +
> +       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 + IOXCFG2);
> +
> +       return 0;
> +}
> +
> +static int npcm_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
> +{
> +       struct npcm_sgpio *gpio = gpiochip_get_data(gc);
> +
> +       if (offset < gpio->nout_sgpio)
> +               return -EINVAL;
> +
> +       return 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->nout_sgpio)
> +               return 0;
> +       return 1;
> +}
> +
> +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 = 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));
> +               iowrite8(reg, addr);
> +       } 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 dir, reg;
> +
> +       dir = npcm_sgpio_get_direction(gc, offset);
> +       if (dir == 0) {
> +               bank = 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 = 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 + IOXCTS);
> +       reg = reg & ~IOXCTS_RD_MODE;
> +       reg = reg | IOXCTS_RD_MODE_PERIODIC;
> +
> +       if (enable) {
> +               reg |= IOXCTS_IOXIF_EN;
> +               iowrite8(reg, gpio->base + IOXCTS);
> +       } else {
> +               reg &= ~IOXCTS_IOXIF_EN;
> +               iowrite8(reg, gpio->base + IOXCTS);
> +       }
> +}
> +
> +static int npcm_sgpio_setup_clk(struct npcm_sgpio *gpio,
> +                               const struct npcm_clk_cfg *clk_cfg, u32 sgpio_freq)
> +{
> +       unsigned long apb_freq;
> +       u32 sgpio_clk_div;
> +       u8 tmp;
> +       int i;
> +
> +       apb_freq = clk_get_rate(gpio->pclk);
> +       sgpio_clk_div = (apb_freq / sgpio_freq);
> +       if ((apb_freq % sgpio_freq) != 0)
> +               sgpio_clk_div += 1;
> +
> +       tmp = ioread8(gpio->base + IOXCFG1) & ~IOXCFG1_SFT_CLK;
> +
> +       for (i = 0; i < clk_cfg->cfg_opt; i++) {
> +               if (sgpio_clk_div >= clk_cfg->SFT_CLK[i]) {
> +                       iowrite8(clk_cfg->CLK_SEL[i] | tmp, gpio->base + 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 &= ~(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 = IXOEVCFG_BOTH;
> +               handler = handle_edge_irq;
> +               break;
> +       case IRQ_TYPE_EDGE_RISING:
> +               val = IXOEVCFG_RISING;
> +               handler = handle_edge_irq;
> +               break;
> +       case IRQ_TYPE_EDGE_FALLING:
> +               val = IXOEVCFG_FALLING;
> +               handler = handle_edge_irq;
> +               break;
> +       case IRQ_TYPE_LEVEL_HIGH:
> +               val = IXOEVCFG_RISING;
> +               handler = handle_level_irq;
> +               break;
> +       case IRQ_TYPE_LEVEL_LOW:
> +               val = 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, &reg, 8) {
> +                       girq = irq_find_mapping(gc->irq.domain, i * 8 + gpio->nout_sgpio + j);
> +                       generic_handle_irq(girq);
> +               }
> +       }
> +
> +       chained_irq_exit(ic, desc);
> +}
> +
> +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;
> +
> +       /* 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));
> +       }
> +
> +       gpio->intc.name = dev_name(&pdev->dev);
> +       gpio->intc.irq_ack = npcm_sgpio_irq_ack;
> +       gpio->intc.irq_mask = npcm_sgpio_irq_mask;
> +       gpio->intc.irq_unmask = npcm_sgpio_irq_unmask;
> +       gpio->intc.irq_set_type = npcm_sgpio_set_type;
> +
> +       irq = &gpio->chip.irq;
> +       irq->chip = &gpio->intc;
> +       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 int npcm7xx_SFT_CLK[] = {
> +               1024, 32, 8, 4, 3, 2,
> +};
> +
> +static const u8 npcm7xx_CLK_SEL[] = {
> +               0x00, 0x05, 0x07, 0x0C, 0x0D, 0x0E,
> +};
> +
> +static const int npcm845_SFT_CLK[] = {
> +               1024, 32, 16, 8, 4,
> +};
> +
> +static const u8 npcm845_CLK_SEL[] = {
> +               0x00, 0x05, 0x06, 0x07, 0x0C,
> +};
> +
> +static const struct npcm_clk_cfg npcm7xx_sgpio_pdata = {
> +       .SFT_CLK = npcm7xx_SFT_CLK,
> +       .CLK_SEL = npcm7xx_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 = &npcm7xx_sgpio_pdata, },
> +       { .compatible = "nuvoton,npcm845-sgpio", .data = &npcm845_sgpio_pdata, },
> +       {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, npcm_sgpio_of_table);
> +
> +static int __init npcm_sgpio_probe(struct platform_device *pdev)

With 99% confidence you shouldn't have __init here.

> +{
> +       struct npcm_sgpio *gpio;
> +       const struct npcm_clk_cfg *clk_cfg;
> +       int rc;
> +       u32 nin_gpios, nout_gpios, sgpio_freq;
> +
> +       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, "nin_gpios", &nin_gpios);
> +       if (rc < 0) {
> +               dev_err(&pdev->dev, "Could not read ngpios property\n");
> +               return -EINVAL;
> +       }
> +       rc = device_property_read_u32(&pdev->dev, "nout_gpios", &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;
> +       }
> +
> +       rc = device_property_read_u32(&pdev->dev, "bus-frequency", &sgpio_freq);
> +       if (rc < 0) {
> +               dev_err(&pdev->dev, "Could not read bus-frequency property\n");
> +               return -EINVAL;
> +       }
> +
> +       gpio->pclk = devm_clk_get(&pdev->dev, NULL);
> +       if (IS_ERR(gpio->pclk)) {
> +               dev_err(&pdev->dev, "devm_clk_get failed\n");
> +               return PTR_ERR(gpio->pclk);
> +       }
> +
> +       rc = npcm_sgpio_setup_clk(gpio, clk_cfg, sgpio_freq);
> +       if (rc < 0) {
> +               dev_err(&pdev->dev, "Failed to setup clock\n");
> +               return -EINVAL;
> +       }
> +       spin_lock_init(&gpio->lock);

This lock is used very inconsistently. Don't you need protection when
reading/writing to the input and output registers?

> +       gpio->chip.parent = &pdev->dev;
> +       gpio->chip.ngpio = gpio->nin_sgpio + gpio->nout_sgpio;
> +       gpio->chip.init_valid_mask = npcm_sgpio_init_valid_mask;
> +       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_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);
> +       return 0;
> +}
> +
> +static struct platform_driver npcm_sgpio_driver = {
> +       .driver = {
> +               .name = KBUILD_MODNAME,
> +               .of_match_table = npcm_sgpio_of_table,
> +       },
> +};
> +
> +module_platform_driver_probe(npcm_sgpio_driver, npcm_sgpio_probe);

Any reason to use this instead of module_platform_driver()? It's much
clearer IMO and almost all platform drivers use it instead.

Newline here please too.

> +MODULE_AUTHOR("Jim Liu <jjliu0@nuvoton.com>");
> +MODULE_AUTHOR("Joseph Liu <kwliu@nuvoton.com>");
> +MODULE_DESCRIPTION("Nuvoton NPCM Serial GPIO Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 3/3] gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver
@ 2022-04-05 15:00     ` Bartosz Golaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2022-04-05 15:00 UTC (permalink / raw)
  To: jimliu2
  Cc: JJLIU0, KWLIU, Linus Walleij, Rob Herring, Avi Fishman,
	Tomer Maimon, Tali Perry, Patrick Venture, Nancy Yuen,
	Benjamin Fair, CTCCHIEN, open list:GPIO SUBSYSTEM, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist

On Fri, Mar 11, 2022 at 7:17 AM jimliu2 <jim.t90615@gmail.com> wrote:
>
> Add Nuvoton sgpio driver and add config to Kconfig/Makefile
>
> Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> ---
>  drivers/gpio/Kconfig           |  12 +
>  drivers/gpio/Makefile          |   1 +
>  drivers/gpio/gpio-npcm-sgpio.c | 634 +++++++++++++++++++++++++++++++++
>  3 files changed, 647 insertions(+)
>  create mode 100644 drivers/gpio/gpio-npcm-sgpio.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1c211b4c63be..f4d97016d184 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1575,6 +1575,18 @@ config GPIO_SODAVILLE
>
>  endmenu
>
> +config GPIO_NUVOTON_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.
> +         If unsure, say N.
> +         NPCM7XX/NPCM8XX can use same serial GPIO driver.
> +         NPCM7XX/NPCM8XX includes two SGPIO modules, SIOX1 ans SIOX2 are
> +         under BMC control.
> +
>  menu "SPI GPIO expanders"
>         depends on SPI_MASTER
>
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index edbaa3cb343c..109c4a5c845b 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -106,6 +106,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_NUVOTON_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..e8c13dd51564
> --- /dev/null
> +++ b/drivers/gpio/gpio-npcm-sgpio.c
> @@ -0,0 +1,634 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nuvoton NPCM Serial GPIO Driver
> + *
> + * Copyright (C) 2021 Nuvoton Technologies
> + */

Add a newline here.

> +#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  IOXCFG1 0x2A
> +#define  IOXCFG1_SFT_CLK GENMASK(3, 0)
> +#define  IOXCFG1_SCLK_POL BIT(4)
> +#define  IOXCFG1_LDSH_POL BIT(5)
> +
> +#define  IOXCTS 0x28
> +#define  IOXCTS_IOXIF_EN BIT(7)
> +#define  IOXCTS_RD_MODE GENMASK(2, 1)
> +#define  IOXCTS_RD_MODE_PERIODIC BIT(2)
> +#define  IOXCTS_RD_MODE_CONTINUOUS GENMASK(2, 1)
> +
> +#define  IOXCFG2 0x2B
> +#define  IXOEVCFG_MASK 0x3
> +#define  IXOEVCFG_BOTH 0x3
> +#define  IXOEVCFG_FALLING 0x2
> +#define  IXOEVCFG_RISING 0x1
> +
> +#define GPIO_BANK(x)    ((x) / 8)
> +#define GPIO_BIT(x)     ((x) % 8)
> +

Would you mind adding the NPCM_ prefix to the above? So that's it's
clear they come from this driver?

> +/*
> + * Slect the freqency of shift clock.
> + * The shift clock is a division of the APB clock.
> + */
> +struct npcm_clk_cfg {
> +       const int *SFT_CLK;
> +       const u8 *CLK_SEL;
> +       const u8 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;

Don't use tabs like this, please.

> +       u8      wdata_reg;
> +       u8      event_config;
> +       u8      event_status;
> +};
> +
> +enum npcm_sgpio_reg {
> +       read_data,
> +       write_data,
> +       event_cfg,
> +       event_sts,
> +};

For consistency write the enum values in all capitals.

> +
> +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:
> +               /* acturally if code runs to here, it's an error case */
> +               BUG();

But maybe it shouldn't crash the kernel? How about a WARN?

> +       }
> +}
> +
> +static const struct npcm_sgpio_bank *to_bank(unsigned int offset)
> +{
> +       unsigned int bank = GPIO_BANK(offset);
> +
> +       return &npcm_sgpio_banks[bank];
> +}

Usually functions starting with to_ cast structure pointers to their
containing parent structures. Can we make it a simple npcm_get_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 = to_bank(*offset);
> +       *bit = GPIO_BIT(*offset);
> +}
> +
> +static int npcm_sgpio_init_valid_mask(struct gpio_chip *gc,
> +                                     unsigned long *valid_mask, unsigned int ngpios)
> +{
> +       struct npcm_sgpio *gpio = gpiochip_get_data(gc);
> +       u8 in_port, out_port, set_port;
> +
> +       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 + IOXCFG2);
> +
> +       return 0;
> +}
> +
> +static int npcm_sgpio_dir_in(struct gpio_chip *gc, unsigned int offset)
> +{
> +       struct npcm_sgpio *gpio = gpiochip_get_data(gc);
> +
> +       if (offset < gpio->nout_sgpio)
> +               return -EINVAL;
> +
> +       return 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->nout_sgpio)
> +               return 0;
> +       return 1;
> +}
> +
> +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 = 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));
> +               iowrite8(reg, addr);
> +       } 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 dir, reg;
> +
> +       dir = npcm_sgpio_get_direction(gc, offset);
> +       if (dir == 0) {
> +               bank = 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 = 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 + IOXCTS);
> +       reg = reg & ~IOXCTS_RD_MODE;
> +       reg = reg | IOXCTS_RD_MODE_PERIODIC;
> +
> +       if (enable) {
> +               reg |= IOXCTS_IOXIF_EN;
> +               iowrite8(reg, gpio->base + IOXCTS);
> +       } else {
> +               reg &= ~IOXCTS_IOXIF_EN;
> +               iowrite8(reg, gpio->base + IOXCTS);
> +       }
> +}
> +
> +static int npcm_sgpio_setup_clk(struct npcm_sgpio *gpio,
> +                               const struct npcm_clk_cfg *clk_cfg, u32 sgpio_freq)
> +{
> +       unsigned long apb_freq;
> +       u32 sgpio_clk_div;
> +       u8 tmp;
> +       int i;
> +
> +       apb_freq = clk_get_rate(gpio->pclk);
> +       sgpio_clk_div = (apb_freq / sgpio_freq);
> +       if ((apb_freq % sgpio_freq) != 0)
> +               sgpio_clk_div += 1;
> +
> +       tmp = ioread8(gpio->base + IOXCFG1) & ~IOXCFG1_SFT_CLK;
> +
> +       for (i = 0; i < clk_cfg->cfg_opt; i++) {
> +               if (sgpio_clk_div >= clk_cfg->SFT_CLK[i]) {
> +                       iowrite8(clk_cfg->CLK_SEL[i] | tmp, gpio->base + 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 &= ~(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 = IXOEVCFG_BOTH;
> +               handler = handle_edge_irq;
> +               break;
> +       case IRQ_TYPE_EDGE_RISING:
> +               val = IXOEVCFG_RISING;
> +               handler = handle_edge_irq;
> +               break;
> +       case IRQ_TYPE_EDGE_FALLING:
> +               val = IXOEVCFG_FALLING;
> +               handler = handle_edge_irq;
> +               break;
> +       case IRQ_TYPE_LEVEL_HIGH:
> +               val = IXOEVCFG_RISING;
> +               handler = handle_level_irq;
> +               break;
> +       case IRQ_TYPE_LEVEL_LOW:
> +               val = 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, &reg, 8) {
> +                       girq = irq_find_mapping(gc->irq.domain, i * 8 + gpio->nout_sgpio + j);
> +                       generic_handle_irq(girq);
> +               }
> +       }
> +
> +       chained_irq_exit(ic, desc);
> +}
> +
> +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;
> +
> +       /* 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));
> +       }
> +
> +       gpio->intc.name = dev_name(&pdev->dev);
> +       gpio->intc.irq_ack = npcm_sgpio_irq_ack;
> +       gpio->intc.irq_mask = npcm_sgpio_irq_mask;
> +       gpio->intc.irq_unmask = npcm_sgpio_irq_unmask;
> +       gpio->intc.irq_set_type = npcm_sgpio_set_type;
> +
> +       irq = &gpio->chip.irq;
> +       irq->chip = &gpio->intc;
> +       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 int npcm7xx_SFT_CLK[] = {
> +               1024, 32, 8, 4, 3, 2,
> +};
> +
> +static const u8 npcm7xx_CLK_SEL[] = {
> +               0x00, 0x05, 0x07, 0x0C, 0x0D, 0x0E,
> +};
> +
> +static const int npcm845_SFT_CLK[] = {
> +               1024, 32, 16, 8, 4,
> +};
> +
> +static const u8 npcm845_CLK_SEL[] = {
> +               0x00, 0x05, 0x06, 0x07, 0x0C,
> +};
> +
> +static const struct npcm_clk_cfg npcm7xx_sgpio_pdata = {
> +       .SFT_CLK = npcm7xx_SFT_CLK,
> +       .CLK_SEL = npcm7xx_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 = &npcm7xx_sgpio_pdata, },
> +       { .compatible = "nuvoton,npcm845-sgpio", .data = &npcm845_sgpio_pdata, },
> +       {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, npcm_sgpio_of_table);
> +
> +static int __init npcm_sgpio_probe(struct platform_device *pdev)

With 99% confidence you shouldn't have __init here.

> +{
> +       struct npcm_sgpio *gpio;
> +       const struct npcm_clk_cfg *clk_cfg;
> +       int rc;
> +       u32 nin_gpios, nout_gpios, sgpio_freq;
> +
> +       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, "nin_gpios", &nin_gpios);
> +       if (rc < 0) {
> +               dev_err(&pdev->dev, "Could not read ngpios property\n");
> +               return -EINVAL;
> +       }
> +       rc = device_property_read_u32(&pdev->dev, "nout_gpios", &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;
> +       }
> +
> +       rc = device_property_read_u32(&pdev->dev, "bus-frequency", &sgpio_freq);
> +       if (rc < 0) {
> +               dev_err(&pdev->dev, "Could not read bus-frequency property\n");
> +               return -EINVAL;
> +       }
> +
> +       gpio->pclk = devm_clk_get(&pdev->dev, NULL);
> +       if (IS_ERR(gpio->pclk)) {
> +               dev_err(&pdev->dev, "devm_clk_get failed\n");
> +               return PTR_ERR(gpio->pclk);
> +       }
> +
> +       rc = npcm_sgpio_setup_clk(gpio, clk_cfg, sgpio_freq);
> +       if (rc < 0) {
> +               dev_err(&pdev->dev, "Failed to setup clock\n");
> +               return -EINVAL;
> +       }
> +       spin_lock_init(&gpio->lock);

This lock is used very inconsistently. Don't you need protection when
reading/writing to the input and output registers?

> +       gpio->chip.parent = &pdev->dev;
> +       gpio->chip.ngpio = gpio->nin_sgpio + gpio->nout_sgpio;
> +       gpio->chip.init_valid_mask = npcm_sgpio_init_valid_mask;
> +       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_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);
> +       return 0;
> +}
> +
> +static struct platform_driver npcm_sgpio_driver = {
> +       .driver = {
> +               .name = KBUILD_MODNAME,
> +               .of_match_table = npcm_sgpio_of_table,
> +       },
> +};
> +
> +module_platform_driver_probe(npcm_sgpio_driver, npcm_sgpio_probe);

Any reason to use this instead of module_platform_driver()? It's much
clearer IMO and almost all platform drivers use it instead.

Newline here please too.

> +MODULE_AUTHOR("Jim Liu <jjliu0@nuvoton.com>");
> +MODULE_AUTHOR("Joseph Liu <kwliu@nuvoton.com>");
> +MODULE_DESCRIPTION("Nuvoton NPCM Serial GPIO Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/3] dts: add Nuvoton sgpio feature
  2022-03-11  9:18     ` Krzysztof Kozlowski
@ 2022-09-15  3:36       ` Jim Liu
  -1 siblings, 0 replies; 18+ messages in thread
From: Jim Liu @ 2022-09-15  3:36 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: JJLIU0, KWLIU, linus.walleij, brgl, robh+dt, avifishman70,
	tmaimon77, tali.perry1, venture, yuenn, benjaminfair, CTCCHIEN,
	linux-gpio, devicetree, linux-kernel, openbmc

Hi Krzysztof Kozlowski

Thanks for your review.
I am modifying this driver now, and i have some questions.

what's mean "Generic node name." ?
Nuvoton NPCM750 SGPIO module is base on serial to parallel IC (HC595)
and parallel to serial IC (HC165).
and dts node name is followed aspeed dts node name.

Could you give more information??

Best regards,
Jim

On Fri, Mar 11, 2022 at 5:18 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 11/03/2022 07:09, jimliu2 wrote:
> > add Nuvoton sgpio feature
> >
> > Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> > ---
> >  arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 30 +++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > index 3696980a3da1..58f4b463c745 100644
> > --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > @@ -329,6 +329,36 @@
> >                               status = "disabled";
> >                       };
> >
> > +                     sgpio1: sgpio@101000 {
>
> Generic node name.
>
> > +                             clocks = <&clk NPCM7XX_CLK_APB3>;
> > +                             compatible = "nuvoton,npcm750-sgpio";
> > +                             interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> > +                             gpio-controller;
> > +                             #gpio-cells = <2>;
> > +                             pinctrl-names = "default";
> > +                             pinctrl-0 = <&iox1_pins>;
> > +                             bus-frequency = <16000000>;
> > +                             nin_gpios = <64>;
> > +                             nout_gpios = <64>;
> > +                             reg = <0x101000 0x200>;
>
> In each node first goes compatible, then reg.
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/3] dts: add Nuvoton sgpio feature
@ 2022-09-15  3:36       ` Jim Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Jim Liu @ 2022-09-15  3:36 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: KWLIU, tmaimon77, linux-gpio, avifishman70, venture,
	linus.walleij, openbmc, JJLIU0, CTCCHIEN, tali.perry1,
	devicetree, robh+dt, brgl, linux-kernel, benjaminfair

Hi Krzysztof Kozlowski

Thanks for your review.
I am modifying this driver now, and i have some questions.

what's mean "Generic node name." ?
Nuvoton NPCM750 SGPIO module is base on serial to parallel IC (HC595)
and parallel to serial IC (HC165).
and dts node name is followed aspeed dts node name.

Could you give more information??

Best regards,
Jim

On Fri, Mar 11, 2022 at 5:18 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 11/03/2022 07:09, jimliu2 wrote:
> > add Nuvoton sgpio feature
> >
> > Signed-off-by: jimliu2 <JJLIU0@nuvoton.com>
> > ---
> >  arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 30 +++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > index 3696980a3da1..58f4b463c745 100644
> > --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > @@ -329,6 +329,36 @@
> >                               status = "disabled";
> >                       };
> >
> > +                     sgpio1: sgpio@101000 {
>
> Generic node name.
>
> > +                             clocks = <&clk NPCM7XX_CLK_APB3>;
> > +                             compatible = "nuvoton,npcm750-sgpio";
> > +                             interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
> > +                             gpio-controller;
> > +                             #gpio-cells = <2>;
> > +                             pinctrl-names = "default";
> > +                             pinctrl-0 = <&iox1_pins>;
> > +                             bus-frequency = <16000000>;
> > +                             nin_gpios = <64>;
> > +                             nout_gpios = <64>;
> > +                             reg = <0x101000 0x200>;
>
> In each node first goes compatible, then reg.
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-09-15 23:44 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-11  6:09 [PATCH v1 0/3]sgpio:nuvoton:add support for Nuvoton NPCM SoCs jimliu2
2022-03-11  6:09 ` jimliu2
2022-03-11  6:09 ` [PATCH v1 1/3] dts: add Nuvoton sgpio feature jimliu2
2022-03-11  6:09   ` jimliu2
2022-03-11  9:18   ` Krzysztof Kozlowski
2022-03-11  9:18     ` Krzysztof Kozlowski
2022-09-15  3:36     ` Jim Liu
2022-09-15  3:36       ` Jim Liu
2022-03-11  6:09 ` [PATCH v1 2/3] dt-bindings: support Nuvoton sgpio jimliu2
2022-03-11  6:09   ` jimliu2
2022-03-11  9:25   ` Krzysztof Kozlowski
2022-03-11  9:25     ` Krzysztof Kozlowski
2022-03-11  6:09 ` [PATCH v1 3/3] gpio:gpio-npcm-sgpio: Add Nuvoton sgpio driver jimliu2
2022-03-11  6:09   ` jimliu2
2022-04-05 15:00   ` Bartosz Golaszewski
2022-04-05 15:00     ` Bartosz Golaszewski
2022-03-11  9:19 ` [PATCH v1 0/3]sgpio:nuvoton:add support for Nuvoton NPCM SoCs Krzysztof Kozlowski
2022-03-11  9:19   ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.