[PATCH 0/4] Devicetree support for Loongson-1 GPIO

[PATCH 0/4] Devicetree support for Loongson-1 GPIO

[Keguang Zhang]

Update the driver to add DT support and dt-binding document,
along with other minor changes.

Keguang Zhang (4):
  gpio: loongson1: Update copyright
  gpio: loongson1: Introduce ls1x_gpio_chip struct
  gpio: loongson1: Add DT support
  dt-bindings: gpio: Add Loongson-1 GPIO

 .../bindings/gpio/loongson,ls1x-gpio.yaml     | 60 ++++++++++++++
 drivers/gpio/gpio-loongson1.c                 | 83 ++++++++++++-------
 2 files changed, 115 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml


base-commit: 4827aae061337251bb91801b316157a78b845ec7
-- 
2.34.1

[PATCH 1/4] gpio: loongson1: Update copyright

[Keguang Zhang]

This patch updates copyright and author information.

Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
---
 drivers/gpio/gpio-loongson1.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
index 5d90b3bc5a25..7ecbc43f8b38 100644
--- a/drivers/gpio/gpio-loongson1.c
+++ b/drivers/gpio/gpio-loongson1.c
@@ -1,11 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * GPIO Driver for Loongson 1 SoC
  *
- * Copyright (C) 2015-2016 Zhang, Keguang <keguang.zhang@gmail.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
+ * Copyright (C) 2015-2023 Keguang Zhang <keguang.zhang@gmail.com>
  */
 
 #include <linux/module.h>
@@ -90,6 +87,6 @@ static struct platform_driver ls1x_gpio_driver = {
 
 module_platform_driver(ls1x_gpio_driver);
 
-MODULE_AUTHOR("Kelvin Cheung <keguang.zhang@gmail.com>");
+MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
 MODULE_DESCRIPTION("Loongson1 GPIO driver");
 MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH 1/4] gpio: loongson1: Update copyright

[andy.shevchenko]

Wed, Feb 22, 2023 at 07:12:10PM +0800, Keguang Zhang kirjoitti:
> This patch updates copyright and author information.

...

> +// SPDX-License-Identifier: GPL-2.0-or-later

Have you talked to your lawers? This is an inequivalent to what was written
below.

>  /*
>   * GPIO Driver for Loongson 1 SoC
>   *
> - * Copyright (C) 2015-2016 Zhang, Keguang <keguang.zhang@gmail.com>
> - *
> - * This file is licensed under the terms of the GNU General Public
> - * License version 2. This program is licensed "as is" without any
> - * warranty of any kind, whether express or implied.
> + * Copyright (C) 2015-2023 Keguang Zhang <keguang.zhang@gmail.com>
>   */

...

> -MODULE_AUTHOR("Kelvin Cheung <keguang.zhang@gmail.com>");
> +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");

So, which name here is in your official documents?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/4] gpio: loongson1: Update copyright

[Kelvin Cheung]

On Wed, Feb 22, 2023 at 8:18 PM <andy.shevchenko@gmail.com> wrote:
>
> Wed, Feb 22, 2023 at 07:12:10PM +0800, Keguang Zhang kirjoitti:
> > This patch updates copyright and author information.
>
> ...
>
> > +// SPDX-License-Identifier: GPL-2.0-or-later
>
> Have you talked to your lawers? This is an inequivalent to what was written
> below.
>
Yes.
> >  /*
> >   * GPIO Driver for Loongson 1 SoC
> >   *
> > - * Copyright (C) 2015-2016 Zhang, Keguang <keguang.zhang@gmail.com>
> > - *
> > - * This file is licensed under the terms of the GNU General Public
> > - * License version 2. This program is licensed "as is" without any
> > - * warranty of any kind, whether express or implied.
> > + * Copyright (C) 2015-2023 Keguang Zhang <keguang.zhang@gmail.com>
> >   */
>
> ...
>
> > -MODULE_AUTHOR("Kelvin Cheung <keguang.zhang@gmail.com>");
> > +MODULE_AUTHOR("Keguang Zhang <keguang.zhang@gmail.com>");
>
> So, which name here is in your official documents?
>
Keguang Zhang is my official.

