Skip to contentSkip to conversation

drivers: gpio: zevio: drop of_gpio.h header

PATCH Superseded Moses Christopher Bollavarapu 4 participants 8 messages May 10, 2022 – May 14, 2022 GPIO

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers: gpio: zevio: drop of_gpio.h header

remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives

---
 drivers/gpio/gpio-zevio.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)
drivers/gpio/gpio-zevio.c +17 -11
index f6f8a541348f..ec774fefb0ff 100644
@@ -11,7 +11,6 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/gpio/driver.h>
 
@@ -54,21 +53,22 @@
 
 struct zevio_gpio {
 	spinlock_t		lock;
-	struct of_mm_gpio_chip	chip;
+	struct gpio_chip	chip;
+	void __iomem		*regs;
 };
 
 static inline u32 zevio_gpio_port_get(struct zevio_gpio *c, unsigned pin,
 					unsigned port_offset)
 {
 	unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-	return readl(IOMEM(c->chip.regs + section_offset + port_offset));
+	return readl(IOMEM(c->regs + section_offset + port_offset));
 }
 
 static inline void zevio_gpio_port_set(struct zevio_gpio *c, unsigned pin,
 					unsigned port_offset, u32 val)
 {
 	unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-	writel(val, IOMEM(c->chip.regs + section_offset + port_offset));
+	writel(val, IOMEM(c->regs + section_offset + port_offset));
 }
 
 /* Functions for struct gpio_chip */
@@ -178,12 +178,18 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, controller);
 
 	/* Copy our reference */
-	controller->chip.gc = zevio_gpio_chip;
-	controller->chip.gc.parent = &pdev->dev;
+	controller->chip = zevio_gpio_chip;
+	controller->chip.parent = &pdev->dev;
 
-	status = of_mm_gpiochip_add_data(pdev->dev.of_node,
-					 &(controller->chip),
-					 controller);
+	controller->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(controller->regs)) {
+		dev_err(&pdev->dev, "failed to ioremap memory resource\n");
+		return PTR_ERR(controller->regs);
+	}
+
+	status = devm_gpiochip_add_data(&pdev->dev,
+					&(controller->chip),
+					controller);
 	if (status) {
 		dev_err(&pdev->dev, "failed to add gpiochip: %d\n", status);
 		return status;
@@ -192,10 +198,10 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	spin_lock_init(&controller->lock);
 
 	/* Disable interrupts, they only cause errors */
-	for (i = 0; i < controller->chip.gc.ngpio; i += 8)
+	for (i = 0; i < controller->chip.ngpio; i += 8)
 		zevio_gpio_port_set(controller, i, ZEVIO_GPIO_INT_MASK, 0xFF);
 
-	dev_dbg(controller->chip.gc.parent, "ZEVIO GPIO controller set up!\n");
+	dev_dbg(controller->chip.parent, "ZEVIO GPIO controller set up!\n");
 
 	return 0;
 }
Signature · 1 line
2.30.2
#1 lore raw

On Wed, May 11, 2022 at 12:31 AM Moses Christopher Bollavarapu <[email protected]> wrote:

I like the idea and the patch, but a few comments below.

Quoted 2 lines

remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives

Please respect English grammar, i.e. Capital letters at the beginning of the sentences and period at the ends.

...

drivers/gpio/gpio-zevio.c line 57

Quoted 5 lines
 struct zevio_gpio {
        spinlock_t              lock;
drivers/gpio/gpio-zevio.c +2 -1
-       struct of_mm_gpio_chip  chip;
+       struct gpio_chip        chip;
+       void __iomem            *regs;

While at it, please move the chip member to be the first one. It will optimize container_of() in case it's being called against a zevio_gpio object instance.

Quoted 1 line

};

...

drivers/gpio/gpio-zevio.c static int zevio_gpio_probe(struct platform_device *pdev) · line 187

