mei: make hdcp and pxp depend on X86 && PCI

  From: Adam Borowski <kilobyte@angband.pl>

All other MEI configs do so already.  This fixes a Kconfig gripe if I915
gets ported to other archs (such as RISC-V in Intel Horse Creek...).

Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
 drivers/misc/mei/hdcp/Kconfig | 1 +
 drivers/misc/mei/pxp/Kconfig  | 1 +
 2 files changed, 2 insertions(+)
  

What is the exact issue you are experiencing, can you add the error message this fixes? 

> 
> From: Adam Borowski <kilobyte@angband.pl>
> 
> All other MEI configs do so already.  This fixes a Kconfig gripe if I915 gets
> ported to other archs (such as RISC-V in Intel Horse Creek...).
> 
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> ---
>  drivers/misc/mei/hdcp/Kconfig | 1 +
>  drivers/misc/mei/pxp/Kconfig  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/misc/mei/hdcp/Kconfig b/drivers/misc/mei/hdcp/Kconfig
> index 54e1c9526909..83e67771ac6b 100644
> --- a/drivers/misc/mei/hdcp/Kconfig
> +++ b/drivers/misc/mei/hdcp/Kconfig
> @@ -4,6 +4,7 @@
>  config INTEL_MEI_HDCP
>  	tristate "Intel HDCP2.2 services of ME Interface"
>  	select INTEL_MEI_ME
> +	depends on X86 && PCI
>  	depends on DRM_I915
>  	help
>  	  MEI Support for HDCP2.2 Services on Intel platforms.
> diff --git a/drivers/misc/mei/pxp/Kconfig b/drivers/misc/mei/pxp/Kconfig
> index 4029b96afc04..95f2c2470d28 100644
> --- a/drivers/misc/mei/pxp/Kconfig
> +++ b/drivers/misc/mei/pxp/Kconfig
> @@ -5,6 +5,7 @@
>  config INTEL_MEI_PXP
>  	tristate "Intel PXP services of ME Interface"
>  	select INTEL_MEI_ME
> +	depends on X86 && PCI
>  	depends on DRM_I915
>  	help
>  	  MEI Support for PXP Services on Intel platforms.
> --
> 2.40.0
  

On Tue, Apr 25, 2023 at 04:39:23AM +0000, Winkler, Tomas wrote:
> What is the exact issue you are experiencing, can you add the error message this fixes? 

