diff mbox series

[1/4] i2c: designware: Create shared header hosting driver name

Message ID 20240423233622.1494708-2-florian.fainelli@broadcom.com (mailing list archive)
State Superseded
Headers show
Series Define i2c_designware in a header file | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Florian Fainelli April 23, 2024, 11:36 p.m. UTC 
We have a number of drivers that reference the string "i2c_designware"
yet this is copied all over the places with opportunities for this
string being mis-used. Create a shared header that defines this as a
constant that other drivers can reference.

Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 MAINTAINERS                                 | 1 +
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 5 +++--
 drivers/i2c/busses/i2c-designware-platdrv.c | 5 +++--
 include/linux/i2c-designware.h              | 7 +++++++
 4 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/i2c-designware.h

Comments

Andy Shevchenko April 23, 2024, 11:59 p.m. UTC | #1 
Tue, Apr 23, 2024 at 04:36:19PM -0700, Florian Fainelli kirjoitti:
> We have a number of drivers that reference the string "i2c_designware"
> yet this is copied all over the places with opportunities for this
> string being mis-used. Create a shared header that defines this as a
> constant that other drivers can reference.

...

>  #include <linux/i2c.h>
> +#include <linux/i2c-designware.h>

Can it be hidden in the subfolder?

...

> -#define DRIVER_NAME "i2c-designware-pci"
> +#define DRIVER_NAME I2C_DESIGNWARE_NAME "-pci"

Oh, this makes all the things hard to read.

>  /* Work with hotplug and coldplug */
> -MODULE_ALIAS("i2c_designware-pci");
> +MODULE_ALIAS(DRIVER_NAME);

I believe we shouldn't use MODULE_ALIAS() without real justification.

...

> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c

All as per above.
Florian Fainelli April 24, 2024, 1:22 a.m. UTC | #2 
On 4/23/2024 4:59 PM, Andy Shevchenko wrote:
> Tue, Apr 23, 2024 at 04:36:19PM -0700, Florian Fainelli kirjoitti:
>> We have a number of drivers that reference the string "i2c_designware"
>> yet this is copied all over the places with opportunities for this
>> string being mis-used. Create a shared header that defines this as a
>> constant that other drivers can reference.
> 
> ...
> 
>>   #include <linux/i2c.h>
>> +#include <linux/i2c-designware.h>
> 
> Can it be hidden in the subfolder?

That would require the MFD and ethernet drivers to include relative to 
where they are in the source tree, do we really want that?

> 
> ...
> 
>> -#define DRIVER_NAME "i2c-designware-pci"
>> +#define DRIVER_NAME I2C_DESIGNWARE_NAME "-pci"
> 
> Oh, this makes all the things hard to read.

OK, besides there is a change for '_' when it was a '-' before, so maybe 
I should drop that hunk.

> 
>>   /* Work with hotplug and coldplug */
>> -MODULE_ALIAS("i2c_designware-pci");
>> +MODULE_ALIAS(DRIVER_NAME);
> 
> I believe we shouldn't use MODULE_ALIAS() without real justification.

Pre-existing change.
Andrew Lunn April 24, 2024, 12:18 p.m. UTC | #3 
> > >   #include <linux/i2c.h>
> > > +#include <linux/i2c-designware.h>
> > 
> > Can it be hidden in the subfolder?
> 
> That would require the MFD and ethernet drivers to include relative to where
> they are in the source tree, do we really want that?

Maybe linux/platform_data/i2c-designware.h ? There are a few NAME
macros in there.

       Andrew
Andy Shevchenko April 24, 2024, 12:45 p.m. UTC | #4 
On Wed, Apr 24, 2024 at 3:18 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > >   #include <linux/i2c.h>
> > > > +#include <linux/i2c-designware.h>
> > >
> > > Can it be hidden in the subfolder?
> >
> > That would require the MFD and ethernet drivers to include relative to where
> > they are in the source tree, do we really want that?
>
> Maybe linux/platform_data/i2c-designware.h ? There are a few NAME
> macros in there.

