Re: [PATCH v3 2/4] i2c: piix4: Move SB800_PIIX4_FCH_PM_ADDR definition to amd_node.h

From: Ingo Molnar
Date: Sun Apr 13 2025 - 15:32:29 EST



* Mario Limonciello <superm1@xxxxxxxxxx> wrote:

>
>
> On 4/13/25 03:44, Ingo Molnar wrote:
> >
> > * Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > >
> > > * Borislav Petkov <bp@xxxxxxxxx> wrote:
> > >
> > > > I was aiming more for a header which contains non-CPU defines -
> > > > i.e., platform. But the FCH is only one part of that platform. But
> > > > let's start with amd/fch.h - "amd/" subpath element would allow us
> > > > to trivially put other headers there too - and see where it gets
> > > > us. We can (and will) always refactor later if needed...
> > >
> > > Yeah, agreed on opening the <asm/amd/> namespace for this.
> >
> > Here's a tree that establishes <asm/amd/> and moves existing headers
> > there:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/platform
> >
> > Mario, could you base your series on top of this tree?
> >
>
> Sure.
>
> One problem that I notice though is that by using <asm/amd/fch.h> that
> drivers/i2c/busses/i2c-piix4.c has some compile failures on non-x86.

Hm, should these I2C drivers even be built on non-x86 systems?

> 1) Add '#if CONFIG_X86' around all related code.
>
> 2) Move applicable code to drivers/i2c/busses/i2c-amd-fch.c (similar to how
> we have i2c-amd-asf-plat.c) but modify drivers/i2c/busses/Makefile to only
> compile it for x86.
>
> 3) Idea two but also add a new Kconfig for CONFIG_I2C_AMD_FCH that depends
> on CONFIG_X86.
>
> I am /leaning/ on the refactor with idea 3.

I'd go for something like the patch below. There's X86 dependencies for
other I2C drivers as well, so it's not unprecedented.

Thanks,

Ingo

==============>
drivers/i2c/busses/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 83c88c79afe2..bbbd6240fa6e 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -200,7 +200,7 @@ config I2C_ISMT

config I2C_PIIX4
tristate "Intel PIIX4 and compatible (ATI/AMD/Serverworks/Broadcom/SMSC)"
- depends on PCI && HAS_IOPORT
+ depends on PCI && HAS_IOPORT && X86
select I2C_SMBUS
help
If you say yes to this option, support will be included for the Intel