[PATCH 1/2] [v2] mei: fix vsc dependency

[PATCH 1/2] [v2] mei: fix vsc dependency

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m,
but then the driver is not built because Kbuild never enters the
drivers/misc/mei directory for built-in files, leading to a link
failure:

ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko] undefined!
ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko] undefined!

Add an explicit dependency on CONFIG_MEI that was apparently missing,
to ensure the VSC_HW driver cannot be built-in with MEI itself being
a loadable module.

Fixes: 566f5ca97680 ("mei: Add transport driver for IVSC device")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/misc/mei/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 858bd701d68c..1e28ca23a74a 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -62,6 +62,7 @@ config INTEL_MEI_GSC
 
 config INTEL_MEI_VSC_HW
 	tristate "Intel visual sensing controller device transport driver"
+	depends on INTEL_MEI
 	depends on ACPI && SPI
 	depends on GPIOLIB || COMPILE_TEST
 	help
-- 
2.39.2

[PATCH 2/2] mei: rework Kconfig dependencies

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The dependencies in the mei framework are inconsistent, with some symbols
using 'select INTEL_MEI' to force it being enabled and others using
'depends on INTEL_MEI'.

In general, one should not select user-visible symbols, so change all
of these to normal dependencies, but change the default on INTEL_MEI to
be enabled when building a kernel for an Intel CPU with ME or a generic
x86 kernel.

Having consistent dependencies makes the 'menuconfig' listing more
readable by using proper indentation.

A large if/endif block is just a simpler syntax than repeating the
dependencies for each symbol.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This does not fix a bug, but seems like a sensible cleanup to me,
making the logic less error-prone for future changes. Feel free
to just take the first patch and ignore this one if I missed
an important reason for the original variant, or if you prefer
a different method.
---
 drivers/misc/mei/Kconfig           | 14 ++++++--------
 drivers/misc/mei/gsc_proxy/Kconfig |  2 +-
 drivers/misc/mei/hdcp/Kconfig      |  2 +-
 drivers/misc/mei/pxp/Kconfig       |  2 +-
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 1e28ca23a74a..67d9391f1855 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -3,6 +3,7 @@
 config INTEL_MEI
 	tristate "Intel Management Engine Interface"
 	depends on X86 && PCI
+	default GENERIC_CPU || MCORE2 || MATOM || X86_GENERIC
 	help
 	  The Intel Management Engine (Intel ME) provides Manageability,
 	  Security and Media services for system containing Intel chipsets.
@@ -11,10 +12,11 @@ config INTEL_MEI
 	  For more information see
 	  <https://software.intel.com/en-us/manageability/>
 
+if INTEL_MEI
+
 config INTEL_MEI_ME
 	tristate "ME Enabled Intel Chipsets"
-	select INTEL_MEI
-	depends on X86 && PCI
+	default y
 	help
 	  MEI support for ME Enabled Intel chipsets.
 
@@ -38,8 +40,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 +48,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.
@@ -62,7 +60,6 @@ config INTEL_MEI_GSC
 
 config INTEL_MEI_VSC_HW
 	tristate "Intel visual sensing controller device transport driver"
-	depends on INTEL_MEI
 	depends on ACPI && SPI
 	depends on GPIOLIB || COMPILE_TEST
 	help
@@ -75,7 +72,6 @@ config INTEL_MEI_VSC_HW
 config INTEL_MEI_VSC
 	tristate "Intel visual sensing controller device with ME interface"
 	depends on INTEL_MEI_VSC_HW
-	depends on INTEL_MEI
 	help
 	  Intel MEI over SPI driver for Intel visual sensing controller
 	  (IVSC) device embedded in IA platform. It supports camera sharing
@@ -88,3 +84,5 @@ config INTEL_MEI_VSC
 source "drivers/misc/mei/hdcp/Kconfig"
 source "drivers/misc/mei/pxp/Kconfig"
 source "drivers/misc/mei/gsc_proxy/Kconfig"
+
+endif
diff --git a/drivers/misc/mei/gsc_proxy/Kconfig b/drivers/misc/mei/gsc_proxy/Kconfig
index 5f68d9f3d691..ac78b9d1eccd 100644
--- a/drivers/misc/mei/gsc_proxy/Kconfig
+++ b/drivers/misc/mei/gsc_proxy/Kconfig
@@ -3,7 +3,7 @@
 #
 config INTEL_MEI_GSC_PROXY
 	tristate "Intel GSC Proxy services of ME Interface"