Quoted 5 lines
drivers/gpio/gpio-zevio.c +4 -0
+       controller->regs = devm_platform_ioremap_resource(pdev, 0);
+       if (IS_ERR(controller->regs)) {
 
+               dev_err(&pdev->dev, "failed to ioremap memory resource\n");
+               return PTR_ERR(controller->regs);

return dev_err_probe(&pdev->dev, ...);

Quoted 1 line

+ }

...

drivers/gpio/gpio-zevio.c static int zevio_gpio_probe(struct platform_device *pdev) · line 191

Quoted 2 lines
drivers/gpio/gpio-zevio.c +2 -0
+       status = devm_gpiochip_add_data(&pdev->dev,
+                                       &(controller->chip),

Too many parentheses.

Quoted 1 line

+ controller);

Also, combine all of them to be located on a single line.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#2 lore raw

PATCH v2 1/1 drivers: gpio: zevio: drop of_gpio.h header

In reply to Moses Christopher Bollavarapu

Remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives.

---
V1 -> V2: Move gpio_chip member to top of the struct
Use dev_error_probe instead of dev_err Minor style fixes

 drivers/gpio/gpio-zevio.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/gpio/gpio-zevio.c +14 -11
index f6f8a541348f..df22f79e8210 100644
@@ -11,7 +11,6 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/gpio/driver.h>
 
@@ -53,22 +52,23 @@
 #define ZEVIO_GPIO_BIT(gpio) (gpio&7)
 
 struct zevio_gpio {
+	struct gpio_chip        chip;
 	spinlock_t		lock;
-	struct of_mm_gpio_chip	chip;
+	void __iomem		*regs;
 };
 
 static inline u32 zevio_gpio_port_get(struct zevio_gpio *c, unsigned pin,
 					unsigned port_offset)
 {
 	unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-	return readl(IOMEM(c->chip.regs + section_offset + port_offset));
+	return readl(IOMEM(c->regs + section_offset + port_offset));
 }
 
 static inline void zevio_gpio_port_set(struct zevio_gpio *c, unsigned pin,
 					unsigned port_offset, u32 val)
 {
 	unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-	writel(val, IOMEM(c->chip.regs + section_offset + port_offset));
+	writel(val, IOMEM(c->regs + section_offset + port_offset));
 }
 
 /* Functions for struct gpio_chip */
@@ -178,12 +178,15 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, controller);
 
 	/* Copy our reference */
-	controller->chip.gc = zevio_gpio_chip;
-	controller->chip.gc.parent = &pdev->dev;
+	controller->chip = zevio_gpio_chip;
+	controller->chip.parent = &pdev->dev;
 
-	status = of_mm_gpiochip_add_data(pdev->dev.of_node,
-					 &(controller->chip),
-					 controller);
+	controller->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(controller->regs))
+		dev_err_probe(&pdev->dev, PTR_ERR(controller->regs),
+			      "failed to ioremap memory resource\n");
+
+	status = devm_gpiochip_add_data(&pdev->dev, &controller->chip, controller);
 	if (status) {
 		dev_err(&pdev->dev, "failed to add gpiochip: %d\n", status);
 		return status;
@@ -192,10 +195,10 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	spin_lock_init(&controller->lock);
 
 	/* Disable interrupts, they only cause errors */
-	for (i = 0; i < controller->chip.gc.ngpio; i += 8)
+	for (i = 0; i < controller->chip.ngpio; i += 8)
 		zevio_gpio_port_set(controller, i, ZEVIO_GPIO_INT_MASK, 0xFF);
 
-	dev_dbg(controller->chip.gc.parent, "ZEVIO GPIO controller set up!\n");
+	dev_dbg(controller->chip.parent, "ZEVIO GPIO controller set up!\n");
 
 	return 0;
 }
Signature · 1 line
2.30.2
#3 lore raw
Quoted 3 lines · Moses Christopher Bollavarapu

Remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives.

...

Quoted 3 lines

V1 -> V2: Move gpio_chip member to top of the struct
Use dev_error_probe instead of dev_err Minor style fixes

Almost, see below.

...

