[PATCH 0/2] gpio: Add the Siflower SF19A2890 GPIO controller

[PATCH 0/2] gpio: Add the Siflower SF19A2890 GPIO controller

[Joseph Telaak via B4 Relay]

This series adds a binding and a driver for the GPIO controller of the
Siflower SoCs, starting with the SF19A2890, a MIPS SoC for home routers.
Each line has a register block of its own, and each group of 16 lines an
interrupt.

The binding is Chuanhong Guo's and the driver Qingfang Deng's, both from
Chuanhong's SF19A2890 branch [1], with my changes noted in each. The
same controller is in Siflower's SF21 RISC-V SoCs; the binding has only
the SF19A2890's compatible, the one I can test.

The SoC support itself (MACH_SIFLOWER_MIPS) goes through the MIPS tree;
until then the driver builds with COMPILE_TEST.

The MAINTAINERS entry that covers these files comes with the MIPS series
"MIPS: Add support for the Siflower SF19A2890"; this series applies on
its own. That entry is also why checkpatch's question about updating
MAINTAINERS for the new files is left unanswered here.

Tested on a GL.iNet GL-SFT1200: gpio-keys sees both buttons on both
edges (the reset button on GPIO 27 as KEY_RESTART, the mode slide switch
on GPIO 1 as BTN_0, 1 and 0 per slide); /sys/kernel/debug/gpio lists the
49 lines; the GSW150 switch comes out of reset through GPIO 0 and the
SPI NAND uses GPIO 8 as its chip select.

[1] https://github.com/981213/linux/commits/siflower-7.1

---
Chuanhong Guo (1):
      dt-bindings: gpio: Add Siflower SF19A2890 GPIO controller

Qingfang Deng (1):
      gpio: Add Siflower GPIO controller driver

 .../bindings/gpio/siflower,sf19a2890-gpio.yaml     |  84 ++++++
 drivers/gpio/Kconfig                               |  11 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-siflower.c                       | 325 +++++++++++++++++++++
 4 files changed, 421 insertions(+)
---
base-commit: d66b6b86d5a6278ca930112c634d03725beb7d9a
change-id: 20261102-sf19a2890-gpio-b21562e52f88

Best regards,
--  
Joseph Telaak <joseph@telaak.dev>

[PATCH 1/2] dt-bindings: gpio: Add Siflower SF19A2890 GPIO controller

[Joseph Telaak via B4 Relay]

From: Chuanhong Guo <gch981213@gmail.com>

Document the GPIO controller found on the Siflower SF19A2890 and other
Siflower MIPS and RISC-V SoCs.

Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
[jt: from Chuanhong's siflower-7.1 branch; Joseph Telaak as the binding's
 maintainer, GPL-2.0-only for the deprecated GPL-2.0 SPDX identifier,
 example indented by four spaces, commit message, subject]
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 .../bindings/gpio/siflower,sf19a2890-gpio.yaml     | 84 ++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml b/Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml
new file mode 100644
index 000000000..6708d92fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/siflower,sf19a2890-gpio.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/siflower,sf19a2890-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Siflower SF19A2890 GPIO controller
+
+maintainers:
+  - Joseph Telaak <joseph@telaak.dev>
+
+properties:
+  compatible:
+    const: siflower,sf19a2890-gpio
+
+  gpio-controller: true
+  "#gpio-cells":
+    const: 2
+
+  gpio-ranges: true
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt mapping, one interrupt per 16 GPIOs.
+    minItems: 1
+    maxItems: 10
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  ngpios:
+    description:
+      The number of GPIOs available on the controller implementation.
+    minimum: 1
+
+required:
+  - compatible
+  - clocks
+  - gpio-controller
+  - gpio-ranges
+  - interrupt-controller
+  - interrupts
+  - ngpios
+  - reg
+  - resets
+  - "#gpio-cells"
+  - "#interrupt-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/mips-gic.h>
+
+    gpio@19d00000 {
+        compatible = "siflower,sf19a2890-gpio";
+        reg = <0x19d00000 0x100000>;
+        interrupts = <GIC_SHARED 246 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SHARED 247 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SHARED 248 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SHARED 249 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&gpioclk 0>;
+        resets = <&gpiorst 0>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+        ngpios = <49>;
+        gpio-ranges = <&pinctrl 0 0 49>;
+
+        interrupt-controller;
+        #interrupt-cells = <2>;
+    };

-- 
Git-155)

[PATCH 2/2] gpio: Add Siflower GPIO controller driver

[Joseph Telaak via B4 Relay]

From: Qingfang Deng <qingfang.deng@siflower.com.cn>

