Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Geert Uytterhoeven]

Hi Mario,

CC mips, loongarch

On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> PIIX4 and compatible controllers are only for X86. As some headers are
> being moved into x86 specific headers PIIX4 won't compile on non-x86.
>
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Thanks for your patch, which is now commit 7e173eb82ae97175
("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
in v6.16-rc1.

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

Are you sure this south-bridge is not used on non-x86 platforms?
It is enabled in several non-x86 defconfigs:

    arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
    arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
    arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
    arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y

The loongarch and loongson entries are probably bogus, but I wouldn't
be surprised if the SGI Onyx and Origin do use Intel south-bridges.

>         select I2C_SMBUS
>         help
>           If you say yes to this option, support will be included for the Intel

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Huacai Chen]

On Tue, Jun 10, 2025 at 5:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Mario,
>
> CC mips, loongarch
>
> On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
> > From: Mario Limonciello <mario.limonciello@amd.com>
> >
> > PIIX4 and compatible controllers are only for X86. As some headers are
> > being moved into x86 specific headers PIIX4 won't compile on non-x86.
> >
> > Suggested-by: Ingo Molnar <mingo@kernel.org>
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>
> Thanks for your patch, which is now commit 7e173eb82ae97175
> ("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
> in v6.16-rc1.
>
> > --- 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
>
> Are you sure this south-bridge is not used on non-x86 platforms?
> It is enabled in several non-x86 defconfigs:
>
>     arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>     arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
>     arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
>     arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>
> The loongarch and loongson entries are probably bogus, but I wouldn't
> be surprised if the SGI Onyx and Origin do use Intel south-bridges.
Loongson can use AMD SB700/SB800 south bridges, which have I2C_PIIX4.

Huacai

>
> >         select I2C_SMBUS
> >         help
> >           If you say yes to this option, support will be included for the Intel
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Mario Limonciello]

On 6/10/2025 2:24 AM, Huacai Chen wrote:
> On Tue, Jun 10, 2025 at 5:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>
>> Hi Mario,
>>
>> CC mips, loongarch
>>
>> On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> PIIX4 and compatible controllers are only for X86. As some headers are
>>> being moved into x86 specific headers PIIX4 won't compile on non-x86.
>>>
>>> Suggested-by: Ingo Molnar <mingo@kernel.org>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>
>> Thanks for your patch, which is now commit 7e173eb82ae97175
>> ("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
>> in v6.16-rc1.
>>
>>> --- 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
>>
>> Are you sure this south-bridge is not used on non-x86 platforms?
>> It is enabled in several non-x86 defconfigs:
>>
>>      arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>      arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
>>      arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
>>      arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>
>> The loongarch and loongson entries are probably bogus, but I wouldn't
>> be surprised if the SGI Onyx and Origin do use Intel south-bridges.
> Loongson can use AMD SB700/SB800 south bridges, which have I2C_PIIX4.

Well we could revert this patch, but it's going to be a compile failure 
because of 624b0d5696a89b138408d385899dd35372db324b and other patches 
that go on top of that.

My current leaning is we make a dummy fch.h header for these archs with 
#defines for 0.

Any thoughts?

> 
> Huacai
> 
>>
>>>          select I2C_SMBUS
>>>          help
>>>            If you say yes to this option, support will be included for the Intel
>>
>> Gr{oetje,eeting}s,
>>
>>                          Geert
>>
>> --
>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>>
>> In personal conversations with technical people, I call myself a hacker. But
>> when I'm talking to journalists I just say "programmer" or something like that.
>>                                  -- Linus Torvalds
>>

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Hans de Goede]

Hi,

