[v2,07/15] media: ipu-bridge: Only keep PLD around while parsing
Commit Message
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(-)
Comments
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];
@@ -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;
@@ -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];