Add a driver for the GPIO controller on Siflower SoCs.
This controller is found on all current Siflower MIPS and RISC-V
chips including SF19A2890, SF21A6826 and SF21H8898.

Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
[jt: from Chuanhong's siflower-7.1 branch; depend on MACH_SIFLOWER_MIPS,
 longer Kconfig help, assert the reset through a devm action instead of in
 .remove, which ran before the GPIO chip was removed, drop .owner and the
 unused drvdata, drop the irq_set_affinity callback, which reached the
 parent through irq_to_desc() (not exported with SPARSE_IRQ), call
 gpiochip_disable_irq() and gpiochip_enable_irq() from mask and unmask as
 an immutable irqchip must, find the group by its parent IRQ instead of
 assuming consecutive numbers, return 0 or 1 from .get, read ngpios with
 device_property_read_u32(), linux/math64.h and sorted includes, "Siflower"
 spelling, subject]
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/gpio/Kconfig         |  11 ++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-siflower.c | 325 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 55e428002..6125a8452 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -721,6 +721,17 @@ config GPIO_SIFIVE
 	help
 	  Say yes here to support the GPIO device on SiFive SoCs.
 
+config GPIO_SIFLOWER
+	tristate "Siflower GPIO support"
+	depends on OF_GPIO
+	depends on MACH_SIFLOWER_MIPS || COMPILE_TEST
+	select GPIOLIB_IRQCHIP
+	help
+	  GPIO controller driver for Siflower MIPS and RISC-V SoCs
+	  including SF19A2890, SF21A6826 and SF21H8898. Each line can be an
+	  input or an output and raise an interrupt on either edge or level,
+	  and inputs can be debounced in hardware.
+
 config GPIO_SIOX
 	tristate "SIOX GPIO support"
 	depends on SIOX
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 88f748846..bde618108 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -171,6 +171,7 @@ obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
 obj-$(CONFIG_GPIO_SCH)			+= gpio-sch.o
 obj-$(CONFIG_GPIO_SHARED_PROXY)		+= gpio-shared-proxy.o
 obj-$(CONFIG_GPIO_SIFIVE)		+= gpio-sifive.o
+obj-$(CONFIG_GPIO_SIFLOWER)		+= gpio-siflower.o
 obj-$(CONFIG_GPIO_SIM)			+= gpio-sim.o
 obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
 obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