> --
> With Best Regards,
> Andy Shevchenko
>
>


--
Best regards,

Kelvin Cheung

Re: [PATCH 1/4] gpio: loongson1: Update copyright

[Krzysztof Kozlowski]

On 02/03/2023 12:09, Kelvin Cheung wrote:
> On Wed, Feb 22, 2023 at 8:18 PM <andy.shevchenko@gmail.com> wrote:
>>
>> Wed, Feb 22, 2023 at 07:12:10PM +0800, Keguang Zhang kirjoitti:
>>> This patch updates copyright and author information.
>>
>> ...
>>
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>
>> Have you talked to your lawers? This is an inequivalent to what was written
>> below.
>>
> Yes.

Yes to what? You now change the license... and commit msg does not
explain it and does not justifies it. What is even weirder that your
lawyers agreed on GPLv3! With GPLv3 you need to open a lot from your
products... Not mentioning that they agreed on future GPLv4 and GPLv5,
that's even weirder because GPLv4 might be saying you need to buy me
flowers...

Best regards,
Krzysztof

Re: [PATCH 1/4] gpio: loongson1: Update copyright

[Kelvin Cheung]

On Thu, Mar 2, 2023 at 7:40 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/03/2023 12:09, Kelvin Cheung wrote:
> > On Wed, Feb 22, 2023 at 8:18 PM <andy.shevchenko@gmail.com> wrote:
> >>
> >> Wed, Feb 22, 2023 at 07:12:10PM +0800, Keguang Zhang kirjoitti:
> >>> This patch updates copyright and author information.
> >>
> >> ...
> >>
> >>> +// SPDX-License-Identifier: GPL-2.0-or-later
> >>
> >> Have you talked to your lawers? This is an inequivalent to what was written
> >> below.
> >>
> > Yes.
>
> Yes to what? You now change the license... and commit msg does not
> explain it and does not justifies it. What is even weirder that your
> lawyers agreed on GPLv3! With GPLv3 you need to open a lot from your
> products... Not mentioning that they agreed on future GPLv4 and GPLv5,
> that's even weirder because GPLv4 might be saying you need to buy me
> flowers...
>
If so, I choose to leave the license as is.
Is "GPL-2.0-only" right?

> Best regards,
> Krzysztof
>


-- 
Best regards,

Kelvin Cheung

Re: [PATCH 1/4] gpio: loongson1: Update copyright

[Andy Shevchenko]

On Thu, Mar 2, 2023 at 2:16 PM Kelvin Cheung <keguang.zhang@gmail.com> wrote:
> On Thu, Mar 2, 2023 at 7:40 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 02/03/2023 12:09, Kelvin Cheung wrote:
> > > On Wed, Feb 22, 2023 at 8:18 PM <andy.shevchenko@gmail.com> wrote:
> > >> Wed, Feb 22, 2023 at 07:12:10PM +0800, Keguang Zhang kirjoitti:
> > >>> This patch updates copyright and author information.
> > >>
> > >> ...
> > >>
> > >>> +// SPDX-License-Identifier: GPL-2.0-or-later
> > >>
> > >> Have you talked to your lawyers? This is an inequivalent to what was written
> > >> below.
> > >>
> > > Yes.
> >
> > Yes to what? You now change the license... and commit msg does not
> > explain it and does not justify it. What is even weirder that your
> > lawyers agreed on GPLv3! With GPLv3 you need to open a lot from your
> > products... Not mentioning that they agreed on future GPLv4 and GPLv5,
> > that's even weirder because GPLv4 might be saying you need to buy me
> > flowers...
> >
> If so, I choose to leave the license as is.
> Is "GPL-2.0-only" right?

We do not know.
It depends on what your company lawyers told you.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct

[Keguang Zhang]

Introduce and allocate ls1x_gpio_chip struct containing
gpio_chip and reg_base to avoid global gpio_reg_base.

Use readl() & writel() instead of __raw_readl() & __raw_writel().

Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
---
 drivers/gpio/gpio-loongson1.c | 45 +++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
index 7ecbc43f8b38..b950bcfd78ce 100644
--- a/drivers/gpio/gpio-loongson1.c
+++ b/drivers/gpio/gpio-loongson1.c
@@ -16,15 +16,19 @@
 #define GPIO_DATA		0x20
 #define GPIO_OUTPUT		0x30
 
-static void __iomem *gpio_reg_base;
+struct ls1x_gpio_chip {
+	struct gpio_chip gc;
+	void __iomem *reg_base;
+};
 
 static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
 {
+	struct ls1x_gpio_chip *ls1x_gc = gpiochip_get_data(gc);
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
-	__raw_writel(__raw_readl(gpio_reg_base + GPIO_CFG) | BIT(offset),
-		     gpio_reg_base + GPIO_CFG);
+	writel(readl(ls1x_gc->reg_base + GPIO_CFG) | BIT(offset),
+	       ls1x_gc->reg_base + GPIO_CFG);
 	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
 
 	return 0;
@@ -32,44 +36,45 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
 
 static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
 {
+	struct ls1x_gpio_chip *ls1x_gc = gpiochip_get_data(gc);
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
-	__raw_writel(__raw_readl(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
-		     gpio_reg_base + GPIO_CFG);
+	writel(readl(ls1x_gc->reg_base + GPIO_CFG) & ~BIT(offset),
+	       ls1x_gc->reg_base + GPIO_CFG);
 	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
 }
 
 static int ls1x_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct gpio_chip *gc;
+	struct ls1x_gpio_chip *ls1x_gc;
 	int ret;
 
-	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
-	if (!gc)
+	ls1x_gc = devm_kzalloc(dev, sizeof(*ls1x_gc), GFP_KERNEL);
+	if (!ls1x_gc)
 		return -ENOMEM;
 
-	gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(gpio_reg_base))
-		return PTR_ERR(gpio_reg_base);
+	ls1x_gc->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ls1x_gc->reg_base))
+		return PTR_ERR(ls1x_gc->reg_base);
 
-	ret = bgpio_init(gc, dev, 4, gpio_reg_base + GPIO_DATA,
-			 gpio_reg_base + GPIO_OUTPUT, NULL,
-			 NULL, gpio_reg_base + GPIO_DIR, 0);
+	ret = bgpio_init(&ls1x_gc->gc, dev, 4, ls1x_gc->reg_base + GPIO_DATA,
+			 ls1x_gc->reg_base + GPIO_OUTPUT, NULL,
+			 NULL, ls1x_gc->reg_base + GPIO_DIR, 0);
 	if (ret)
 		goto err;
 
-	gc->owner = THIS_MODULE;
-	gc->request = ls1x_gpio_request;
-	gc->free = ls1x_gpio_free;
-	gc->base = pdev->id * 32;
+	ls1x_gc->gc.owner = THIS_MODULE;
+	ls1x_gc->gc.request = ls1x_gpio_request;
+	ls1x_gc->gc.free = ls1x_gpio_free;
+	ls1x_gc->gc.base = pdev->id * 32;
 
-	ret = devm_gpiochip_add_data(dev, gc, NULL);
+	ret = devm_gpiochip_add_data(dev, &ls1x_gc->gc, ls1x_gc);
 	if (ret)
 		goto err;
 
-	platform_set_drvdata(pdev, gc);
+	platform_set_drvdata(pdev, ls1x_gc);
 	dev_info(dev, "Loongson1 GPIO driver registered\n");
 
 	return 0;
-- 
2.34.1

