Skip to contentSkip to conversation

gpio: adp5588: Switch from of headers to mod_devicetable.h

PATCH Applied Andy Shevchenko 6 participants 14 messages Jun 28, 2022 – Jul 6, 2022 GPIO

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 gpio: adp5588: Switch from of headers to mod_devicetable.h

There is nothing directly using of specific interfaces in this driver, so lets not include the headers.

---
 drivers/gpio/gpio-adp5588.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/gpio/gpio-adp5588.c +3 -5
index e388e75103f4..51ed23ba4645 100644
@@ -6,7 +6,6 @@
  * Copyright 2009-2010 Analog Devices Inc.
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/init.h>
@@ -14,7 +13,8 @@
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/of_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 
 #include <linux/platform_data/adp5588.h>
 
@@ -427,18 +427,16 @@ static const struct i2c_device_id adp5588_gpio_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, adp5588_gpio_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id adp5588_gpio_of_id[] = {
 	{ .compatible = "adi," DRV_NAME, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, adp5588_gpio_of_id);
-#endif
 
 static struct i2c_driver adp5588_gpio_driver = {
 	.driver = {
 		.name = DRV_NAME,
-		.of_match_table = of_match_ptr(adp5588_gpio_of_id),
+		.of_match_table = adp5588_gpio_of_id,
 	},
 	.probe_new = adp5588_gpio_probe,
 	.remove = adp5588_gpio_remove,
Signature · 1 line
2.35.1
#1 lore raw

PATCH 2/3 gpio: adp5588: Do not use defined value for driver name and compatible

It's wrong to use defined string literal for three semantically different cases, i.e.:
1) compatible string, which is part of ABI and has to have specific format;
2) I2C ID, which is user space visible and also ABI;
3) driver name, that can be changed.

Drop the define and use appropriate string literals in place.

While at it, drop comma at terminator entry of OF ID table.

---
 drivers/gpio/gpio-adp5588.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpio/gpio-adp5588.c +4 -6
index 51ed23ba4645..d6a229a67044 100644
@@ -18,8 +18,6 @@
 
 #include <linux/platform_data/adp5588.h>
 
-#define DRV_NAME	"adp5588-gpio"
-
 /*
  * Early pre 4.0 Silicon required to delay readout by at least 25ms,
  * since the Event Counter Register updated 25ms after the interrupt
@@ -422,20 +420,20 @@ static int adp5588_gpio_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id adp5588_gpio_id[] = {
-	{DRV_NAME, 0},
+	{ "adp5588-gpio" },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, adp5588_gpio_id);
 
 static const struct of_device_id adp5588_gpio_of_id[] = {
-	{ .compatible = "adi," DRV_NAME, },
-	{},
+	{ .compatible = "adi,adp5588-gpio" },
+	{}
 };
 MODULE_DEVICE_TABLE(of, adp5588_gpio_of_id);
 
 static struct i2c_driver adp5588_gpio_driver = {
 	.driver = {
-		.name = DRV_NAME,
+		.name = "adp5588-gpio",
 		.of_match_table = adp5588_gpio_of_id,
 	},
 	.probe_new = adp5588_gpio_probe,
Signature · 1 line
2.35.1
Reviewed-by on v1 2/3
#2 lore raw

Re: [PATCH v1 2/3] gpio: adp5588: Do not use defined value for driver name and compatible

Quoted 11 lines · Andy Shevchenko

It's wrong to use defined string literal for three semantically different cases, i.e.:
1) compatible string, which is part of ABI and has to have specific format;
2) I2C ID, which is user space visible and also ABI;
3) driver name, that can be changed.

Drop the define and use appropriate string literals in place.

While at it, drop comma at terminator entry of OF ID table.

Yours,
Linus Walleij

#3 lore raw

PATCH 3/3 gpio: adp5588: sort header inclusion alphabetically

In reply to Andy Shevchenko

Sort header inclusion alphabetically.

---
 drivers/gpio/gpio-adp5588.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpio/gpio-adp5588.c +4 -4
index d6a229a67044..d49f12560cde 100644
@@ -6,15 +6,15 @@
  * Copyright 2009-2010 Analog Devices Inc.
  */
 
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/init.h>
-#include <linux/i2c.h>
 #include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/kernel.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #include <linux/platform_data/adp5588.h>
 
