MMIO support for I2C-PIIX4 and SP5100-TCO

MMIO support for I2C-PIIX4 and SP5100-TCO

[Limonciello, Mario]

[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://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/1984
https://bugs.launchpad.net/amd/+bug/1950062

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

Thanks,

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

[Greg KH]

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://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/1984
> https://bugs.launchpad.net/amd/+bug/1950062
> 
> 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?  And why
can't people with newer hardware just use 5.18 and newer releases like
they do for other more complex hardware additions?

thanks,

greg k-h

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

[Limonciello, Mario]

[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.

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

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

[Greg KH]

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