[PATCH v2 0/2] gpio: loongson: add firmware offset parse support

[PATCH v2 0/2] gpio: loongson: add firmware offset parse support

[Yinbo Zhu]

Add some GPIO register offset value parse support for device properties
allowing to specify them in ACPI or DT.

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 parsing of loongson gpio offset
  gpio: loongson: add firmware offset parse support

 .../bindings/gpio/loongson,ls-gpio.yaml       | 37 ++++++++++
 drivers/gpio/gpio-loongson-64bit.c            | 71 +++++++++++++++++--
 2 files changed, 101 insertions(+), 7 deletions(-)

-- 
2.20.1

[PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

Add parsing GPIO configure, input, output, interrupt register offset
address and GPIO control mode support.

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

diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
index fb86e8ce6349..cad67f8bfe6e 100644
--- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
@@ -29,6 +29,33 @@ properties:
 
   gpio-ranges: true
 
+  loongson,gpio-conf-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      This option indicate this GPIO configuration register offset address.
+
+  loongson,gpio-out-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      This option indicate this GPIO output register offset address.
+
+  loongson,gpio-in-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      This option indicate this GPIO input register offset address.
+
+  loongson,gpio-ctrl-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      This option indicate this GPIO control mode, where '0' represents
+      bit control mode and '1' represents byte control mode.
+
+  loongson,gpio-inten-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      This option indicate this GPIO interrupt enable register offset
+      address.
+
   interrupts:
     minItems: 1
     maxItems: 64
@@ -39,6 +66,11 @@ required:
   - ngpios
   - "#gpio-cells"
   - gpio-controller
+  - loongson,gpio-conf-offset
+  - loongson,gpio-in-offset
+  - loongson,gpio-out-offset
+  - loongson,gpio-ctrl-mode
+  - loongson,gpio-inten-offset
   - gpio-ranges
   - interrupts
 
@@ -54,6 +86,11 @@ examples:
       ngpios = <64>;
       #gpio-cells = <2>;
       gpio-controller;
+      loongson,gpio-conf-offset = <0>;
+      loongson,gpio-in-offset = <0x20>;
+      loongson,gpio-out-offset = <0x10>;
+      loongson,gpio-ctrl-mode = <0>;
+      loongson,gpio-inten-offset = <0x30>;
       gpio-ranges = <&pctrl 0 0 15>,
                     <&pctrl 16 16 15>,
                     <&pctrl 32 32 10>,
-- 
2.20.1

[PATCH v2 2/2] gpio: loongson: add firmware offset parse support

[Yinbo Zhu]

Loongson GPIO controllers come in multiple variants that are compatible
except for certain register offset values.  Add support for device
properties allowing to specify them in ACPI or DT.

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

diff --git a/drivers/gpio/gpio-loongson-64bit.c b/drivers/gpio/gpio-loongson-64bit.c
index 06213bbfabdd..7f92cb6205b2 100644
--- a/drivers/gpio/gpio-loongson-64bit.c
+++ b/drivers/gpio/gpio-loongson-64bit.c
@@ -26,6 +26,7 @@ struct loongson_gpio_chip_data {
 	unsigned int		conf_offset;
 	unsigned int		out_offset;
 	unsigned int		in_offset;
+	unsigned int		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,16 +181,35 @@ 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->label)
+		lgpio->chip.label = lgpio->chip_data->label;
+	else
+		lgpio->chip.label = kstrdup(to_platform_device(dev)->name, GFP_KERNEL);
+
+	if (lgpio->chip_data->inten_offset)
+		lgpio->chip.to_irq = loongson_gpio_to_irq;
 
 	return devm_gpiochip_add_data(dev, &lgpio->chip, lgpio);
 }
 
+static int loongson_gpio_get_props(struct device *dev,
+				    struct loongson_gpio_chip *lgpio)
+{
+	const struct loongson_gpio_chip_data *d = lgpio->chip_data;
+
+	if (device_property_read_u32(dev, "loongson,gpio-conf-offset", (u32 *)&d->conf_offset)
+	    || device_property_read_u32(dev, "loongson,gpio-in-offset", (u32 *)&d->in_offset)
+	    || device_property_read_u32(dev, "loongson,gpio-out-offset", (u32 *)&d->out_offset)
+	    || device_property_read_u32(dev, "loongson,gpio-ctrl-mode", (u32 *)&d->mode))
+		return -EINVAL;
+
+	device_property_read_u32(dev, "loongson,gpio-inten-offset", (u32 *)&d->inten_offset);
+
+	return 0;
+}
+
 static int loongson_gpio_probe(struct platform_device *pdev)
 {
 	void __iomem *reg_base;
@@ -172,7 +221,12 @@ static int loongson_gpio_probe(struct platform_device *pdev)
 	if (!lgpio)
 		return -ENOMEM;
 
-	lgpio->chip_data = device_get_match_data(dev);
+	lgpio->chip_data = devm_kzalloc(dev, sizeof(*lgpio->chip_data), GFP_KERNEL);
+	if (!lgpio->chip_data)
+		return -ENOMEM;
+
+	if (loongson_gpio_get_props(dev, lgpio))
+		lgpio->chip_data = device_get_match_data(dev);
 
 	reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(reg_base))
@@ -215,6 +269,9 @@ static const struct acpi_device_id loongson_gpio_acpi_match[] = {
 		.id = "LOON0002",
 		.driver_data = (kernel_ulong_t)&loongson_gpio_ls7a_data,
 	},