On 10-Jun-25 16:12, Mario Limonciello wrote:
> On 6/10/2025 2:24 AM, Huacai Chen wrote:
>> On Tue, Jun 10, 2025 at 5:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>
>>> Hi Mario,
>>>
>>> CC mips, loongarch
>>>
>>> On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> PIIX4 and compatible controllers are only for X86. As some headers are
>>>> being moved into x86 specific headers PIIX4 won't compile on non-x86.
>>>>
>>>> Suggested-by: Ingo Molnar <mingo@kernel.org>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> Thanks for your patch, which is now commit 7e173eb82ae97175
>>> ("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
>>> in v6.16-rc1.
>>>
>>>> --- 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
>>>
>>> Are you sure this south-bridge is not used on non-x86 platforms?
>>> It is enabled in several non-x86 defconfigs:
>>>
>>>      arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>>      arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
>>>      arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
>>>      arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>>
>>> The loongarch and loongson entries are probably bogus, but I wouldn't
>>> be surprised if the SGI Onyx and Origin do use Intel south-bridges.
>> Loongson can use AMD SB700/SB800 south bridges, which have I2C_PIIX4.
> 
> Well we could revert this patch, but it's going to be a compile failure because of 624b0d5696a89b138408d385899dd35372db324b and other patches that go on top of that.
> 
> My current leaning is we make a dummy fch.h header for these archs with #defines for 0.

Move "fch.h" to include/linux/platform/x86/ so that it is available on all arches
and if necessary ifdef out anything x86 specific in the C-code referencing it ?

Regards,

Hans

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Hans de Goede]

Hi,

On 10-Jun-25 16:53, Hans de Goede wrote:
> Hi,
> 
> On 10-Jun-25 16:12, Mario Limonciello wrote:
>> On 6/10/2025 2:24 AM, Huacai Chen wrote:
>>> On Tue, Jun 10, 2025 at 5:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>
>>>> Hi Mario,
>>>>
>>>> CC mips, loongarch
>>>>
>>>> On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
>>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>>
>>>>> PIIX4 and compatible controllers are only for X86. As some headers are
>>>>> being moved into x86 specific headers PIIX4 won't compile on non-x86.
>>>>>
>>>>> Suggested-by: Ingo Molnar <mingo@kernel.org>
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>
>>>> Thanks for your patch, which is now commit 7e173eb82ae97175
>>>> ("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
>>>> in v6.16-rc1.
>>>>
>>>>> --- 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
>>>>
>>>> Are you sure this south-bridge is not used on non-x86 platforms?
>>>> It is enabled in several non-x86 defconfigs:
>>>>
>>>>      arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>>>      arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
>>>>      arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
>>>>      arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>>>
>>>> The loongarch and loongson entries are probably bogus, but I wouldn't
>>>> be surprised if the SGI Onyx and Origin do use Intel south-bridges.
>>> Loongson can use AMD SB700/SB800 south bridges, which have I2C_PIIX4.
>>
>> Well we could revert this patch, but it's going to be a compile failure because of 624b0d5696a89b138408d385899dd35372db324b and other patches that go on top of that.
>>
>> My current leaning is we make a dummy fch.h header for these archs with #defines for 0.
> 
> Move "fch.h" to include/linux/platform/x86/ so that it is available on all arches
> and if necessary ifdef out anything x86 specific in the C-code referencing it ?

Correction that should be include/linux/platform_data/x86/

Regards,

Hans

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Geert Uytterhoeven]

Hi Hans,

On Tue, 10 Jun 2025 at 16:55, Hans de Goede <hansg@kernel.org> wrote:
> On 10-Jun-25 16:53, Hans de Goede wrote:
> > On 10-Jun-25 16:12, Mario Limonciello wrote:
> >> On 6/10/2025 2:24 AM, Huacai Chen wrote:
> >>> On Tue, Jun 10, 2025 at 5:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >>>> On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
> >>>>> From: Mario Limonciello <mario.limonciello@amd.com>
> >>>>>
> >>>>> PIIX4 and compatible controllers are only for X86. As some headers are
> >>>>> being moved into x86 specific headers PIIX4 won't compile on non-x86.
> >>>>>
> >>>>> Suggested-by: Ingo Molnar <mingo@kernel.org>
> >>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >>>>
> >>>> Thanks for your patch, which is now commit 7e173eb82ae97175
> >>>> ("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
> >>>> in v6.16-rc1.
> >>>>
> >>>>> --- 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
> >>>>
> >>>> Are you sure this south-bridge is not used on non-x86 platforms?
> >>>> It is enabled in several non-x86 defconfigs:
> >>>>
> >>>>      arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
> >>>>      arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
> >>>>      arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
> >>>>      arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
> >>>>
> >>>> The loongarch and loongson entries are probably bogus, but I wouldn't
> >>>> be surprised if the SGI Onyx and Origin do use Intel south-bridges.
> >>> Loongson can use AMD SB700/SB800 south bridges, which have I2C_PIIX4.
> >>
> >> Well we could revert this patch, but it's going to be a compile failure because of 624b0d5696a89b138408d385899dd35372db324b and other patches that go on top of that.
> >>
> >> My current leaning is we make a dummy fch.h header for these archs with #defines for 0.
> >
> > Move "fch.h" to include/linux/platform/x86/ so that it is available on all arches
> > and if necessary ifdef out anything x86 specific in the C-code referencing it ?
>
> Correction that should be include/linux/platform_data/x86/

Is that a good location?
It is not x86-specific, and I see no platform_data (struct) definitions in
arch/x86/include/asm/amd/fch.h?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v5 2/5] i2c: piix4: Depends on X86

