linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Support GPIO controller of Loongson 2K0300 SoC
@ 2025-09-01 13:38 Yao Zi
  2025-09-01 13:38 ` [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of " Yao Zi
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Yao Zi @ 2025-09-01 13:38 UTC (permalink / raw)
  To: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Huacai Chen, WANG Xuerui,
	Philipp Zabel
  Cc: linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit, Yao Zi

This series adds support for Loongson 2K0300's GPIO controller. While
being mostly identical to previous implementation, its interrupt
functionality hasn't been implemented in gpio-loongson-64bit.c. PATCH 2
implements its interrupt support with an IRQCHIP, and the code could be
reused for other Loongson SoCs with similar interrupt functionality like
2K1500 and 2K2000.

Tested on CTCISZ Forever Pi, reading/writing GPIOs works correctly, and
both level and edge interrupts could be triggered.

The devicetree patch depends on series "Support reset controller of
Loongson 2K0300 SoC"[1] for a clean apply. Thanks for your time and
review.

Changed from v1:
- Rebase on top of gpio/for-next, adapt changes that convert
  gpio-loongson-64bit.c to use generic gpiochip.
- Collect review tags
- Link to v1: https://lore.kernel.org/all/20250816035027.11727-2-ziyao@disroot.org/

[1]: https://lore.kernel.org/all/20250816033327.11359-2-ziyao@disroot.org/

Yao Zi (3):
  dt-bindings: gpio: loongson: Document GPIO controller of 2K0300 SoC
  gpio: loongson-64bit: Add support for Loongson 2K0300 SoC
  LoongArch: dts: Add GPIO controller for Loongson 2K0300

 .../bindings/gpio/loongson,ls-gpio.yaml       |  28 ++-
 arch/loongarch/boot/dts/loongson-2k0300.dtsi  |  20 ++
 drivers/gpio/Kconfig                          |   1 +
 drivers/gpio/gpio-loongson-64bit.c            | 191 +++++++++++++++++-
 4 files changed, 232 insertions(+), 8 deletions(-)

-- 
2.50.1


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

* [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of 2K0300 SoC
  2025-09-01 13:38 [PATCH v2 0/3] Support GPIO controller of Loongson 2K0300 SoC Yao Zi
@ 2025-09-01 13:38 ` Yao Zi
  2025-09-01 14:22   ` Huacai Chen
  2025-09-01 13:38 ` [PATCH v2 2/3] gpio: loongson-64bit: Add support for Loongson " Yao Zi
  2025-09-01 13:38 ` [PATCH v2 3/3] LoongArch: dts: Add GPIO controller for Loongson 2K0300 Yao Zi
  2 siblings, 1 reply; 7+ messages in thread
From: Yao Zi @ 2025-09-01 13:38 UTC (permalink / raw)
  To: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Huacai Chen, WANG Xuerui,
	Philipp Zabel
  Cc: linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit, Yao Zi, Krzysztof Kozlowski

Loongson 2K0300 ships a GPIO controller whose input/output control logic
is similar to previous generation of SoCs. Additionally, it acts as an
interrupt-controller supporting both level and edge interrupts and has a
distinct reset signal.

Describe its compatible in devicetree. We enlarge the maximum value of
ngpios to 128, since the controller technically supports at most 128
pins, although only 106 are routed out of the package. Properties for
interrupt-controllers and resets are introduced and limited as 2K0300
only.

Signed-off-by: Yao Zi <ziyao@disroot.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/gpio/loongson,ls-gpio.yaml       | 28 ++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
index b68159600e2b..69852444df23 100644
--- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
@@ -14,6 +14,7 @@ properties:
     oneOf:
       - enum:
           - loongson,ls2k-gpio
+          - loongson,ls2k0300-gpio
           - loongson,ls2k0500-gpio0
           - loongson,ls2k0500-gpio1
           - loongson,ls2k2000-gpio0
@@ -36,7 +37,7 @@ properties:
 
   ngpios:
     minimum: 1
-    maximum: 64
+    maximum: 128
 
   "#gpio-cells":
     const: 2
@@ -49,6 +50,14 @@ properties:
     minItems: 1
     maxItems: 64
 
+  "#interrupt-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  resets:
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -58,6 +67,23 @@ required:
   - gpio-ranges
   - interrupts
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: loongson,ls2k0300-gpio
+    then:
+      required:
+        - "#interrupt-cells"
+        - interrupt-controller
+        - resets
+    else:
+      properties:
+        "#interrupts-cells": false
+        interrupt-controller: false
+        resets: false
+
 additionalProperties: false
 
 examples:
-- 
2.50.1


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

* [PATCH v2 2/3] gpio: loongson-64bit: Add support for Loongson 2K0300 SoC
  2025-09-01 13:38 [PATCH v2 0/3] Support GPIO controller of Loongson 2K0300 SoC Yao Zi
  2025-09-01 13:38 ` [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of " Yao Zi
@ 2025-09-01 13:38 ` Yao Zi
  2025-09-01 14:24   ` Huacai Chen
  2025-09-01 13:38 ` [PATCH v2 3/3] LoongArch: dts: Add GPIO controller for Loongson 2K0300 Yao Zi
  2 siblings, 1 reply; 7+ messages in thread
From: Yao Zi @ 2025-09-01 13:38 UTC (permalink / raw)
  To: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Huacai Chen, WANG Xuerui,
	Philipp Zabel
  Cc: linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit, Yao Zi

This controller's input and output logic is similar to previous
generations of SoCs. Additionally, it's capable of interrupt masking,
and could be configured to detect levels and edges, and is supplied with
a distinct reset signal.

The interrupt functionality is implemented through an irqchip, whose
operations are written with previous generation SoCs in mind and could
be reused. Since all Loongson SoCs with similar interrupt capability
(2K1500, 2K2000) support byte-control mode, these operations are for
byte-control mode only for simplicity.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/gpio/Kconfig               |   1 +
 drivers/gpio/gpio-loongson-64bit.c | 191 +++++++++++++++++++++++++++--
 2 files changed, 185 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a437fe652dbc..c55173643eb4 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -437,6 +437,7 @@ config GPIO_LOONGSON_64BIT
 	depends on LOONGARCH || COMPILE_TEST
 	depends on OF_GPIO
 	select GPIO_GENERIC
+	select GPIOLIB_IRQCHIP
 	help
 	  Say yes here to support the GPIO functionality of a number of
 	  Loongson series of chips. The Loongson GPIO controller supports
diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index 482e64ba9b42..7fb712e101ce 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -7,6 +7,8 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
 #include <linux/err.h>
@@ -14,6 +16,7 @@
 #include <linux/gpio/generic.h>
 #include <linux/platform_device.h>
 #include <linux/bitops.h>
+#include <linux/reset.h>
 #include <asm/types.h>
 
 enum loongson_gpio_mode {
@@ -28,6 +31,14 @@ struct loongson_gpio_chip_data {
 	unsigned int		out_offset;
 	unsigned int		in_offset;
 	unsigned int		inten_offset;
+	unsigned int		intpol_offset;
+	unsigned int		intedge_offset;
+	unsigned int		intclr_offset;
+	unsigned int		intsts_offset;
+	unsigned int		intdual_offset;
+	unsigned int		intr_num;
+	irq_flow_handler_t	irq_handler;
+	const struct irq_chip	*girqchip;
 };
 
 struct loongson_gpio_chip {
@@ -137,7 +148,140 @@ static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
 	return platform_get_irq(pdev, offset);
 }
 
-static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgpio,
+static void loongson_gpio_irq_ack(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	writeb(0x1, lgpio->reg_base + lgpio->chip_data->intclr_offset + hwirq);
+}
+
+static void loongson_gpio_irq_mask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	writeb(0x0, lgpio->reg_base + lgpio->chip_data->inten_offset + hwirq);
+}
+
+static void loongson_gpio_irq_unmask(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+
+	writeb(0x1, lgpio->reg_base + lgpio->chip_data->inten_offset + hwirq);
+}
+
+static int loongson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+	irq_hw_number_t hwirq = irqd_to_hwirq(data);
+	u8 pol = 0, edge = 0, dual = 0;
+
+	if ((type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH) {
+		edge = 1;
+		dual = 1;
+		irq_set_handler_locked(data, handle_edge_irq);
+	} else {
+		switch (type) {
+		case IRQ_TYPE_LEVEL_HIGH:
+			pol = 1;
+			fallthrough;
+		case IRQ_TYPE_LEVEL_LOW:
+			irq_set_handler_locked(data, handle_level_irq);
+			break;
+
+		case IRQ_TYPE_EDGE_RISING:
+			pol = 1;
+			fallthrough;
+		case IRQ_TYPE_EDGE_FALLING:
+			edge = 1;
+			irq_set_handler_locked(data, handle_edge_irq);
+			break;
+
+		default:
+			return -EINVAL;
+		};
+	}
+
+	writeb(pol, lgpio->reg_base + lgpio->chip_data->intpol_offset + hwirq);
+	writeb(edge, lgpio->reg_base + lgpio->chip_data->intedge_offset + hwirq);
+	writeb(dual, lgpio->reg_base + lgpio->chip_data->intdual_offset + hwirq);
+
+	return 0;
+}
+
+static void loongson_gpio_ls2k0300_irq_handler(struct irq_desc *desc)
+{
+	struct loongson_gpio_chip *lgpio = irq_desc_get_handler_data(desc);
+	struct irq_chip *girqchip = irq_desc_get_chip(desc);
+	int i;
+
+	chained_irq_enter(girqchip, desc);
+
+	for (i = 0; i < lgpio->chip.gc.ngpio; i++) {
+		/*
+		 * For the GPIO controller of 2K0300, interrupts status bits
+		 * may be wrongly set even if the corresponding interrupt is
+		 * disabled. Thus interrupt enable bits are checked along with
+		 * status bits to detect interrupts reliably.
+		 */
+		if (readb(lgpio->reg_base + lgpio->chip_data->intsts_offset + i) &&
+		    readb(lgpio->reg_base + lgpio->chip_data->inten_offset + i))
+			generic_handle_domain_irq(lgpio->chip.gc.irq.domain, i);
+	}
+
+	chained_irq_exit(girqchip, desc);
+}
+
+static const struct irq_chip loongson_gpio_ls2k0300_irqchip = {
+	.irq_ack	= loongson_gpio_irq_ack,
+	.irq_mask	= loongson_gpio_irq_mask,
+	.irq_unmask	= loongson_gpio_irq_unmask,
+	.irq_set_type	= loongson_gpio_irq_set_type,
+	.flags		= IRQCHIP_IMMUTABLE | IRQCHIP_SKIP_SET_WAKE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int loongson_gpio_init_irqchip(struct platform_device *pdev,
+				      struct loongson_gpio_chip *lgpio)
+{
+	const struct loongson_gpio_chip_data *data = lgpio->chip_data;
+	struct gpio_chip *chip = &lgpio->chip.gc;
+	int i;
+
+	chip->irq.default_type = IRQ_TYPE_NONE;
+	chip->irq.handler = handle_bad_irq;
+	chip->irq.parent_handler = data->irq_handler;
+	chip->irq.parent_handler_data = lgpio;
+	gpio_irq_chip_set_chip(&chip->irq, data->girqchip);
+
+	chip->irq.num_parents = data->intr_num;
+	chip->irq.parents = devm_kcalloc(&pdev->dev, data->intr_num,
+					 sizeof(*chip->irq.parents), GFP_KERNEL);
+	if (!chip->parent)
+		return -ENOMEM;
+
+	for (i = 0; i < data->intr_num; i++) {
+		chip->irq.parents[i] = platform_get_irq(pdev, i);
+		if (chip->irq.parents[i] < 0)
+			return dev_err_probe(&pdev->dev, chip->irq.parents[i],
+					     "failed to get IRQ %d\n", i);
+	}
+
+	for (i = 0; i < data->intr_num; i++) {
+		writeb(0x0, lgpio->reg_base + data->inten_offset + i);
+		writeb(0x1, lgpio->reg_base + data->intclr_offset + i);
+	}
+
+	return 0;
+}
+
+static int loongson_gpio_init(struct platform_device *pdev, struct loongson_gpio_chip *lgpio,
 			      void __iomem *reg_base)
 {
 	struct gpio_generic_chip_config config;
@@ -146,7 +290,7 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
 	lgpio->reg_base = reg_base;
 	if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
 		config = (typeof(config)){
-			.dev = dev,
+			.dev = &pdev->dev,
 			.sz = 8,
 			.dat = lgpio->reg_base + lgpio->chip_data->in_offset,
 			.set = lgpio->reg_base + lgpio->chip_data->out_offset,
@@ -155,7 +299,7 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
 
 		ret = gpio_generic_chip_init(&lgpio->chip, &config);
 		if (ret) {
-			dev_err(dev, "unable to init generic GPIO\n");
+			dev_err(&pdev->dev, "unable to init generic GPIO\n");
 			return ret;
 		}
 	} else {
@@ -164,16 +308,22 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
 		lgpio->chip.gc.get_direction = loongson_gpio_get_direction;
 		lgpio->chip.gc.direction_output = loongson_gpio_direction_output;
 		lgpio->chip.gc.set = loongson_gpio_set;
-		lgpio->chip.gc.parent = dev;
+		lgpio->chip.gc.parent = &pdev->dev;
 		spin_lock_init(&lgpio->lock);
 	}
 
 	lgpio->chip.gc.label = lgpio->chip_data->label;
 	lgpio->chip.gc.can_sleep = false;
-	if (lgpio->chip_data->inten_offset)
+	if (lgpio->chip_data->girqchip) {
+		ret = loongson_gpio_init_irqchip(pdev, lgpio);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "failed to initialize irqchip\n");
+	} else if (lgpio->chip_data->inten_offset) {
 		lgpio->chip.gc.to_irq = loongson_gpio_to_irq;
+	}
 
-	return devm_gpiochip_add_data(dev, &lgpio->chip.gc, lgpio);
+	return devm_gpiochip_add_data(&pdev->dev, &lgpio->chip.gc, lgpio);
 }
 
 static int loongson_gpio_probe(struct platform_device *pdev)
@@ -181,6 +331,7 @@ static int loongson_gpio_probe(struct platform_device *pdev)
 	void __iomem *reg_base;
 	struct loongson_gpio_chip *lgpio;
 	struct device *dev = &pdev->dev;
+	struct reset_control *rst;
 
 	lgpio = devm_kzalloc(dev, sizeof(*lgpio), GFP_KERNEL);
 	if (!lgpio)
@@ -192,7 +343,12 @@ static int loongson_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(reg_base))
 		return PTR_ERR(reg_base);
 
-	return loongson_gpio_init(dev, lgpio, reg_base);
+	rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, NULL);
+	if (IS_ERR(rst))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rst),
+				     "failed to get reset control\n");
+
+	return loongson_gpio_init(pdev, lgpio, reg_base);
 }
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k_data = {
@@ -204,6 +360,23 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k_data = {
 	.inten_offset = 0x30,
 };
 
+static const struct loongson_gpio_chip_data loongson_gpio_ls2k0300_data = {
+	.label = "ls2k0300_gpio",
+	.mode = BYTE_CTRL_MODE,
+	.conf_offset = 0x800,
+	.in_offset = 0xa00,
+	.out_offset = 0x900,
+	.inten_offset = 0xb00,
+	.intpol_offset = 0xc00,
+	.intedge_offset = 0xd00,
+	.intclr_offset = 0xe00,
+	.intsts_offset = 0xf00,
+	.intdual_offset = 0xf80,
+	.intr_num = 7,
+	.irq_handler = loongson_gpio_ls2k0300_irq_handler,
+	.girqchip = &loongson_gpio_ls2k0300_irqchip,
+};
+
 static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data0 = {
 	.label = "ls2k0500_gpio",
 	.mode = BIT_CTRL_MODE,
@@ -300,6 +473,10 @@ static const struct of_device_id loongson_gpio_of_match[] = {
 		.compatible = "loongson,ls2k-gpio",
 		.data = &loongson_gpio_ls2k_data,
 	},
+	{
+		.compatible = "loongson,ls2k0300-gpio",
+		.data = &loongson_gpio_ls2k0300_data,
+	},
 	{
 		.compatible = "loongson,ls2k0500-gpio0",
 		.data = &loongson_gpio_ls2k0500_data0,
-- 
2.50.1


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

* [PATCH v2 3/3] LoongArch: dts: Add GPIO controller for Loongson 2K0300
  2025-09-01 13:38 [PATCH v2 0/3] Support GPIO controller of Loongson 2K0300 SoC Yao Zi
  2025-09-01 13:38 ` [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of " Yao Zi
  2025-09-01 13:38 ` [PATCH v2 2/3] gpio: loongson-64bit: Add support for Loongson " Yao Zi
@ 2025-09-01 13:38 ` Yao Zi
  2 siblings, 0 replies; 7+ messages in thread
From: Yao Zi @ 2025-09-01 13:38 UTC (permalink / raw)
  To: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Huacai Chen, WANG Xuerui,
	Philipp Zabel
  Cc: linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit, Yao Zi

Describe Loongson 2K0300's GPIO controller in devicetree.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/loongarch/boot/dts/loongson-2k0300.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/loongarch/boot/dts/loongson-2k0300.dtsi b/arch/loongarch/boot/dts/loongson-2k0300.dtsi
index 835d3c63537b..60a055b05c44 100644
--- a/arch/loongarch/boot/dts/loongson-2k0300.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k0300.dtsi
@@ -240,6 +240,26 @@ uart9: serial@16102400 {
 			status = "disabled";
 		};
 
+		gpio: gpio@16104000 {
+			compatible = "loongson,ls2k0300-gpio";
+			reg = <0x0 0x16104000 0x0 0x4000>;
+			interrupt-parent = <&liointc1>;
+			interrupts = <21 IRQ_TYPE_LEVEL_HIGH>,
+				     <22 IRQ_TYPE_LEVEL_HIGH>,
+				     <23 IRQ_TYPE_LEVEL_HIGH>,
+				     <24 IRQ_TYPE_LEVEL_HIGH>,
+				     <25 IRQ_TYPE_LEVEL_HIGH>,
+				     <26 IRQ_TYPE_LEVEL_HIGH>,
+				     <27 IRQ_TYPE_LEVEL_HIGH>;
+			ngpios = <106>;
+			gpio-controller;
+			gpio-ranges = <&pinctrl 0 0 106>;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			resets = <&rst RST_GPIO>;
+		};
+
 		isa@16400000 {
 			compatible = "isa";
 			#address-cells = <2>;
-- 
2.50.1


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

* Re: [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of 2K0300 SoC
  2025-09-01 13:38 ` [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of " Yao Zi
@ 2025-09-01 14:22   ` Huacai Chen
  2025-09-01 14:58     ` Yao Zi
  0 siblings, 1 reply; 7+ messages in thread
From: Huacai Chen @ 2025-09-01 14:22 UTC (permalink / raw)
  To: Yao Zi
  Cc: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, WANG Xuerui, Philipp Zabel,
	linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit, Krzysztof Kozlowski

Hi, Yao,

On Mon, Sep 1, 2025 at 9:38 PM Yao Zi <ziyao@disroot.org> wrote:
>
> Loongson 2K0300 ships a GPIO controller whose input/output control logic
> is similar to previous generation of SoCs. Additionally, it acts as an
> interrupt-controller supporting both level and edge interrupts and has a
> distinct reset signal.
>
> Describe its compatible in devicetree. We enlarge the maximum value of
> ngpios to 128, since the controller technically supports at most 128
> pins, although only 106 are routed out of the package. Properties for
> interrupt-controllers and resets are introduced and limited as 2K0300
> only.
Replace the full name with "Loongson-2K0300" and short name with
"LS2K0300", others look good to me.
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>

>
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/gpio/loongson,ls-gpio.yaml       | 28 ++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> index b68159600e2b..69852444df23 100644
> --- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> @@ -14,6 +14,7 @@ properties:
>      oneOf:
>        - enum:
>            - loongson,ls2k-gpio
> +          - loongson,ls2k0300-gpio
>            - loongson,ls2k0500-gpio0
>            - loongson,ls2k0500-gpio1
>            - loongson,ls2k2000-gpio0
> @@ -36,7 +37,7 @@ properties:
>
>    ngpios:
>      minimum: 1
> -    maximum: 64
> +    maximum: 128
>
>    "#gpio-cells":
>      const: 2
> @@ -49,6 +50,14 @@ properties:
>      minItems: 1
>      maxItems: 64
>
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  resets:
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - reg
> @@ -58,6 +67,23 @@ required:
>    - gpio-ranges
>    - interrupts
>
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: loongson,ls2k0300-gpio
> +    then:
> +      required:
> +        - "#interrupt-cells"
> +        - interrupt-controller
> +        - resets
> +    else:
> +      properties:
> +        "#interrupts-cells": false
> +        interrupt-controller: false
> +        resets: false
> +
>  additionalProperties: false
>
>  examples:
> --
> 2.50.1
>
>

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

* Re: [PATCH v2 2/3] gpio: loongson-64bit: Add support for Loongson 2K0300 SoC
  2025-09-01 13:38 ` [PATCH v2 2/3] gpio: loongson-64bit: Add support for Loongson " Yao Zi
@ 2025-09-01 14:24   ` Huacai Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Huacai Chen @ 2025-09-01 14:24 UTC (permalink / raw)
  To: Yao Zi
  Cc: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, WANG Xuerui, Philipp Zabel,
	linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit

Hi, Yao,

On Mon, Sep 1, 2025 at 9:38 PM Yao Zi <ziyao@disroot.org> wrote:
>
> This controller's input and output logic is similar to previous
> generations of SoCs. Additionally, it's capable of interrupt masking,
> and could be configured to detect levels and edges, and is supplied with
> a distinct reset signal.
>
> The interrupt functionality is implemented through an irqchip, whose
> operations are written with previous generation SoCs in mind and could
> be reused. Since all Loongson SoCs with similar interrupt capability
> (2K1500, 2K2000) support byte-control mode, these operations are for
> byte-control mode only for simplicity.
Modify the name style the same as in Patch-1.

>
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/gpio/Kconfig               |   1 +
>  drivers/gpio/gpio-loongson-64bit.c | 191 +++++++++++++++++++++++++++--
>  2 files changed, 185 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index a437fe652dbc..c55173643eb4 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -437,6 +437,7 @@ config GPIO_LOONGSON_64BIT
>         depends on LOONGARCH || COMPILE_TEST
>         depends on OF_GPIO
>         select GPIO_GENERIC
> +       select GPIOLIB_IRQCHIP
>         help
>           Say yes here to support the GPIO functionality of a number of
>           Loongson series of chips. The Loongson GPIO controller supports
> diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
> index 482e64ba9b42..7fb712e101ce 100644
> --- a/drivers/gpio/gpio-loongson-64bit.c
> +++ b/drivers/gpio/gpio-loongson-64bit.c
> @@ -7,6 +7,8 @@
>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/irqdesc.h>
>  #include <linux/module.h>
>  #include <linux/spinlock.h>
>  #include <linux/err.h>
> @@ -14,6 +16,7 @@
>  #include <linux/gpio/generic.h>
>  #include <linux/platform_device.h>
>  #include <linux/bitops.h>
> +#include <linux/reset.h>
>  #include <asm/types.h>
>
>  enum loongson_gpio_mode {
> @@ -28,6 +31,14 @@ struct loongson_gpio_chip_data {
>         unsigned int            out_offset;
>         unsigned int            in_offset;
>         unsigned int            inten_offset;
> +       unsigned int            intpol_offset;
> +       unsigned int            intedge_offset;
> +       unsigned int            intclr_offset;
> +       unsigned int            intsts_offset;
> +       unsigned int            intdual_offset;
> +       unsigned int            intr_num;
> +       irq_flow_handler_t      irq_handler;
> +       const struct irq_chip   *girqchip;
>  };
>
>  struct loongson_gpio_chip {
> @@ -137,7 +148,140 @@ static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
>         return platform_get_irq(pdev, offset);
>  }
>
> -static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgpio,
> +static void loongson_gpio_irq_ack(struct irq_data *data)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +
> +       writeb(0x1, lgpio->reg_base + lgpio->chip_data->intclr_offset + hwirq);
> +}
> +
> +static void loongson_gpio_irq_mask(struct irq_data *data)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +
> +       writeb(0x0, lgpio->reg_base + lgpio->chip_data->inten_offset + hwirq);
> +}
> +
> +static void loongson_gpio_irq_unmask(struct irq_data *data)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +
> +       writeb(0x1, lgpio->reg_base + lgpio->chip_data->inten_offset + hwirq);
> +}
> +
> +static int loongson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> +       struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +       irq_hw_number_t hwirq = irqd_to_hwirq(data);
> +       u8 pol = 0, edge = 0, dual = 0;
> +
> +       if ((type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH) {
> +               edge = 1;
> +               dual = 1;
> +               irq_set_handler_locked(data, handle_edge_irq);
> +       } else {
> +               switch (type) {
> +               case IRQ_TYPE_LEVEL_HIGH:
> +                       pol = 1;
> +                       fallthrough;
> +               case IRQ_TYPE_LEVEL_LOW:
> +                       irq_set_handler_locked(data, handle_level_irq);
> +                       break;
> +
> +               case IRQ_TYPE_EDGE_RISING:
> +                       pol = 1;
> +                       fallthrough;
> +               case IRQ_TYPE_EDGE_FALLING:
> +                       edge = 1;
> +                       irq_set_handler_locked(data, handle_edge_irq);
> +                       break;
> +
> +               default:
> +                       return -EINVAL;
> +               };
> +       }
> +
> +       writeb(pol, lgpio->reg_base + lgpio->chip_data->intpol_offset + hwirq);
> +       writeb(edge, lgpio->reg_base + lgpio->chip_data->intedge_offset + hwirq);
> +       writeb(dual, lgpio->reg_base + lgpio->chip_data->intdual_offset + hwirq);
> +
> +       return 0;
> +}
> +
> +static void loongson_gpio_ls2k0300_irq_handler(struct irq_desc *desc)
> +{
> +       struct loongson_gpio_chip *lgpio = irq_desc_get_handler_data(desc);
> +       struct irq_chip *girqchip = irq_desc_get_chip(desc);
> +       int i;
> +
> +       chained_irq_enter(girqchip, desc);
> +
> +       for (i = 0; i < lgpio->chip.gc.ngpio; i++) {
> +               /*
> +                * For the GPIO controller of 2K0300, interrupts status bits
s/2K0300/LS2K0300/g.

> +                * may be wrongly set even if the corresponding interrupt is
> +                * disabled. Thus interrupt enable bits are checked along with
> +                * status bits to detect interrupts reliably.
> +                */
> +               if (readb(lgpio->reg_base + lgpio->chip_data->intsts_offset + i) &&
> +                   readb(lgpio->reg_base + lgpio->chip_data->inten_offset + i))
> +                       generic_handle_domain_irq(lgpio->chip.gc.irq.domain, i);
> +       }
> +
> +       chained_irq_exit(girqchip, desc);
> +}
> +
> +static const struct irq_chip loongson_gpio_ls2k0300_irqchip = {
> +       .irq_ack        = loongson_gpio_irq_ack,
> +       .irq_mask       = loongson_gpio_irq_mask,
> +       .irq_unmask     = loongson_gpio_irq_unmask,
> +       .irq_set_type   = loongson_gpio_irq_set_type,
> +       .flags          = IRQCHIP_IMMUTABLE | IRQCHIP_SKIP_SET_WAKE,
> +       GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +static int loongson_gpio_init_irqchip(struct platform_device *pdev,
> +                                     struct loongson_gpio_chip *lgpio)
> +{
> +       const struct loongson_gpio_chip_data *data = lgpio->chip_data;
> +       struct gpio_chip *chip = &lgpio->chip.gc;
> +       int i;
> +
> +       chip->irq.default_type = IRQ_TYPE_NONE;
> +       chip->irq.handler = handle_bad_irq;
> +       chip->irq.parent_handler = data->irq_handler;
> +       chip->irq.parent_handler_data = lgpio;
> +       gpio_irq_chip_set_chip(&chip->irq, data->girqchip);
> +
> +       chip->irq.num_parents = data->intr_num;
> +       chip->irq.parents = devm_kcalloc(&pdev->dev, data->intr_num,
> +                                        sizeof(*chip->irq.parents), GFP_KERNEL);
> +       if (!chip->parent)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < data->intr_num; i++) {
> +               chip->irq.parents[i] = platform_get_irq(pdev, i);
> +               if (chip->irq.parents[i] < 0)
> +                       return dev_err_probe(&pdev->dev, chip->irq.parents[i],
> +                                            "failed to get IRQ %d\n", i);
> +       }
> +
> +       for (i = 0; i < data->intr_num; i++) {
> +               writeb(0x0, lgpio->reg_base + data->inten_offset + i);
> +               writeb(0x1, lgpio->reg_base + data->intclr_offset + i);
> +       }
> +
> +       return 0;
> +}
> +
> +static int loongson_gpio_init(struct platform_device *pdev, struct loongson_gpio_chip *lgpio,
>                               void __iomem *reg_base)
>  {
>         struct gpio_generic_chip_config config;
> @@ -146,7 +290,7 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
>         lgpio->reg_base = reg_base;
>         if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
>                 config = (typeof(config)){
> -                       .dev = dev,
> +                       .dev = &pdev->dev,
>                         .sz = 8,
>                         .dat = lgpio->reg_base + lgpio->chip_data->in_offset,
>                         .set = lgpio->reg_base + lgpio->chip_data->out_offset,
> @@ -155,7 +299,7 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
>
>                 ret = gpio_generic_chip_init(&lgpio->chip, &config);
>                 if (ret) {
> -                       dev_err(dev, "unable to init generic GPIO\n");
> +                       dev_err(&pdev->dev, "unable to init generic GPIO\n");
>                         return ret;
>                 }
>         } else {
> @@ -164,16 +308,22 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
>                 lgpio->chip.gc.get_direction = loongson_gpio_get_direction;
>                 lgpio->chip.gc.direction_output = loongson_gpio_direction_output;
>                 lgpio->chip.gc.set = loongson_gpio_set;
> -               lgpio->chip.gc.parent = dev;
> +               lgpio->chip.gc.parent = &pdev->dev;
>                 spin_lock_init(&lgpio->lock);
>         }
>
>         lgpio->chip.gc.label = lgpio->chip_data->label;
>         lgpio->chip.gc.can_sleep = false;
> -       if (lgpio->chip_data->inten_offset)
> +       if (lgpio->chip_data->girqchip) {
> +               ret = loongson_gpio_init_irqchip(pdev, lgpio);
> +               if (ret)
> +                       return dev_err_probe(&pdev->dev, ret,
> +                                            "failed to initialize irqchip\n");
One line is enough.

> +       } else if (lgpio->chip_data->inten_offset) {
>                 lgpio->chip.gc.to_irq = loongson_gpio_to_irq;
> +       }
>
> -       return devm_gpiochip_add_data(dev, &lgpio->chip.gc, lgpio);
> +       return devm_gpiochip_add_data(&pdev->dev, &lgpio->chip.gc, lgpio);
>  }
>
>  static int loongson_gpio_probe(struct platform_device *pdev)
> @@ -181,6 +331,7 @@ static int loongson_gpio_probe(struct platform_device *pdev)
>         void __iomem *reg_base;
>         struct loongson_gpio_chip *lgpio;
>         struct device *dev = &pdev->dev;
> +       struct reset_control *rst;
>
>         lgpio = devm_kzalloc(dev, sizeof(*lgpio), GFP_KERNEL);
>         if (!lgpio)
> @@ -192,7 +343,12 @@ static int loongson_gpio_probe(struct platform_device *pdev)
>         if (IS_ERR(reg_base))
>                 return PTR_ERR(reg_base);
>
> -       return loongson_gpio_init(dev, lgpio, reg_base);
> +       rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, NULL);
> +       if (IS_ERR(rst))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(rst),
> +                                    "failed to get reset control\n");
One line is enough.

With above changes,
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>

> +
> +       return loongson_gpio_init(pdev, lgpio, reg_base);
>  }
>
>  static const struct loongson_gpio_chip_data loongson_gpio_ls2k_data = {
> @@ -204,6 +360,23 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k_data = {
>         .inten_offset = 0x30,
>  };
>
> +static const struct loongson_gpio_chip_data loongson_gpio_ls2k0300_data = {
> +       .label = "ls2k0300_gpio",
> +       .mode = BYTE_CTRL_MODE,
> +       .conf_offset = 0x800,
> +       .in_offset = 0xa00,
> +       .out_offset = 0x900,
> +       .inten_offset = 0xb00,
> +       .intpol_offset = 0xc00,
> +       .intedge_offset = 0xd00,
> +       .intclr_offset = 0xe00,
> +       .intsts_offset = 0xf00,
> +       .intdual_offset = 0xf80,
> +       .intr_num = 7,
> +       .irq_handler = loongson_gpio_ls2k0300_irq_handler,
> +       .girqchip = &loongson_gpio_ls2k0300_irqchip,
> +};
> +
>  static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data0 = {
>         .label = "ls2k0500_gpio",
>         .mode = BIT_CTRL_MODE,
> @@ -300,6 +473,10 @@ static const struct of_device_id loongson_gpio_of_match[] = {
>                 .compatible = "loongson,ls2k-gpio",
>                 .data = &loongson_gpio_ls2k_data,
>         },
> +       {
> +               .compatible = "loongson,ls2k0300-gpio",
> +               .data = &loongson_gpio_ls2k0300_data,
> +       },
>         {
>                 .compatible = "loongson,ls2k0500-gpio0",
>                 .data = &loongson_gpio_ls2k0500_data0,
> --
> 2.50.1
>

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

* Re: [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of 2K0300 SoC
  2025-09-01 14:22   ` Huacai Chen
@ 2025-09-01 14:58     ` Yao Zi
  0 siblings, 0 replies; 7+ messages in thread
From: Yao Zi @ 2025-09-01 14:58 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Yinbo Zhu, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, WANG Xuerui, Philipp Zabel,
	linux-gpio, devicetree, linux-kernel, loongarch, Mingcong Bai,
	Kexy Biscuit, Krzysztof Kozlowski

On Mon, Sep 01, 2025 at 10:22:04PM +0800, Huacai Chen wrote:
> Hi, Yao,
> 
> On Mon, Sep 1, 2025 at 9:38 PM Yao Zi <ziyao@disroot.org> wrote:
> >
> > Loongson 2K0300 ships a GPIO controller whose input/output control logic
> > is similar to previous generation of SoCs. Additionally, it acts as an
> > interrupt-controller supporting both level and edge interrupts and has a
> > distinct reset signal.
> >
> > Describe its compatible in devicetree. We enlarge the maximum value of
> > ngpios to 128, since the controller technically supports at most 128
> > pins, although only 106 are routed out of the package. Properties for
> > interrupt-controllers and resets are introduced and limited as 2K0300
> > only.
> Replace the full name with "Loongson-2K0300" and short name with
> "LS2K0300", others look good to me.
> Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>

I don't understand what is improved with this. For the fullname, I don't
see any difference between "Loongson 2K0300" and "Loongson-2K0300". And
for the short one, omitting the "LS" prefix doesn't introduce any
ambiguity, either.

I did a quick search through git log, and found many commits for Loongson
2K SoCs do include a hyphen for the fullname and "LS" prefix for
abbreviation in messages, while some merge commits don't do so.

Even the official production page for Loongson 2K0300 refers to it
without a hyphen[1]. Thus I cannot find out of the point of rewording...

> Loongson 2K0300 is a multi-function SoC build upon the LA264 processor
> core, ...

I'll appreciate and adapt it if you could explain why the change is
necessary. Thanks for your patience.

Best regards,
Yao Zi

[1]: https://loongson.cn/EN/product/show?id=35

> >
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > ---
> >  .../bindings/gpio/loongson,ls-gpio.yaml       | 28 ++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> > index b68159600e2b..69852444df23 100644
> > --- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> > +++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> > @@ -14,6 +14,7 @@ properties:
> >      oneOf:
> >        - enum:
> >            - loongson,ls2k-gpio
> > +          - loongson,ls2k0300-gpio
> >            - loongson,ls2k0500-gpio0
> >            - loongson,ls2k0500-gpio1
> >            - loongson,ls2k2000-gpio0
> > @@ -36,7 +37,7 @@ properties:
> >
> >    ngpios:
> >      minimum: 1
> > -    maximum: 64
> > +    maximum: 128
> >
> >    "#gpio-cells":
> >      const: 2
> > @@ -49,6 +50,14 @@ properties:
> >      minItems: 1
> >      maxItems: 64
> >
> > +  "#interrupt-cells":
> > +    const: 2
> > +
> > +  interrupt-controller: true
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -58,6 +67,23 @@ required:
> >    - gpio-ranges
> >    - interrupts
> >
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: loongson,ls2k0300-gpio
> > +    then:
> > +      required:
> > +        - "#interrupt-cells"
> > +        - interrupt-controller
> > +        - resets
> > +    else:
> > +      properties:
> > +        "#interrupts-cells": false
> > +        interrupt-controller: false
> > +        resets: false
> > +
> >  additionalProperties: false
> >
> >  examples:
> > --
> > 2.50.1
> >
> >
> 

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

end of thread, other threads:[~2025-09-01 14:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 13:38 [PATCH v2 0/3] Support GPIO controller of Loongson 2K0300 SoC Yao Zi
2025-09-01 13:38 ` [PATCH v2 1/3] dt-bindings: gpio: loongson: Document GPIO controller of " Yao Zi
2025-09-01 14:22   ` Huacai Chen
2025-09-01 14:58     ` Yao Zi
2025-09-01 13:38 ` [PATCH v2 2/3] gpio: loongson-64bit: Add support for Loongson " Yao Zi
2025-09-01 14:24   ` Huacai Chen
2025-09-01 13:38 ` [PATCH v2 3/3] LoongArch: dts: Add GPIO controller for Loongson 2K0300 Yao Zi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).