diff --git a/drivers/gpio/gpio-siflower.c b/drivers/gpio/gpio-siflower.c
new file mode 100644
index 000000000..6eb560da7
--- /dev/null
+++ b/drivers/gpio/gpio-siflower.c
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * GPIO controller driver for Siflower MIPS and RISC-V SoCs including:
+ *  Siflower SF19A2890
+ *  Siflower SF21A6826
+ *  Siflower SF21H8898
+ *
+ */
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/reset.h>
+
+#define GPIO_IR(n)	(0x40 * (n) + 0x00)
+#define GPIO_OR(n)	(0x40 * (n) + 0x04)
+#define GPIO_OEN(n)	(0x40 * (n) + 0x08)
+#define GPIO_IMR(n)	(0x40 * (n) + 0x0c)
+#define GPIO_GPIMR(n)	(0x40 * (n) + 0x10)
+#define GPIO_PIR(n)	(0x40 * (n) + 0x14)
+#define GPIO_ITR(n)	(0x40 * (n) + 0x18)
+#define GPIO_IFR(n)	(0x40 * (n) + 0x1c)
+#define GPIO_ICR(n)	(0x40 * (n) + 0x20)
+#define GPIO_GPxIR(n)	(0x4 * (n) + 0x4000)
+
+#define GPIOS_PER_GROUP	16
+
+struct sf_gpio_priv {
+	struct gpio_chip gc;
+	void __iomem *base;
+	struct clk *clk;
+	struct reset_control *rstc;
+	unsigned int irq[];
+};
+
+#define to_sf_gpio(x)	container_of(x, struct sf_gpio_priv, gc)
+
+static u32 sf_gpio_rd(struct sf_gpio_priv *priv, unsigned long reg)
+{
+	return readl_relaxed(priv->base + reg);
+}
+
+static void sf_gpio_wr(struct sf_gpio_priv *priv, unsigned long reg,
+		       u32 val)
+{
+	writel_relaxed(val, priv->base + reg);
+}
+
+static int sf_gpio_get_value(struct gpio_chip *gc, unsigned int offset)
+{
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+
+	return !!sf_gpio_rd(priv, GPIO_IR(offset));
+}
+
+static int sf_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
+			     int value)
+{
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+
+	sf_gpio_wr(priv, GPIO_OR(offset), value);
+	return 0;
+}
+
+static int sf_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+
+	if (sf_gpio_rd(priv, GPIO_OEN(offset)))
+		return GPIO_LINE_DIRECTION_IN;
+	else
+		return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int sf_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+
+	sf_gpio_wr(priv, GPIO_OEN(offset), 1);
+	return 0;
+}
+
+static int sf_gpio_direction_output(struct gpio_chip *gc, unsigned int offset,
+				    int value)
+{
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+
+	sf_gpio_wr(priv, GPIO_OR(offset), value);
+	sf_gpio_wr(priv, GPIO_OEN(offset), 0);
+	return 0;
+}
+
+static int sf_gpio_set_debounce(struct gpio_chip *gc, unsigned int offset,
+				u32 debounce)
+{
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+	unsigned long freq = clk_get_rate(priv->clk);
+	u64 mul;
+
+	/* (ICR + 1) * IFR = debounce_us * clkfreq_mhz / 4 */
+	mul = (u64)debounce * freq;
+	do_div(mul, 1000000 * 4);
+	if (mul > 0xff00)
+		return -EINVAL;
+
+	sf_gpio_wr(priv, GPIO_ICR(offset), 0xff);
+	sf_gpio_wr(priv, GPIO_IFR(offset), DIV_ROUND_UP(mul, 0x100));
+
+	return 0;
+}
+
+static int sf_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
+			      unsigned long config)
+{
+	switch (pinconf_to_config_param(config)) {
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		return sf_gpio_set_debounce(gc, offset,
+			pinconf_to_config_argument(config));
+	default:
+		return gpiochip_generic_config(gc, offset, config);
+	}
+}
+
+static void sf_gpio_irq_ack(struct irq_data *data)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+	unsigned long offset = irqd_to_hwirq(data);
+
+	sf_gpio_wr(priv, GPIO_PIR(offset), 0);
+}
+
+static void sf_gpio_irq_mask(struct irq_data *data)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+	unsigned long offset = irqd_to_hwirq(data);
+
+	sf_gpio_wr(priv, GPIO_IMR(offset), 1);
+	sf_gpio_wr(priv, GPIO_GPIMR(offset), 1);
+	gpiochip_disable_irq(gc, offset);
+}
+
+static void sf_gpio_irq_unmask(struct irq_data *data)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+	unsigned long offset = irqd_to_hwirq(data);
+
+	gpiochip_enable_irq(gc, offset);
+	sf_gpio_wr(priv, GPIO_IMR(offset), 0);
+	sf_gpio_wr(priv, GPIO_GPIMR(offset), 0);
+}
+
+static int sf_gpio_irq_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+	struct sf_gpio_priv *priv = to_sf_gpio(gc);
+	unsigned long offset = irqd_to_hwirq(data);
+	u32 val;
+
+	switch (flow_type) {
+	case IRQ_TYPE_EDGE_RISING:
+		val = 4;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		val = 2;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		val = 6;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		val = 1;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		val = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+	sf_gpio_wr(priv, GPIO_ITR(offset), val);
+
+	if (flow_type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(data, handle_level_irq);
+	else
+		irq_set_handler_locked(data, handle_edge_irq);
+
+	return 0;
+}
+
+static const struct irq_chip sf_gpio_irqchip = {
+	.name			= KBUILD_MODNAME,
+	.irq_ack		= sf_gpio_irq_ack,
+	.irq_mask		= sf_gpio_irq_mask,
+	.irq_unmask		= sf_gpio_irq_unmask,
+	.irq_set_type		= sf_gpio_irq_set_type,
+	.flags			= IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static void sf_gpio_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 sf_gpio_priv *priv = to_sf_gpio(gc);
+	unsigned int irq = irq_desc_get_irq(desc);
+	unsigned long pending;
+	unsigned int group, n;
+
+	for (group = 0; group < gc->irq.num_parents; group++)
+		if (priv->irq[group] == irq)
+			break;
+	if (WARN_ON_ONCE(group == gc->irq.num_parents))
+		return;
+
+	chained_irq_enter(ic, desc);
+
+	pending = sf_gpio_rd(priv, GPIO_GPxIR(group));
+	for_each_set_bit(n, &pending, GPIOS_PER_GROUP) {
+		generic_handle_domain_irq(gc->irq.domain,
+					  n + group * GPIOS_PER_GROUP);
+	}
+
+	chained_irq_exit(ic, desc);
+}
+
+static void sf_gpio_reset_assert(void *data)
+{
+	reset_control_assert(data);
+}
+
+static int sf_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sf_gpio_priv *priv;
+	struct gpio_irq_chip *girq;
+	struct gpio_chip *gc;
+	u32 ngpios, ngroups;
+	int ret, i;
+
+	ret = device_property_read_u32(dev, "ngpios", &ngpios);
+	if (ret)
+		return ret;
+
+	ngroups = DIV_ROUND_UP(ngpios, GPIOS_PER_GROUP);
+	priv = devm_kzalloc(dev, struct_size(priv, irq, ngroups), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	priv->rstc = devm_reset_control_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(priv->rstc))
+		return PTR_ERR(priv->rstc);
+
+	ret = reset_control_deassert(priv->rstc);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, sf_gpio_reset_assert, priv->rstc);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ngroups; i++) {
+		ret = platform_get_irq(pdev, i);
+		if (ret < 0)
+			return ret;
+
+		priv->irq[i] = ret;
+	}
+
+	gc = &priv->gc;
+	gc->label = KBUILD_MODNAME;
+	gc->parent = dev;
+	gc->owner = THIS_MODULE;
+	gc->request = gpiochip_generic_request;
+	gc->free = gpiochip_generic_free;
+	gc->get_direction = sf_gpio_get_direction;
+	gc->direction_input = sf_gpio_direction_input;
+	gc->direction_output = sf_gpio_direction_output;
+	gc->get = sf_gpio_get_value;
+	gc->set = sf_gpio_set_value;
+	gc->set_config = sf_gpio_set_config;
+	gc->base = -1;
+	gc->ngpio = ngpios;
+
+	girq = &gc->irq;
+	gpio_irq_chip_set_chip(girq, &sf_gpio_irqchip);
+	girq->num_parents = ngroups;
+	girq->parents = priv->irq;
+	girq->parent_handler = sf_gpio_irq_handler;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_bad_irq;
+
+	return devm_gpiochip_add_data(dev, gc, priv);
+}
+
+static const struct of_device_id sf_gpio_ids[] = {
+	{ .compatible = "siflower,sf19a2890-gpio" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sf_gpio_ids);
+
+static struct platform_driver sf_gpio_driver = {
+	.probe		= sf_gpio_probe,
+	.driver = {
+		.name		= "siflower_gpio",
+		.of_match_table	= sf_gpio_ids,
+	},
+};
+module_platform_driver(sf_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Qingfang Deng <qingfang.deng@siflower.com.cn>");
+MODULE_DESCRIPTION("GPIO driver for Siflower SoCs");

-- 
Git-155)

Re: [PATCH 2/2] gpio: Add Siflower GPIO controller driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] gpio: siflower: Kconfig depends on undefined MACH_SIFLOWER_MIPS
- [Low] gpio: siflower: sf_gpio_probe() open-codes deasserted reset helper
--

