x86 : Kconfig : INTEL_PMC_CORE not specific enough

x86 : Kconfig : INTEL_PMC_CORE not specific enough

[tedheadster]

Intel PMC Core and CLK API teams,
  I am trying to build a minimal kernel config on old
regression-testing hardware. Presently there is the following
drivers/platform/x86/Kconfig item:

config PMC_ATOM
       def_bool y
       depends on PCI
       select COMMON_CLK

This creates a cascade of Kconfig "select" statements and pulls in two files:

drivers/clk/x86/clk-pmc-atom.c
drivers/platform/x86/intel_pmc_core.c

The Kconfig seems too general to me. Compiling for my i586 Pentium-I
pulls in these files (I think) unnecessarily. Why does _anybody_ with
a PCI bus need these ATOM-specific files?

Would this change (or something similar) make sense?

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index d10ffe5..6a026fa 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1217,5 +1217,5 @@ endif # X86_PLATFORM_DEVICES

 config PMC_ATOM
        def_bool y
-       depends on PCI
+       depends on PCI && MATOM
        select COMMON_CLK
-- 
1.8.3.1

- Matthew Whitehead

Re: x86 : Kconfig : INTEL_PMC_CORE not specific enough

[Andy Shevchenko]

+Cc: Alan (perhaps he has something to add or correct me)

On Tue, Mar 13, 2018 at 1:20 PM, tedheadster <tedheadster@gmail.com> wrote:
> Intel PMC Core and CLK API teams,
>   I am trying to build a minimal kernel config on old
> regression-testing hardware. Presently there is the following
> drivers/platform/x86/Kconfig item:
>
> config PMC_ATOM
>        def_bool y
>        depends on PCI
>        select COMMON_CLK
>
> This creates a cascade of Kconfig "select" statements and pulls in two files:
>
> drivers/clk/x86/clk-pmc-atom.c
> drivers/platform/x86/intel_pmc_core.c
>
> The Kconfig seems too general to me. Compiling for my i586 Pentium-I
> pulls in these files (I think) unnecessarily. Why does _anybody_ with
> a PCI bus need these ATOM-specific files?

Long story, but main concern here, that if you do s/ATOM/X86/ it will
be more clear since we provide as less kernels as possible to cover
all x86 architectures.

Currently we have one kernel per:
- any x86_64 case
- any i686 case
- uniprocessor i586 TSC case (Intel Quark)
- (the rest exotic cases which is probably no-one using anymore)

Your change disrupts this quite badly.

So, I think your point that you don't need it by default, and here is
another story related to the design of Intel BayTrail / CherryTrail
platforms where we have a nasty bug and we can't use the driver as a
module.

> Would this change (or something similar) make sense?

No.

> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1217,5 +1217,5 @@ endif # X86_PLATFORM_DEVICES
>
>  config PMC_ATOM
>         def_bool y
> -       depends on PCI
> +       depends on PCI && MATOM

MATOM is for really old CPUs. Mentioned BayTrail et al rather normal
x86 in that sense.

>         select COMMON_CLK

Definitely NAK by the reasons mentioned above.

-- 
With Best Regards,
Andy Shevchenko

Re: x86 : Kconfig : INTEL_PMC_CORE not specific enough

[Andy Shevchenko]

On Tue, Mar 13, 2018 at 1:20 PM, tedheadster <tedheadster@gmail.com> wrote:

> drivers/platform/x86/intel_pmc_core.c

Apparently you meant pmc_atom.c in the same folder.

>  config PMC_ATOM

-- 
With Best Regards,
Andy Shevchenko

Re: x86 : Kconfig : INTEL_PMC_CORE not specific enough

[tedheadster]

> Long story, but main concern here, that if you do s/ATOM/X86/ it will
> be more clear since we provide as less kernels as possible to cover
> all x86 architectures.
>
> Currently we have one kernel per:
> - any x86_64 case
> - any i686 case
> - uniprocessor i586 TSC case (Intel Quark)
> - (the rest exotic cases which is probably no-one using anymore)
>
> Your change disrupts this quite badly.

This simplifies your situation, but it forces everybody else to
include extra code they will never use.

>
> So, I think your point that you don't need it by default, and here is
> another story related to the design of Intel BayTrail / CherryTrail
> platforms where we have a nasty bug and we can't use the driver as a
> module.
>

Please explain the history so we can understand.


In more detail, here are the Kconfig dependencies:

- Symbol: PMC_ATOM [=y]
- Depends on: X86 [=y] && PCI [=y]
- Selects: COMMON_CLK [=y]

Symbol: COMMON_CLK [=y]
- Selected by:  PMC_ATOM [=y] && X86 [=y] && PCI [=y]
- Selects: HAVE_CLK_PREPARE [=y] && CLKDEV_LOOKUP [=y] && SRCU [=y] &&
RATIONAL [=y]

So having _any_ X86 and PCI pulls in these four files:

drivers/clk/x86/clk-pmc-atom.c
drivers/platform/x86/pmc_atom.c (you are right, I cut-and-pasted the
wrong file earlier)
drivers/clk/clkdev.c
lib/rational.c


>> Would this change (or something similar) make sense?
>
> No.
>

Okay, my novice solution isn't the right way. There must be an elegant
way to leave this configured in by default (what you want), and still
be able to turn it off if you desire (what I want).


> MATOM is for really old CPUs. Mentioned BayTrail et al rather normal
> x86 in that sense.
>
>>         select COMMON_CLK
>
> Definitely NAK by the reasons mentioned above.
>


- Matthew