Re: [PATCH v3 7/7] media: ipu-bridge: Remove unneeded conditional compilations

Mauro Carvalho Chehab
2024-12-11T08:20:00Z

Em Tue, 10 Dec 2024 19:56:04 +0000
Ricardo Ribalda <ribalda@chromium.org> escreveu:

> The ACPI headers have introduced implementations for some of their
> functions when the kernel is not configured with ACPI.
>
> Let's use them instead of our conditional compilation. It is easier to
> maintain and less prone to errors.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/pci/intel/ipu-bridge.c | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
> index be82bc3e27d0..1db994338fdf 100644
> --- a/drivers/media/pci/intel/ipu-bridge.c
> +++ b/drivers/media/pci/intel/ipu-bridge.c
> @@ -2,6 +2,7 @@
> /* Author: Dan Scally <djrscally@gmail.com> */
>
> #include <linux/acpi.h>
> +#include <acpi/acpi_bus.h>
> #include <linux/cleanup.h>
> #include <linux/device.h>
> #include <linux/i2c.h>
> @@ -107,7 +108,6 @@ static const char * const ipu_vcm_types[] = {
> "lc898212axb",
> };
>
> -#if IS_ENABLED(CONFIG_ACPI)
> /*
> * Used to figure out IVSC acpi device by ipu_bridge_get_ivsc_acpi_dev()
> * instead of device and driver match to probe IVSC device.
> @@ -127,11 +127,11 @@ static struct acpi_device *ipu_bridge_get_ivsc_acpi_dev(struct acpi_device *adev
> const struct acpi_device_id *acpi_id = &ivsc_acpi_ids[i];
> struct acpi_device *consumer, *ivsc_adev;
>
> - acpi_handle handle = acpi_device_handle(adev);
> + acpi_handle handle = acpi_device_handle(ACPI_PTR(adev));
> for_each_acpi_dev_match(ivsc_adev, acpi_id->id, NULL, -1)
> /* camera sensor depends on IVSC in DSDT if exist */
> for_each_acpi_consumer_dev(ivsc_adev, consumer)
> - if (consumer->handle == handle) {
> + if (ACPI_PTR(consumer->handle) == handle) {
> acpi_dev_put(consumer);
> return ivsc_adev;
> }
> @@ -139,12 +139,6 @@ static struct acpi_device *ipu_bridge_get_ivsc_acpi_dev(struct acpi_device *adev
>
> return NULL;
> }
> -#else
> -static struct acpi_device *ipu_bridge_get_ivsc_acpi_dev(struct acpi_device *adev)
> -{
> - return NULL;
> -}
> -#endif
>
> static int ipu_bridge_match_ivsc_dev(struct device *dev, const void *adev)
> {
> @@ -261,9 +255,8 @@ static enum v4l2_fwnode_orientation ipu_bridge_parse_orientation(struct acpi_dev
> struct acpi_pld_info *pld = NULL;
> acpi_status status = AE_ERROR;
>
> -#if IS_ENABLED(CONFIG_ACPI)
> - status = acpi_get_physical_device_location(adev->handle, &pld);
> -#endif
> + status = acpi_get_physical_device_location(ACPI_PTR(adev->handle),
> + &pld);
> if (ACPI_FAILURE(status)) {
> dev_warn(ADEV_DEV(adev), "_PLD call failed, using default orientation\n");
> return V4L2_FWNODE_ORIENTATION_EXTERNAL;
> @@ -498,9 +491,7 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge,
> if (sensor->csi_dev) {
> const char *device_hid = "";
>
> -#if IS_ENABLED(CONFIG_ACPI)
> device_hid = acpi_device_hid(sensor->ivsc_adev);
> -#endif
>
> snprintf(sensor->ivsc_name, sizeof(sensor->ivsc_name), "%s-%u",
> device_hid, sensor->link);
> @@ -671,11 +662,7 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg,
> struct acpi_device *adev = NULL;
> int ret;
>
> -#if IS_ENABLED(CONFIG_ACPI)
> for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
> -#else
> - while (true) {
> -#endif
> if (!ACPI_PTR(adev->status.enabled))
> continue;

Heh, that's what I pointed on patch 1: you don't need it there, as this
will be handled on patch 2.

>
> @@ -768,15 +755,10 @@ static int ipu_bridge_ivsc_is_ready(void)
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(ipu_supported_sensors); i++) {
> -#if IS_ENABLED(CONFIG_ACPI)
> const struct ipu_sensor_config *cfg =
> &ipu_supported_sensors[i];
>
> for_each_acpi_dev_match(sensor_adev, cfg->hid, NULL, -1) {
> -#else
> - while (false) {
> - sensor_adev = NULL;
> -#endif
> if (!ACPI_PTR(sensor_adev->status.enabled))
> continue;
>
>

Considering that you drop patch 1, and keep the ACPI dependencies
at the header, as proposed by patches 2-6:

Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kerenel.org>

Thanks,
Mauro