diff mbox series

[v1,1/1] media: ipu3-cio2: Fix referece counting when looping over ACPI devices

Message ID 20210404181409.1451026-1-andy.shevchenko@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/1] media: ipu3-cio2: Fix referece counting when looping over ACPI devices | expand

Commit Message

Andy Shevchenko April 4, 2021, 6:14 p.m. UTC
When we continue, due to device is disabled, loop we have to drop reference count.
When we have an array full of devices we have to also drop the reference count.
Note, in this case the cio2_bridge_unregister_sensors() is called by the caller.

Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
Cc: Daniel Scally <djrscally@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/media/pci/intel/ipu3/cio2-bridge.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Daniel Scally April 5, 2021, 8:37 p.m. UTC | #1
Hi Andy

On 04/04/2021 19:14, Andy Shevchenko wrote:
> When we continue, due to device is disabled, loop we have to drop reference count.
> When we have an array full of devices we have to also drop the reference count.
> Note, in this case the cio2_bridge_unregister_sensors() is called by the caller.
>
> Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
> Cc: Daniel Scally <djrscally@gmail.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>


Ah; thanks for catching those, I'm annoyed to have missed the
acpi_dev_put() calls in particular

Reviewed-by: Daniel Scally <djrscally@gmail.com>
Andy Shevchenko April 13, 2021, 4:10 p.m. UTC | #2
On Mon, Apr 05, 2021 at 09:37:28PM +0100, Daniel Scally wrote:
> On 04/04/2021 19:14, Andy Shevchenko wrote:
> > When we continue, due to device is disabled, loop we have to drop reference count.
> > When we have an array full of devices we have to also drop the reference count.
> > Note, in this case the cio2_bridge_unregister_sensors() is called by the caller.
> >
> > Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
> > Cc: Daniel Scally <djrscally@gmail.com>
> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Ah; thanks for catching those, I'm annoyed to have missed the
> acpi_dev_put() calls in particular
> 
> Reviewed-by: Daniel Scally <djrscally@gmail.com>

Sakari, Mauro, can this be applied as v5.12 material, please?
Sakari Ailus April 21, 2021, 2:03 p.m. UTC | #3
Hi Andy,

Thanks for the patch.

On Sun, Apr 04, 2021 at 09:14:09PM +0300, Andy Shevchenko wrote:
> When we continue, due to device is disabled, loop we have to drop reference count.
> When we have an array full of devices we have to also drop the reference count.
> Note, in this case the cio2_bridge_unregister_sensors() is called by the caller.

Please limit the commit message to 75 chars, at least if there's no reason
to do otherwise.

I can wrap it this time.

> 
> Fixes: 803abec64ef9 ("media: ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver")
> Cc: Daniel Scally <djrscally@gmail.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/media/pci/intel/ipu3/cio2-bridge.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> index c2199042d3db..85f8b587405e 100644
> --- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> @@ -173,14 +173,15 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
>  	int ret;
>  
>  	for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
> -		if (!adev->status.enabled)
> +		if (!adev->status.enabled) {
> +			acpi_dev_put(adev);
>  			continue;
> +		}
>  
>  		if (bridge->n_sensors >= CIO2_NUM_PORTS) {
> +			acpi_dev_put(adev);
>  			dev_err(&cio2->dev, "Exceeded available CIO2 ports\n");
> -			cio2_bridge_unregister_sensors(bridge);
> -			ret = -EINVAL;
> -			goto err_out;
> +			return -EINVAL;
>  		}
>  
>  		sensor = &bridge->sensors[bridge->n_sensors];
> @@ -228,7 +229,6 @@ static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
>  	software_node_unregister_nodes(sensor->swnodes);
>  err_put_adev:
>  	acpi_dev_put(sensor->adev);
> -err_out:
>  	return ret;
>  }
>
Andy Shevchenko April 21, 2021, 3:05 p.m. UTC | #4
On Wed, Apr 21, 2021 at 5:04 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Sun, Apr 04, 2021 at 09:14:09PM +0300, Andy Shevchenko wrote:
> > When we continue, due to device is disabled, loop we have to drop reference count.
> > When we have an array full of devices we have to also drop the reference count.
> > Note, in this case the cio2_bridge_unregister_sensors() is called by the caller.
>
> Please limit the commit message to 75 chars, at least if there's no reason
> to do otherwise.

OK.

> I can wrap it this time.

Thanks!
diff mbox series

Patch

diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
index c2199042d3db..85f8b587405e 100644
--- a/drivers/media/pci/intel/ipu3/cio2-bridge.c
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
@@ -173,14 +173,15 @@  static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
 	int ret;
 
 	for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
-		if (!adev->status.enabled)
+		if (!adev->status.enabled) {
+			acpi_dev_put(adev);
 			continue;
+		}
 
 		if (bridge->n_sensors >= CIO2_NUM_PORTS) {
+			acpi_dev_put(adev);
 			dev_err(&cio2->dev, "Exceeded available CIO2 ports\n");
-			cio2_bridge_unregister_sensors(bridge);
-			ret = -EINVAL;
-			goto err_out;
+			return -EINVAL;
 		}
 
 		sensor = &bridge->sensors[bridge->n_sensors];
@@ -228,7 +229,6 @@  static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
 	software_node_unregister_nodes(sensor->swnodes);
 err_put_adev:
 	acpi_dev_put(sensor->adev);
-err_out:
 	return ret;
 }