+	{
+		.id = "LOON0007",
+	},
 	{}
 };
 MODULE_DEVICE_TABLE(acpi, loongson_gpio_acpi_match);
-- 
2.20.1

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
> Add parsing GPIO configure, input, output, interrupt register offset
> address and GPIO control mode support.

This reeks of insufficient use of SoC specific compatibles. Do GPIO
controllers on the same SoC have different register offsets?
Where are the users for this?

Cheers,
Conor.

> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  .../bindings/gpio/loongson,ls-gpio.yaml       | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> index fb86e8ce6349..cad67f8bfe6e 100644
> --- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
> @@ -29,6 +29,33 @@ properties:
>  
>    gpio-ranges: true
>  
> +  loongson,gpio-conf-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This option indicate this GPIO configuration register offset address.
> +
> +  loongson,gpio-out-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This option indicate this GPIO output register offset address.
> +
> +  loongson,gpio-in-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This option indicate this GPIO input register offset address.
> +
> +  loongson,gpio-ctrl-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This option indicate this GPIO control mode, where '0' represents
> +      bit control mode and '1' represents byte control mode.
> +
> +  loongson,gpio-inten-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This option indicate this GPIO interrupt enable register offset
> +      address.
> +
>    interrupts:
>      minItems: 1
>      maxItems: 64
> @@ -39,6 +66,11 @@ required:
>    - ngpios
>    - "#gpio-cells"
>    - gpio-controller
> +  - loongson,gpio-conf-offset
> +  - loongson,gpio-in-offset
> +  - loongson,gpio-out-offset
> +  - loongson,gpio-ctrl-mode
> +  - loongson,gpio-inten-offset
>    - gpio-ranges
>    - interrupts
>  
> @@ -54,6 +86,11 @@ examples:
>        ngpios = <64>;
>        #gpio-cells = <2>;
>        gpio-controller;
> +      loongson,gpio-conf-offset = <0>;
> +      loongson,gpio-in-offset = <0x20>;
> +      loongson,gpio-out-offset = <0x10>;
> +      loongson,gpio-ctrl-mode = <0>;
> +      loongson,gpio-inten-offset = <0x30>;
>        gpio-ranges = <&pctrl 0 0 15>,
>                      <&pctrl 16 16 15>,
>                      <&pctrl 32 32 10>,
> -- 
> 2.20.1
> 

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/7/31 下午11:55, Conor Dooley 写道:
> On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
>> Add parsing GPIO configure, input, output, interrupt register offset
>> address and GPIO control mode support.
> 
> This reeks of insufficient use of SoC specific compatibles. Do GPIO
> controllers on the same SoC have different register offsets?


Yes,

> Where are the users for this?


For example, ls2k500 contains multiple GPIO chips with different
(configure, input, output, interrupt) offset addresses, but all others
are the same.

> 
> Cheers,
> Conor.
> 
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> ---
>>   .../bindings/gpio/loongson,ls-gpio.yaml       | 37 +++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
>> index fb86e8ce6349..cad67f8bfe6e 100644
>> --- a/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
>> +++ b/Documentation/devicetree/bindings/gpio/loongson,ls-gpio.yaml
>> @@ -29,6 +29,33 @@ properties:
>>   
>>     gpio-ranges: true
>>   
>> +  loongson,gpio-conf-offset:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      This option indicate this GPIO configuration register offset address.
>> +
>> +  loongson,gpio-out-offset:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      This option indicate this GPIO output register offset address.
>> +
>> +  loongson,gpio-in-offset:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      This option indicate this GPIO input register offset address.
>> +
>> +  loongson,gpio-ctrl-mode:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      This option indicate this GPIO control mode, where '0' represents
>> +      bit control mode and '1' represents byte control mode.
>> +
>> +  loongson,gpio-inten-offset:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      This option indicate this GPIO interrupt enable register offset
>> +      address.
>> +
>>     interrupts:
>>       minItems: 1
>>       maxItems: 64
>> @@ -39,6 +66,11 @@ required:
>>     - ngpios
>>     - "#gpio-cells"
>>     - gpio-controller
>> +  - loongson,gpio-conf-offset
>> +  - loongson,gpio-in-offset
>> +  - loongson,gpio-out-offset
>> +  - loongson,gpio-ctrl-mode
>> +  - loongson,gpio-inten-offset
>>     - gpio-ranges
>>     - interrupts
>>   
>> @@ -54,6 +86,11 @@ examples:
>>         ngpios = <64>;
>>         #gpio-cells = <2>;
>>         gpio-controller;
>> +      loongson,gpio-conf-offset = <0>;
>> +      loongson,gpio-in-offset = <0x20>;
>> +      loongson,gpio-out-offset = <0x10>;
>> +      loongson,gpio-ctrl-mode = <0>;
>> +      loongson,gpio-inten-offset = <0x30>;
>>         gpio-ranges = <&pctrl 0 0 15>,
>>                       <&pctrl 16 16 15>,
>>                       <&pctrl 32 32 10>,
>> -- 
>> 2.20.1
>>

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Tue, Aug 01, 2023 at 02:39:49PM +0800, Yinbo Zhu wrote:
> 
> 
> 在 2023/7/31 下午11:55, Conor Dooley 写道:
> > On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
> > > Add parsing GPIO configure, input, output, interrupt register offset
> > > address and GPIO control mode support.
> > 
> > This reeks of insufficient use of SoC specific compatibles. Do GPIO
> > controllers on the same SoC have different register offsets?
> 
> 
> Yes,
> 
> > Where are the users for this?
> 
> 
> For example, ls2k500 contains multiple GPIO chips with different
> (configure, input, output, interrupt) offset addresses, but all others
> are the same.