Signature · 1 line
2.35.1
#4 lore raw

RE: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 12 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Dienstag, 28. Juni 2022 21:39
To: Andy Shevchenko <[email protected]>; linux- [email protected]; [email protected]
Cc: Hennerich, Michael <[email protected]>; Linus Walleij <[email protected]>; Bartosz Golaszewski <[email protected]>
Subject: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 1 line

Sort header inclusion alphabetically.

Hi Andy,

Thanks for the patches, they look good.
However, Nuno is currently working on getting the irqchip support into the adp5588 input driver. In his patch series this driver is going away.

https://lore.kernel.org/linux-input/[email protected]/

Michael

Quoted 32 lines
---
 drivers/gpio/gpio-adp5588.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpio/gpio-adp5588.c index renamed from drivers/gpio/gpio-adp5588.c +0 -0

d6a229a67044..d49f12560cde 100644

drivers/gpio/gpio-adp5588.c +4 -5
@@ -6,15 +6,15 @@
  * Copyright 2009-2010 Analog Devices Inc.
  */
 
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/init.h>
-#include <linux/i2c.h>
 #include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/kernel.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #include <linux/platform_data/adp5588.h>
 
--

2.35.1

#5 lore raw

Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 12 lines · Michael
Quoted 12 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Dienstag, 28. Juni 2022 21:39
To: Andy Shevchenko <[email protected]>; linux- [email protected]; [email protected]
Cc: Hennerich, Michael <[email protected]>; Linus Walleij <[email protected]>; Bartosz Golaszewski <[email protected]>
Subject: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 7 lines
Quoted 1 line

Sort header inclusion alphabetically.

Thanks for the patches, they look good.
However, Nuno is currently working on getting the irqchip support into the adp5588 input driver. In his patch series this driver is going away.

https://lore.kernel.org/linux-input/[email protected]/

While that work is ongoing, and most likely won't make v5.20-rc1, for the v5.20-rc1 I think my patches are good to go to avoid a bad (or rather very old) example on how to do GPIO drivers. What do you think?

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

RE: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Hi Andy,

Quoted 42 lines

From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:00 PM
To: Hennerich, Michael <[email protected]>
Cc: Andy Shevchenko <[email protected]>; linux- [email protected]; [email protected]; Sa, Nuno <[email protected]>; Linus Walleij <[email protected]>; Bartosz Golaszewski <[email protected]>
Subject: Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

[External]

Quoted 7 lines · Michael
Quoted 7 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Dienstag, 28. Juni 2022 21:39
To: Andy Shevchenko <[email protected]>;

linux-

Quoted 2 lines
Quoted 2 lines

[email protected]; [email protected]
Cc: Hennerich, Michael <[email protected]>; Linus

Walleij

Quoted 2 lines
Quoted 2 lines

<[email protected]>; Bartosz Golaszewski <[email protected]>
Subject: [PATCH v1 3/3] gpio: adp5588: sort header inclusion

alphabetically

Quoted 5 lines
Quoted 2 lines

Sort header inclusion alphabetically.

Thanks for the patches, they look good.
However, Nuno is currently working on getting the irqchip support

into the

Quoted 3 lines

adp5588 input driver. In his patch series this driver is going away.

https://urldefense.com/v3/__https://lore.kernel.org/linux-

input/YpMCh1Xje*[email protected]/__;Kw!!A3Ni8CS0y2Y!_Cb1uAs D- Z6iz_zSDfrd5Va6zLmdoxjv1vLYWQGHDOqYniaXVVHl3Ou2lOUQIjwhSN Ku2aZkYrOb8xMi2cZtfmsv$

Quoted 5 lines

While that work is ongoing, and most likely won't make v5.20-rc1, for the v5.20-rc1 I think my patches are good to go to avoid a bad (or rather very old) example on how to do GPIO drivers. What do you think?

