Re: MMIO support for I2C-PIIX4 and SP5100-TCO

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, May 18, 2022 at 07:35:41PM +0000, Limonciello, Mario wrote:
> [Public]
> 
> 
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Wednesday, May 18, 2022 14:25
> > To: Limonciello, Mario <Mario.Limonciello@amd.com>
> > Cc: stable@vger.kernel.org
> > Subject: Re: MMIO support for I2C-PIIX4 and SP5100-TCO
> > 
> > On Wed, May 18, 2022 at 06:49:59PM +0000, Limonciello, Mario wrote:
> > > [Public]
> > >
> > > Hi,
> > >
> > > Some users have complained that i2c the controller doesn't work on newer
> > designs.  This is because the system can be configured by an OEM to not
> > allow access to the I2C controller registers via legacy methods and instead
> > requires MMIO.
> > >
> > > Some bug reports collecting this problem (which have had duplicates
> > brought in)
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> > b.com%2FCalcProgrammer1%2FOpenRGB%2F-
> > %2Fissues%2F1984&amp;data=05%7C01%7CMario.Limonciello%40amd.com
> > %7C5c8827b7f53e479d364108da39042a91%7C3dd8961fe4884e608e11a82d994
> > e183d%7C0%7C0%7C637884987298944924%7CUnknown%7CTWFpbGZsb3d8
> > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C3000%7C%7C%7C&amp;sdata=JQrvW5I0iSF14zm64ijMegqFSX6YTF1p6Y
> > c8mub7DgI%3D&amp;reserved=0
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> > .launchpad.net%2Famd%2F%2Bbug%2F1950062&amp;data=05%7C01%7CMa
> > rio.Limonciello%40amd.com%7C5c8827b7f53e479d364108da39042a91%7C3dd
> > 8961fe4884e608e11a82d994e183d%7C0%7C0%7C637884987298944924%7CUn
> > known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> > Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=z3Ba6AfJE41z
> > cZfpTd1Fkp5jg1sKPEMHy%2BWDIgCv6tE%3D&amp;reserved=0
> > >
> > > These commits that have landed into 5.18 fix this issue both for i2c-piix4
> > and for sp5100-tco (which suffers the same fate).
> > >
> > > Would you take them back to stable 5.15.y and 5.17.y?  The series comes
> > back cleanly to both.
> > >
> > > 27c196c7b73c kernel/resource: Introduce request_mem_region_muxed()
> > > 93102cb44978 i2c: piix4: Replace hardcoded memory map size with a
> > #define
> > > a3325d225b00 i2c: piix4: Move port I/O region request/release code into
> > functions
> > > 0a59a24e14e9 i2c: piix4: Move SMBus controller base address detect into
> > function
> > > fbafbd51bff5 i2c: piix4: Move SMBus port selection into function
> > > 7c148722d074 i2c: piix4: Add EFCH MMIO support to region request and
> > release
> > > 46967bc1ee93 i2c: piix4: Add EFCH MMIO support to SMBus base address
> > detect
> > > 381a3083c674 i2c: piix4: Add EFCH MMIO support for SMBus port select
> > > 6cf72f41808a i2c: piix4: Enable EFCH MMIO for Family 17h+
> > > abd71a948f7a Watchdog: sp5100_tco: Move timer initialization into
> > function
> > > 1f182aca2300 Watchdog: sp5100_tco: Refactor MMIO base address
> > initialization
> > > 0578fff4aae5 Watchdog: sp5100_tco: Add initialization using EFCH MMIO
> > > 826270373f17 Watchdog: sp5100_tco: Enable Family 17h+ CPUs
> > 
> > What is the overall diffstat of all of these commits applied?  
> 
> $ git diff --stat linux-5.15.y HEAD^
>  drivers/i2c/busses/i2c-piix4.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
>  drivers/watchdog/sp5100_tco.c  | 318 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------
>  drivers/watchdog/sp5100_tco.h  |   6 +++
>  include/linux/ioport.h         |   2 +
>  4 files changed, 390 insertions(+), 149 deletions(-)
> 
> > And why
> > can't people with newer hardware just use 5.18 and newer releases like
> > they do for other more complex hardware additions?
> > 
> 
> I think that argument is fine for not taking it to 5.17.y as 5.18 is right around the
> corner.  I do think there is a strong case for 5.15.y though for a few reasons:
> 
> 1) The same problem happens on client (Ryzen) and server (EPYC).  Most people
> will otherwise stick to the LTS kernel for the stability purposes.
> 
> 2) The affected hardware here isn't new hardware, it's just a solution to an old
> problem.  For client chips I2C not working means common things like the I2C
> trackpads won't work if they are on such a system.   Also rarer things like RGB
> lighting (which was the OpenRGB bug I linked) don't work.

Ah, RGB lighting, I missed that link, sorry about that.

I'll gladly fix up a kernel for stuff like this anyday over boring old
"enterprise" features, as this is what makes computers fun.

> Not having the watchdog timer hardware working is probably more important for
> the server chips though.

Ah, bonus then, the enterprise people will not feel left out.

all queued up, thanks.

greg k-h