Right. That's admittedly not what I expected to hear! Can you firstly
explain this in the commit message, and secondly add a soc-specific
compatible for the ls2k500 and only allow these properties on that SoC?

Thanks,
Conor.

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/8/1 下午3:23, Conor Dooley 写道:
> On Tue, Aug 01, 2023 at 02:39:49PM +0800, Yinbo Zhu wrote:
>>
>>
>> 在 2023/7/31 下午11:55, Conor Dooley 写道:
>>> On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
>>>> Add parsing GPIO configure, input, output, interrupt register offset
>>>> address and GPIO control mode support.
>>>
>>> This reeks of insufficient use of SoC specific compatibles. Do GPIO
>>> controllers on the same SoC have different register offsets?
>>
>>
>> Yes,
>>
>>> Where are the users for this?
>>
>>
>> For example, ls2k500 contains multiple GPIO chips with different
>> (configure, input, output, interrupt) offset addresses, but all others
>> are the same.
> 
> Right. That's admittedly not what I expected to hear! Can you firstly
> explain this in the commit message,


I will add following explain in the commit message. Do you think it's
suitable?

Loongson GPIO controllers come in multiple variants that are compatible
except for certain register offset values.  Add support in yaml file for
device properties allowing to specify them in DT.


> and secondly add a soc-specific
> compatible for the ls2k500 and only allow these properties on that SoC?
> 


Sorry, I may not have described it clearly before, the ls2k500 was only
as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
in multiple variants that are compatible except for certain register
offset values.  So above all offset device property was used to in all
loongson gpio controller.

Thanks,
Yinbo

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
> 
> 
> 在 2023/8/1 下午3:23, Conor Dooley 写道:
> > On Tue, Aug 01, 2023 at 02:39:49PM +0800, Yinbo Zhu wrote:
> > > 
> > > 
> > > 在 2023/7/31 下午11:55, Conor Dooley 写道:
> > > > On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
> > > > > Add parsing GPIO configure, input, output, interrupt register offset
> > > > > address and GPIO control mode support.
> > > > 
> > > > This reeks of insufficient use of SoC specific compatibles. Do GPIO
> > > > controllers on the same SoC have different register offsets?
> > > 
> > > 
> > > Yes,
> > > 
> > > > Where are the users for this?
> > > 
> > > 
> > > For example, ls2k500 contains multiple GPIO chips with different
> > > (configure, input, output, interrupt) offset addresses, but all others
> > > are the same.
> > 
> > Right. That's admittedly not what I expected to hear! Can you firstly
> > explain this in the commit message,
> 
> 
> I will add following explain in the commit message. Do you think it's
> suitable?
> 
> Loongson GPIO controllers come in multiple variants that are compatible
> except for certain register offset values.  Add support in yaml file for
> device properties allowing to specify them in DT.

Sure, that would be helpful. 

> > and secondly add a soc-specific
> > compatible for the ls2k500 and only allow these properties on that SoC?

> Sorry, I may not have described it clearly before, the ls2k500 was only
> as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
> in multiple variants that are compatible except for certain register
> offset values.  So above all offset device property was used to in all
> loongson gpio controller.

But it would be good to know why they are different. Do they each
support some different features, or was there some other reason for
making controllers like this?

Thanks,
Conor.

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/8/1 下午11:54, Conor Dooley 写道:
> On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
>>
>>
>> 在 2023/8/1 下午3:23, Conor Dooley 写道:
>>> On Tue, Aug 01, 2023 at 02:39:49PM +0800, Yinbo Zhu wrote:
>>>>
>>>>
>>>> 在 2023/7/31 下午11:55, Conor Dooley 写道:
>>>>> On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
>>>>>> Add parsing GPIO configure, input, output, interrupt register offset
>>>>>> address and GPIO control mode support.
>>>>>
>>>>> This reeks of insufficient use of SoC specific compatibles. Do GPIO
>>>>> controllers on the same SoC have different register offsets?
>>>>
>>>>
>>>> Yes,
>>>>
>>>>> Where are the users for this?
>>>>
>>>>
>>>> For example, ls2k500 contains multiple GPIO chips with different
>>>> (configure, input, output, interrupt) offset addresses, but all others
>>>> are the same.
>>>
>>> Right. That's admittedly not what I expected to hear! Can you firstly
>>> explain this in the commit message,
>>
>>
>> I will add following explain in the commit message. Do you think it's
>> suitable?
>>
>> Loongson GPIO controllers come in multiple variants that are compatible
>> except for certain register offset values.  Add support in yaml file for
>> device properties allowing to specify them in DT.
> 
> Sure, that would be helpful.
> 
>>> and secondly add a soc-specific
>>> compatible for the ls2k500 and only allow these properties on that SoC?
> 
>> Sorry, I may not have described it clearly before, the ls2k500 was only
>> as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
>> in multiple variants that are compatible except for certain register
>> offset values.  So above all offset device property was used to in all
>> loongson gpio controller.
> 
> But it would be good to know why they are different. Do they each
> support some different features, or was there some other reason for
> making controllers like this?


There are no other reasons, just differences in these offset addresses.