Just as note, If nothing unexpected happens, I'm planning in sending this out today. If you still think this makes sense, fine by me. It's a very minimal change that won't give much pain to deal with...

- Nuno Sá

#7 lore raw

Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 7 lines · Nuno
Quoted 7 lines

From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:00 PM

Quoted 3 lines · Michael
Quoted 3 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Dienstag, 28. Juni 2022 21:39

...

Quoted 13 lines
Quoted 13 lines
Quoted 4 lines
Quoted 1 line

Sort header inclusion alphabetically.

Thanks for the patches, they look good.
However, Nuno is currently working on getting the irqchip support

into the

Quoted 3 lines

adp5588 input driver. In his patch series this driver is going away.

https://urldefense.com/v3/__https://lore.kernel.org/linux-

input/YpMCh1Xje*[email protected]/__;Kw!!A3Ni8CS0y2Y!_Cb1uAs D- Z6iz_zSDfrd5Va6zLmdoxjv1vLYWQGHDOqYniaXVVHl3Ou2lOUQIjwhSN Ku2aZkYrOb8xMi2cZtfmsv$

Quoted 8 lines
Quoted 4 lines

While that work is ongoing, and most likely won't make v5.20-rc1, for the v5.20-rc1 I think my patches are good to go to avoid a bad (or rather very old) example on how to do GPIO drivers. What do you think?

Just as note, If nothing unexpected happens, I'm planning in sending this out today. If you still think this makes sense, fine by me. It's a very minimal change that won't give much pain to deal with...

Ah, Okay, I was thinking about a week or two from now. Let's see how it will go and let Linus decide how to proceed.

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

Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 30 lines · Andy Shevchenko
Quoted 7 lines · Nuno
Quoted 7 lines

From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:00 PM

Quoted 3 lines · Michael
Quoted 3 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Dienstag, 28. Juni 2022 21:39

...

Quoted 18 lines
Quoted 17 lines
Quoted 4 lines
Quoted 1 line

Sort header inclusion alphabetically.

Thanks for the patches, they look good.
However, Nuno is currently working on getting the irqchip support

into the

Quoted 3 lines

adp5588 input driver. In his patch series this driver is going away.

https://urldefense.com/v3/__https://lore.kernel.org/linux-

input/YpMCh1Xje*[email protected]/__;Kw!!A3Ni8CS0y2Y!_Cb1uAs D- Z6iz_zSDfrd5Va6zLmdoxjv1vLYWQGHDOqYniaXVVHl3Ou2lOUQIjwhSN Ku2aZkYrOb8xMi2cZtfmsv$

While that work is ongoing, and most likely won't make v5.20-rc1, for the v5.20-rc1 I think my patches are good to go to avoid a bad (or rather very old) example on how to do GPIO drivers. What do you think?

Quoted 6 lines
Quoted 3 lines

Just as note, If nothing unexpected happens, I'm planning in sending this out today. If you still think this makes sense, fine by me. It's a very minimal change that won't give much pain to deal with...

Ah, Okay, I was thinking about a week or two from now. Let's see how it will go and let Linus decide how to proceed.

With all respect to Linus, I had to put Bart :-)

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

RE: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

In reply to Andy Shevchenko
Quoted 49 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:33 PM
To: Sa, Nuno <[email protected]>
Cc: Hennerich, Michael <[email protected]>; Andy Shevchenko <[email protected]>; linux- [email protected]; [email protected]; Linus Walleij <[email protected]>; Bartosz Golaszewski <[email protected]>
Subject: Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

[External]

Quoted 7 lines · Nuno
Quoted 7 lines

From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:00 PM

Quoted 3 lines · Michael
Quoted 3 lines

-----Original Message-----
From: Andy Shevchenko <[email protected]>
Sent: Dienstag, 28. Juni 2022 21:39

...

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line

Sort header inclusion alphabetically.

Thanks for the patches, they look good.
However, Nuno is currently working on getting the irqchip

support

Quoted 5 lines
Quoted 5 lines

into the

Quoted 3 lines

adp5588 input driver. In his patch series this driver is going away.