commit 22b6fa5e482223ae6c4b545f3ef7b8a0b9775f11
Author: Qingfang Deng <qingfang.deng@siflower.com.cn>

gpio: Add Siflower GPIO controller driver

This commit adds a GPIO and interrupt controller driver for Siflower MIPS
and RISC-V SoCs, including SF19A2890, SF21A6826, and SF21H8898.

> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 55e4280029d86..6125a8452a5bf 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -721,6 +721,17 @@ config GPIO_SIFIVE
>  	help
>  	  Say yes here to support the GPIO device on SiFive SoCs.
>  
> +config GPIO_SIFLOWER
> +	tristate "Siflower GPIO support"
> +	depends on OF_GPIO
> +	depends on MACH_SIFLOWER_MIPS || COMPILE_TEST

[Severity: Medium]
Can CONFIG_GPIO_SIFLOWER be enabled without CONFIG_COMPILE_TEST?

MACH_SIFLOWER_MIPS is not defined anywhere in the kernel tree, so this
dependency always evaluates to n unless COMPILE_TEST is set, and causes
scripts/checkkconfigsymbols.py to report an undefined Kconfig symbol.

Additionally, the commit description, Kconfig help text, and driver header
state that this controller is also used on Siflower RISC-V SoCs, which would
be excluded by a MIPS-only machine dependency, and the driver only uses
generic device property and platform APIs rather than OF_GPIO internals.

> diff --git a/drivers/gpio/gpio-siflower.c b/drivers/gpio/gpio-siflower.c
> new file mode 100644
> index 0000000000000..6eb560da7ffd5
> --- /dev/null
> +++ b/drivers/gpio/gpio-siflower.c

[ ... ]

> +static void sf_gpio_reset_assert(void *data)
> +{
> +	reset_control_assert(data);
> +}
> +
> +static int sf_gpio_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	priv->rstc = devm_reset_control_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(priv->rstc))
> +		return PTR_ERR(priv->rstc);
> +
> +	ret = reset_control_deassert(priv->rstc);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, sf_gpio_reset_assert, priv->rstc);
> +	if (ret)
> +		return ret;

[Severity: Low]
This isn't a bug, but could sf_gpio_probe() use
devm_reset_control_get_optional_exclusive_deasserted() (or
devm_reset_control_get_exclusive_deasserted(), since resets is required in
the devicetree binding) instead of open-coding the deassertion and custom
sf_gpio_reset_assert() devres action on top of the deprecated
devm_reset_control_get_optional() wrapper? That would also allow dropping
rstc from struct sf_gpio_priv.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-sf19a2890-gpio-v1-0-45e463c48728@telaak.dev?part=2

Re: [PATCH 1/2] dt-bindings: gpio: Add Siflower SF19A2890 GPIO controller

[Bartosz Golaszewski]

On Wed, Nov 04, 2026 at 02:21:37AM -0500, Joseph Telaak via B4 Relay wrote:
> From: Chuanhong Guo <gch981213@gmail.com>
> 
> Document the GPIO controller found on the Siflower SF19A2890 and other
> Siflower MIPS and RISC-V SoCs.
> 
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> [jt: from Chuanhong's siflower-7.1 branch; Joseph Telaak as the binding's
>  maintainer, GPL-2.0-only for the deprecated GPL-2.0 SPDX identifier,
>  example indented by four spaces, commit message, subject]
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> ---