Thanks,
Yinbo.

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
> 
> 
> 在 2023/8/1 下午11:54, Conor Dooley 写道:
> > On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
> > > 
> > > 
> > > 在 2023/8/1 下午3:23, Conor Dooley 写道:
> > > > On Tue, Aug 01, 2023 at 02:39:49PM +0800, Yinbo Zhu wrote:
> > > > > 
> > > > > 
> > > > > 在 2023/7/31 下午11:55, Conor Dooley 写道:
> > > > > > On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
> > > > > > > Add parsing GPIO configure, input, output, interrupt register offset
> > > > > > > address and GPIO control mode support.
> > > > > > 
> > > > > > This reeks of insufficient use of SoC specific compatibles. Do GPIO
> > > > > > controllers on the same SoC have different register offsets?
> > > > > 
> > > > > 
> > > > > Yes,
> > > > > 
> > > > > > Where are the users for this?
> > > > > 
> > > > > 
> > > > > For example, ls2k500 contains multiple GPIO chips with different
> > > > > (configure, input, output, interrupt) offset addresses, but all others
> > > > > are the same.
> > > > 
> > > > Right. That's admittedly not what I expected to hear! Can you firstly
> > > > explain this in the commit message,
> > > 
> > > 
> > > I will add following explain in the commit message. Do you think it's
> > > suitable?
> > > 
> > > Loongson GPIO controllers come in multiple variants that are compatible
> > > except for certain register offset values.  Add support in yaml file for
> > > device properties allowing to specify them in DT.
> > 
> > Sure, that would be helpful.
> > 
> > > > and secondly add a soc-specific
> > > > compatible for the ls2k500 and only allow these properties on that SoC?
> > 
> > > Sorry, I may not have described it clearly before, the ls2k500 was only
> > > as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
> > > in multiple variants that are compatible except for certain register
> > > offset values.  So above all offset device property was used to in all
> > > loongson gpio controller.
> > 
> > But it would be good to know why they are different. Do they each
> > support some different features, or was there some other reason for
> > making controllers like this?
> 
> 
> There are no other reasons, just differences in these offset addresses.

Huh. Do you have a link to a devicetree for the ls2k500?

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/8/2 下午3:22, Conor Dooley 写道:
> On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
>>
>>
>> 在 2023/8/1 下午11:54, Conor Dooley 写道:
>>> On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
>>>>
>>>>
>>>> 在 2023/8/1 下午3:23, Conor Dooley 写道:
>>>>> On Tue, Aug 01, 2023 at 02:39:49PM +0800, Yinbo Zhu wrote:
>>>>>>
>>>>>>
>>>>>> 在 2023/7/31 下午11:55, Conor Dooley 写道:
>>>>>>> On Mon, Jul 31, 2023 at 05:10:58PM +0800, Yinbo Zhu wrote:
>>>>>>>> Add parsing GPIO configure, input, output, interrupt register offset
>>>>>>>> address and GPIO control mode support.
>>>>>>>
>>>>>>> This reeks of insufficient use of SoC specific compatibles. Do GPIO
>>>>>>> controllers on the same SoC have different register offsets?
>>>>>>
>>>>>>
>>>>>> Yes,
>>>>>>
>>>>>>> Where are the users for this?
>>>>>>
>>>>>>
>>>>>> For example, ls2k500 contains multiple GPIO chips with different
>>>>>> (configure, input, output, interrupt) offset addresses, but all others
>>>>>> are the same.
>>>>>
>>>>> Right. That's admittedly not what I expected to hear! Can you firstly
>>>>> explain this in the commit message,
>>>>
>>>>
>>>> I will add following explain in the commit message. Do you think it's
>>>> suitable?
>>>>
>>>> Loongson GPIO controllers come in multiple variants that are compatible
>>>> except for certain register offset values.  Add support in yaml file for
>>>> device properties allowing to specify them in DT.
>>>
>>> Sure, that would be helpful.
>>>
>>>>> and secondly add a soc-specific
>>>>> compatible for the ls2k500 and only allow these properties on that SoC?
>>>
>>>> Sorry, I may not have described it clearly before, the ls2k500 was only
>>>> as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
>>>> in multiple variants that are compatible except for certain register
>>>> offset values.  So above all offset device property was used to in all
>>>> loongson gpio controller.
>>>
>>> But it would be good to know why they are different. Do they each
>>> support some different features, or was there some other reason for
>>> making controllers like this?
>>
>>
>> There are no other reasons, just differences in these offset addresses.
> 
> Huh. Do you have a link to a devicetree for the ls2k500?


Yes,  there was a link about ls2k500 dts,  but that ls2k500 dts has not
yet added a gpio node.  this gpio node will be added later.

Thanks,
Yinbo

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Wed, Aug 02, 2023 at 03:44:17PM +0800, Yinbo Zhu wrote:
> 在 2023/8/2 下午3:22, Conor Dooley 写道:
> > On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
> > > 在 2023/8/1 下午11:54, Conor Dooley 写道:
> > > > On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:

> > > > > Sorry, I may not have described it clearly before, the ls2k500 was only
> > > > > as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
> > > > > in multiple variants that are compatible except for certain register
> > > > > offset values.  So above all offset device property was used to in all
> > > > > loongson gpio controller.
> > > > 
> > > > But it would be good to know why they are different. Do they each
> > > > support some different features, or was there some other reason for
> > > > making controllers like this?
> > > 
> > > 
> > > There are no other reasons, just differences in these offset addresses.
> > 
> > Huh. Do you have a link to a devicetree for the ls2k500?
> 
> 
> Yes,  there was a link about ls2k500 dts,  but that ls2k500 dts has not
> yet added a gpio node.  this gpio node will be added later.