[Hans de Goede]

Hi Geert,

On 10-Jun-25 18:59, Geert Uytterhoeven wrote:
> Hi Hans,
> 
> On Tue, 10 Jun 2025 at 16:55, Hans de Goede <hansg@kernel.org> wrote:
>> On 10-Jun-25 16:53, Hans de Goede wrote:
>>> On 10-Jun-25 16:12, Mario Limonciello wrote:
>>>> On 6/10/2025 2:24 AM, Huacai Chen wrote:
>>>>> On Tue, Jun 10, 2025 at 5:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>>> On Wed, 23 Apr 2025 at 01:49, Mario Limonciello <superm1@kernel.org> wrote:
>>>>>>> From: Mario Limonciello <mario.limonciello@amd.com>
>>>>>>>
>>>>>>> PIIX4 and compatible controllers are only for X86. As some headers are
>>>>>>> being moved into x86 specific headers PIIX4 won't compile on non-x86.
>>>>>>>
>>>>>>> Suggested-by: Ingo Molnar <mingo@kernel.org>
>>>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>>>
>>>>>> Thanks for your patch, which is now commit 7e173eb82ae97175
>>>>>> ("i2c: piix4: Make CONFIG_I2C_PIIX4 dependent on CONFIG_X86")
>>>>>> in v6.16-rc1.
>>>>>>
>>>>>>> --- 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
>>>>>>
>>>>>> Are you sure this south-bridge is not used on non-x86 platforms?
>>>>>> It is enabled in several non-x86 defconfigs:
>>>>>>
>>>>>>      arch/loongarch/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>>>>>      arch/mips/configs/ip27_defconfig:CONFIG_I2C_PIIX4=m
>>>>>>      arch/mips/configs/loongson2k_defconfig:CONFIG_I2C_PIIX4=y
>>>>>>      arch/mips/configs/loongson3_defconfig:CONFIG_I2C_PIIX4=y
>>>>>>
>>>>>> The loongarch and loongson entries are probably bogus, but I wouldn't
>>>>>> be surprised if the SGI Onyx and Origin do use Intel south-bridges.
>>>>> Loongson can use AMD SB700/SB800 south bridges, which have I2C_PIIX4.
>>>>
>>>> Well we could revert this patch, but it's going to be a compile failure because of 624b0d5696a89b138408d385899dd35372db324b and other patches that go on top of that.
>>>>
>>>> My current leaning is we make a dummy fch.h header for these archs with #defines for 0.
>>>
>>> Move "fch.h" to include/linux/platform/x86/ so that it is available on all arches
>>> and if necessary ifdef out anything x86 specific in the C-code referencing it ?
>>
>> Correction that should be include/linux/platform_data/x86/
> 
> Is that a good location?

It is a location typically used for x86 headers which we want to be
available when not building on x86 too.

> It is not x86-specific, and I see no platform_data (struct) definitions in
> arch/x86/include/asm/amd/fch.h?

If it is not x86 specific then maybe just include/linux/amd_fch.h ?

Anyways I don't really give much about the exact name, the essence of
my suggestion is that we can fix this by moving fch.h to some place
(and maybe a new name) under include/linux so that the header is also
available when not building for x86.

Regards,

Hans





> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>