[PATCH v4 0/2] gpio: loongson: add more gpio chip support

[PATCH v4 0/2] gpio: loongson: add more gpio chip support

[Yinbo Zhu]

This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
driver support.

Change in v4:
		1. Reword the title and commit log information.
		2. Remove the offset parse in DT and add it in of_device_id and
		   acpi_device_id's data field.
		3. Add more gpio chip dt-bindings support in yaml file.
Change in v3:
		1. Reword the dt-bindings patch commit log information.
		2. Add "loongson,ls2k1000-gpio" compatible.
Change in v2:
		1. Reword the patch commit log information.
		2. Add some GPIO register offset description in yaml.

Yinbo Zhu (2):
  gpio: dt-bindings: add more loongson gpio chip support
  gpio: loongson: add more gpio chip support

 .../bindings/gpio/loongson,ls-gpio.yaml       |  23 ++-
 drivers/gpio/gpio-loongson-64bit.c            | 136 ++++++++++++++++--
 2 files changed, 147 insertions(+), 12 deletions(-)

-- 
2.20.1

[PATCH v4 1/2] gpio: dt-bindings: add more loongson gpio chip support

[Yinbo Zhu]

This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
dt-bindings support in yaml file.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
 .../bindings/gpio/loongson,ls-gpio.yaml       | 23 +++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
index fb86e8ce6349..97472f1529a0 100644
--- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
@@ -11,9 +11,24 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - loongson,ls2k-gpio
-      - loongson,ls7a-gpio
+    oneOf:
+      - enum:
+          - loongson,ls2k-gpio
+          - loongson,ls2k0500-gpio0
+          - loongson,ls2k0500-gpio1
+          - loongson,ls2k2000-gpio0
+          - loongson,ls2k2000-gpio1
+          - loongson,ls2k2000-gpio2
+          - loongson,ls3a5000-gpio
+          - loongson,ls7a-gpio
+      - items:
+          - enum:
+              - loongson,ls2k1000-gpio
+          - const: loongson,ls2k-gpio
+      - items:
+          - enum:
+              - loongson,ls7a1000-gpio
+          - const: loongson,ls7a-gpio
 
   reg:
     maxItems: 1
@@ -49,7 +64,7 @@ examples:
     #include <dt-bindings/interrupt-controller/irq.h>
 
     gpio0: gpio@1fe00500 {
-      compatible = "loongson,ls2k-gpio";
+      compatible = "loongson,ls2k1000-gpio", "loongson,ls2k-gpio";
       reg = <0x1fe00500 0x38>;
       ngpios = <64>;
       #gpio-cells = <2>;
-- 
2.20.1

Re: [PATCH v4 1/2] gpio: dt-bindings: add more loongson gpio chip support

[Linus Walleij]

On Wed, Aug 23, 2023 at 5:34 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:

> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
> dt-bindings support in yaml file.
>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>

Thanks Yinbo,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v4 1/2] gpio: dt-bindings: add more loongson gpio chip support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On Wed, Aug 23, 2023 at 11:34:26AM +0800, Yinbo Zhu wrote:
> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
> dt-bindings support in yaml file.
> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  .../bindings/gpio/loongson,ls-gpio.yaml       | 23 +++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> index fb86e8ce6349..97472f1529a0 100644
> --- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> @@ -11,9 +11,24 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - loongson,ls2k-gpio
> -      - loongson,ls7a-gpio
> +    oneOf:
> +      - enum:
> +          - loongson,ls2k-gpio
> +          - loongson,ls2k0500-gpio0
> +          - loongson,ls2k0500-gpio1
> +          - loongson,ls2k2000-gpio0
> +          - loongson,ls2k2000-gpio1
> +          - loongson,ls2k2000-gpio2
> +          - loongson,ls3a5000-gpio
> +          - loongson,ls7a-gpio

> +      - items:
> +          - enum:
> +              - loongson,ls2k1000-gpio
> +          - const: loongson,ls2k-gpio
> +      - items:
> +          - enum:
> +              - loongson,ls7a1000-gpio
> +          - const: loongson,ls7a-gpio

Are there going to be more controllers that are compatible with
"ls7a-gpio"? If not, you can simplify both of these to have 2 const:
entries, like:

      - items:
          - const: loongson,ls2k1000-gpio
          - const: loongson,ls2k-gpio

      - items:
          - const: loongson,ls7a1000-gpio
          - const: loongson,ls7a-gpio