You must have something that you used to test with, no? I don't mind if
it is not a patch, but rather is some WIP - I'd just like to see user of
the binding :)

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/8/2 下午3:50, Conor Dooley 写道:
> On Wed, Aug 02, 2023 at 03:44:17PM +0800, Yinbo Zhu wrote:
>> 在 2023/8/2 下午3:22, Conor Dooley 写道:
>>> On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
>>>> 在 2023/8/1 下午11:54, Conor Dooley 写道:
>>>>> On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
> 
>>>>>> Sorry, I may not have described it clearly before, the ls2k500 was only
>>>>>> as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
>>>>>> in multiple variants that are compatible except for certain register
>>>>>> offset values.  So above all offset device property was used to in all
>>>>>> loongson gpio controller.
>>>>>
>>>>> But it would be good to know why they are different. Do they each
>>>>> support some different features, or was there some other reason for
>>>>> making controllers like this?
>>>>
>>>>
>>>> There are no other reasons, just differences in these offset addresses.
>>>
>>> Huh. Do you have a link to a devicetree for the ls2k500?
>>
>>
>> Yes,  there was a link about ls2k500 dts,  but that ls2k500 dts has not
>> yet added a gpio node.  this gpio node will be added later.
> 
> You must have something that you used to test with, no? I don't mind if
> it is not a patch, but rather is some WIP - I'd just like to see user of
> the binding :)


yes, I have a test, for 2k0500, that gpio dts as follows:

                 gpio0:gpio@0x1fe10430 {
                         compatible = "loongson,ls2k-gpio";
                         reg = <0 0x1fe10430 0 0x20>;
                         gpio-controller;
                         #gpio-cells = <2>;
			interrupt-parent = <&liointc1>;
                         ngpios = <64>;
                         loongson,gpio-conf-offset = <0>;
                         loongson,gpio-out-offset = <0x10>;
                         loongson,gpio-in-offset = <0x8>;
                         loongson,gpio-inten-offset = <0xb0>;
			loongson,gpio-ctrl-mode = <0x0>;
                         ...
		  }

                 gpio1:gpio@0x1fe10450 {
                         compatible = "loongson,ls2k-gpio";
                         reg = <0 0x1fe10450 0 0x20>;
                         gpio-controller;
                         #gpio-cells = <2>;
			interrupt-parent = <&liointc1>;
                         ngpios = <64>;
                         loongson,gpio-conf-offset = <0>;
                         loongson,gpio-out-offset = <0x10>;
                         loongson,gpio-in-offset = <0x8>;
                         loongson,gpio-inten-offset = <0x98>;
			loongson,gpio-ctrl-mode = <0x0>;
                         ...
	        }