Re: [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct

[andy.shevchenko]

Wed, Feb 22, 2023 at 07:12:11PM +0800, Keguang Zhang kirjoitti:
> Introduce and allocate ls1x_gpio_chip struct containing
> gpio_chip and reg_base to avoid global gpio_reg_base.
> 
> Use readl() & writel() instead of __raw_readl() & __raw_writel().

Please, split this to two pathes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct

[Kelvin Cheung]

On Wed, Feb 22, 2023 at 8:18 PM <andy.shevchenko@gmail.com> wrote:
>
> Wed, Feb 22, 2023 at 07:12:11PM +0800, Keguang Zhang kirjoitti:
> > Introduce and allocate ls1x_gpio_chip struct containing
> > gpio_chip and reg_base to avoid global gpio_reg_base.
> >
> > Use readl() & writel() instead of __raw_readl() & __raw_writel().
>
> Please, split this to two pathes.
>
Sure.
Thanks!

> --
> With Best Regards,
> Andy Shevchenko
>
>


-- 
Best regards,

Kelvin Cheung

[PATCH 3/4] gpio: loongson1: Add DT support

[Keguang Zhang]

This patch adds DT support for Loongson-1 GPIO driver,
including the following changes.
- Add the of_match_table
- Parse the ngpios property
- Parse the alias id

Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
---
 drivers/gpio/gpio-loongson1.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
index b950bcfd78ce..92ad31f61bf0 100644
--- a/drivers/gpio/gpio-loongson1.c
+++ b/drivers/gpio/gpio-loongson1.c
@@ -48,7 +48,10 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
 static int ls1x_gpio_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *dn = pdev->dev.of_node;
 	struct ls1x_gpio_chip *ls1x_gc;
+	unsigned int ngpios;
+	int id;
 	int ret;
 
 	ls1x_gc = devm_kzalloc(dev, sizeof(*ls1x_gc), GFP_KERNEL);
@@ -59,34 +62,56 @@ static int ls1x_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(ls1x_gc->reg_base))
 		return PTR_ERR(ls1x_gc->reg_base);
 
+	if (of_property_read_u32(dn, "ngpios", &ngpios)) {
+		dev_err(dev, "Missing ngpios OF property\n");
+		return -ENODEV;
+	}
+
+	id = of_alias_get_id(dn, "gpio");
+	if (id < 0) {
+		dev_err(dev, "Couldn't get OF id\n");
+		return id;
+	}
+
 	ret = bgpio_init(&ls1x_gc->gc, dev, 4, ls1x_gc->reg_base + GPIO_DATA,
 			 ls1x_gc->reg_base + GPIO_OUTPUT, NULL,
 			 NULL, ls1x_gc->reg_base + GPIO_DIR, 0);
 	if (ret)
 		goto err;
 
+	ls1x_gc->gc.label = dev_name(&pdev->dev);
+	ls1x_gc->gc.ngpio = ngpios;
 	ls1x_gc->gc.owner = THIS_MODULE;
+	ls1x_gc->gc.parent = dev;
+	ls1x_gc->gc.base = pdev->id * BITS_PER_LONG;
 	ls1x_gc->gc.request = ls1x_gpio_request;
 	ls1x_gc->gc.free = ls1x_gpio_free;
-	ls1x_gc->gc.base = pdev->id * 32;
 
 	ret = devm_gpiochip_add_data(dev, &ls1x_gc->gc, ls1x_gc);
 	if (ret)
 		goto err;
 
 	platform_set_drvdata(pdev, ls1x_gc);
-	dev_info(dev, "Loongson1 GPIO driver registered\n");
+
+	dev_info(dev, "GPIO controller %d registered with %d pins\n", id,
+		 ngpios);
 
 	return 0;
 err:
-	dev_err(dev, "failed to register GPIO device\n");
+	dev_err(dev, "failed to register GPIO controller\n");
 	return ret;
 }
 
