linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Devicetree support for Loongson-1 GPIO
@ 2023-02-22 11:12 Keguang Zhang
  2023-02-22 11:12 ` [PATCH 1/4] gpio: loongson1: Update copyright Keguang Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Keguang Zhang @ 2023-02-22 11:12 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-mips, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, 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


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

* [PATCH 1/4] gpio: loongson1: Update copyright
  2023-02-22 11:12 [PATCH 0/4] Devicetree support for Loongson-1 GPIO Keguang Zhang
@ 2023-02-22 11:12 ` Keguang Zhang
  2023-02-22 12:17   ` andy.shevchenko
  2023-02-22 11:12 ` [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct Keguang Zhang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Keguang Zhang @ 2023-02-22 11:12 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-mips, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, 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


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

* [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct
  2023-02-22 11:12 [PATCH 0/4] Devicetree support for Loongson-1 GPIO Keguang Zhang
  2023-02-22 11:12 ` [PATCH 1/4] gpio: loongson1: Update copyright Keguang Zhang
@ 2023-02-22 11:12 ` Keguang Zhang
  2023-02-22 12:18   ` andy.shevchenko
  2023-02-22 11:12 ` [PATCH 3/4] gpio: loongson1: Add DT support Keguang Zhang
  2023-02-22 11:12 ` [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO Keguang Zhang
  3 siblings, 1 reply; 16+ messages in thread
From: Keguang Zhang @ 2023-02-22 11:12 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-mips, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, 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


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

* [PATCH 3/4] gpio: loongson1: Add DT support
  2023-02-22 11:12 [PATCH 0/4] Devicetree support for Loongson-1 GPIO Keguang Zhang
  2023-02-22 11:12 ` [PATCH 1/4] gpio: loongson1: Update copyright Keguang Zhang
  2023-02-22 11:12 ` [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct Keguang Zhang
@ 2023-02-22 11:12 ` Keguang Zhang
  2023-02-22 12:20   ` andy.shevchenko
  2023-02-22 11:12 ` [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO Keguang Zhang
  3 siblings, 1 reply; 16+ messages in thread
From: Keguang Zhang @ 2023-02-22 11:12 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-mips, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, 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


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

* [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO
  2023-02-22 11:12 [PATCH 0/4] Devicetree support for Loongson-1 GPIO Keguang Zhang
                   ` (2 preceding siblings ...)
  2023-02-22 11:12 ` [PATCH 3/4] gpio: loongson1: Add DT support Keguang Zhang
@ 2023-02-22 11:12 ` Keguang Zhang
  2023-02-22 12:35   ` Krzysztof Kozlowski
  3 siblings, 1 reply; 16+ messages in thread
From: Keguang Zhang @ 2023-02-22 11:12 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-mips, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, 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


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

* Re: [PATCH 1/4] gpio: loongson1: Update copyright
  2023-02-22 11:12 ` [PATCH 1/4] gpio: loongson1: Update copyright Keguang Zhang
@ 2023-02-22 12:17   ` andy.shevchenko
  2023-03-02 11:09     ` Kelvin Cheung
  0 siblings, 1 reply; 16+ messages in thread
From: andy.shevchenko @ 2023-02-22 12:17 UTC (permalink / raw)
  To: Keguang Zhang
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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



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

* Re: [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct
  2023-02-22 11:12 ` [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct Keguang Zhang
@ 2023-02-22 12:18   ` andy.shevchenko
  2023-03-02 11:11     ` Kelvin Cheung
  0 siblings, 1 reply; 16+ messages in thread
From: andy.shevchenko @ 2023-02-22 12:18 UTC (permalink / raw)
  To: Keguang Zhang
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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



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

* Re: [PATCH 3/4] gpio: loongson1: Add DT support
  2023-02-22 11:12 ` [PATCH 3/4] gpio: loongson1: Add DT support Keguang Zhang
@ 2023-02-22 12:20   ` andy.shevchenko
  2023-03-02 11:08     ` Kelvin Cheung
  0 siblings, 1 reply; 16+ messages in thread
From: andy.shevchenko @ 2023-02-22 12:20 UTC (permalink / raw)
  To: Keguang Zhang
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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



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

* Re: [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO
  2023-02-22 11:12 ` [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO Keguang Zhang
@ 2023-02-22 12:35   ` Krzysztof Kozlowski
  2023-03-01  9:50     ` Kelvin Cheung
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-02-22 12:35 UTC (permalink / raw)
  To: Keguang Zhang, linux-gpio, devicetree, linux-mips, linux-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, 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


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

* Re: [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO
  2023-02-22 12:35   ` Krzysztof Kozlowski
@ 2023-03-01  9:50     ` Kelvin Cheung
  0 siblings, 0 replies; 16+ messages in thread
From: Kelvin Cheung @ 2023-03-01  9:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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

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

* Re: [PATCH 3/4] gpio: loongson1: Add DT support
  2023-02-22 12:20   ` andy.shevchenko
@ 2023-03-02 11:08     ` Kelvin Cheung
  0 siblings, 0 replies; 16+ messages in thread
From: Kelvin Cheung @ 2023-03-02 11:08 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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

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

* Re: [PATCH 1/4] gpio: loongson1: Update copyright
  2023-02-22 12:17   ` andy.shevchenko
@ 2023-03-02 11:09     ` Kelvin Cheung
  2023-03-02 11:40       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Kelvin Cheung @ 2023-03-02 11:09 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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

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

* Re: [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct
  2023-02-22 12:18   ` andy.shevchenko
@ 2023-03-02 11:11     ` Kelvin Cheung
  0 siblings, 0 replies; 16+ messages in thread
From: Kelvin Cheung @ 2023-03-02 11:11 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski

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

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

* Re: [PATCH 1/4] gpio: loongson1: Update copyright
  2023-03-02 11:09     ` Kelvin Cheung
@ 2023-03-02 11:40       ` Krzysztof Kozlowski
  2023-03-02 12:16         ` Kelvin Cheung
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-02 11:40 UTC (permalink / raw)
  To: Kelvin Cheung, andy.shevchenko
  Cc: linux-gpio, devicetree, linux-mips, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Rob Herring, 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


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

* Re: [PATCH 1/4] gpio: loongson1: Update copyright
  2023-03-02 11:40       ` Krzysztof Kozlowski
@ 2023-03-02 12:16         ` Kelvin Cheung
  2023-03-02 19:02           ` Andy Shevchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Kelvin Cheung @ 2023-03-02 12:16 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: andy.shevchenko, linux-gpio, devicetree, linux-mips,
	linux-kernel, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski

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

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

* Re: [PATCH 1/4] gpio: loongson1: Update copyright
  2023-03-02 12:16         ` Kelvin Cheung
@ 2023-03-02 19:02           ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2023-03-02 19:02 UTC (permalink / raw)
  To: Kelvin Cheung
  Cc: Krzysztof Kozlowski, linux-gpio, devicetree, linux-mips,
	linux-kernel, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski

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

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

end of thread, other threads:[~2023-03-02 19:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-22 11:12 [PATCH 0/4] Devicetree support for Loongson-1 GPIO Keguang Zhang
2023-02-22 11:12 ` [PATCH 1/4] gpio: loongson1: Update copyright Keguang Zhang
2023-02-22 12:17   ` andy.shevchenko
2023-03-02 11:09     ` Kelvin Cheung
2023-03-02 11:40       ` Krzysztof Kozlowski
2023-03-02 12:16         ` Kelvin Cheung
2023-03-02 19:02           ` Andy Shevchenko
2023-02-22 11:12 ` [PATCH 2/4] gpio: loongson1: Introduce ls1x_gpio_chip struct Keguang Zhang
2023-02-22 12:18   ` andy.shevchenko
2023-03-02 11:11     ` Kelvin Cheung
2023-02-22 11:12 ` [PATCH 3/4] gpio: loongson1: Add DT support Keguang Zhang
2023-02-22 12:20   ` andy.shevchenko
2023-03-02 11:08     ` Kelvin Cheung
2023-02-22 11:12 ` [PATCH 4/4] dt-bindings: gpio: Add Loongson-1 GPIO Keguang Zhang
2023-02-22 12:35   ` Krzysztof Kozlowski
2023-03-01  9:50     ` Kelvin Cheung

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).