Thanks,
Yinbo.
> 

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Wed, Aug 02, 2023 at 04:37:50PM +0800, Yinbo Zhu wrote:
> 
> 
> 在 2023/8/2 下午3:50, Conor Dooley 写道:
> > On Wed, Aug 02, 2023 at 03:44:17PM +0800, Yinbo Zhu wrote:
> > > 在 2023/8/2 下午3:22, Conor Dooley 写道:
> > > > On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
> > > > > 在 2023/8/1 下午11:54, Conor Dooley 写道:
> > > > > > On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
> > 
> > > > > > > Sorry, I may not have described it clearly before, the ls2k500 was only
> > > > > > > as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
> > > > > > > in multiple variants that are compatible except for certain register
> > > > > > > offset values.  So above all offset device property was used to in all
> > > > > > > loongson gpio controller.
> > > > > > 
> > > > > > But it would be good to know why they are different. Do they each
> > > > > > support some different features, or was there some other reason for
> > > > > > making controllers like this?
> > > > > 
> > > > > 
> > > > > There are no other reasons, just differences in these offset addresses.
> > > > 
> > > > Huh. Do you have a link to a devicetree for the ls2k500?
> > > 
> > > 
> > > Yes,  there was a link about ls2k500 dts,  but that ls2k500 dts has not
> > > yet added a gpio node.  this gpio node will be added later.
> > 
> > You must have something that you used to test with, no? I don't mind if
> > it is not a patch, but rather is some WIP - I'd just like to see user of
> > the binding :)
> 
> 
> yes, I have a test, for 2k0500, that gpio dts as follows:
> 
>                 gpio0:gpio@0x1fe10430 {
>                         compatible = "loongson,ls2k-gpio";
>                         reg = <0 0x1fe10430 0 0x20>;
>                         gpio-controller;
>                         #gpio-cells = <2>;
> 			interrupt-parent = <&liointc1>;
>                         ngpios = <64>;
>                         loongson,gpio-conf-offset = <0>;
>                         loongson,gpio-out-offset = <0x10>;
>                         loongson,gpio-in-offset = <0x8>;
>                         loongson,gpio-inten-offset = <0xb0>;
> 			loongson,gpio-ctrl-mode = <0x0>;
>                         ...
> 		  }
> 
>                 gpio1:gpio@0x1fe10450 {
>                         compatible = "loongson,ls2k-gpio";
>                         reg = <0 0x1fe10450 0 0x20>;
>                         gpio-controller;
>                         #gpio-cells = <2>;
> 			interrupt-parent = <&liointc1>;
>                         ngpios = <64>;
>                         loongson,gpio-conf-offset = <0>;
>                         loongson,gpio-out-offset = <0x10>;
>                         loongson,gpio-in-offset = <0x8>;

These 3 are the same for both controllers, no?
Is only the inten-offset a variable?

>                         loongson,gpio-inten-offset = <0x98>;

These offsets exceed the region that you've got in the reg property for
this controller, do they not?

Is there some sort of "miscellaneous register area" at 0x1FE104E0, or
just those two interrupt registers and nothing else?

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/8/2 下午11:36, Conor Dooley 写道:
> On Wed, Aug 02, 2023 at 04:37:50PM +0800, Yinbo Zhu wrote:
>>
>>
>> 在 2023/8/2 下午3:50, Conor Dooley 写道:
>>> On Wed, Aug 02, 2023 at 03:44:17PM +0800, Yinbo Zhu wrote:
>>>> 在 2023/8/2 下午3:22, Conor Dooley 写道:
>>>>> On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
>>>>>> 在 2023/8/1 下午11:54, Conor Dooley 写道:
>>>>>>> On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
>>>
>>>>>>>> Sorry, I may not have described it clearly before, the ls2k500 was only
>>>>>>>> as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
>>>>>>>> in multiple variants that are compatible except for certain register
>>>>>>>> offset values.  So above all offset device property was used to in all
>>>>>>>> loongson gpio controller.
>>>>>>>
>>>>>>> But it would be good to know why they are different. Do they each
>>>>>>> support some different features, or was there some other reason for
>>>>>>> making controllers like this?
>>>>>>
>>>>>>
>>>>>> There are no other reasons, just differences in these offset addresses.
>>>>>
>>>>> Huh. Do you have a link to a devicetree for the ls2k500?
>>>>
>>>>
>>>> Yes,  there was a link about ls2k500 dts,  but that ls2k500 dts has not
>>>> yet added a gpio node.  this gpio node will be added later.
>>>
>>> You must have something that you used to test with, no? I don't mind if
>>> it is not a patch, but rather is some WIP - I'd just like to see user of
>>> the binding :)
>>
>>
>> yes, I have a test, for 2k0500, that gpio dts as follows:
>>
>>                  gpio0:gpio@0x1fe10430 {
>>                          compatible = "loongson,ls2k-gpio";
>>                          reg = <0 0x1fe10430 0 0x20>;
>>                          gpio-controller;
>>                          #gpio-cells = <2>;
>> 			interrupt-parent = <&liointc1>;
>>                          ngpios = <64>;
>>                          loongson,gpio-conf-offset = <0>;
>>                          loongson,gpio-out-offset = <0x10>;
>>                          loongson,gpio-in-offset = <0x8>;
>>                          loongson,gpio-inten-offset = <0xb0>;
>> 			loongson,gpio-ctrl-mode = <0x0>;
>>                          ...
>> 		  }
>>
>>                  gpio1:gpio@0x1fe10450 {
>>                          compatible = "loongson,ls2k-gpio";
>>                          reg = <0 0x1fe10450 0 0x20>;
>>                          gpio-controller;
>>                          #gpio-cells = <2>;
>> 			interrupt-parent = <&liointc1>;
>>                          ngpios = <64>;
>>                          loongson,gpio-conf-offset = <0>;
>>                          loongson,gpio-out-offset = <0x10>;
>>                          loongson,gpio-in-offset = <0x8>;
> 
> These 3 are the same for both controllers, no?
> Is only the inten-offset a variable?
> 
>>                          loongson,gpio-inten-offset = <0x98>;
> 
> These offsets exceed the region that you've got in the reg property for
> this controller, do they not?
> 
> Is there some sort of "miscellaneous register area" at 0x1FE104E0, or
> just those two interrupt registers and nothing else?


2k500 gpio dts is just an example, like 3a5000, or more other platform,
above offset was different but the gpio controller was compatible.

                 gpio: gpio@1fe00500 {
                         compatible = "loongson,ls2k-gpio";
                         reg = <0 0x1fe00500 0xc00>;
                         gpio-controller;
                         #gpio-cells = <2>;
                         ngpios = <16>;
                         loongson,gpio-conf-offset = <0x0>;
                         loongson,gpio-out-offset = <0x8>;
                         loongson,gpio-in-offset = <0xc>;
			...
			}


Thanks,
Yinbo


> 

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Conor Dooley]

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

