Re: [PATCH 1/2] i2c: i801: Switch to iomapped register access

[Heiner Kallweit <hkallweit1@gmail.com>]

On 19.03.2025 20:48, Andy Shevchenko wrote:
> On Wed, Mar 19, 2025 at 9:33 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> On 19.03.2025 09:23, Andy Shevchenko wrote:
>>> On Wed, Mar 19, 2025 at 9:17 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>> On 19.03.2025 00:22, Andi Shyti wrote:
>>>>> On Wed, Mar 19, 2025 at 12:18:47AM +0200, Andy Shevchenko wrote:
>>>>>> Wed, Mar 12, 2025 at 08:07:23PM +0100, Heiner Kallweit kirjoitti:
>>>
>>>>>>> Switch to iomapped register access as a prerequisite for adding
>>>>>>> support for MMIO register access.
>>>>>>
>>>>>> I believe that I at least discussed the similar change a few years ago or even
>>>>>> proposed a one. The problem here is that *_p() variants of IO port accessors
>>>>>> are not the same as non-_p ones. And commit message is kept silent about
>>>>>> possible consequences of this change.
>>>>>>
>>>>>> So, at bare minumum it would be good to test for some period of time before
>>>>>> going for it.
>>>>>
>>>>> How would you do it?
>>>>
>>>> Documentation/driver-api/device-io.rst states that the artificially delayed
>>>> _p versions were needed on ISA devices. And in general I didn't find any hint
>>>> that the non-delayed versions ever caused issues on PCI devices.
>>>
>>> At least put this in the commit message. It will show that you were aware of _p.
>>>
>>>> On my system using the non-delayed version works fine, but I can't say 100%
>>>> that it's the same for the very first (> 25 yrs ago) chipsets supported by i801.
>>>>
>>>> Likely users with old systems don't run -next kernels, therefore leaving
>>>> this change a full cycle in -next may not really help. We can argue that
>>>> we have the -rc period for testing (and reverting if needed).
>>>
>>> My main concern is to make no regressions for most currently used
>>> cases, that's why one cycle in Linux Next is better than none.
>>
>> Even ICH7 datasheet from 2012 mentions that SMBus register space is also
>> memory-mapped. So all systems from at least the last 10 yrs should use MMIO
>> instead of PMIO now, and therefore not be affected by switching to non-delayed
>> PMIO access. This should significantly reduce the risk you're referring to.
> 
> Cool! So, can we just put a summary into the commit message of all
> findings, worries (or their absence)?
> 
Sure. Would be a question to Andi how this should be done technically.