Update the driver to add DT support and dt-binding document,
along with other minor changes.
Keguang Zhang (5):
gpio: loongson1: Convert to SPDX identifier
gpio: loongson1: Use readl() & writel()
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 | 49 +++++++++++++
drivers/gpio/gpio-loongson1.c | 71 +++++++++++--------
2 files changed, 92 insertions(+), 28 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
base-commit: 4827aae061337251bb91801b316157a78b845ec7
--
2.34.1
Use SPDX-License-Identifier instead of the license text and
update the author information.
Signed-off-by: Keguang Zhang <[email protected]>
---
V1 -> V2: Keep GPLv2, just convert to SPDX identifier
---
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..8862c9ea0d41 100644
--- a/drivers/gpio/gpio-loongson1.c
+++ b/drivers/gpio/gpio-loongson1.c
@@ -1,11 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
/*
* GPIO Driver for Loongson 1 SoC
*
- * Copyright (C) 2015-2016 Zhang, Keguang <[email protected]>
- *
- * 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 <[email protected]>
*/
#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 <[email protected]>");
+MODULE_AUTHOR("Keguang Zhang <[email protected]>");
MODULE_DESCRIPTION("Loongson1 GPIO driver");
MODULE_LICENSE("GPL");
--
2.34.1
This patch replace __raw_readl() & __raw_writel() with readl() & writel().
Signed-off-by: Keguang Zhang <[email protected]>
---
V1 -> V2: Split this change to a separate patch
---
drivers/gpio/gpio-loongson1.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
index 8862c9ea0d41..b6c11caa3ade 100644
--- a/drivers/gpio/gpio-loongson1.c
+++ b/drivers/gpio/gpio-loongson1.c
@@ -23,8 +23,8 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
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(gpio_reg_base + GPIO_CFG) | BIT(offset),
+ gpio_reg_base + GPIO_CFG);
raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
return 0;
@@ -35,8 +35,8 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
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(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
+ gpio_reg_base + GPIO_CFG);
raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
}
--
2.34.1
This patch introduces and allocates ls1x_gpio_chip struct containing
gpio_chip and reg_base to avoid global gpio_reg_base.
Signed-off-by: Keguang Zhang <[email protected]>
---
V1 -> V2: Split this change to a separate patch
---
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 b6c11caa3ade..3ac9e49e7efb 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);
- writel(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);
- writel(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
This patch adds DT support for Loongson-1 GPIO driver.
Signed-off-by: Keguang Zhang <[email protected]>
---
V1 -> V2: Let gpiolib parse ngpios property
Remove unnecessary alias id parsing
Remove superfluous initialization done by bgpio_init()
Add MODULE_DEVICE_TABLE()
Other minor fixes
---
drivers/gpio/gpio-loongson1.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
index 3ac9e49e7efb..94ac0ccb450f 100644
--- a/drivers/gpio/gpio-loongson1.c
+++ b/drivers/gpio/gpio-loongson1.c
@@ -68,25 +68,38 @@ static int ls1x_gpio_probe(struct platform_device *pdev)
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;
+ /*
+ * Clear ngpio to let gpiolib get the correct number
+ * by reading ngpios property
+ */
+ ls1x_gc->gc.ngpio = 0;
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 registered with %d pins\n",
+ ls1x_gc->gc.ngpio);
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 */ }
+};
+MODULE_DEVICE_TABLE(of, ls1x_gpio_dt_ids);
+
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
Add devicetree binding document for Loongson-1 GPIO.
Signed-off-by: Keguang Zhang <[email protected]>
---
V1 -> V2: Use the same consistent quotes
Delete superfluous examples
---
.../bindings/gpio/loongson,ls1x-gpio.yaml | 49 +++++++++++++++++++
1 file changed, 49 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..1a472c05697c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/loongson,ls1x-gpio.yaml
@@ -0,0 +1,49 @@
+# 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 <[email protected]>
+
+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>;
+ };
+
+...
--
2.34.1
On 02/03/2023 13:52, Keguang Zhang wrote:
> Add devicetree binding document for Loongson-1 GPIO.
>
> Signed-off-by: Keguang Zhang <[email protected]>
> ---
> V1 -> V2: Use the same consistent quotes
> Delete superfluous examples
> ---
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
>
> Use SPDX-License-Identifier instead of the license text and
> update the author information.
>
> Signed-off-by: Keguang Zhang <[email protected]>
> ---
> V1 -> V2: Keep GPLv2, just convert to SPDX identifier
> ---
> 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..8862c9ea0d41 100644
> --- a/drivers/gpio/gpio-loongson1.c
> +++ b/drivers/gpio/gpio-loongson1.c
> @@ -1,11 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> /*
> * GPIO Driver for Loongson 1 SoC
> *
> - * Copyright (C) 2015-2016 Zhang, Keguang <[email protected]>
> - *
> - * 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 <[email protected]>
> */
>
> #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 <[email protected]>");
Why are you removing credits of the old author?
Bart
> +MODULE_AUTHOR("Keguang Zhang <[email protected]>");
> MODULE_DESCRIPTION("Loongson1 GPIO driver");
> MODULE_LICENSE("GPL");
> --
> 2.34.1
>
On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
>
> This patch replace __raw_readl() & __raw_writel() with readl() & writel().
>
Please say WHY you're doing this.
Bart
> Signed-off-by: Keguang Zhang <[email protected]>
> ---
> V1 -> V2: Split this change to a separate patch
> ---
> drivers/gpio/gpio-loongson1.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> index 8862c9ea0d41..b6c11caa3ade 100644
> --- a/drivers/gpio/gpio-loongson1.c
> +++ b/drivers/gpio/gpio-loongson1.c
> @@ -23,8 +23,8 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> 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(gpio_reg_base + GPIO_CFG) | BIT(offset),
> + gpio_reg_base + GPIO_CFG);
> raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>
> return 0;
> @@ -35,8 +35,8 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
> 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(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
> + gpio_reg_base + GPIO_CFG);
> raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> }
>
> --
> 2.34.1
>
On Thu, Mar 2, 2023 at 1:53 PM Keguang Zhang <[email protected]> wrote:
>
> This patch adds DT support for Loongson-1 GPIO driver.
>
> Signed-off-by: Keguang Zhang <[email protected]>
> ---
> V1 -> V2: Let gpiolib parse ngpios property
> Remove unnecessary alias id parsing
> Remove superfluous initialization done by bgpio_init()
> Add MODULE_DEVICE_TABLE()
> Other minor fixes
> ---
> drivers/gpio/gpio-loongson1.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> index 3ac9e49e7efb..94ac0ccb450f 100644
> --- a/drivers/gpio/gpio-loongson1.c
> +++ b/drivers/gpio/gpio-loongson1.c
> @@ -68,25 +68,38 @@ static int ls1x_gpio_probe(struct platform_device *pdev)
> 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;
> + /*
> + * Clear ngpio to let gpiolib get the correct number
> + * by reading ngpios property
> + */
> + ls1x_gc->gc.ngpio = 0;
>
Who could have set it before and why would this information need to be
unconditionally discarded?
Bart
> 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 registered with %d pins\n",
> + ls1x_gc->gc.ngpio);
>
> 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 */ }
> +};
> +MODULE_DEVICE_TABLE(of, ls1x_gpio_dt_ids);
> +
> 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
>
On Mon, Mar 6, 2023 at 5:29 PM Bartosz Golaszewski <[email protected]> wrote:
>
> On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> >
> > Use SPDX-License-Identifier instead of the license text and
> > update the author information.
> >
> > Signed-off-by: Keguang Zhang <[email protected]>
> > ---
> > V1 -> V2: Keep GPLv2, just convert to SPDX identifier
> > ---
> > 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..8862c9ea0d41 100644
> > --- a/drivers/gpio/gpio-loongson1.c
> > +++ b/drivers/gpio/gpio-loongson1.c
> > @@ -1,11 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > /*
> > * GPIO Driver for Loongson 1 SoC
> > *
> > - * Copyright (C) 2015-2016 Zhang, Keguang <[email protected]>
> > - *
> > - * 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 <[email protected]>
> > */
> >
> > #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 <[email protected]>");
>
> Why are you removing credits of the old author?
Kelvin Cheung and Keguang Zhang are the same person.
This change is to keep pace with the related entry of MAINTAINERS.
>
> Bart
>
> > +MODULE_AUTHOR("Keguang Zhang <[email protected]>");
> > MODULE_DESCRIPTION("Loongson1 GPIO driver");
> > MODULE_LICENSE("GPL");
> > --
> > 2.34.1
> >
--
Best regards,
Kelvin Cheung
On Mon, Mar 6, 2023 at 5:39 PM Bartosz Golaszewski <[email protected]> wrote:
>
> On Thu, Mar 2, 2023 at 1:53 PM Keguang Zhang <[email protected]> wrote:
> >
> > This patch adds DT support for Loongson-1 GPIO driver.
> >
> > Signed-off-by: Keguang Zhang <[email protected]>
> > ---
> > V1 -> V2: Let gpiolib parse ngpios property
> > Remove unnecessary alias id parsing
> > Remove superfluous initialization done by bgpio_init()
> > Add MODULE_DEVICE_TABLE()
> > Other minor fixes
> > ---
> > drivers/gpio/gpio-loongson1.c | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> > index 3ac9e49e7efb..94ac0ccb450f 100644
> > --- a/drivers/gpio/gpio-loongson1.c
> > +++ b/drivers/gpio/gpio-loongson1.c
> > @@ -68,25 +68,38 @@ static int ls1x_gpio_probe(struct platform_device *pdev)
> > 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;
> > + /*
> > + * Clear ngpio to let gpiolib get the correct number
> > + * by reading ngpios property
> > + */
> > + ls1x_gc->gc.ngpio = 0;
> >
>
> Who could have set it before and why would this information need to be
> unconditionally discarded?
>
'ngpio' has been set to 32 by bgpio_init().
But this is incorrect for LS1B who has different number of GPIOs for each group.
To get the right number, I have to discard this default value to let
gpiolib parse 'ngpios' property.
> Bart
>
> > 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 registered with %d pins\n",
> > + ls1x_gc->gc.ngpio);
> >
> > 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 */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ls1x_gpio_dt_ids);
> > +
> > 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
> >
--
Best regards,
Kelvin Cheung
On Mon, Mar 6, 2023 at 5:30 PM Bartosz Golaszewski <[email protected]> wrote:
>
> On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> >
> > This patch replace __raw_readl() & __raw_writel() with readl() & writel().
> >
>
> Please say WHY you're doing this.
>
readl & writel contain memory barriers which can guarantee access order.
> Bart
>
> > Signed-off-by: Keguang Zhang <[email protected]>
> > ---
> > V1 -> V2: Split this change to a separate patch
> > ---
> > drivers/gpio/gpio-loongson1.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> > index 8862c9ea0d41..b6c11caa3ade 100644
> > --- a/drivers/gpio/gpio-loongson1.c
> > +++ b/drivers/gpio/gpio-loongson1.c
> > @@ -23,8 +23,8 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> > 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(gpio_reg_base + GPIO_CFG) | BIT(offset),
> > + gpio_reg_base + GPIO_CFG);
> > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> >
> > return 0;
> > @@ -35,8 +35,8 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
> > 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(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
> > + gpio_reg_base + GPIO_CFG);
> > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > }
> >
> > --
> > 2.34.1
> >
--
Best regards,
Kelvin Cheung
On Tue, Mar 7, 2023 at 3:25 AM Keguang Zhang <[email protected]> wrote:
> On Mon, Mar 6, 2023 at 5:29 PM Bartosz Golaszewski <[email protected]> wrote:
> > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> > >
> > > Use SPDX-License-Identifier instead of the license text and
> > > update the author information.
> > >
> > > Signed-off-by: Keguang Zhang <[email protected]>
> > Why are you removing credits of the old author?
> Kelvin Cheung and Keguang Zhang are the same person.
> This change is to keep pace with the related entry of MAINTAINERS.
That's a pretty interesting change!
Is Kelvin Cheung the "westernized" name and Keguang Zhang the
closer to the real name, such as pinyin form? That would make
a lot of sense.
I think some authors even use the native characters these days,
as git and all tools and terminals should support Unicode now.
It might make it hard for us to answer mails (not knowing which
characters to refer to as given name) but I kind of like it when I
see it.
Yours,
Linus Walleij
On Tue, Mar 7, 2023 at 3:25 AM Keguang Zhang <[email protected]> wrote:
>
> On Mon, Mar 6, 2023 at 5:29 PM Bartosz Golaszewski <[email protected]> wrote:
> >
> > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> > >
> > > Use SPDX-License-Identifier instead of the license text and
> > > update the author information.
> > >
> > > Signed-off-by: Keguang Zhang <[email protected]>
> > > ---
> > > V1 -> V2: Keep GPLv2, just convert to SPDX identifier
> > > ---
> > > 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..8862c9ea0d41 100644
> > > --- a/drivers/gpio/gpio-loongson1.c
> > > +++ b/drivers/gpio/gpio-loongson1.c
> > > @@ -1,11 +1,8 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > /*
> > > * GPIO Driver for Loongson 1 SoC
> > > *
> > > - * Copyright (C) 2015-2016 Zhang, Keguang <[email protected]>
> > > - *
> > > - * 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 <[email protected]>
> > > */
> > >
> > > #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 <[email protected]>");
> >
> > Why are you removing credits of the old author?
> Kelvin Cheung and Keguang Zhang are the same person.
> This change is to keep pace with the related entry of MAINTAINERS.
>
Even so - how could I have possibly known this? Please put it into the
commit message, it's crucial information for this change.
Bart
On Tue, Mar 7, 2023 at 9:31 PM Linus Walleij <[email protected]> wrote:
>
> On Tue, Mar 7, 2023 at 3:25 AM Keguang Zhang <[email protected]> wrote:
> > On Mon, Mar 6, 2023 at 5:29 PM Bartosz Golaszewski <[email protected]> wrote:
> > > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> > > >
> > > > Use SPDX-License-Identifier instead of the license text and
> > > > update the author information.
> > > >
> > > > Signed-off-by: Keguang Zhang <[email protected]>
>
> > > Why are you removing credits of the old author?
>
> > Kelvin Cheung and Keguang Zhang are the same person.
> > This change is to keep pace with the related entry of MAINTAINERS.
>
> That's a pretty interesting change!
>
> Is Kelvin Cheung the "westernized" name and Keguang Zhang the
> closer to the real name, such as pinyin form? That would make
> a lot of sense.
>
Exactly.
Kelvin Cheung is easy to pronounce, but has no direct relationship
with my real name.
Keguang Zhang, the Pinyin form of my real name, is the official name.
That is why I'd like to make this change.
> I think some authors even use the native characters these days,
> as git and all tools and terminals should support Unicode now.
> It might make it hard for us to answer mails (not knowing which
> characters to refer to as given name) but I kind of like it when I
> see it.
>
Yes, I did see the names written in native characters.
But it's even harder for western people to identify.
Maybe the real name with native characters + "westernized" name is better.
> Yours,
> Linus Walleij
--
Best regards,
Kelvin Cheung
On Wed, Mar 8, 2023 at 12:49 AM Bartosz Golaszewski <[email protected]> wrote:
>
> On Tue, Mar 7, 2023 at 3:25 AM Keguang Zhang <[email protected]> wrote:
> >
> > On Mon, Mar 6, 2023 at 5:29 PM Bartosz Golaszewski <[email protected]> wrote:
> > >
> > > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> > > >
> > > > Use SPDX-License-Identifier instead of the license text and
> > > > update the author information.
> > > >
> > > > Signed-off-by: Keguang Zhang <[email protected]>
> > > > ---
> > > > V1 -> V2: Keep GPLv2, just convert to SPDX identifier
> > > > ---
> > > > 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..8862c9ea0d41 100644
> > > > --- a/drivers/gpio/gpio-loongson1.c
> > > > +++ b/drivers/gpio/gpio-loongson1.c
> > > > @@ -1,11 +1,8 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > /*
> > > > * GPIO Driver for Loongson 1 SoC
> > > > *
> > > > - * Copyright (C) 2015-2016 Zhang, Keguang <[email protected]>
> > > > - *
> > > > - * 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 <[email protected]>
> > > > */
> > > >
> > > > #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 <[email protected]>");
> > >
> > > Why are you removing credits of the old author?
> > Kelvin Cheung and Keguang Zhang are the same person.
> > This change is to keep pace with the related entry of MAINTAINERS.
> >
>
> Even so - how could I have possibly known this? Please put it into the
> commit message, it's crucial information for this change.
>
Sure. I will amend the commit message.
In addition, should I update this patch only? Or the whole patch series?
> Bart
--
Best regards,
Kelvin Cheung
From: Keguang Zhang
> Sent: 07 March 2023 03:46
>
> On Mon, Mar 6, 2023 at 5:30 PM Bartosz Golaszewski <[email protected]> wrote:
> >
> > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> > >
> > > This patch replace __raw_readl() & __raw_writel() with readl() & writel().
> > >
> >
> > Please say WHY you're doing this.
> >
> readl & writel contain memory barriers which can guarantee access order.
So what...
There is a data dependency between the read and write.
The read can't be scheduled before the lock is acquired.
The write can't be scheduled after the lock is released.
So any barriers in readl()/writel() aren't needed.
If they are only compile barriers they'll have no real effect.
OTOH if the cpu needs actual synchronising instructions (as some
ppc do) then they will slow things down.
David
>
> > Bart
> >
> > > Signed-off-by: Keguang Zhang <[email protected]>
> > > ---
> > > V1 -> V2: Split this change to a separate patch
> > > ---
> > > drivers/gpio/gpio-loongson1.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> > > index 8862c9ea0d41..b6c11caa3ade 100644
> > > --- a/drivers/gpio/gpio-loongson1.c
> > > +++ b/drivers/gpio/gpio-loongson1.c
> > > @@ -23,8 +23,8 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> > > 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(gpio_reg_base + GPIO_CFG) | BIT(offset),
> > > + gpio_reg_base + GPIO_CFG);
> > > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > >
> > > return 0;
> > > @@ -35,8 +35,8 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
> > > 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(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
> > > + gpio_reg_base + GPIO_CFG);
> > > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > > }
> > >
> > > --
> > > 2.34.1
> > >
>
>
>
> --
> Best regards,
>
> Kelvin Cheung
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Wed, Mar 8, 2023 at 5:42 PM David Laight <[email protected]> wrote:
>
> From: Keguang Zhang
> > Sent: 07 March 2023 03:46
> >
> > On Mon, Mar 6, 2023 at 5:30 PM Bartosz Golaszewski <[email protected]> wrote:
> > >
> > > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <[email protected]> wrote:
> > > >
> > > > This patch replace __raw_readl() & __raw_writel() with readl() & writel().
> > > >
> > >
> > > Please say WHY you're doing this.
> > >
> > readl & writel contain memory barriers which can guarantee access order.
>
> So what...
>
> There is a data dependency between the read and write.
> The read can't be scheduled before the lock is acquired.
> The write can't be scheduled after the lock is released.
>
> So any barriers in readl()/writel() aren't needed.
>
> If they are only compile barriers they'll have no real effect.
> OTOH if the cpu needs actual synchronising instructions (as some
> ppc do) then they will slow things down.
>
Thanks for the explanation.
The intention of this change is to prevent possible order issues.
At present, __raw_readl() & __raw_writel() do work fine.
I will drop this patch in the next version.
> David
>
> >
> > > Bart
> > >
> > > > Signed-off-by: Keguang Zhang <[email protected]>
> > > > ---
> > > > V1 -> V2: Split this change to a separate patch
> > > > ---
> > > > drivers/gpio/gpio-loongson1.c | 8 ++++----
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> > > > index 8862c9ea0d41..b6c11caa3ade 100644
> > > > --- a/drivers/gpio/gpio-loongson1.c
> > > > +++ b/drivers/gpio/gpio-loongson1.c
> > > > @@ -23,8 +23,8 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> > > > 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(gpio_reg_base + GPIO_CFG) | BIT(offset),
> > > > + gpio_reg_base + GPIO_CFG);
> > > > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > > >
> > > > return 0;
> > > > @@ -35,8 +35,8 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
> > > > 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(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
> > > > + gpio_reg_base + GPIO_CFG);
> > > > raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > > > }
> > > >
> > > > --
> > > > 2.34.1
> > > >
> >
> >
> >
> > --
> > Best regards,
> >
> > Kelvin Cheung
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
--
Best regards,
Kelvin Cheung