2023-07-27 12:30:41

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] media: ipu3-cio2: allow ipu_bridge to be a module again

From: Arnd Bergmann <[email protected]>

This code was previously part of the VIDEO_IPU3_CIO2 driver, which could
be built-in or a loadable module, but after the move it turned into a
builtin-only driver. This fails to link when the I2C subsystem is a
module:

x86_64-linux-ld: drivers/media/pci/intel/ipu-bridge.o: in function `ipu_bridge_unregister_sensors':
ipu-bridge.c:(.text+0x50): undefined reference to `i2c_unregister_device'
x86_64-linux-ld: drivers/media/pci/intel/ipu-bridge.o: in function `ipu_bridge_init':
ipu-bridge.c:(.text+0x9c9): undefined reference to `i2c_acpi_new_device_by_fwnode'

In general, drivers should not have to be built-in, so change the option
to a tristate with the corresponding dependency. This in turn opens a
new problem with the dependency, as the IPU bridge can be a loadable module
while the ipu3 driver itself is built-in, producing a new link failure:

86_64-linux-ld: drivers/media/pci/intel/ipu3/ipu3-cio2.o: in function `cio2_pci_probe':
ipu3-cio2.c:(.text+0x197e): undefined reference to `ipu_bridge_init'

In order to fix this, restore the old Kconfig option that controlled
the ipu bridge driver before it was split out, but make it select a
hidden symbol that now corresponds to the bridge driver.

When other drivers get added that share ipu-bridge, this should cover
all corner cases, and allow any combination of them to be built-in
or modular.

Fixes: 881ca25978c6f ("media: ipu3-cio2: rename cio2 bridge to ipu bridge and move out of ipu3")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/media/pci/intel/Kconfig | 21 +++++----------------
drivers/media/pci/intel/ipu-bridge.c | 3 +++
drivers/media/pci/intel/ipu3/Kconfig | 20 ++++++++++++++++++++
3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/media/pci/intel/Kconfig b/drivers/media/pci/intel/Kconfig
index 64a29b0b7033b..51b18fce6a1de 100644
--- a/drivers/media/pci/intel/Kconfig
+++ b/drivers/media/pci/intel/Kconfig
@@ -1,21 +1,10 @@
# SPDX-License-Identifier: GPL-2.0-only
config IPU_BRIDGE
- bool "Intel IPU Sensors Bridge"
- depends on VIDEO_IPU3_CIO2 && ACPI
- depends on I2C
+ tristate
+ depends on I2C && ACPI
help
- This extension provides an API for the Intel IPU driver to create
- connections to cameras that are hidden in the SSDB buffer in ACPI.
- It can be used to enable support for cameras in detachable / hybrid
- devices that ship with Windows.
-
- Say Y here if your device is a detachable / hybrid laptop that comes
- with Windows installed by the OEM, for example:
-
- - Microsoft Surface models (except Surface Pro 3)
- - The Lenovo Miix line (for example the 510, 520, 710 and 720)
- - Dell 7285
-
- If in doubt, say N here.
+ This is a helper module for the IPU bridge, which can be
+ used by ipu3 and other drivers. In order to handle module
+ dependencies, this is selected by each driver that needs it.

source "drivers/media/pci/intel/ipu3/Kconfig"
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 62daa8c1f6b18..bd67c3e990ea8 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -493,3 +493,6 @@ int ipu_bridge_init(struct pci_dev *ipu)
return ret;
}
EXPORT_SYMBOL_NS_GPL(ipu_bridge_init, INTEL_IPU_BRIDGE);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Intel IPU Sensors Bridge driver");
diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig
index 9be06ee81ff05..0951545eab21a 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -8,6 +8,7 @@ config VIDEO_IPU3_CIO2
select VIDEO_V4L2_SUBDEV_API
select V4L2_FWNODE
select VIDEOBUF2_DMA_SG
+ select IPU_BRIDGE if CIO2_BRIDGE

help
This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
@@ -17,3 +18,22 @@ config VIDEO_IPU3_CIO2
Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
connected camera.
The module will be called ipu3-cio2.
+
+config CIO2_BRIDGE
+ bool "IPU3 CIO2 Sensors Bridge"
+ depends on VIDEO_IPU3_CIO2 && ACPI
+ depends on I2C
+ help
+ This extension provides an API for the ipu3-cio2 driver to create
+ connections to cameras that are hidden in the SSDB buffer in ACPI.
+ It can be used to enable support for cameras in detachable / hybrid
+ devices that ship with Windows.
+
+ Say Y here if your device is a detachable / hybrid laptop that comes
+ with Windows installed by the OEM, for example:
+
+ - Microsoft Surface models (except Surface Pro 3)
+ - The Lenovo Miix line (for example the 510, 520, 710 and 720)
+ - Dell 7285
+
+ If in doubt, say N here.
--
2.39.2



2023-07-27 13:57:56

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] media: ipu3-cio2: allow ipu_bridge to be a module again

On Thu, Jul 27, 2023 at 02:22:58PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> This code was previously part of the VIDEO_IPU3_CIO2 driver, which could
> be built-in or a loadable module, but after the move it turned into a
> builtin-only driver. This fails to link when the I2C subsystem is a
> module:

Which probably never happen in (real, like distro kernel) x86 builds due to
other x86 platforms that require I2C to be built-in. Nevertheless, I agree
with your patch.

Reviewed-by: Andy Shevchenko <[email protected]>

> x86_64-linux-ld: drivers/media/pci/intel/ipu-bridge.o: in function `ipu_bridge_unregister_sensors':
> ipu-bridge.c:(.text+0x50): undefined reference to `i2c_unregister_device'
> x86_64-linux-ld: drivers/media/pci/intel/ipu-bridge.o: in function `ipu_bridge_init':
> ipu-bridge.c:(.text+0x9c9): undefined reference to `i2c_acpi_new_device_by_fwnode'
>
> In general, drivers should not have to be built-in, so change the option
> to a tristate with the corresponding dependency. This in turn opens a
> new problem with the dependency, as the IPU bridge can be a loadable module
> while the ipu3 driver itself is built-in, producing a new link failure:
>
> 86_64-linux-ld: drivers/media/pci/intel/ipu3/ipu3-cio2.o: in function `cio2_pci_probe':
> ipu3-cio2.c:(.text+0x197e): undefined reference to `ipu_bridge_init'
>
> In order to fix this, restore the old Kconfig option that controlled
> the ipu bridge driver before it was split out, but make it select a
> hidden symbol that now corresponds to the bridge driver.
>
> When other drivers get added that share ipu-bridge, this should cover
> all corner cases, and allow any combination of them to be built-in
> or modular.
>
> Fixes: 881ca25978c6f ("media: ipu3-cio2: rename cio2 bridge to ipu bridge and move out of ipu3")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/media/pci/intel/Kconfig | 21 +++++----------------
> drivers/media/pci/intel/ipu-bridge.c | 3 +++
> drivers/media/pci/intel/ipu3/Kconfig | 20 ++++++++++++++++++++
> 3 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/pci/intel/Kconfig b/drivers/media/pci/intel/Kconfig
> index 64a29b0b7033b..51b18fce6a1de 100644
> --- a/drivers/media/pci/intel/Kconfig
> +++ b/drivers/media/pci/intel/Kconfig
> @@ -1,21 +1,10 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config IPU_BRIDGE
> - bool "Intel IPU Sensors Bridge"
> - depends on VIDEO_IPU3_CIO2 && ACPI
> - depends on I2C
> + tristate
> + depends on I2C && ACPI
> help
> - This extension provides an API for the Intel IPU driver to create
> - connections to cameras that are hidden in the SSDB buffer in ACPI.
> - It can be used to enable support for cameras in detachable / hybrid
> - devices that ship with Windows.
> -
> - Say Y here if your device is a detachable / hybrid laptop that comes
> - with Windows installed by the OEM, for example:
> -
> - - Microsoft Surface models (except Surface Pro 3)
> - - The Lenovo Miix line (for example the 510, 520, 710 and 720)
> - - Dell 7285
> -
> - If in doubt, say N here.
> + This is a helper module for the IPU bridge, which can be
> + used by ipu3 and other drivers. In order to handle module
> + dependencies, this is selected by each driver that needs it.
>
> source "drivers/media/pci/intel/ipu3/Kconfig"
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index 62daa8c1f6b18..bd67c3e990ea8 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -493,3 +493,6 @@ int ipu_bridge_init(struct pci_dev *ipu)
> return ret;
> }
> EXPORT_SYMBOL_NS_GPL(ipu_bridge_init, INTEL_IPU_BRIDGE);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Intel IPU Sensors Bridge driver");
> diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig
> index 9be06ee81ff05..0951545eab21a 100644
> --- a/drivers/media/pci/intel/ipu3/Kconfig
> +++ b/drivers/media/pci/intel/ipu3/Kconfig
> @@ -8,6 +8,7 @@ config VIDEO_IPU3_CIO2
> select VIDEO_V4L2_SUBDEV_API
> select V4L2_FWNODE
> select VIDEOBUF2_DMA_SG
> + select IPU_BRIDGE if CIO2_BRIDGE
>
> help
> This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
> @@ -17,3 +18,22 @@ config VIDEO_IPU3_CIO2
> Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
> connected camera.
> The module will be called ipu3-cio2.
> +
> +config CIO2_BRIDGE
> + bool "IPU3 CIO2 Sensors Bridge"
> + depends on VIDEO_IPU3_CIO2 && ACPI
> + depends on I2C
> + help
> + This extension provides an API for the ipu3-cio2 driver to create
> + connections to cameras that are hidden in the SSDB buffer in ACPI.
> + It can be used to enable support for cameras in detachable / hybrid
> + devices that ship with Windows.
> +
> + Say Y here if your device is a detachable / hybrid laptop that comes
> + with Windows installed by the OEM, for example:
> +
> + - Microsoft Surface models (except Surface Pro 3)
> + - The Lenovo Miix line (for example the 510, 520, 710 and 720)
> + - Dell 7285
> +
> + If in doubt, say N here.
> --
> 2.39.2
>

--
With Best Regards,
Andy Shevchenko