https://urldefense.com/v3/__https://lore.kernel.org/linux-

input/YpMCh1Xje*[email protected]/__;Kw!!A3Ni8CS0y2Y!_Cb1uAs

Quoted 2 lines
Quoted 2 lines

D-

Z6iz_zSDfrd5Va6zLmdoxjv1vLYWQGHDOqYniaXVVHl3Ou2lOUQIjwhSN

Quoted 3 lines
Quoted 3 lines

Ku2aZkYrOb8xMi2cZtfmsv$

While that work is ongoing, and most likely won't make v5.20-rc1,

for

Quoted 4 lines
Quoted 3 lines

the v5.20-rc1 I think my patches are good to go to avoid a bad (or rather very old) example on how to do GPIO drivers. What do you think?

Quoted 9 lines
Quoted 1 line

Just as note, If nothing unexpected happens, I'm planning in sending

this

Quoted 1 line

out today. If you still think this makes sense, fine by me. It's a very

minimal

Quoted 1 line

change that won't give much pain to deal with...

Ah, Okay, I was thinking about a week or two from now. Let's see how it will go and let Linus decide how to proceed.

Yeah, if it does not happen today, I will only get into it again next Wednesday (so in this case, might make sense).

- Nuno Sá

#10 lore raw

Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 4 lines · Nuno
Quoted 4 lines

From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:33 PM
On Wed, Jun 29, 2022 at 12:10 PM Sa, Nuno <[email protected]> wrote:

...

Quoted 5 lines
Quoted 2 lines

Ah, Okay, I was thinking about a week or two from now. Let's see how it will go and let Linus decide how to proceed.

Yeah, if it does not happen today, I will only get into it again next Wednesday (so in this case, might make sense).

If I'm not mistaken, that didn't happen.

Bart, can you apply this series then?

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

Re: [PATCH v1 3/3] gpio: adp5588: sort header inclusion alphabetically

Quoted 15 lines · Andy Shevchenko
Quoted 4 lines · Nuno
Quoted 4 lines

From: Andy Shevchenko <[email protected]>
Sent: Wednesday, June 29, 2022 12:33 PM
On Wed, Jun 29, 2022 at 12:10 PM Sa, Nuno <[email protected]> wrote:

...

Quoted 5 lines
Quoted 2 lines

Ah, Okay, I was thinking about a week or two from now. Let's see how it will go and let Linus decide how to proceed.

Yeah, if it does not happen today, I will only get into it again next Wednesday (so in this case, might make sense).

Quoted 9 lines

If I'm not mistaken, that didn't happen.

Bart, can you apply this series then?

--
With Best Regards,
Andy Shevchenko

Now queued.

Bart

#13 lore raw
In reply to Andy Shevchenko
Quoted 52 lines · Andy Shevchenko

There is nothing directly using of specific interfaces in this driver, so lets not include the headers.

---
 drivers/gpio/gpio-adp5588.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/gpio/gpio-adp5588.c +3 -6
index e388e75103f4..51ed23ba4645 100644
@@ -6,7 +6,6 @@
  * Copyright 2009-2010 Analog Devices Inc.
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/init.h>
@@ -14,7 +13,8 @@
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
-#include <linux/of_device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
 
 #include <linux/platform_data/adp5588.h>
 
@@ -427,18 +427,16 @@ static const struct i2c_device_id adp5588_gpio_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, adp5588_gpio_id);
 
-#ifdef CONFIG_OF
 static const struct of_device_id adp5588_gpio_of_id[] = {
        { .compatible = "adi," DRV_NAME, },
        {},
 };
 MODULE_DEVICE_TABLE(of, adp5588_gpio_of_id);
-#endif
 
 static struct i2c_driver adp5588_gpio_driver = {
        .driver = {
                .name = DRV_NAME,
-               .of_match_table = of_match_ptr(adp5588_gpio_of_id),
+               .of_match_table = adp5588_gpio_of_id,
        },
        .probe_new = adp5588_gpio_probe,
        .remove = adp5588_gpio_remove,
--
Quoted 2 lines

2.35.1

Applied all three, thanks!

Bart