Either way,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks for sticking at this one :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/2] gpio: dt-bindings: add more loongson gpio chip support

[Yinbo Zhu]

在 2023/8/23 下午11:08, Conor Dooley 写道:
> On Wed, Aug 23, 2023 at 11:34:26AM +0800, Yinbo Zhu wrote:
>> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
>> dt-bindings support in yaml file.
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> ---
>>   .../bindings/gpio/loongson,ls-gpio.yaml       | 23 +++++++++++++++----
>>   1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
>> index fb86e8ce6349..97472f1529a0 100644
>> --- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
>> +++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
>> @@ -11,9 +11,24 @@ maintainers:
>>   
>>   properties:
>>     compatible:
>> -    enum:
>> -      - loongson,ls2k-gpio
>> -      - loongson,ls7a-gpio
>> +    oneOf:
>> +      - enum:
>> +          - loongson,ls2k-gpio
>> +          - loongson,ls2k0500-gpio0
>> +          - loongson,ls2k0500-gpio1
>> +          - loongson,ls2k2000-gpio0
>> +          - loongson,ls2k2000-gpio1
>> +          - loongson,ls2k2000-gpio2
>> +          - loongson,ls3a5000-gpio
>> +          - loongson,ls7a-gpio
> 
>> +      - items:
>> +          - enum:
>> +              - loongson,ls2k1000-gpio
>> +          - const: loongson,ls2k-gpio
>> +      - items:
>> +          - enum:
>> +              - loongson,ls7a1000-gpio
>> +          - const: loongson,ls7a-gpio
> 
> Are there going to be more controllers that are compatible with
> "ls7a-gpio"? If not, you can simplify both of these to have 2 const:
> entries, like:
> 
>        - items:
>            - const: loongson,ls2k1000-gpio
>            - const: loongson,ls2k-gpio
> 
>        - items:
>            - const: loongson,ls7a1000-gpio
>            - const: loongson,ls7a-gpio
> 
> Either way,
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>


okay, I got it.

Thanks,
Yinbo
> 

[PATCH v4 2/2] gpio: loongson: add more gpio chip support

[Yinbo Zhu]

