[only Mauro]
On Wed, 11 Dec 2024 at 09:20, Mauro Carvalho Chehab
<mchehab+huawei@xxxxxxxxxx> wrote:
>
> Em Tue, 10 Dec 2024 19:56:04 +0000
> Ricardo Ribalda <ribalda@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxx> */
> >
> > #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@xxxxxxxxxxx>
I will fix the typo in your email
>
> Thanks,
> Mauro
--
Ricardo Ribalda
