| Message ID | 20230630110643.209761-8-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: ipu-bridge: Shared with atomisp, rework VCM instantiation | expand |
Hi Hans On 30/06/2023 13:06, Hans de Goede wrote: > There is no need to keep a reference to the PLD struct around, > it is only used once the get the sensor orientation. > > Make ipu_bridge_parse_orientation() also get + put the PLD. > > This is a preparation patch for making the ipu-bridge code more generic > so that it can be shared with the atomisp driver. > > Reviewed-by: Andy Shevchenko <andy@kernel.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/pci/intel/ipu-bridge.c | 48 ++++++++++++++++------------ > drivers/media/pci/intel/ipu-bridge.h | 1 - > 2 files changed, 27 insertions(+), 22 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index 8e91d9b3e0fe..3a984d688b42 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c > @@ -112,23 +112,39 @@ static u32 ipu_bridge_parse_rotation(struct ipu_sensor *sensor) > } > } > > -static enum v4l2_fwnode_orientation ipu_bridge_parse_orientation(struct ipu_sensor *sensor) > +static enum v4l2_fwnode_orientation ipu_bridge_parse_orientation(struct acpi_device *adev) > { > - switch (sensor->pld->panel) { > + enum v4l2_fwnode_orientation orientation; > + struct acpi_pld_info *pld; > + acpi_status status; > + > + status = acpi_get_physical_device_location(adev->handle, &pld); > + if (ACPI_FAILURE(status)) { > + dev_warn(&adev->dev, "_PLD call failed using unknown orientation\n"); I think some punctuation after "_PLD call failed", and I'd probably say "default" rather than "unknown". Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > + return V4L2_FWNODE_ORIENTATION_EXTERNAL; > + } > + > + switch (pld->panel) { > case ACPI_PLD_PANEL_FRONT: > - return V4L2_FWNODE_ORIENTATION_FRONT; > + orientation = V4L2_FWNODE_ORIENTATION_FRONT; > + break; > case ACPI_PLD_PANEL_BACK: > - return V4L2_FWNODE_ORIENTATION_BACK; > + orientation = V4L2_FWNODE_ORIENTATION_BACK; > + break; > case ACPI_PLD_PANEL_TOP: > case ACPI_PLD_PANEL_LEFT: > case ACPI_PLD_PANEL_RIGHT: > case ACPI_PLD_PANEL_UNKNOWN: > - return V4L2_FWNODE_ORIENTATION_EXTERNAL; > + orientation = V4L2_FWNODE_ORIENTATION_EXTERNAL; > + break; > default: > - dev_warn(&sensor->adev->dev, "Unknown _PLD panel value %d\n", > - sensor->pld->panel); > - return V4L2_FWNODE_ORIENTATION_EXTERNAL; > + dev_warn(&adev->dev, "Unknown _PLD panel val %d\n", pld->panel); > + orientation = V4L2_FWNODE_ORIENTATION_EXTERNAL; > + break; > } > + > + ACPI_FREE(pld); > + return orientation; > } > > static void ipu_bridge_create_fwnode_properties( > @@ -140,7 +156,7 @@ static void ipu_bridge_create_fwnode_properties( > enum v4l2_fwnode_orientation orientation; > > rotation = ipu_bridge_parse_rotation(sensor); > - orientation = ipu_bridge_parse_orientation(sensor); > + orientation = ipu_bridge_parse_orientation(sensor->adev); > > sensor->prop_names = prop_names; > > @@ -279,7 +295,6 @@ static void ipu_bridge_unregister_sensors(struct ipu_bridge *bridge) > for (i = 0; i < bridge->n_sensors; i++) { > sensor = &bridge->sensors[i]; > software_node_unregister_node_group(sensor->group); > - ACPI_FREE(sensor->pld); > acpi_dev_put(sensor->adev); > i2c_unregister_device(sensor->vcm_i2c_client); > } > @@ -291,7 +306,6 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, > struct fwnode_handle *fwnode, *primary; > struct ipu_sensor *sensor; > struct acpi_device *adev; > - acpi_status status; > int ret; > > for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) { > @@ -326,17 +340,11 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, > sensor->ssdb.vcmtype = 0; > } > > - status = acpi_get_physical_device_location(adev->handle, &sensor->pld); > - if (ACPI_FAILURE(status)) { > - ret = -ENODEV; > - goto err_put_adev; > - } > - > if (sensor->ssdb.lanes > IPU_MAX_LANES) { > dev_err(&adev->dev, > "Number of lanes in SSDB is invalid\n"); > ret = -EINVAL; > - goto err_free_pld; > + goto err_put_adev; > } > > ipu_bridge_create_fwnode_properties(sensor, bridge, cfg); > @@ -344,7 +352,7 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, > > ret = software_node_register_node_group(sensor->group); > if (ret) > - goto err_free_pld; > + goto err_put_adev; > > fwnode = software_node_fwnode(&sensor->swnodes[ > SWNODE_SENSOR_HID]); > @@ -370,8 +378,6 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, > > err_free_swnodes: > software_node_unregister_node_group(sensor->group); > -err_free_pld: > - ACPI_FREE(sensor->pld); > err_put_adev: > acpi_dev_put(adev); > return ret; > diff --git a/drivers/media/pci/intel/ipu-bridge.h b/drivers/media/pci/intel/ipu-bridge.h > index 6cb68e3344dc..907ca833a7c1 100644 > --- a/drivers/media/pci/intel/ipu-bridge.h > +++ b/drivers/media/pci/intel/ipu-bridge.h > @@ -124,7 +124,6 @@ struct ipu_sensor { > struct ipu_node_names node_names; > > struct ipu_sensor_ssdb ssdb; > - struct acpi_pld_info *pld; > > struct ipu_property_names prop_names; > struct property_entry ep_properties[5];
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c index 8e91d9b3e0fe..3a984d688b42 100644 --- a/drivers/media/pci/intel/ipu-bridge.c +++ b/drivers/media/pci/intel/ipu-bridge.c @@ -112,23 +112,39 @@ static u32 ipu_bridge_parse_rotation(struct ipu_sensor *sensor) } } -static enum v4l2_fwnode_orientation ipu_bridge_parse_orientation(struct ipu_sensor *sensor) +static enum v4l2_fwnode_orientation ipu_bridge_parse_orientation(struct acpi_device *adev) { - switch (sensor->pld->panel) { + enum v4l2_fwnode_orientation orientation; + struct acpi_pld_info *pld; + acpi_status status; + + status = acpi_get_physical_device_location(adev->handle, &pld); + if (ACPI_FAILURE(status)) { + dev_warn(&adev->dev, "_PLD call failed using unknown orientation\n"); + return V4L2_FWNODE_ORIENTATION_EXTERNAL; + } + + switch (pld->panel) { case ACPI_PLD_PANEL_FRONT: - return V4L2_FWNODE_ORIENTATION_FRONT; + orientation = V4L2_FWNODE_ORIENTATION_FRONT; + break; case ACPI_PLD_PANEL_BACK: - return V4L2_FWNODE_ORIENTATION_BACK; + orientation = V4L2_FWNODE_ORIENTATION_BACK; + break; case ACPI_PLD_PANEL_TOP: case ACPI_PLD_PANEL_LEFT: case ACPI_PLD_PANEL_RIGHT: case ACPI_PLD_PANEL_UNKNOWN: - return V4L2_FWNODE_ORIENTATION_EXTERNAL; + orientation = V4L2_FWNODE_ORIENTATION_EXTERNAL; + break; default: - dev_warn(&sensor->adev->dev, "Unknown _PLD panel value %d\n", - sensor->pld->panel); - return V4L2_FWNODE_ORIENTATION_EXTERNAL; + dev_warn(&adev->dev, "Unknown _PLD panel val %d\n", pld->panel); + orientation = V4L2_FWNODE_ORIENTATION_EXTERNAL; + break; } + + ACPI_FREE(pld); + return orientation; } static void ipu_bridge_create_fwnode_properties( @@ -140,7 +156,7 @@ static void ipu_bridge_create_fwnode_properties( enum v4l2_fwnode_orientation orientation; rotation = ipu_bridge_parse_rotation(sensor); - orientation = ipu_bridge_parse_orientation(sensor); + orientation = ipu_bridge_parse_orientation(sensor->adev); sensor->prop_names = prop_names; @@ -279,7 +295,6 @@ static void ipu_bridge_unregister_sensors(struct ipu_bridge *bridge) for (i = 0; i < bridge->n_sensors; i++) { sensor = &bridge->sensors[i]; software_node_unregister_node_group(sensor->group); - ACPI_FREE(sensor->pld); acpi_dev_put(sensor->adev); i2c_unregister_device(sensor->vcm_i2c_client); } @@ -291,7 +306,6 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, struct fwnode_handle *fwnode, *primary; struct ipu_sensor *sensor; struct acpi_device *adev; - acpi_status status; int ret; for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) { @@ -326,17 +340,11 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, sensor->ssdb.vcmtype = 0; } - status = acpi_get_physical_device_location(adev->handle, &sensor->pld); - if (ACPI_FAILURE(status)) { - ret = -ENODEV; - goto err_put_adev; - } - if (sensor->ssdb.lanes > IPU_MAX_LANES) { dev_err(&adev->dev, "Number of lanes in SSDB is invalid\n"); ret = -EINVAL; - goto err_free_pld; + goto err_put_adev; } ipu_bridge_create_fwnode_properties(sensor, bridge, cfg); @@ -344,7 +352,7 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, ret = software_node_register_node_group(sensor->group); if (ret) - goto err_free_pld; + goto err_put_adev; fwnode = software_node_fwnode(&sensor->swnodes[ SWNODE_SENSOR_HID]); @@ -370,8 +378,6 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg, err_free_swnodes: software_node_unregister_node_group(sensor->group); -err_free_pld: - ACPI_FREE(sensor->pld); err_put_adev: acpi_dev_put(adev); return ret; diff --git a/drivers/media/pci/intel/ipu-bridge.h b/drivers/media/pci/intel/ipu-bridge.h index 6cb68e3344dc..907ca833a7c1 100644 --- a/drivers/media/pci/intel/ipu-bridge.h +++ b/drivers/media/pci/intel/ipu-bridge.h @@ -124,7 +124,6 @@ struct ipu_sensor { struct ipu_node_names node_names; struct ipu_sensor_ssdb ssdb; - struct acpi_pld_info *pld; struct ipu_property_names prop_names; struct property_entry ep_properties[5];