On Thu, Aug 03, 2023 at 09:56:02AM +0800, Yinbo Zhu wrote:
> 
> 
> 在 2023/8/2 下午11:36, Conor Dooley 写道:
> > On Wed, Aug 02, 2023 at 04:37:50PM +0800, Yinbo Zhu wrote:
> > > 
> > > 
> > > 在 2023/8/2 下午3:50, Conor Dooley 写道:
> > > > On Wed, Aug 02, 2023 at 03:44:17PM +0800, Yinbo Zhu wrote:
> > > > > 在 2023/8/2 下午3:22, Conor Dooley 写道:
> > > > > > On Wed, Aug 02, 2023 at 09:38:34AM +0800, Yinbo Zhu wrote:
> > > > > > > 在 2023/8/1 下午11:54, Conor Dooley 写道:
> > > > > > > > On Tue, Aug 01, 2023 at 04:34:30PM +0800, Yinbo Zhu wrote:
> > > > 
> > > > > > > > > Sorry, I may not have described it clearly before, the ls2k500 was only
> > > > > > > > > as a example, actually, Loongson GPIO controllers (2k500,2k1000,eg)come
> > > > > > > > > in multiple variants that are compatible except for certain register
> > > > > > > > > offset values.  So above all offset device property was used to in all
> > > > > > > > > loongson gpio controller.
> > > > > > > > 
> > > > > > > > But it would be good to know why they are different. Do they each
> > > > > > > > support some different features, or was there some other reason for
> > > > > > > > making controllers like this?
> > > > > > > 
> > > > > > > 
> > > > > > > There are no other reasons, just differences in these offset addresses.
> > > > > > 
> > > > > > Huh. Do you have a link to a devicetree for the ls2k500?
> > > > > 
> > > > > 
> > > > > Yes,  there was a link about ls2k500 dts,  but that ls2k500 dts has not
> > > > > yet added a gpio node.  this gpio node will be added later.
> > > > 
> > > > You must have something that you used to test with, no? I don't mind if
> > > > it is not a patch, but rather is some WIP - I'd just like to see user of
> > > > the binding :)
> > > 
> > > 
> > > yes, I have a test, for 2k0500, that gpio dts as follows:
> > > 
> > >                  gpio0:gpio@0x1fe10430 {
> > >                          compatible = "loongson,ls2k-gpio";
> > >                          reg = <0 0x1fe10430 0 0x20>;
> > >                          gpio-controller;
> > >                          #gpio-cells = <2>;
> > > 			interrupt-parent = <&liointc1>;
> > >                          ngpios = <64>;
> > >                          loongson,gpio-conf-offset = <0>;
> > >                          loongson,gpio-out-offset = <0x10>;
> > >                          loongson,gpio-in-offset = <0x8>;
> > >                          loongson,gpio-inten-offset = <0xb0>;
> > > 			loongson,gpio-ctrl-mode = <0x0>;
> > >                          ...
> > > 		  }
> > > 
> > >                  gpio1:gpio@0x1fe10450 {
> > >                          compatible = "loongson,ls2k-gpio";
> > >                          reg = <0 0x1fe10450 0 0x20>;
> > >                          gpio-controller;
> > >                          #gpio-cells = <2>;
> > > 			interrupt-parent = <&liointc1>;
> > >                          ngpios = <64>;
> > >                          loongson,gpio-conf-offset = <0>;
> > >                          loongson,gpio-out-offset = <0x10>;
> > >                          loongson,gpio-in-offset = <0x8>;
> > 
> > These 3 are the same for both controllers, no?
> > Is only the inten-offset a variable?
> > 
> > >                          loongson,gpio-inten-offset = <0x98>;
> > 
> > These offsets exceed the region that you've got in the reg property for
> > this controller, do they not?
> > 
> > Is there some sort of "miscellaneous register area" at 0x1FE104E0, or
> > just those two interrupt registers and nothing else?
> 
> 
> 2k500 gpio dts is just an example, like 3a5000, or more other platform,
> above offset was different but the gpio controller was compatible.
> 
>                 gpio: gpio@1fe00500 {
>                         compatible = "loongson,ls2k-gpio";
>                         reg = <0 0x1fe00500 0xc00>;
>                         gpio-controller;
>                         #gpio-cells = <2>;
>                         ngpios = <16>;
>                         loongson,gpio-conf-offset = <0x0>;
>                         loongson,gpio-out-offset = <0x8>;
>                         loongson,gpio-in-offset = <0xc>;
> 			...
> 			}

That is a different SoC and needs to have a different compatible string.
"loongson,ls2k-foo" compatible strings were a mistake that only got past
us because we were not aware it was a family, rather than a specific
SoC. They certainly should not be used in isolation on a 3a5000!

Are there more than one GPIO controllers on the 3a5000? If so, what do
those nodes look like.

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

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Krzysztof Kozlowski]