If you're a co-developer, please add your Co-developed-by: prefixed with
a list that looks more like:

[Joseph Telaak:
  - did foo
  - did bar]

The above is completely unreadable.

Thanks,
Bartosz

Re: [PATCH 2/2] gpio: Add Siflower GPIO controller driver

[Bartosz Golaszewski]

On Wed, Nov 04, 2026 at 02:21:38AM -0500, Joseph Telaak via B4 Relay wrote:
> From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> 
> Add a driver for the GPIO controller on Siflower SoCs.
> This controller is found on all current Siflower MIPS and RISC-V
> chips including SF19A2890, SF21A6826 and SF21H8898.
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> [jt: from Chuanhong's siflower-7.1 branch; depend on MACH_SIFLOWER_MIPS,
>  longer Kconfig help, assert the reset through a devm action instead of in
>  .remove, which ran before the GPIO chip was removed, drop .owner and the
>  unused drvdata, drop the irq_set_affinity callback, which reached the
>  parent through irq_to_desc() (not exported with SPARSE_IRQ), call
>  gpiochip_disable_irq() and gpiochip_enable_irq() from mask and unmask as
>  an immutable irqchip must, find the group by its parent IRQ instead of
>  assuming consecutive numbers, return 0 or 1 from .get, read ngpios with
>  device_property_read_u32(), linux/math64.h and sorted includes, "Siflower"
>  spelling, subject]

Same comment as for the DT binding patch.

> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> ---
>  drivers/gpio/Kconfig         |  11 ++
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/gpio-siflower.c | 325 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 337 insertions(+)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 55e428002..6125a8452 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -721,6 +721,17 @@ config GPIO_SIFIVE
>  	help
>  	  Say yes here to support the GPIO device on SiFive SoCs.
>  
> +config GPIO_SIFLOWER
> +	tristate "Siflower GPIO support"
> +	depends on OF_GPIO
> +	depends on MACH_SIFLOWER_MIPS || COMPILE_TEST
> +	select GPIOLIB_IRQCHIP
> +	help
> +	  GPIO controller driver for Siflower MIPS and RISC-V SoCs
> +	  including SF19A2890, SF21A6826 and SF21H8898. Each line can be an
> +	  input or an output and raise an interrupt on either edge or level,
> +	  and inputs can be debounced in hardware.
> +
>  config GPIO_SIOX
>  	tristate "SIOX GPIO support"
>  	depends on SIOX
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 88f748846..bde618108 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -171,6 +171,7 @@ obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
>  obj-$(CONFIG_GPIO_SCH)			+= gpio-sch.o
>  obj-$(CONFIG_GPIO_SHARED_PROXY)		+= gpio-shared-proxy.o
>  obj-$(CONFIG_GPIO_SIFIVE)		+= gpio-sifive.o
> +obj-$(CONFIG_GPIO_SIFLOWER)		+= gpio-siflower.o
>  obj-$(CONFIG_GPIO_SIM)			+= gpio-sim.o
>  obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
>  obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
> diff --git a/drivers/gpio/gpio-siflower.c b/drivers/gpio/gpio-siflower.c
> new file mode 100644
> index 000000000..6eb560da7
> --- /dev/null
> +++ b/drivers/gpio/gpio-siflower.c
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * GPIO controller driver for Siflower MIPS and RISC-V SoCs including:
> + *  Siflower SF19A2890
> + *  Siflower SF21A6826
> + *  Siflower SF21H8898
> + *
> + */

Newline.

> +#include <linux/clk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/reset.h>
> +
> +#define GPIO_IR(n)	(0x40 * (n) + 0x00)
> +#define GPIO_OR(n)	(0x40 * (n) + 0x04)
> +#define GPIO_OEN(n)	(0x40 * (n) + 0x08)
> +#define GPIO_IMR(n)	(0x40 * (n) + 0x0c)
> +#define GPIO_GPIMR(n)	(0x40 * (n) + 0x10)
> +#define GPIO_PIR(n)	(0x40 * (n) + 0x14)
> +#define GPIO_ITR(n)	(0x40 * (n) + 0x18)
> +#define GPIO_IFR(n)	(0x40 * (n) + 0x1c)
> +#define GPIO_ICR(n)	(0x40 * (n) + 0x20)
> +#define GPIO_GPxIR(n)	(0x4 * (n) + 0x4000)
> +
> +#define GPIOS_PER_GROUP	16
> +
> +struct sf_gpio_priv {
> +	struct gpio_chip gc;
> +	void __iomem *base;
> +	struct clk *clk;
> +	struct reset_control *rstc;
> +	unsigned int irq[];
> +};
> +
> +#define to_sf_gpio(x)	container_of(x, struct sf_gpio_priv, gc)
> +
> +static u32 sf_gpio_rd(struct sf_gpio_priv *priv, unsigned long reg)
> +{
> +	return readl_relaxed(priv->base + reg);
> +}
> +
> +static void sf_gpio_wr(struct sf_gpio_priv *priv, unsigned long reg,
> +		       u32 val)
> +{
> +	writel_relaxed(val, priv->base + reg);
> +}
> +
> +static int sf_gpio_get_value(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);

