Em Wed, 11 Dec 2024 09:25:33 +0100
Ricardo Ribalda <ribalda@xxxxxxxxxxxx> escreveu:
> [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
Heh, slow start... I didn't take any caffeine today yet :-)
Thanks for noticing it!
Thanks,
Mauro