The problem doesn't trigger in mainline, as DRM_I915 can't be enabled on
!X86; https://github.com/kilobyte/linux branch i915 has a WIP port.  Some
unrelated piece (I suspect ACPI changes, didn't bother to investigate) has
in the meantime grown enough bits to enable prerequsites that trigger:

WARNING: unmet direct dependencies detected for INTEL_MEI_ME
  Depends on [n]: X86 && PCI [=y]
  Selected by [y]:
  - INTEL_MEI_HDCP [=y] && DRM_I915 [=y]
  - INTEL_MEI_PXP [=y] && DRM_I915 [=y]

This doesn't cause an actual build failure, as drivers/misc/Makefile has
> obj-$(CONFIG_INTEL_MEI)         += mei/
which won't try to build mei/hdcp nor mei/pxp despite them being enabled
in the config.

Of all the settings in drivers/misc/mei/Kconfig, only these two (that
reside in their own subdirectories) lack this stanza.  You'd want to
deduplicate the selects one day -- but alas, "select" relations are not as
straightforward as "depends", thus I only did the same as for all other
settings in drivers/misc/mei/Kconfig.

Should I quote this warning in full in the commit message?  I thought that
"make ... depend on ..." already explains enough.

Also, as this lack of dependency is an obvious and self-contained change,
I decided to submit it directly via mei, rather than ask for an ACK then
spam you with every iteration of the patchset.  Should it go via drm/i915
instead?

(I'd also like to apologize for sitting on this patchset so long, my
brain's firmware is faulty :/ ).

> > From: Adam Borowski <kilobyte@angband.pl>
> > 
> > All other MEI configs do so already.  This fixes a Kconfig gripe if I915 gets
> > ported to other archs (such as RISC-V in Intel Horse Creek...).
> > 
> > Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> > ---
> >  drivers/misc/mei/hdcp/Kconfig | 1 +
> >  drivers/misc/mei/pxp/Kconfig  | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/drivers/misc/mei/hdcp/Kconfig b/drivers/misc/mei/hdcp/Kconfig
> > index 54e1c9526909..83e67771ac6b 100644
> > --- a/drivers/misc/mei/hdcp/Kconfig
> > +++ b/drivers/misc/mei/hdcp/Kconfig
> > @@ -4,6 +4,7 @@
> >  config INTEL_MEI_HDCP
> >  	tristate "Intel HDCP2.2 services of ME Interface"
> >  	select INTEL_MEI_ME
> > +	depends on X86 && PCI
> >  	depends on DRM_I915
> >  	help
> >  	  MEI Support for HDCP2.2 Services on Intel platforms.
> > diff --git a/drivers/misc/mei/pxp/Kconfig b/drivers/misc/mei/pxp/Kconfig
> > index 4029b96afc04..95f2c2470d28 100644
> > --- a/drivers/misc/mei/pxp/Kconfig
> > +++ b/drivers/misc/mei/pxp/Kconfig
> > @@ -5,6 +5,7 @@
> >  config INTEL_MEI_PXP
> >  	tristate "Intel PXP services of ME Interface"
> >  	select INTEL_MEI_ME
> > +	depends on X86 && PCI
> >  	depends on DRM_I915
> >  	help
> >  	  MEI Support for PXP Services on Intel platforms.
> > --
> > 2.40.0

Meow!
  

On Tue, Apr 25, 2023 at 01:52:10PM +0200, Adam Borowski wrote:
> On Tue, Apr 25, 2023 at 04:39:23AM +0000, Winkler, Tomas wrote:
> > What is the exact issue you are experiencing, can you add the error message this fixes? 
> 
> The problem doesn't trigger in mainline

Then it's nothing we need to worry about in mainline.  If/when other
changes ever happen to need it here in mainline, we will gladly take the
change.

For obvious reasons, we can't take patches for issues outside of our
codebase.  Nor do you want us to, as that way lies madness and an
unmaintainable mess.

thanks,

greg k-h
  

On Tue, Apr 25, 2023 at 03:40:10PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 25, 2023 at 01:52:10PM +0200, Adam Borowski wrote:
> > On Tue, Apr 25, 2023 at 04:39:23AM +0000, Winkler, Tomas wrote:
> > > What is the exact issue you are experiencing, can you add the error message this fixes? 
> > 
> > The problem doesn't trigger in mainline
> 
> Then it's nothing we need to worry about in mainline.  If/when other
> changes ever happen to need it here in mainline, we will gladly take the
> change.
> 
> For obvious reasons, we can't take patches for issues outside of our
> codebase.  Nor do you want us to, as that way lies madness and an
> unmaintainable mess.

The problem in mainline is inconsistency: out of 6 config items, 4 repeat
the "depends on X86 && PCI" line, the other 2 do not.  There's indeed no
immediate functional issue, but I'd argue that a dormant bug is still a bug.

So we can fix the cosmetic (currently) issue on its own, or as part of the
large patchset -- the latter having a side effect of stuffing your mailboxes
more than needed (neither Greg, Arnd, nor Tomas are involved in other bits).

But do the selects really require so much duplication?  Perhaps I'm trying
to fix the underlying issue wrong? 


Meow!
  

On Tue, Apr 25, 2023, at 16:27, Adam Borowski wrote:
> On Tue, Apr 25, 2023 at 03:40:10PM +0200, Greg Kroah-Hartman wrote:
>> On Tue, Apr 25, 2023 at 01:52:10PM +0200, Adam Borowski wrote:
>> > On Tue, Apr 25, 2023 at 04:39:23AM +0000, Winkler, Tomas wrote:
>> > > What is the exact issue you are experiencing, can you add the error message this fixes? 
>> > 
>> > The problem doesn't trigger in mainline
>> 
>> Then it's nothing we need to worry about in mainline.  If/when other
>> changes ever happen to need it here in mainline, we will gladly take the
>> change.
>> 
>> For obvious reasons, we can't take patches for issues outside of our
>> codebase.  Nor do you want us to, as that way lies madness and an
>> unmaintainable mess.
>
> The problem in mainline is inconsistency: out of 6 config items, 4 repeat
> the "depends on X86 && PCI" line, the other 2 do not.  There's indeed no
> immediate functional issue, but I'd argue that a dormant bug is still a bug.
>
> So we can fix the cosmetic (currently) issue on its own, or as part of the
> large patchset -- the latter having a side effect of stuffing your mailboxes
> more than needed (neither Greg, Arnd, nor Tomas are involved in other bits).
>
> But do the selects really require so much duplication?  Perhaps I'm trying
> to fix the underlying issue wrong? 

I think something along these lines would do (untested):

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index d21486d69df2..8e5d79cff80b 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 # Copyright (c) 2003-2019, Intel Corporation. All rights reserved.
-config INTEL_MEI
+menuconfig INTEL_MEI
        tristate "Intel Management Engine Interface"
        depends on X86 && PCI
        help
@@ -13,8 +13,6 @@ config INTEL_MEI
 
 config INTEL_MEI_ME
        tristate "ME Enabled Intel Chipsets"
-       select INTEL_MEI
-       depends on X86 && PCI
        help
          MEI support for ME Enabled Intel chipsets.
 
@@ -38,8 +36,6 @@ config INTEL_MEI_ME
 
 config INTEL_MEI_TXE
        tristate "Intel Trusted Execution Environment with ME Interface"
-       select INTEL_MEI
-       depends on X86 && PCI
        help
          MEI Support for Trusted Execution Environment device on Intel SoCs
 
@@ -48,9 +44,7 @@ config INTEL_MEI_TXE
 
 config INTEL_MEI_GSC
        tristate "Intel MEI GSC embedded device"
-       depends on INTEL_MEI
        depends on INTEL_MEI_ME
-       depends on X86 && PCI
        depends on DRM_I915
        help
          Intel auxiliary driver for GSC devices embedded in Intel graphics devices.
@@ -63,3 +57,4 @@ config INTEL_MEI_GSC
 source "drivers/misc/mei/hdcp/Kconfig"
 source "drivers/misc/mei/pxp/Kconfig"
 
+endmenu
  

> 
> On Tue, Apr 25, 2023, at 16:27, Adam Borowski wrote:
> > On Tue, Apr 25, 2023 at 03:40:10PM +0200, Greg Kroah-Hartman wrote:
> >> On Tue, Apr 25, 2023 at 01:52:10PM +0200, Adam Borowski wrote:
> >> > On Tue, Apr 25, 2023 at 04:39:23AM +0000, Winkler, Tomas wrote:
> >> > > What is the exact issue you are experiencing, can you add the error
> message this fixes?
> >> >
> >> > The problem doesn't trigger in mainline
> >>
> >> Then it's nothing we need to worry about in mainline.  If/when other
> >> changes ever happen to need it here in mainline, we will gladly take
> >> the change.
> >>
> >> For obvious reasons, we can't take patches for issues outside of our
> >> codebase.  Nor do you want us to, as that way lies madness and an
> >> unmaintainable mess.
> >
> > The problem in mainline is inconsistency: out of 6 config items, 4
> > repeat the "depends on X86 && PCI" line, the other 2 do not.  There's
> > indeed no immediate functional issue, but I'd argue that a dormant bug is
> still a bug.

The MEI protocol (CONFIG_INTEL_MEI) is not PCI or X86 dependent. 
INTEL_MEI_GSC is PCI dependent but not X86. (Hope I correct also on implementation side)
They HW layers are currently X86 dependent. 

> >
> > So we can fix the cosmetic (currently) issue on its own, or as part of
> > the large patchset -- the latter having a side effect of stuffing your
> > mailboxes more than needed (neither Greg, Arnd, nor Tomas are involved
> in other bits).
> >
> > But do the selects really require so much duplication?  Perhaps I'm
> > trying to fix the underlying issue wrong?
> 
> I think something along these lines would do (untested):
> 
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> d21486d69df2..8e5d79cff80b 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Copyright (c) 2003-2019, Intel Corporation. All rights reserved.
> -config INTEL_MEI
> +menuconfig INTEL_MEI
>         tristate "Intel Management Engine Interface"
>         depends on X86 && PCI
>         help
> @@ -13,8 +13,6 @@ config INTEL_MEI
> 
>  config INTEL_MEI_ME
>         tristate "ME Enabled Intel Chipsets"
> -       select INTEL_MEI
> -       depends on X86 && PCI
>         help
>           MEI support for ME Enabled Intel chipsets.
> 
> @@ -38,8 +36,6 @@ config INTEL_MEI_ME
> 
>  config INTEL_MEI_TXE
>         tristate "Intel Trusted Execution Environment with ME Interface"
> -       select INTEL_MEI
> -       depends on X86 && PCI
>         help
>           MEI Support for Trusted Execution Environment device on Intel SoCs
> 
> @@ -48,9 +44,7 @@ config INTEL_MEI_TXE
> 
>  config INTEL_MEI_GSC
>         tristate "Intel MEI GSC embedded device"
> -       depends on INTEL_MEI
>         depends on INTEL_MEI_ME
> -       depends on X86 && PCI
>         depends on DRM_I915
>         help
>           Intel auxiliary driver for GSC devices embedded in Intel graphics devices.
> @@ -63,3 +57,4 @@ config INTEL_MEI_GSC
>  source "drivers/misc/mei/hdcp/Kconfig"
>  source "drivers/misc/mei/pxp/Kconfig"
> 
> +endmenu
  

On Wed, Apr 26, 2023, at 19:11, Winkler, Tomas wrote:
>> 
>> On Tue, Apr 25, 2023, at 16:27, Adam Borowski wrote:
>> > The problem in mainline is inconsistency: out of 6 config items, 4
>> > repeat the "depends on X86 && PCI" line, the other 2 do not.  There's
>> > indeed no immediate functional issue, but I'd argue that a dormant bug is
>> still a bug.
>
> The MEI protocol (CONFIG_INTEL_MEI) is not PCI or X86 dependent. 
> INTEL_MEI_GSC is PCI dependent but not X86. (Hope I correct also on 
> implementation side)
> They HW layers are currently X86 dependent. 

Ok, so in that case the dependencies should be relaxed like below
I guess, in order to allow using the MEI on non-x86 i915 devices.

      Arnd

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index d21486d69df2..7c6e3b4588d0 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -2,7 +2,6 @@
 # Copyright (c) 2003-2019, Intel Corporation. All rights reserved.
 config INTEL_MEI
        tristate "Intel Management Engine Interface"
-       depends on X86 && PCI
        help
          The Intel Management Engine (Intel ME) provides Manageability,
          Security and Media services for system containing Intel chipsets.
@@ -39,7 +38,7 @@ config INTEL_MEI_ME
 config INTEL_MEI_TXE
        tristate "Intel Trusted Execution Environment with ME Interface"
        select INTEL_MEI
-       depends on X86 && PCI
+       depends on X86
        help
          MEI Support for Trusted Execution Environment device on Intel SoCs
 
@@ -50,7 +49,7 @@ config INTEL_MEI_GSC
        tristate "Intel MEI GSC embedded device"
        depends on INTEL_MEI
        depends on INTEL_MEI_ME
-       depends on X86 && PCI
+       depends on PCI
        depends on DRM_I915
        help
          Intel auxiliary driver for GSC devices embedded in Intel graphics devices.
  

> On Wed, Apr 26, 2023, at 19:11, Winkler, Tomas wrote:
> >>
> >> On Tue, Apr 25, 2023, at 16:27, Adam Borowski wrote:
> >> > The problem in mainline is inconsistency: out of 6 config items, 4
> >> > repeat the "depends on X86 && PCI" line, the other 2 do not.
> >> > There's indeed no immediate functional issue, but I'd argue that a
> >> > dormant bug is
> >> still a bug.
> >
> > The MEI protocol (CONFIG_INTEL_MEI) is not PCI or X86 dependent.
> > INTEL_MEI_GSC is PCI dependent but not X86. (Hope I correct also on
> > implementation side) They HW layers are currently X86 dependent.
> 
> Ok, so in that case the dependencies should be relaxed like below I guess, in
> order to allow using the MEI on non-x86 i915 devices.
> 
>       Arnd
> 
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> d21486d69df2..7c6e3b4588d0 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -2,7 +2,6 @@
>  # Copyright (c) 2003-2019, Intel Corporation. All rights reserved.
>  config INTEL_MEI
>         tristate "Intel Management Engine Interface"
> -       depends on X86 && PCI
>         help
>           The Intel Management Engine (Intel ME) provides Manageability,
>           Security and Media services for system containing Intel chipsets.
> @@ -39,7 +38,7 @@ config INTEL_MEI_ME
>  config INTEL_MEI_TXE
>         tristate "Intel Trusted Execution Environment with ME Interface"
>         select INTEL_MEI
> -       depends on X86 && PCI
> +       depends on X86
This should be both X86 and PCI, this is  a HW dependent module 
>         help
>           MEI Support for Trusted Execution Environment device on Intel SoCs
> 
> @@ -50,7 +49,7 @@ config INTEL_MEI_GSC
>         tristate "Intel MEI GSC embedded device"
>         depends on INTEL_MEI
>         depends on INTEL_MEI_ME
> -       depends on X86 && PCI
> +       depends on PCI
>         depends on DRM_I915
>         help
>           Intel auxiliary driver for GSC devices embedded in Intel graphics devices.