mbox series

[v3,0/5] Define i2c_designware in a header file

Message ID 20240425214438.2100534-1-florian.fainelli@broadcom.com
Headers show
Series Define i2c_designware in a header file | expand

Message

Florian Fainelli April 25, 2024, 9:44 p.m. UTC
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.

Given the dependency on the two other patches above, it would make sense
to route this via the networking tree, or wait until a v6.9-rc
containing the above two changes gets merged into one of the i2c/mfd
trees.

Changes in v3:

- incorporate Andy's change removing the MODULE_ALIAS
- created a separate inclusion group as requested by Andy
- changed the string format in txgbe_phy.c

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

Andy Shevchenko (1):
  i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()

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     |  2 --
 drivers/i2c/busses/i2c-designware-platdrv.c    | 12 +++++++++---
 drivers/mfd/intel-lpss.c                       |  3 ++-
 drivers/mfd/intel_quark_i2c_gpio.c             |  6 ++++--
 drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c |  6 ++++--
 include/linux/platform_data/i2c-designware.h   | 12 ++++++++++++
 7 files changed, 32 insertions(+), 10 deletions(-)
 create mode 100644 include/linux/platform_data/i2c-designware.h

Comments

Jarkko Nikula May 2, 2024, 9:19 a.m. UTC | #1
On 4/30/24 12:36 PM, Andi Shyti wrote:
> Hi Andy,
> 
> On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
>> On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
>>> Rather than open code the i2c_designware string, utilize the newly
>>> defined constant in i2c-designware.h.
>>
>> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> P.S>
>> Note, my tags for MFD patches does not imply that I agree on the general idea
>> of this series, it's just in case if it will be approved by the respective
>> maintainers (I²C / MFD / etc).
> 
> I waited a bit to see if more comments were coming.
> 
Well I had doubts about the idea will it help maintaining code or do the 
opposite but didn't receive a reply from the patch author:

https://lore.kernel.org/linux-i2c/62e58960-f568-459d-8690-0c9c608a4d9f@linux.intel.com/

Also Lee Jones have similar concerns:

https://lore.kernel.org/linux-i2c/20240502071751.GA5338@google.com/
Lee Jones May 2, 2024, 10:11 a.m. UTC | #2
On Thu, 02 May 2024, Jarkko Nikula wrote:

> On 4/30/24 12:36 PM, Andi Shyti wrote:
> > Hi Andy,
> > 
> > On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
> > > On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
> > > > Rather than open code the i2c_designware string, utilize the newly
> > > > defined constant in i2c-designware.h.
> > > 
> > > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 
> > > P.S>
> > > Note, my tags for MFD patches does not imply that I agree on the general idea
> > > of this series, it's just in case if it will be approved by the respective
> > > maintainers (I²C / MFD / etc).
> > 
> > I waited a bit to see if more comments were coming.
> > 
> Well I had doubts about the idea will it help maintaining code or do the
> opposite but didn't receive a reply from the patch author:
> 
> https://lore.kernel.org/linux-i2c/62e58960-f568-459d-8690-0c9c608a4d9f@linux.intel.com/
> 
> Also Lee Jones have similar concerns:
> 
> https://lore.kernel.org/linux-i2c/20240502071751.GA5338@google.com/

Right.  It's a NACK from me, sorry.
Florian Fainelli May 2, 2024, 4:08 p.m. UTC | #3
On 5/2/24 02:19, Jarkko Nikula wrote:
> On 4/30/24 12:36 PM, Andi Shyti wrote:
>> Hi Andy,
>>
>> On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
>>> On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
>>>> Rather than open code the i2c_designware string, utilize the newly
>>>> defined constant in i2c-designware.h.
>>>
>>> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>>
>>> P.S>
>>> Note, my tags for MFD patches does not imply that I agree on the 
>>> general idea
>>> of this series, it's just in case if it will be approved by the 
>>> respective
>>> maintainers (I²C / MFD / etc).
>>
>> I waited a bit to see if more comments were coming.
>>
> Well I had doubts about the idea will it help maintaining code or do the 
> opposite but didn't receive a reply from the patch author:
> 
> https://lore.kernel.org/linux-i2c/62e58960-f568-459d-8690-0c9c608a4d9f@linux.intel.com/