Yes, under subfolder I meant something like
include/linux/$SOMETHING/_this_header_.h or even deeper.
Jarkko Nikula April 25, 2024, 8:33 a.m. UTC | #5 
On 4/24/24 2:59 AM, Andy Shevchenko wrote:
> Tue, Apr 23, 2024 at 04:36:19PM -0700, Florian Fainelli kirjoitti:
>> We have a number of drivers that reference the string "i2c_designware"
>> yet this is copied all over the places with opportunities for this
>> string being mis-used. Create a shared header that defines this as a
>> constant that other drivers can reference.
> 
> ...
> 
>>   #include <linux/i2c.h>
>> +#include <linux/i2c-designware.h>
> 
> Can it be hidden in the subfolder?
> 
> ...
> 
>> -#define DRIVER_NAME "i2c-designware-pci"
>> +#define DRIVER_NAME I2C_DESIGNWARE_NAME "-pci"
> 
> Oh, this makes all the things hard to read.
> 
>>   /* Work with hotplug and coldplug */
>> -MODULE_ALIAS("i2c_designware-pci");
>> +MODULE_ALIAS(DRIVER_NAME);
> 
> I believe we shouldn't use MODULE_ALIAS() without real justification.
> 
I think MODULE_ALIAS() is even needless here since this device is not 
added from another driver but loaded only for known PCI IDs in device table.
Andy Shevchenko April 25, 2024, 9:20 a.m. UTC | #6 
On Thu, Apr 25, 2024 at 11:33:02AM +0300, Jarkko Nikula wrote:
> On 4/24/24 2:59 AM, Andy Shevchenko wrote:
> > Tue, Apr 23, 2024 at 04:36:19PM -0700, Florian Fainelli kirjoitti:
> > > We have a number of drivers that reference the string "i2c_designware"
> > > yet this is copied all over the places with opportunities for this
> > > string being mis-used. Create a shared header that defines this as a
> > > constant that other drivers can reference.

...

> > >   #include <linux/i2c.h>
> > > +#include <linux/i2c-designware.h>
> > 
> > Can it be hidden in the subfolder?

...

> > > -#define DRIVER_NAME "i2c-designware-pci"
> > > +#define DRIVER_NAME I2C_DESIGNWARE_NAME "-pci"
> > 
> > Oh, this makes all the things hard to read.
> > 
> > >   /* Work with hotplug and coldplug */
> > > -MODULE_ALIAS("i2c_designware-pci");
> > > +MODULE_ALIAS(DRIVER_NAME);
> > 
> > I believe we shouldn't use MODULE_ALIAS() without real justification.
> > 
> I think MODULE_ALIAS() is even needless here since this device is not added
> from another driver but loaded only for known PCI IDs in device table.

The patch that removes it got reverted :-( and then removed completely from PR.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d5acd6d98c4..d5e10c747f65 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21438,6 +21438,7 @@  R:	Jan Dabros <jsd@semihalf.com>
 L:	linux-i2c@vger.kernel.org
 S:	Supported
 F:	drivers/i2c/busses/i2c-designware-*
+F:	include/linux/i2c-designware.h
 
 SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER
 M:	Jaehoon Chung <jh80.chung@samsung.com>
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 9be9a2658e1f..948669265375 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -14,6 +14,7 @@ 
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/i2c-designware.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -27,7 +28,7 @@ 
 #include "i2c-designware-core.h"
 #include "i2c-ccgx-ucsi.h"
 
-#define DRIVER_NAME "i2c-designware-pci"
+#define DRIVER_NAME I2C_DESIGNWARE_NAME "-pci"
 
 enum dw_pci_ctl_id_t {
 	medfield,
@@ -425,7 +426,7 @@  static struct pci_driver dw_i2c_driver = {
 module_pci_driver(dw_i2c_driver);
 
 /* Work with hotplug and coldplug */
-MODULE_ALIAS("i2c_designware-pci");
+MODULE_ALIAS(DRIVER_NAME);
 MODULE_AUTHOR("Baruch Siach <baruch@tkos.co.il>");
 MODULE_DESCRIPTION("Synopsys DesignWare PCI I2C bus adapter");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 4ab41ba39d55..dc335a22546b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -16,6 +16,7 @@ 
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/i2c-designware.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -480,13 +481,13 @@  static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 };
 
 /* Work with hotplug and coldplug */
-MODULE_ALIAS("platform:i2c_designware");
+MODULE_ALIAS("platform:" I2C_DESIGNWARE_NAME);
 
 static struct platform_driver dw_i2c_driver = {
 	.probe = dw_i2c_plat_probe,
 	.remove_new = dw_i2c_plat_remove,
 	.driver		= {
-		.name	= "i2c_designware",
+		.name	= I2C_DESIGNWARE_NAME,
 		.of_match_table = of_match_ptr(dw_i2c_of_match),
 		.acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
 		.pm	= pm_ptr(&dw_i2c_dev_pm_ops),
diff --git a/include/linux/i2c-designware.h b/include/linux/i2c-designware.h
new file mode 100644
index 000000000000..f20240ac89c4
--- /dev/null
+++ b/include/linux/i2c-designware.h
@@ -0,0 +1,7 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __I2C_DESIGNWARE_H__
+#define __I2C_DESIGNWARE_H__
+
+#define I2C_DESIGNWARE_NAME	"i2c_designware"
+
+#endif /* __I2C_DESIGNWARE_H__ */