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.
--
With Best Regards,
Andy Shevchenko
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.
--
Florian
> > > #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
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.