-	select INTEL_MEI_ME
+	depends on INTEL_MEI_ME
 	depends on DRM_I915
 	help
          MEI Support for GSC Proxy Services on Intel platforms.
diff --git a/drivers/misc/mei/hdcp/Kconfig b/drivers/misc/mei/hdcp/Kconfig
index 54e1c9526909..9be312ec798d 100644
--- a/drivers/misc/mei/hdcp/Kconfig
+++ b/drivers/misc/mei/hdcp/Kconfig
@@ -3,7 +3,7 @@
 #
 config INTEL_MEI_HDCP
 	tristate "Intel HDCP2.2 services of ME Interface"
-	select INTEL_MEI_ME
+	depends on INTEL_MEI_ME
 	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..838eae556dd4 100644
--- a/drivers/misc/mei/pxp/Kconfig
+++ b/drivers/misc/mei/pxp/Kconfig
@@ -4,7 +4,7 @@
 #
 config INTEL_MEI_PXP
 	tristate "Intel PXP services of ME Interface"
-	select INTEL_MEI_ME
+	depends on INTEL_MEI_ME
 	depends on DRM_I915
 	help
 	  MEI Support for PXP Services on Intel platforms.
-- 
2.39.2

RE: [PATCH 2/2] mei: rework Kconfig dependencies

[Wu, Wentong]

> From: Arnd Bergmann <arnd@kernel.org> Sent: Friday, December 15, 2023 2:40 AM
> 
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The dependencies in the mei framework are inconsistent, with some symbols
> using 'select INTEL_MEI' to force it being enabled and others using 'depends on
> INTEL_MEI'.
> 
> In general, one should not select user-visible symbols, so change all of these to
> normal dependencies, but change the default on INTEL_MEI to be enabled when
> building a kernel for an Intel CPU with ME or a generic
> x86 kernel.
> 
> Having consistent dependencies makes the 'menuconfig' listing more readable
> by using proper indentation.
> 
> A large if/endif block is just a simpler syntax than repeating the dependencies for
> each symbol.

Yes, I agree it will make future changes more easier.

Reviewed-by: Wentong Wu <wentong.wu@intel.com>

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This does not fix a bug, but seems like a sensible cleanup to me, making the logic
> less error-prone for future changes. Feel free to just take the first patch and
> ignore this one if I missed an important reason for the original variant, or if you
> prefer a different method.
> ---
>  drivers/misc/mei/Kconfig           | 14 ++++++--------
>  drivers/misc/mei/gsc_proxy/Kconfig |  2 +-
>  drivers/misc/mei/hdcp/Kconfig      |  2 +-
>  drivers/misc/mei/pxp/Kconfig       |  2 +-
>  4 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> 1e28ca23a74a..67d9391f1855 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -3,6 +3,7 @@
>  config INTEL_MEI
>  	tristate "Intel Management Engine Interface"
>  	depends on X86 && PCI
> +	default GENERIC_CPU || MCORE2 || MATOM || X86_GENERIC
>  	help
>  	  The Intel Management Engine (Intel ME) provides Manageability,
>  	  Security and Media services for system containing Intel chipsets.
> @@ -11,10 +12,11 @@ config INTEL_MEI
>  	  For more information see
>  	  <https://software.intel.com/en-us/manageability/>
> 
> +if INTEL_MEI
> +
>  config INTEL_MEI_ME
>  	tristate "ME Enabled Intel Chipsets"
> -	select INTEL_MEI
> -	depends on X86 && PCI
> +	default y
>  	help
>  	  MEI support for ME Enabled Intel chipsets.
> 
> @@ -38,8 +40,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 +48,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.
> @@ -62,7 +60,6 @@ config INTEL_MEI_GSC
> 
>  config INTEL_MEI_VSC_HW
>  	tristate "Intel visual sensing controller device transport driver"
> -	depends on INTEL_MEI
>  	depends on ACPI && SPI
>  	depends on GPIOLIB || COMPILE_TEST
>  	help
> @@ -75,7 +72,6 @@ config INTEL_MEI_VSC_HW  config INTEL_MEI_VSC
>  	tristate "Intel visual sensing controller device with ME interface"
>  	depends on INTEL_MEI_VSC_HW
> -	depends on INTEL_MEI
>  	help
>  	  Intel MEI over SPI driver for Intel visual sensing controller
>  	  (IVSC) device embedded in IA platform. It supports camera sharing @@
> -88,3 +84,5 @@ config INTEL_MEI_VSC  source
> "drivers/misc/mei/hdcp/Kconfig"
>  source "drivers/misc/mei/pxp/Kconfig"
>  source "drivers/misc/mei/gsc_proxy/Kconfig"
> +
> +endif
> diff --git a/drivers/misc/mei/gsc_proxy/Kconfig
> b/drivers/misc/mei/gsc_proxy/Kconfig
> index 5f68d9f3d691..ac78b9d1eccd 100644
> --- a/drivers/misc/mei/gsc_proxy/Kconfig
> +++ b/drivers/misc/mei/gsc_proxy/Kconfig
> @@ -3,7 +3,7 @@
>  #
>  config INTEL_MEI_GSC_PROXY
>  	tristate "Intel GSC Proxy services of ME Interface"
> -	select INTEL_MEI_ME
> +	depends on INTEL_MEI_ME
>  	depends on DRM_I915
>  	help
>           MEI Support for GSC Proxy Services on Intel platforms.
> diff --git a/drivers/misc/mei/hdcp/Kconfig b/drivers/misc/mei/hdcp/Kconfig
> index 54e1c9526909..9be312ec798d 100644
> --- a/drivers/misc/mei/hdcp/Kconfig
> +++ b/drivers/misc/mei/hdcp/Kconfig
> @@ -3,7 +3,7 @@
>  #
>  config INTEL_MEI_HDCP
>  	tristate "Intel HDCP2.2 services of ME Interface"
> -	select INTEL_MEI_ME
> +	depends on INTEL_MEI_ME
>  	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..838eae556dd4 100644
> --- a/drivers/misc/mei/pxp/Kconfig
> +++ b/drivers/misc/mei/pxp/Kconfig
> @@ -4,7 +4,7 @@
>  #
>  config INTEL_MEI_PXP
>  	tristate "Intel PXP services of ME Interface"
> -	select INTEL_MEI_ME
> +	depends on INTEL_MEI_ME
>  	depends on DRM_I915
>  	help
>  	  MEI Support for PXP Services on Intel platforms.
> --
> 2.39.2