On 03/08/2023 08:30, Conor Dooley wrote:
>>>>                  gpio0:gpio@0x1fe10430 {
>>>>                          compatible = "loongson,ls2k-gpio";
>>>>                          reg = <0 0x1fe10430 0 0x20>;
>>>>                          gpio-controller;
>>>>                          #gpio-cells = <2>;
>>>> 			interrupt-parent = <&liointc1>;
>>>>                          ngpios = <64>;
>>>>                          loongson,gpio-conf-offset = <0>;
>>>>                          loongson,gpio-out-offset = <0x10>;
>>>>                          loongson,gpio-in-offset = <0x8>;
>>>>                          loongson,gpio-inten-offset = <0xb0>;
>>>> 			loongson,gpio-ctrl-mode = <0x0>;
>>>>                          ...
>>>> 		  }
>>>>
>>>>                  gpio1:gpio@0x1fe10450 {
>>>>                          compatible = "loongson,ls2k-gpio";
>>>>                          reg = <0 0x1fe10450 0 0x20>;
>>>>                          gpio-controller;
>>>>                          #gpio-cells = <2>;
>>>> 			interrupt-parent = <&liointc1>;
>>>>                          ngpios = <64>;
>>>>                          loongson,gpio-conf-offset = <0>;
>>>>                          loongson,gpio-out-offset = <0x10>;
>>>>                          loongson,gpio-in-offset = <0x8>;
>>>
>>> These 3 are the same for both controllers, no?
>>> Is only the inten-offset a variable?
>>>
>>>>                          loongson,gpio-inten-offset = <0x98>;
>>>
>>> These offsets exceed the region that you've got in the reg property for
>>> this controller, do they not?
>>>
>>> Is there some sort of "miscellaneous register area" at 0x1FE104E0, or
>>> just those two interrupt registers and nothing else?
>>
>>
>> 2k500 gpio dts is just an example, like 3a5000, or more other platform,
>> above offset was different but the gpio controller was compatible.
>>
>>                 gpio: gpio@1fe00500 {
>>                         compatible = "loongson,ls2k-gpio";
>>                         reg = <0 0x1fe00500 0xc00>;
>>                         gpio-controller;
>>                         #gpio-cells = <2>;
>>                         ngpios = <16>;
>>                         loongson,gpio-conf-offset = <0x0>;
>>                         loongson,gpio-out-offset = <0x8>;
>>                         loongson,gpio-in-offset = <0xc>;
>> 			...
>> 			}
> 
> That is a different SoC and needs to have a different compatible string.
> "loongson,ls2k-foo" compatible strings were a mistake that only got past
> us because we were not aware it was a family, rather than a specific
> SoC. They certainly should not be used in isolation on a 3a5000!
> 
> Are there more than one GPIO controllers on the 3a5000? If so, what do
> those nodes look like.

Eh, even for the same SoC having different offsets suggest that
programming model is a bit different. Anyway, who designed such
hardware? Really?

Best regards,
Krzysztof

Re: [PATCH v2 1/2] gpio: dt-bindings: add parsing of loongson gpio offset

[Yinbo Zhu]

在 2023/8/3 下午2:41, Krzysztof Kozlowski 写道:
> On 03/08/2023 08:30, Conor Dooley wrote:
>>>>>                   gpio0:gpio@0x1fe10430 {
>>>>>                           compatible = "loongson,ls2k-gpio";
>>>>>                           reg = <0 0x1fe10430 0 0x20>;
>>>>>                           gpio-controller;
>>>>>                           #gpio-cells = <2>;
>>>>> 			interrupt-parent = <&liointc1>;
>>>>>                           ngpios = <64>;
>>>>>                           loongson,gpio-conf-offset = <0>;
>>>>>                           loongson,gpio-out-offset = <0x10>;
>>>>>                           loongson,gpio-in-offset = <0x8>;
>>>>>                           loongson,gpio-inten-offset = <0xb0>;
>>>>> 			loongson,gpio-ctrl-mode = <0x0>;
>>>>>                           ...
>>>>> 		  }
>>>>>
>>>>>                   gpio1:gpio@0x1fe10450 {
>>>>>                           compatible = "loongson,ls2k-gpio";
>>>>>                           reg = <0 0x1fe10450 0 0x20>;
>>>>>                           gpio-controller;
>>>>>                           #gpio-cells = <2>;
>>>>> 			interrupt-parent = <&liointc1>;
>>>>>                           ngpios = <64>;
>>>>>                           loongson,gpio-conf-offset = <0>;
>>>>>                           loongson,gpio-out-offset = <0x10>;
>>>>>                           loongson,gpio-in-offset = <0x8>;
>>>>
>>>> These 3 are the same for both controllers, no?
>>>> Is only the inten-offset a variable?
>>>>
>>>>>                           loongson,gpio-inten-offset = <0x98>;
>>>>
>>>> These offsets exceed the region that you've got in the reg property for
>>>> this controller, do they not?
>>>>
>>>> Is there some sort of "miscellaneous register area" at 0x1FE104E0, or
>>>> just those two interrupt registers and nothing else?
>>>
>>>
>>> 2k500 gpio dts is just an example, like 3a5000, or more other platform,
>>> above offset was different but the gpio controller was compatible.
>>>
>>>                  gpio: gpio@1fe00500 {
>>>                          compatible = "loongson,ls2k-gpio";
>>>                          reg = <0 0x1fe00500 0xc00>;
>>>                          gpio-controller;
>>>                          #gpio-cells = <2>;
>>>                          ngpios = <16>;
>>>                          loongson,gpio-conf-offset = <0x0>;
>>>                          loongson,gpio-out-offset = <0x8>;
>>>                          loongson,gpio-in-offset = <0xc>;
>>> 			...
>>> 			}
>>
>> That is a different SoC and needs to have a different compatible string.
>> "loongson,ls2k-foo" compatible strings were a mistake that only got past
>> us because we were not aware it was a family, rather than a specific
>> SoC. They certainly should not be used in isolation on a 3a5000!
>>
>> Are there more than one GPIO controllers on the 3a5000? If so, what do
>> those nodes look like.
> 
> Eh, even for the same SoC having different offsets suggest that
> programming model is a bit different. Anyway, who designed such
> hardware? Really?


Hi Krzysztof & Conor,

I'm sorry for the confusion caused to you, such as 2k2000, which has two
gpio controllers with different register offsets.  The gpio node is
following:

         pioA: gpio0@0x1fe00500 {
             compatible = "loongson,ls2k-gpio";
             reg = <0 0x1fe00500 0 0x20>;
             gpio-controller;
             #gpio-cells = <2>;
             ngpios = <32>;
             loongson,gpio-ctrl-mode = <0>;
             loongson,gpio-conf-offset = <0>;
             loongson,gpio-in-offset = <0xc>;
             loongson,gpio-out-offset = <0x8>;
         };

         pioB: gpio1@0x100e0000 {
             compatible = "loongson,ls2k-gpio";
             reg = <0 0x100e0000 0 0x20>;
             gpio-controller;
             #gpio-cells = <2>;
             ngpios = <64>;
             loongson,gpio-ctrl-mode = <0>;
             loongson,gpio-conf-offset = <0>;
             loongson,gpio-in-offset = <0x20>;
             loongson,gpio-out-offset = <0x10>;
         };


In addition, the GPIO controllers between different SoCs are compatible
except for offset, previously, the examples of 3a5000 and 2k0500 gpio
were listed, Of course, it also includes 2k1000, which gpio chip was
compatible but offset was different.

About the "loongson,ls2k-foo" compatible strings were a mistake that I
got it and I will add a specific SoC  "loongson,ls2k1000-foo" compatible
, but from previous community communication, it seems that if different
SoC peripherals are compatible, they can use the same compatible string.

Thanks,
Yinbo