diff mbox series

[v2,07/15] media: ipu-bridge: Only keep PLD around while parsing

Message ID 20230630110643.209761-8-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series media: ipu-bridge: Shared with atomisp, rework VCM instantiation | expand

Commit Message

Hans de Goede June 30, 2023, 11:06 a.m. UTC 
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

Dan Scally July 4, 2023, 11:21 a.m. UTC | #1 
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 mbox series

Patch

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];