Re: [PATCH v2 0/4] Define i2c_designware in a header file

[Jarkko Nikula]

On 4/25/24 3:26 AM, Florian Fainelli wrote:
> This patch series depends upon the following two patches being applied:
>
> https://lore.kernel.org/all/20240422084109.3201-1-duanqiangwen@xxxxxxxxxxxxx/
> https://lore.kernel.org/all/20240422084109.3201-2-duanqiangwen@xxxxxxxxxxxxx/
>
> There is no reason why each driver should have to repeat the
> "i2c_designware" string all over the place, because when that happens we
> see the reverts like the above being necessary.
>
> Changes in v2:
>
> - avoid changing i2c-designware-pcidrv.c more than necessary
> - move constant to include/linux/platform_data/i2c-designware.h
> - add comments as to how this constant is used and why
>
> Florian Fainelli (4):
>    i2c: designware: Create shared header hosting driver name
>    mfd: intel-lpss: Utilize i2c-designware.h
>    mfd: intel_quark_i2c_gpio: Utilize i2c-designware.h
>    net: txgbe: Utilize i2c-designware.h
>
>   MAINTAINERS                                    |  1 +
>   drivers/i2c/busses/i2c-designware-pcidrv.c     |  3 ++-
>   drivers/i2c/busses/i2c-designware-platdrv.c    |  5 +++--
>   drivers/mfd/intel-lpss.c                       |  3 ++-
>   drivers/mfd/intel_quark_i2c_gpio.c             |  5 +++--
>   drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c |  7 ++++---
>   include/linux/platform_data/i2c-designware.h   | 11 +++++++++++
>   7 files changed, 26 insertions(+), 9 deletions(-)
>   create mode 100644 include/linux/platform_data/i2c-designware.h
I have mixed feeling about this set will it help maintaining and 
developing new code or do the opposite. Surely misnaming as referenced 
above can happen but I think it's not too common pattern while having 
single define header put under include/ feels added burden.

Also I personally like explicit strings put into MODULE_ALIAS() since 
they are easier to grep from sources and modules.alias file when 
debugging autoloading issues. So change like below makes that debugging 
one step more labor.

-MODULE_ALIAS("platform:i2c_designware");
+MODULE_ALIAS("platform:" I2C_DESIGNWARE_NAME);