[PATCH] peci: PECI should depend on ARCH_ASPEED

[PATCH] peci: PECI should depend on ARCH_ASPEED

[Geert Uytterhoeven]

The Platform Environment Control Interface (PECI) is only available on
Baseboard Management Controllers (BMC) for Intel processors.  Currently
the only supported BMCs are ASpeed BMC SoCs.  Hence add a dependency on
ARCH_ASPEED, to prevent asking the user about the PECI subsystem when
configuring a kernel without ASpeed SoC support.

Fixes: 6523d3b2ffa238ac ("peci: Add core infrastructure")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/peci/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
index 89872ad833201510..0d3ef8ba0998d649 100644
--- a/drivers/peci/Kconfig
+++ b/drivers/peci/Kconfig
@@ -2,6 +2,7 @@
 
 menuconfig PECI
 	tristate "PECI support"
+	depends on ARCH_ASPEED || COMPILE_TEST
 	help
 	  The Platform Environment Control Interface (PECI) is an interface
 	  that provides a communication channel to Intel processors and
-- 
2.25.1

Re: [PATCH] peci: PECI should depend on ARCH_ASPEED

[Greg Kroah-Hartman]

On Tue, Mar 29, 2022 at 11:21:37AM +0200, Geert Uytterhoeven wrote:
> The Platform Environment Control Interface (PECI) is only available on
> Baseboard Management Controllers (BMC) for Intel processors.  Currently
> the only supported BMCs are ASpeed BMC SoCs.  Hence add a dependency on
> ARCH_ASPEED, to prevent asking the user about the PECI subsystem when
> configuring a kernel without ASpeed SoC support.
> 
> Fixes: 6523d3b2ffa238ac ("peci: Add core infrastructure")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/peci/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
> index 89872ad833201510..0d3ef8ba0998d649 100644
> --- a/drivers/peci/Kconfig
> +++ b/drivers/peci/Kconfig
> @@ -2,6 +2,7 @@
>  
>  menuconfig PECI
>  	tristate "PECI support"
> +	depends on ARCH_ASPEED || COMPILE_TEST

I hate ARCH_ dependencies as there is nothing specific with that one
platform that means that this driver subsystem will only work on that
one.

I'm all for fixing build dependancies, but it should be fine to build
all drivers for all arches.

So sorry, I don't like this change.

greg k-h

Re: [PATCH] peci: PECI should depend on ARCH_ASPEED

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]

On Tue, Mar 29, 2022 at 12:33:14PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Mar 29, 2022 at 11:21:37AM +0200, Geert Uytterhoeven wrote:
> > The Platform Environment Control Interface (PECI) is only available on
> > Baseboard Management Controllers (BMC) for Intel processors.  Currently
> > the only supported BMCs are ASpeed BMC SoCs.  Hence add a dependency on
> > ARCH_ASPEED, to prevent asking the user about the PECI subsystem when
> > configuring a kernel without ASpeed SoC support.
> > 
> > Fixes: 6523d3b2ffa238ac ("peci: Add core infrastructure")
> > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> >  drivers/peci/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
> > index 89872ad833201510..0d3ef8ba0998d649 100644
> > --- a/drivers/peci/Kconfig
> > +++ b/drivers/peci/Kconfig
> > @@ -2,6 +2,7 @@
> >  
> >  menuconfig PECI
> >  	tristate "PECI support"
> > +	depends on ARCH_ASPEED || COMPILE_TEST
> 
> I hate ARCH_ dependencies as there is nothing specific with that one
> platform that means that this driver subsystem will only work on that
> one.

The motivation in the commit message isn't even accurate because the chips
under ARCH_NPCM are usually used as a BMC as well and PECI is just as important
for them.  HPE also has a custom chip they use as BMC and they've started the
process for upstreaming that support.

> I'm all for fixing build dependancies, but it should be fine to build
> all drivers for all arches.

There are a few drivers, like PECI and FSI, that are likely only useful
when being used in the BMC space.  Is it worth having a "config BMC" for
drivers which are likely only useful in a BMC environment and that we can
"if BMC" around these drivers so they get hidden from the menuconfig for
typical use cases?

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] peci: PECI should depend on ARCH_ASPEED

[Winiarska, Iwona]