This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
driver support.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
 drivers/gpio/gpio-loongson-64bit.c | 136 +++++++++++++++++++++++++++--
 1 file changed, 128 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index 06213bbfabdd..8786d0a21bbb 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -23,9 +23,10 @@ enum loongson_gpio_mode {
 struct loongson_gpio_chip_data {
 	const char		*label;
 	enum loongson_gpio_mode	mode;
-	unsigned int		conf_offset;
-	unsigned int		out_offset;
-	unsigned int		in_offset;
+	u32			conf_offset;
+	u32			out_offset;
+	u32			in_offset;
+	u32			inten_offset;
 };
 
 struct loongson_gpio_chip {
@@ -117,7 +118,17 @@ static void loongson_gpio_set(struct gpio_chip *chip, unsigned int pin, int valu
 
 static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
 {
+	unsigned int u;
 	struct platform_device *pdev = to_platform_device(chip->parent);
+	struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
+
+	if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
+		u = readl(lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);
+		u |= BIT(offset % 32);
+		writel(u, lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);
+	} else {
+		writeb(1, lgpio->reg_base + lgpio->chip_data->inten_offset + offset);
+	}
 
 	return platform_get_irq(pdev, offset);
 }
@@ -127,11 +138,30 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
 {
 	int ret;
 	u32 ngpios;
+	unsigned int io_width;
 
 	lgpio->reg_base = reg_base;
+	if (device_property_read_u32(dev, "ngpios", &ngpios) || !ngpios)
+		return -EINVAL;
+
+	ret = DIV_ROUND_UP(ngpios, 8);
+	switch (ret) {
+	case 1 ... 2:
+		io_width = ret;
+		break;
+	case 3 ... 4:
+		io_width = 0x4;
+		break;
+	case 5 ... 8:
+		io_width = 0x8;
+		break;
+	default:
+		dev_err(dev, "unsupported io width\n");
+		return -EINVAL;
+	}
 
 	if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
-		ret = bgpio_init(&lgpio->chip, dev, 8,
+		ret = bgpio_init(&lgpio->chip, dev, io_width,
 				lgpio->reg_base + lgpio->chip_data->in_offset,
 				lgpio->reg_base + lgpio->chip_data->out_offset,
 				NULL, NULL,
@@ -151,12 +181,11 @@ static int loongson_gpio_init(struct device *dev, struct loongson_gpio_chip *lgp
 		spin_lock_init(&lgpio->lock);
 	}
 
-	device_property_read_u32(dev, "ngpios", &ngpios);
-
-	lgpio->chip.can_sleep = 0;
 	lgpio->chip.ngpio = ngpios;
 	lgpio->chip.label = lgpio->chip_data->label;
-	lgpio->chip.to_irq = loongson_gpio_to_irq;
+	lgpio->chip.can_sleep = 0;
+	if (lgpio->chip_data->inten_offset)
+		lgpio->chip.to_irq = loongson_gpio_to_irq;
 
 	return devm_gpiochip_add_data(dev, &lgpio->chip, lgpio);
 }
@@ -187,6 +216,57 @@ static const struct loongson_gpio_chip_data loongson_gpio_ls2k_data = {
 	.conf_offset = 0x0,
 	.in_offset = 0x20,
 	.out_offset = 0x10,
+	.inten_offset = 0x30,
+};
+
+static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data0 = {
+	.label = "ls2k0500_gpio",
+	.mode = BIT_CTRL_MODE,
+	.conf_offset = 0x0,
+	.in_offset = 0x8,
+	.out_offset = 0x10,
+	.inten_offset = 0xb0,
+};
+
+static const struct loongson_gpio_chip_data loongson_gpio_ls2k0500_data1 = {
+	.label = "ls2k0500_gpio",
+	.mode = BIT_CTRL_MODE,
+	.conf_offset = 0x0,
+	.in_offset = 0x8,
+	.out_offset = 0x10,
+	.inten_offset = 0x98,
+};
+
+static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data0 = {
+	.label = "ls2k2000_gpio",
+	.mode = BIT_CTRL_MODE,
+	.conf_offset = 0x0,
+	.in_offset = 0xc,
+	.out_offset = 0x8,
+};
+
+static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data1 = {
+	.label = "ls2k2000_gpio",
+	.mode = BIT_CTRL_MODE,
+	.conf_offset = 0x0,
+	.in_offset = 0x20,
+	.out_offset = 0x10,
+};
+
+static const struct loongson_gpio_chip_data loongson_gpio_ls2k2000_data2 = {
+	.label = "ls2k2000_gpio",
+	.mode = BIT_CTRL_MODE,
+	.conf_offset = 0x84,
+	.in_offset = 0x88,
+	.out_offset = 0x80,
+};
+
+static const struct loongson_gpio_chip_data loongson_gpio_ls3a5000_data = {
+	.label = "ls3a5000_gpio",
+	.mode = BIT_CTRL_MODE,
+	.conf_offset = 0x0,
+	.in_offset = 0xc,
+	.out_offset = 0x8,
 };
 
 static const struct loongson_gpio_chip_data loongson_gpio_ls7a_data = {
@@ -202,6 +282,30 @@ static const struct of_device_id loongson_gpio_of_match[] = {
 		.compatible = "loongson,ls2k-gpio",
 		.data = &loongson_gpio_ls2k_data,
 	},
+	{
+		.compatible = "loongson,ls2k0500-gpio0",
+		.data = &loongson_gpio_ls2k0500_data0,
+	},
+	{
+		.compatible = "loongson,ls2k0500-gpio1",
+		.data = &loongson_gpio_ls2k0500_data1,
+	},
+	{
+		.compatible = "loongson,ls2k2000-gpio0",
+		.data = &loongson_gpio_ls2k2000_data0,
+	},
+	{
+		.compatible = "loongson,ls2k2000-gpio1",
+		.data = &loongson_gpio_ls2k2000_data1,
+	},
+	{
+		.compatible = "loongson,ls2k2000-gpio2",
+		.data = &loongson_gpio_ls2k2000_data2,
+	},
+	{
+		.compatible = "loongson,ls3a5000-gpio",
+		.data = &loongson_gpio_ls3a5000_data,
+	},
 	{
 		.compatible = "loongson,ls7a-gpio",
 		.data = &loongson_gpio_ls7a_data,
@@ -215,6 +319,22 @@ static const struct acpi_device_id loongson_gpio_acpi_match[] = {
 		.id = "LOON0002",
 		.driver_data = (kernel_ulong_t)&loongson_gpio_ls7a_data,
 	},
+	{
+		.id = "LOON0007",
+		.driver_data = (kernel_ulong_t)&loongson_gpio_ls3a5000_data,
+	},
+	{
+		.id = "LOON000A",
+		.driver_data = (kernel_ulong_t)&loongson_gpio_ls2k2000_data0,
+	},
+	{
+		.id = "LOON000B",
+		.driver_data = (kernel_ulong_t)&loongson_gpio_ls2k2000_data1,
+	},
+	{
+		.id = "LOON000C",
+		.driver_data = (kernel_ulong_t)&loongson_gpio_ls2k2000_data2,
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(acpi, loongson_gpio_acpi_match);
-- 
2.20.1

Re: [PATCH v4 2/2] gpio: loongson: add more gpio chip support

[Linus Walleij]

Hi Yinbo,

thanks for the new patch, it's starting to look really good!
The main point with offsets in the match data is very nice.

On Wed, Aug 23, 2023 at 5:34 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:

> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
> driver support.
>
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
(...)


>  static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
>  {
> +       unsigned int u;
>         struct platform_device *pdev = to_platform_device(chip->parent);
> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
> +
> +       if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
> +               u = readl(lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);
> +               u |= BIT(offset % 32);
> +               writel(u, lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);

This offset / 32 * 4 is really hard to read.
What about

/* Get the register index from offset then multiply by bytes per register */
(offset / 32) * 4

>         lgpio->reg_base = reg_base;
> +       if (device_property_read_u32(dev, "ngpios", &ngpios) || !ngpios)
> +               return -EINVAL;
> +
> +       ret = DIV_ROUND_UP(ngpios, 8);
> +       switch (ret) {
> +       case 1 ... 2:
> +               io_width = ret;
> +               break;
> +       case 3 ... 4:
> +               io_width = 0x4;
> +               break;
> +       case 5 ... 8:
> +               io_width = 0x8;
> +               break;
> +       default:
> +               dev_err(dev, "unsupported io width\n");
> +               return -EINVAL;
> +       }

Is it really a good idea to infer the register width from ngpios?

What about just putting this into the struct loongson_gpio_chip_data
as well? Certainly it will be fixed for a certain device.

Yours,
Linus Walleij

Re: [PATCH v4 2/2] gpio: loongson: add more gpio chip support

[Yinbo Zhu]

在 2023/8/23 下午8:29, Linus Walleij 写道:
> Hi Yinbo,
> 
> thanks for the new patch, it's starting to look really good!
> The main point with offsets in the match data is very nice.
> 
> On Wed, Aug 23, 2023 at 5:34 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
> 
>> This patch was to add loongson 2k0500, 2k2000 and 3a5000 gpio chip
>> driver support.
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> (...)
> 
> 
>>   static int loongson_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
>>   {
>> +       unsigned int u;
>>          struct platform_device *pdev = to_platform_device(chip->parent);
>> +       struct loongson_gpio_chip *lgpio = to_loongson_gpio_chip(chip);
>> +
>> +       if (lgpio->chip_data->mode == BIT_CTRL_MODE) {
>> +               u = readl(lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);
>> +               u |= BIT(offset % 32);
>> +               writel(u, lgpio->reg_base + lgpio->chip_data->inten_offset + offset / 32 * 4);
> 
> This offset / 32 * 4 is really hard to read.
> What about
> 
> /* Get the register index from offset then multiply by bytes per register */
> (offset / 32) * 4


okay, I will add it.

> 
>>          lgpio->reg_base = reg_base;
>> +       if (device_property_read_u32(dev, "ngpios", &ngpios) || !ngpios)
>> +               return -EINVAL;
>> +
>> +       ret = DIV_ROUND_UP(ngpios, 8);
>> +       switch (ret) {
>> +       case 1 ... 2:
>> +               io_width = ret;
>> +               break;
>> +       case 3 ... 4:
>> +               io_width = 0x4;
>> +               break;
>> +       case 5 ... 8:
>> +               io_width = 0x8;
>> +               break;
>> +       default:
>> +               dev_err(dev, "unsupported io width\n");
>> +               return -EINVAL;
>> +       }
> 
> Is it really a good idea to infer the register width from ngpios?
> 
> What about just putting this into the struct loongson_gpio_chip_data
> as well? Certainly it will be fixed for a certain device.
> 


Andy's suggestion seems to look better, We have bgpio_init() handle
this switch case. I will remove this switch case.
https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/commit/?h=gpio/for-next&id=55b2395e4e92adc492c6b30ac109eb78250dcd9d

Thanks,
Yinbo