drivers/gpio/gpio-zevio.c static int zevio_gpio_probe(struct platform_device *pdev) · line 187

Quoted 4 lines
drivers/gpio/gpio-zevio.c +4 -0
+       controller->regs = devm_platform_ioremap_resource(pdev, 0);
+       if (IS_ERR(controller->regs))
+               dev_err_probe(&pdev->dev, PTR_ERR(controller->regs),
+                             "failed to ioremap memory resource\n");

You forgot 'return'.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#4 lore raw

PATCH v3 1/1 drivers: gpio: zevio: drop of_gpio.h header

In reply to Moses Christopher Bollavarapu

Remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives.

---
V2 -> V3: Add missing return in front of dev_error_probe V1 -> V2: Move gpio_chip member to top of the struct
Use dev_error_probe instead of dev_err Minor style fixes

 drivers/gpio/gpio-zevio.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/gpio/gpio-zevio.c +14 -11
index f6f8a541348f..ce9d1282165c 100644
@@ -11,7 +11,6 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/gpio/driver.h>
 
@@ -53,22 +52,23 @@
 #define ZEVIO_GPIO_BIT(gpio) (gpio&7)
 
 struct zevio_gpio {
+	struct gpio_chip        chip;
 	spinlock_t		lock;
-	struct of_mm_gpio_chip	chip;
+	void __iomem		*regs;
 };
 
 static inline u32 zevio_gpio_port_get(struct zevio_gpio *c, unsigned pin,
 					unsigned port_offset)
 {
 	unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-	return readl(IOMEM(c->chip.regs + section_offset + port_offset));
+	return readl(IOMEM(c->regs + section_offset + port_offset));
 }
 
 static inline void zevio_gpio_port_set(struct zevio_gpio *c, unsigned pin,
 					unsigned port_offset, u32 val)
 {
 	unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-	writel(val, IOMEM(c->chip.regs + section_offset + port_offset));
+	writel(val, IOMEM(c->regs + section_offset + port_offset));
 }
 
 /* Functions for struct gpio_chip */
@@ -178,12 +178,15 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, controller);
 
 	/* Copy our reference */
-	controller->chip.gc = zevio_gpio_chip;
-	controller->chip.gc.parent = &pdev->dev;
+	controller->chip = zevio_gpio_chip;
+	controller->chip.parent = &pdev->dev;
 
-	status = of_mm_gpiochip_add_data(pdev->dev.of_node,
-					 &(controller->chip),
-					 controller);
+	controller->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(controller->regs))
+		return dev_err_probe(&pdev->dev, PTR_ERR(controller->regs),
+				     "failed to ioremap memory resource\n");
+
+	status = devm_gpiochip_add_data(&pdev->dev, &controller->chip, controller);
 	if (status) {
 		dev_err(&pdev->dev, "failed to add gpiochip: %d\n", status);
 		return status;
@@ -192,10 +195,10 @@ static int zevio_gpio_probe(struct platform_device *pdev)
 	spin_lock_init(&controller->lock);
 
 	/* Disable interrupts, they only cause errors */
-	for (i = 0; i < controller->chip.gc.ngpio; i += 8)
+	for (i = 0; i < controller->chip.ngpio; i += 8)
 		zevio_gpio_port_set(controller, i, ZEVIO_GPIO_INT_MASK, 0xFF);
 
-	dev_dbg(controller->chip.gc.parent, "ZEVIO GPIO controller set up!\n");
+	dev_dbg(controller->chip.parent, "ZEVIO GPIO controller set up!\n");
 
 	return 0;
 }
Signature · 1 line
2.30.2
Reviewed-by on v3 1/1
#5 lore raw
Quoted 3 lines · Moses Christopher Bollavarapu

Remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives.

LGTM,

Quoted 86 lines

---
V2 -> V3: Add missing return in front of dev_error_probe V1 -> V2: Move gpio_chip member to top of the struct
Use dev_error_probe instead of dev_err Minor style fixes

 drivers/gpio/gpio-zevio.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/gpio/gpio-zevio.c +14 -12