+static const struct of_device_id ls1x_gpio_dt_ids[] = {
+	{ .compatible = "loongson,ls1x-gpio", },
+	{ /* sentinel */ }
+};
+
 static struct platform_driver ls1x_gpio_driver = {
 	.probe	= ls1x_gpio_probe,
 	.driver	= {
 		.name	= "ls1x-gpio",
+		.of_match_table = ls1x_gpio_dt_ids,
 	},
 };
 
-- 
2.34.1

Re: [PATCH 3/4] gpio: loongson1: Add DT support

[andy.shevchenko]

Wed, Feb 22, 2023 at 07:12:12PM +0800, Keguang Zhang kirjoitti:
> This patch adds DT support for Loongson-1 GPIO driver,
> including the following changes.
> - Add the of_match_table
> - Parse the ngpios property
> - Parse the alias id

Split!

...

> +	if (of_property_read_u32(dn, "ngpios", &ngpios)) {
> +		dev_err(dev, "Missing ngpios OF property\n");
> +		return -ENODEV;
> +	}

Why?! GPIO library has this already.

...

> +	id = of_alias_get_id(dn, "gpio");
> +	if (id < 0) {
> +		dev_err(dev, "Couldn't get OF id\n");
> +		return id;
> +	}

What is this for?

...

> +	ls1x_gc->gc.base = pdev->id * BITS_PER_LONG;
> -	ls1x_gc->gc.base = pdev->id * 32;

No way. This is change makes me thing that initially it's simply wrong. Please,
just use -1 for the base.

...

> +static const struct of_device_id ls1x_gpio_dt_ids[] = {
> +	{ .compatible = "loongson,ls1x-gpio", },

Inner comma is not needed.

> +	{ /* sentinel */ }
> +};

You missed MODULE_DEVICE_TABLE().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] gpio: loongson1: Add DT support

[Kelvin Cheung]

On Wed, Feb 22, 2023 at 8:21 PM <andy.shevchenko@gmail.com> wrote:
>
> Wed, Feb 22, 2023 at 07:12:12PM +0800, Keguang Zhang kirjoitti:
> > This patch adds DT support for Loongson-1 GPIO driver,
> > including the following changes.
> > - Add the of_match_table
> > - Parse the ngpios property
> > - Parse the alias id
>
> Split!
>
> ...
>
> > +     if (of_property_read_u32(dn, "ngpios", &ngpios)) {
> > +             dev_err(dev, "Missing ngpios OF property\n");
> > +             return -ENODEV;
> > +     }
>
> Why?! GPIO library has this already.
>
Will make use of gpiolib and remove this part.

> ...
>
> > +     id = of_alias_get_id(dn, "gpio");
> > +     if (id < 0) {
> > +             dev_err(dev, "Couldn't get OF id\n");
> > +             return id;
> > +     }
>
> What is this for?
>
Will remove this part.

> ...
>
> > +     ls1x_gc->gc.base = pdev->id * BITS_PER_LONG;
> > -     ls1x_gc->gc.base = pdev->id * 32;
>
> No way. This is change makes me thing that initially it's simply wrong. Please,
> just use -1 for the base.
>
Sure. And this is already done by bgpio_init().
So I will simply remove this line.
> ...
>
> > +static const struct of_device_id ls1x_gpio_dt_ids[] = {
> > +     { .compatible = "loongson,ls1x-gpio", },
>
> Inner comma is not needed.
>
Will do.
> > +     { /* sentinel */ }
> > +};
>
> You missed MODULE_DEVICE_TABLE().
>
Will add MODULE_DEVICE_TABLE().
> --
> With Best Regards,
> Andy Shevchenko
>
>


-- 
Best regards,

Kelvin Cheung

[PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO

[Keguang Zhang]

Add devicetree binding document for Loongson-1 GPIO.

Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
---
 .../bindings/gpio/loongson,ls1x-gpio.yaml     | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
new file mode 100644
index 000000000000..e4ab49d48fae
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/loongson,ls1x-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson-1 GPIO controller
+
+maintainers:
+  - Keguang Zhang <keguang.zhang@gmail.com>
+
+properties:
+  compatible:
+    const: loongson,ls1x-gpio
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  ngpios:
+    minimum: 1
+    maximum: 32
+
+required:
+  - compatible
+  - reg
+  - gpio-controller
+  - '#gpio-cells'
+  - ngpios
+
+additionalProperties: false
+
+examples:
+  - |
+    gpio0: gpio@1fd010c0 {
+        compatible = "loongson,ls1x-gpio";
+        reg = <0x1fd010c0 0x4>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+
+        ngpios = <32>;
+    };
+
+  - |
+    gpio1: gpio@1fd010c4 {
+        compatible = "loongson,ls1x-gpio";
+        reg = <0x1fd010c4 0x4>;
+
+        gpio-controller;
+        #gpio-cells = <2>;
+
+        ngpios = <32>;
+    };
+
+...
-- 
2.34.1

Re: [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO

[Krzysztof Kozlowski]

On 22/02/2023 12:12, Keguang Zhang wrote:
> Add devicetree binding document for Loongson-1 GPIO.
> 
> Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> ---
>  .../bindings/gpio/loongson,ls1x-gpio.yaml     | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
> new file mode 100644
> index 000000000000..e4ab49d48fae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/loongson,ls1x-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson-1 GPIO controller
> +
> +maintainers:
> +  - Keguang Zhang <keguang.zhang@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: loongson,ls1x-gpio
> +
> +  reg:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  ngpios:
> +    minimum: 1
> +    maximum: 32

Isn't this fixed at 32? Do you have hardware with different number of GPIOs?

> +
> +required:
> +  - compatible
> +  - reg
> +  - gpio-controller
> +  - '#gpio-cells'

Use consistent quotes - either " or '

> +  - ngpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    gpio0: gpio@1fd010c0 {
> +        compatible = "loongson,ls1x-gpio";
> +        reg = <0x1fd010c0 0x4>;
> +
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +
> +        ngpios = <32>;
> +    };
> +
> +  - |
> +    gpio1: gpio@1fd010c4 {
> +        compatible = "loongson,ls1x-gpio";
> +        reg = <0x1fd010c4 0x4>;
> +
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +
> +        ngpios = <32>;
> +    };

These are two the same examples, keep only one.

Best regards,
Krzysztof

Re: [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO

[Kelvin Cheung]

On Wed, Feb 22, 2023 at 8:35 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/02/2023 12:12, Keguang Zhang wrote:
> > Add devicetree binding document for Loongson-1 GPIO.
> >
> > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > ---
> >  .../bindings/gpio/loongson,ls1x-gpio.yaml     | 60 +++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
> > new file mode 100644
> > index 000000000000..e4ab49d48fae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/loongson,ls1x-gpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Loongson-1 GPIO controller
> > +
> > +maintainers:
> > +  - Keguang Zhang <keguang.zhang@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: loongson,ls1x-gpio
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  gpio-controller: true
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  ngpios:
> > +    minimum: 1
> > +    maximum: 32
>
> Isn't this fixed at 32? Do you have hardware with different number of GPIOs?
>
Yes. The GPIO number of some groups is less than 32.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - gpio-controller
> > +  - '#gpio-cells'
>
> Use consistent quotes - either " or '
>
Will do.

> > +  - ngpios
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    gpio0: gpio@1fd010c0 {
> > +        compatible = "loongson,ls1x-gpio";
> > +        reg = <0x1fd010c0 0x4>;
> > +
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> > +
> > +        ngpios = <32>;
> > +    };
> > +
> > +  - |
> > +    gpio1: gpio@1fd010c4 {
> > +        compatible = "loongson,ls1x-gpio";
> > +        reg = <0x1fd010c4 0x4>;
> > +
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> > +
> > +        ngpios = <32>;
> > +    };
>
> These are two the same examples, keep only one.
>
Will do.

> Best regards,
> Krzysztof
>


-- 
Best regards,

Kelvin Cheung