linux-kernel - Re: [PATCH v2 0/4] Define i2c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <62e58960-f568-459d-8690-0c9c608a4d9f@linux.intel.com>
Date: Thu, 25 Apr 2024 16:11:58 +0300
From: Jarkko Nikula <jarkko.nikula@...ux.intel.com>
To: Florian Fainelli <florian.fainelli@...adcom.com>,
 linux-kernel@...r.kernel.org
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Mika Westerberg <mika.westerberg@...ux.intel.com>,
 Jan Dabros <jsd@...ihalf.com>, Andi Shyti <andi.shyti@...nel.org>,
 Lee Jones <lee@...nel.org>, Jiawen Wu <jiawenwu@...stnetic.com>,
 Mengyuan Lou <mengyuanlou@...-swift.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
 Andrew Lunn <andrew@...n.ch>, Duanqiang Wen <duanqiangwen@...-swift.com>,
 "open list:SYNOPSYS DESIGNWARE I2C DRIVER" <linux-i2c@...r.kernel.org>,
 "open list:WANGXUN ETHERNET DRIVER" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 0/4] Define i2c_designware in a header file

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@net-swift.com/
> https://lore.kernel.org/all/20240422084109.3201-2-duanqiangwen@net-swift.com/
> 
> 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);