On Tue, 2022-03-29 at 12:08 -0500, Patrick Williams wrote:
> On Tue, Mar 29, 2022 at 12:33:14PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Mar 29, 2022 at 11:21:37AM +0200, Geert Uytterhoeven wrote:
> > > The Platform Environment Control Interface (PECI) is only available on
> > > Baseboard Management Controllers (BMC) for Intel processors.  Currently
> > > the only supported BMCs are ASpeed BMC SoCs.  Hence add a dependency on
> > > ARCH_ASPEED, to prevent asking the user about the PECI subsystem when
> > > configuring a kernel without ASpeed SoC support.
> > > 
> > > Fixes: 6523d3b2ffa238ac ("peci: Add core infrastructure")
> > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > ---
> > >  drivers/peci/Kconfig | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
> > > index 89872ad833201510..0d3ef8ba0998d649 100644
> > > --- a/drivers/peci/Kconfig
> > > +++ b/drivers/peci/Kconfig
> > > @@ -2,6 +2,7 @@
> > >  
> > >  menuconfig PECI
> > >         tristate "PECI support"
> > > +       depends on ARCH_ASPEED || COMPILE_TEST
> > 
> > I hate ARCH_ dependencies as there is nothing specific with that one
> > platform that means that this driver subsystem will only work on that
> > one.
> 
> The motivation in the commit message isn't even accurate because the chips
> under ARCH_NPCM are usually used as a BMC as well and PECI is just as important
> for them.  HPE also has a custom chip they use as BMC and they've started the
> process for upstreaming that support.

"Currently the only supported BMCs are ASpeed BMC SoCs."
From PECI subsystem perspective (not BMC support in general), it is technically
true for now - but I agree with Greg and Patrick, this is just artificially
introducing build-time dependencies where in fact there are none. And yes - we
would then have to add the "depends on ARCH_YET_ANOTHER_ARCH" to generic
subsystem anytime we add a new PECI controller. This belongs in the controller
(and the ASPEED one depends on ARCH_ASPEED).

> 
> > I'm all for fixing build dependancies, but it should be fine to build
> > all drivers for all arches.
> 
> There are a few drivers, like PECI and FSI, that are likely only useful
> when being used in the BMC space.  Is it worth having a "config BMC" for
> drivers which are likely only useful in a BMC environment and that we can
> "if BMC" around these drivers so they get hidden from the menuconfig for
> typical use cases?

We don't have "config SERVER/config DESKTOP/config IOT".
I don't think there's anything special about BMCs to benefit from introducing
that (ultimately, this would be yet another "artificial build-time dependency").

Thanks
-Iwona

> 

Re: [PATCH] peci: PECI should depend on ARCH_ASPEED

[Geert Uytterhoeven]

Hi Iwona,

On Wed, Mar 30, 2022 at 12:37 PM Winiarska, Iwona
<iwona.winiarska@intel.com> wrote:
> On Tue, 2022-03-29 at 12:08 -0500, Patrick Williams wrote:
> > On Tue, Mar 29, 2022 at 12:33:14PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Mar 29, 2022 at 11:21:37AM +0200, Geert Uytterhoeven wrote:
> > > > The Platform Environment Control Interface (PECI) is only available on
> > > > Baseboard Management Controllers (BMC) for Intel processors.  Currently
> > > > the only supported BMCs are ASpeed BMC SoCs.  Hence add a dependency on
> > > > ARCH_ASPEED, to prevent asking the user about the PECI subsystem when
> > > > configuring a kernel without ASpeed SoC support.
> > > >
> > > > Fixes: 6523d3b2ffa238ac ("peci: Add core infrastructure")
> > > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > ---
> > > >  drivers/peci/Kconfig | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/peci/Kconfig b/drivers/peci/Kconfig
> > > > index 89872ad833201510..0d3ef8ba0998d649 100644
> > > > --- a/drivers/peci/Kconfig
> > > > +++ b/drivers/peci/Kconfig
> > > > @@ -2,6 +2,7 @@
> > > >
> > > >  menuconfig PECI
> > > >         tristate "PECI support"
> > > > +       depends on ARCH_ASPEED || COMPILE_TEST
> > >
> > > I hate ARCH_ dependencies as there is nothing specific with that one
> > > platform that means that this driver subsystem will only work on that
> > > one.
> >
> > The motivation in the commit message isn't even accurate because the chips
> > under ARCH_NPCM are usually used as a BMC as well and PECI is just as important
> > for them.  HPE also has a custom chip they use as BMC and they've started the
> > process for upstreaming that support.
>
> "Currently the only supported BMCs are ASpeed BMC SoCs."
> From PECI subsystem perspective (not BMC support in general), it is technically
> true for now - but I agree with Greg and Patrick, this is just artificially
> introducing build-time dependencies where in fact there are none. And yes - we
> would then have to add the "depends on ARCH_YET_ANOTHER_ARCH" to generic
> subsystem anytime we add a new PECI controller. This belongs in the controller
> (and the ASPEED one depends on ARCH_ASPEED).

OK, let's keep it as-is.

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