I read your message and the fact that you provided a diff as a 
suggestion as to what would become acceptable if incorporating your 
suggested change, and made that a v3. If this was not acceptable to you 
from the get go, it would have been clearer with an explicit Nack like 
what others have done now.

> 
> Also Lee Jones have similar concerns:
> 
> https://lore.kernel.org/linux-i2c/20240502071751.GA5338@google.com/
> 

Yes, so clearly I failed to convince all of you that if someone was able 
to trip over that problem, something should be done about it. No 
problem, these are not driver I maintain, so if someone hits the same 
issue, lore has the patches now.
Andi Shyti May 2, 2024, 11:09 p.m. UTC | #4
Hi

On Thu, 25 Apr 2024 14:44:33 -0700, 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.
> 
> [...]

Applied to i2c/i2c-host on

git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git

Thank you,
Andi

Patches applied
===============
[1/5] i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()
      commit: 91647e64f0f5677ace84165dc25dc99579147b8f
[2/5] i2c: designware: Create shared header hosting driver name
      commit: 856cd5f13de7cebca44db5ff4bc2ca73490dd8d7
Jarkko Nikula May 3, 2024, 6:30 a.m. UTC | #5
Hi Andi

On 5/3/24 2:09 AM, Andi Shyti wrote:
> Applied to i2c/i2c-host on
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git
> 
> Thank you,
> Andi
> 
> Patches applied
> ===============
> [1/5] i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()
>        commit: 91647e64f0f5677ace84165dc25dc99579147b8f
> [2/5] i2c: designware: Create shared header hosting driver name
>        commit: 856cd5f13de7cebca44db5ff4bc2ca73490dd8d7
> 
Was the second patch applied accidentally?
Andy Shevchenko May 3, 2024, 3:38 p.m. UTC | #6
On Fri, May 03, 2024 at 09:30:39AM +0300, Jarkko Nikula wrote:
> On 5/3/24 2:09 AM, Andi Shyti wrote:
> > Applied to i2c/i2c-host on
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git
> > 
> > Thank you,
> > Andi
> > 
> > Patches applied
> > ===============
> > [1/5] i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()
> >        commit: 91647e64f0f5677ace84165dc25dc99579147b8f
> > [2/5] i2c: designware: Create shared header hosting driver name
> >        commit: 856cd5f13de7cebca44db5ff4bc2ca73490dd8d7
> > 
> Was the second patch applied accidentally?

+1 here, asked the same in private communication.
Andi Shyti May 3, 2024, 6:26 p.m. UTC | #7
Hi,

On Fri, May 03, 2024 at 06:38:01PM +0300, Andy Shevchenko wrote:
> On Fri, May 03, 2024 at 09:30:39AM +0300, Jarkko Nikula wrote:
> > On 5/3/24 2:09 AM, Andi Shyti wrote:
> > > Applied to i2c/i2c-host on
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git
> > > 
> > > Thank you,
> > > Andi
> > > 
> > > Patches applied
> > > ===============
> > > [1/5] i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()
> > >        commit: 91647e64f0f5677ace84165dc25dc99579147b8f
> > > [2/5] i2c: designware: Create shared header hosting driver name
> > >        commit: 856cd5f13de7cebca44db5ff4bc2ca73490dd8d7
> > > 
> > Was the second patch applied accidentally?
> 
> +1 here, asked the same in private communication.

yes, it is, I had this applied in my branch before reviving the
thread. I then pushed and thanked for everything.

Thanks for your prompt reaction :-)

Andi