Thanks
Wentong

RE: [PATCH 1/2] [v2] mei: fix vsc dependency

[Wu, Wentong]

> From: Arnd Bergmann <arnd@arndb.de>
> 
> CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m,
> but then the driver is not built because Kbuild never enters the drivers/misc/mei
> directory for built-in files, leading to a link
> failure:
> 
> ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko]
> undefined!
> 
> Add an explicit dependency on CONFIG_MEI that was apparently missing, to
> ensure the VSC_HW driver cannot be built-in with MEI itself being a loadable
> module.
> 
> Fixes: 566f5ca97680 ("mei: Add transport driver for IVSC device")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks

Reviewed-by: Wentong Wu <wentong.wu@intel.com>

> ---
>  drivers/misc/mei/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig index
> 858bd701d68c..1e28ca23a74a 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -62,6 +62,7 @@ config INTEL_MEI_GSC
> 
>  config INTEL_MEI_VSC_HW
>  	tristate "Intel visual sensing controller device transport driver"
> +	depends on INTEL_MEI
>  	depends on ACPI && SPI
>  	depends on GPIOLIB || COMPILE_TEST
>  	help
> --
> 2.39.2

Re: [PATCH 1/2] [v2] mei: fix vsc dependency

[Sakari Ailus]

Hi Arnd,

On Thu, Dec 14, 2023 at 06:39:31PM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> CONFIG_INTEL_MEI_VSC_HW can be set to built-in even with CONFIG_MEI=m,
> but then the driver is not built because Kbuild never enters the
> drivers/misc/mei directory for built-in files, leading to a link
> failure:
> 
> ERROR: modpost: "vsc_tp_reset" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_init" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_xfer" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_need_read" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_enable" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_synchronize" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_intr_disable" [drivers/misc/mei/mei-vsc.ko] undefined!
> ERROR: modpost: "vsc_tp_register_event_cb" [drivers/misc/mei/mei-vsc.ko] undefined!
> 
> Add an explicit dependency on CONFIG_MEI that was apparently missing,
> to ensure the VSC_HW driver cannot be built-in with MEI itself being
> a loadable module.

Well, I don't see why someone would build mei as a module can mei-vsc-hw as
builtin but the actual dependencies don't wouldn't prevent it. How about
instead changing the Makefile in the parent directory so mei directory is
always traversed?

Either way, feel free to add:

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Regards,

Sakari Ailus