More common pattern is to retrieve the address of priv with
gpiochip_get_data(). You already pass that address to
devm_gpiochip_add_data().

> +
> +	return !!sf_gpio_rd(priv, GPIO_IR(offset));
> +}
> +
> +static int sf_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
> +			     int value)
> +{
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +
> +	sf_gpio_wr(priv, GPIO_OR(offset), value);
> +	return 0;
> +}
> +
> +static int sf_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +
> +	if (sf_gpio_rd(priv, GPIO_OEN(offset)))
> +		return GPIO_LINE_DIRECTION_IN;
> +	else

Drop the else. Or do:

    return sf_gpio_rd(priv, GPIO_OEN(offset)) ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;

> +		return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int sf_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +
> +	sf_gpio_wr(priv, GPIO_OEN(offset), 1);
> +	return 0;
> +}
> +
> +static int sf_gpio_direction_output(struct gpio_chip *gc, unsigned int offset,
> +				    int value)
> +{
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +
> +	sf_gpio_wr(priv, GPIO_OR(offset), value);
> +	sf_gpio_wr(priv, GPIO_OEN(offset), 0);
> +	return 0;
> +}
> +
> +static int sf_gpio_set_debounce(struct gpio_chip *gc, unsigned int offset,
> +				u32 debounce)
> +{
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +	unsigned long freq = clk_get_rate(priv->clk);
> +	u64 mul;
> +
> +	/* (ICR + 1) * IFR = debounce_us * clkfreq_mhz / 4 */
> +	mul = (u64)debounce * freq;
> +	do_div(mul, 1000000 * 4);
> +	if (mul > 0xff00)
> +		return -EINVAL;
> +
> +	sf_gpio_wr(priv, GPIO_ICR(offset), 0xff);
> +	sf_gpio_wr(priv, GPIO_IFR(offset), DIV_ROUND_UP(mul, 0x100));
> +
> +	return 0;
> +}
> +
> +static int sf_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
> +			      unsigned long config)
> +{
> +	switch (pinconf_to_config_param(config)) {
> +	case PIN_CONFIG_INPUT_DEBOUNCE:
> +		return sf_gpio_set_debounce(gc, offset,
> +			pinconf_to_config_argument(config));
> +	default:
> +		return gpiochip_generic_config(gc, offset, config);
> +	}
> +}
> +
> +static void sf_gpio_irq_ack(struct irq_data *data)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +	unsigned long offset = irqd_to_hwirq(data);
> +
> +	sf_gpio_wr(priv, GPIO_PIR(offset), 0);
> +}
> +
> +static void sf_gpio_irq_mask(struct irq_data *data)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +	unsigned long offset = irqd_to_hwirq(data);
> +
> +	sf_gpio_wr(priv, GPIO_IMR(offset), 1);
> +	sf_gpio_wr(priv, GPIO_GPIMR(offset), 1);
> +	gpiochip_disable_irq(gc, offset);
> +}
> +
> +static void sf_gpio_irq_unmask(struct irq_data *data)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +	unsigned long offset = irqd_to_hwirq(data);
> +
> +	gpiochip_enable_irq(gc, offset);
> +	sf_gpio_wr(priv, GPIO_IMR(offset), 0);
> +	sf_gpio_wr(priv, GPIO_GPIMR(offset), 0);
> +}
> +
> +static int sf_gpio_irq_set_type(struct irq_data *data, unsigned int flow_type)
> +{
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> +	struct sf_gpio_priv *priv = to_sf_gpio(gc);
> +	unsigned long offset = irqd_to_hwirq(data);
> +	u32 val;
> +
> +	switch (flow_type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		val = 4;
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		val = 2;
> +		break;
> +	case IRQ_TYPE_EDGE_BOTH:
> +		val = 6;
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		val = 1;
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		val = 0;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	sf_gpio_wr(priv, GPIO_ITR(offset), val);
> +
> +	if (flow_type & IRQ_TYPE_LEVEL_MASK)
> +		irq_set_handler_locked(data, handle_level_irq);
> +	else

Drop else.

> +		irq_set_handler_locked(data, handle_edge_irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_chip sf_gpio_irqchip = {
> +	.name			= KBUILD_MODNAME,
> +	.irq_ack		= sf_gpio_irq_ack,
> +	.irq_mask		= sf_gpio_irq_mask,
> +	.irq_unmask		= sf_gpio_irq_unmask,
> +	.irq_set_type		= sf_gpio_irq_set_type,
> +	.flags			= IRQCHIP_IMMUTABLE,
> +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +static void sf_gpio_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 sf_gpio_priv *priv = to_sf_gpio(gc);
> +	unsigned int irq = irq_desc_get_irq(desc);
> +	unsigned long pending;
> +	unsigned int group, n;
> +
> +	for (group = 0; group < gc->irq.num_parents; group++)
> +		if (priv->irq[group] == irq)
> +			break;
> +	if (WARN_ON_ONCE(group == gc->irq.num_parents))
> +		return;
> +
> +	chained_irq_enter(ic, desc);
> +
> +	pending = sf_gpio_rd(priv, GPIO_GPxIR(group));
> +	for_each_set_bit(n, &pending, GPIOS_PER_GROUP) {
> +		generic_handle_domain_irq(gc->irq.domain,
> +					  n + group * GPIOS_PER_GROUP);
> +	}
> +
> +	chained_irq_exit(ic, desc);
> +}
> +
> +static void sf_gpio_reset_assert(void *data)
> +{
> +	reset_control_assert(data);
> +}
> +
> +static int sf_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sf_gpio_priv *priv;
> +	struct gpio_irq_chip *girq;
> +	struct gpio_chip *gc;
> +	u32 ngpios, ngroups;
> +	int ret, i;
> +
> +	ret = device_property_read_u32(dev, "ngpios", &ngpios);
> +	if (ret)
> +		return ret;
> +
> +	ngroups = DIV_ROUND_UP(ngpios, GPIOS_PER_GROUP);
> +	priv = devm_kzalloc(dev, struct_size(priv, irq, ngroups), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	priv->rstc = devm_reset_control_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(priv->rstc))
> +		return PTR_ERR(priv->rstc);
> +
> +	ret = reset_control_deassert(priv->rstc);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, sf_gpio_reset_assert, priv->rstc);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < ngroups; i++) {
> +		ret = platform_get_irq(pdev, i);
> +		if (ret < 0)
> +			return ret;
> +
> +		priv->irq[i] = ret;
> +	}
> +
> +	gc = &priv->gc;
> +	gc->label = KBUILD_MODNAME;
> +	gc->parent = dev;
> +	gc->owner = THIS_MODULE;
> +	gc->request = gpiochip_generic_request;
> +	gc->free = gpiochip_generic_free;
> +	gc->get_direction = sf_gpio_get_direction;
> +	gc->direction_input = sf_gpio_direction_input;
> +	gc->direction_output = sf_gpio_direction_output;
> +	gc->get = sf_gpio_get_value;
> +	gc->set = sf_gpio_set_value;
> +	gc->set_config = sf_gpio_set_config;
> +	gc->base = -1;
> +	gc->ngpio = ngpios;
> +
> +	girq = &gc->irq;
> +	gpio_irq_chip_set_chip(girq, &sf_gpio_irqchip);
> +	girq->num_parents = ngroups;
> +	girq->parents = priv->irq;
> +	girq->parent_handler = sf_gpio_irq_handler;
> +	girq->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_bad_irq;
> +
> +	return devm_gpiochip_add_data(dev, gc, priv);
> +}
> +
> +static const struct of_device_id sf_gpio_ids[] = {
> +	{ .compatible = "siflower,sf19a2890-gpio" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, sf_gpio_ids);
> +
> +static struct platform_driver sf_gpio_driver = {
> +	.probe		= sf_gpio_probe,
> +	.driver = {
> +		.name		= "siflower_gpio",
> +		.of_match_table	= sf_gpio_ids,
> +	},
> +};
> +module_platform_driver(sf_gpio_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Qingfang Deng <qingfang.deng@siflower.com.cn>");
> +MODULE_DESCRIPTION("GPIO driver for Siflower SoCs");
> 
> -- 
> Git-155)
> 
> 

Thanks,
Bartosz

Re: [PATCH 1/2] dt-bindings: gpio: Add Siflower SF19A2890 GPIO controller

[Joseph Telaak]

> If you're a co-developer, please add your Co-developed-by: prefixed with
> a list that looks more like:
> [...]
> The above is completely unreadable.

Thanks, v2 will use Co-developed-by with the changes listed that way.

On Wed, Nov 4, 2026 at 3:12 AM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Wed, Nov 04, 2026 at 02:21:37AM -0500, Joseph Telaak via B4 Relay wrote:
> > From: Chuanhong Guo <gch981213@gmail.com>
> >
> > Document the GPIO controller found on the Siflower SF19A2890 and other
> > Siflower MIPS and RISC-V SoCs.
> >
> > Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> > [jt: from Chuanhong's siflower-7.1 branch; Joseph Telaak as the binding's
> >  maintainer, GPL-2.0-only for the deprecated GPL-2.0 SPDX identifier,
> >  example indented by four spaces, commit message, subject]
> > Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> > ---
>
> If you're a co-developer, please add your Co-developed-by: prefixed with
> a list that looks more like:
>
> [Joseph Telaak:
>   - did foo
>   - did bar]
>
> The above is completely unreadable.
>
> Thanks,
> Bartosz
>

Re: [PATCH 2/2] gpio: Add Siflower GPIO controller driver

[Joseph Telaak]

> Same comment as for the DT binding patch.

Will do.

> More common pattern is to retrieve the address of priv with
> gpiochip_get_data(). You already pass that address to
> devm_gpiochip_add_data().

Thanks for the review. v2 will use gpiochip_get_data(), drop both else
branches and add the blank line after the header comment.

Re: [PATCH 2/2] gpio: Add Siflower GPIO controller driver

[Philipp Zabel]

On Mi, 2026-11-04 at 02:21 -0500, Joseph Telaak via B4 Relay wrote:
> From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> 
> Add a driver for the GPIO controller on Siflower SoCs.
> This controller is found on all current Siflower MIPS and RISC-V
> chips including SF19A2890, SF21A6826 and SF21H8898.
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> [jt: from Chuanhong's siflower-7.1 branch; depend on MACH_SIFLOWER_MIPS,
>  longer Kconfig help, assert the reset through a devm action instead of in
>  .remove, which ran before the GPIO chip was removed, drop .owner and the
>  unused drvdata, drop the irq_set_affinity callback, which reached the
>  parent through irq_to_desc() (not exported with SPARSE_IRQ), call
>  gpiochip_disable_irq() and gpiochip_enable_irq() from mask and unmask as
>  an immutable irqchip must, find the group by its parent IRQ instead of
>  assuming consecutive numbers, return 0 or 1 from .get, read ngpios with
>  device_property_read_u32(), linux/math64.h and sorted includes, "Siflower"
>  spelling, subject]
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> ---
>  drivers/gpio/Kconfig         |  11 ++
>  drivers/gpio/Makefile        |   1 +
>  drivers/gpio/gpio-siflower.c | 325 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 337 insertions(+)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 55e428002..6125a8452 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -721,6 +721,17 @@ config GPIO_SIFIVE
>  	help
>  	  Say yes here to support the GPIO device on SiFive SoCs.
>  
> +config GPIO_SIFLOWER
> +	tristate "Siflower GPIO support"
> +	depends on OF_GPIO
> +	depends on MACH_SIFLOWER_MIPS || COMPILE_TEST
> +	select GPIOLIB_IRQCHIP
> +	help
> +	  GPIO controller driver for Siflower MIPS and RISC-V SoCs
> +	  including SF19A2890, SF21A6826 and SF21H8898. Each line can be an
> +	  input or an output and raise an interrupt on either edge or level,
> +	  and inputs can be debounced in hardware.
> +
>  config GPIO_SIOX
>  	tristate "SIOX GPIO support"
>  	depends on SIOX
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 88f748846..bde618108 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -171,6 +171,7 @@ obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
>  obj-$(CONFIG_GPIO_SCH)			+= gpio-sch.o
>  obj-$(CONFIG_GPIO_SHARED_PROXY)		+= gpio-shared-proxy.o
>  obj-$(CONFIG_GPIO_SIFIVE)		+= gpio-sifive.o
> +obj-$(CONFIG_GPIO_SIFLOWER)		+= gpio-siflower.o
>  obj-$(CONFIG_GPIO_SIM)			+= gpio-sim.o
>  obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
>  obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
> diff --git a/drivers/gpio/gpio-siflower.c b/drivers/gpio/gpio-siflower.c
> new file mode 100644
> index 000000000..6eb560da7
> --- /dev/null
> +++ b/drivers/gpio/gpio-siflower.c
> @@ -0,0 +1,325 @@
[...]
> +struct sf_gpio_priv {
> +	struct gpio_chip gc;
> +	void __iomem *base;
> +	struct clk *clk;
> +	struct reset_control *rstc;

rstc seems to be unused outside of the probe function.

[...]
> +static void sf_gpio_reset_assert(void *data)
> +{
> +	reset_control_assert(data);
> +}
> +
> +static int sf_gpio_probe(struct platform_device *pdev)
> +{
[...]
> +	priv->rstc = devm_reset_control_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(priv->rstc))
> +		return PTR_ERR(priv->rstc);
> +
> +	ret = reset_control_deassert(priv->rstc);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, sf_gpio_reset_assert, priv->rstc);
> +	if (ret)
> +		return ret;

Consider replacing this with
devm_reset_control_get_optional_exclusive_deasserted().
rstc can be a local variable then.

Don't use devm_reset_control_get_optional().

regards
Philipp

Re: [PATCH 2/2] gpio: Add Siflower GPIO controller driver

[Joseph Telaak]

> Consider replacing this with
> devm_reset_control_get_optional_exclusive_deasserted().
> rstc can be a local variable then.
>
> Don't use devm_reset_control_get_optional().

Thanks, v2 will do this. The binding makes resets required, so it will
use devm_reset_control_get_exclusive_deasserted(), with rstc as a local
in probe.