index f6f8a541348f..ce9d1282165c 100644
@@ -11,7 +11,6 @@
 #include <linux/bitops.h>
 #include <linux/io.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/gpio/driver.h>
 
@@ -53,22 +52,23 @@
 #define ZEVIO_GPIO_BIT(gpio) (gpio&7)
 
 struct zevio_gpio {
+       struct gpio_chip        chip;
        spinlock_t              lock;
-       struct of_mm_gpio_chip  chip;
+       void __iomem            *regs;
 };
 
 static inline u32 zevio_gpio_port_get(struct zevio_gpio *c, unsigned pin,
                                        unsigned port_offset)
 {
        unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-       return readl(IOMEM(c->chip.regs + section_offset + port_offset));
+       return readl(IOMEM(c->regs + section_offset + port_offset));
 }
 
 static inline void zevio_gpio_port_set(struct zevio_gpio *c, unsigned pin,
                                        unsigned port_offset, u32 val)
 {
        unsigned section_offset = ((pin >> 3) & 3)*ZEVIO_GPIO_SECTION_SIZE;
-       writel(val, IOMEM(c->chip.regs + section_offset + port_offset));
+       writel(val, IOMEM(c->regs + section_offset + port_offset));
 }
 
 /* Functions for struct gpio_chip */
@@ -178,12 +178,15 @@ static int zevio_gpio_probe(struct platform_device *pdev)
        platform_set_drvdata(pdev, controller);
 
        /* Copy our reference */
-       controller->chip.gc = zevio_gpio_chip;
-       controller->chip.gc.parent = &pdev->dev;
+       controller->chip = zevio_gpio_chip;
+       controller->chip.parent = &pdev->dev;
 
-       status = of_mm_gpiochip_add_data(pdev->dev.of_node,
-                                        &(controller->chip),
-                                        controller);
+       controller->regs = devm_platform_ioremap_resource(pdev, 0);
+       if (IS_ERR(controller->regs))
+               return dev_err_probe(&pdev->dev, PTR_ERR(controller->regs),
+                                    "failed to ioremap memory resource\n");
+
+       status = devm_gpiochip_add_data(&pdev->dev, &controller->chip, controller);
        if (status) {
                dev_err(&pdev->dev, "failed to add gpiochip: %d\n", status);
                return status;
@@ -192,10 +195,10 @@ static int zevio_gpio_probe(struct platform_device *pdev)
        spin_lock_init(&controller->lock);
 
        /* Disable interrupts, they only cause errors */
-       for (i = 0; i < controller->chip.gc.ngpio; i += 8)
+       for (i = 0; i < controller->chip.ngpio; i += 8)
                zevio_gpio_port_set(controller, i, ZEVIO_GPIO_INT_MASK, 0xFF);
 
-       dev_dbg(controller->chip.gc.parent, "ZEVIO GPIO controller set up!\n");
+       dev_dbg(controller->chip.parent, "ZEVIO GPIO controller set up!\n");
 
        return 0;
 }
--

2.30.2

Signature · 2 lines
With Best Regards,
Andy Shevchenko
Reviewed-by on v3 1/1
#6 lore raw
In reply to Moses Christopher Bollavarapu
Quoted 9 lines · Moses Christopher Bollavarapu

Remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives.

---
V2 -> V3: Add missing return in front of dev_error_probe V1 -> V2: Move gpio_chip member to top of the struct
Use dev_error_probe instead of dev_err Minor style fixes

Yours,
Linus Walleij

#7 lore raw
In reply to Moses Christopher Bollavarapu
Quoted 11 lines · Moses Christopher Bollavarapu

Remove of_gpio.h header file, replace of_* functions and structs with appropriate alternatives.

---
V2 -> V3: Add missing return in front of dev_error_probe V1 -> V2: Move gpio_chip member to top of the struct
Use dev_error_probe instead of dev_err Minor style fixes

Applied and tweaked the commit subject a bit (dropped "drivers: ").

Bart