[PATCH 1/2] iio: adc: adi-axi-adc: Fix object reference counting

[PATCH 1/2] iio: adc: adi-axi-adc: Fix object reference counting

[Nuno Sá]

When looking for a registered client to attach with, the wrong reference
counters are being grabbed. The idea is to increment the module and device
counters of the client device and not the counters of the axi device being
probed.

Fixes: ef04070692a2 (iio: adc: adi-axi-adc: add support for AXI ADC IP core)

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 63d926e86798..86b6b65916ee 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -332,12 +332,12 @@ static struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
 		if (cl->dev->of_node != cln)
 			continue;
 
-		if (!try_module_get(dev->driver->owner)) {
+		if (!try_module_get(cl->dev->driver->owner)) {
 			mutex_unlock(&registered_clients_lock);
 			return ERR_PTR(-ENODEV);
 		}
 
-		get_device(dev);
+		get_device(cl->dev);
 		cl->info = info;
 		mutex_unlock(&registered_clients_lock);
 		return cl;
-- 
2.25.1

[PATCH 2/2] iio: adc: adi-axi-adc: Remove unneeded check

[Nuno Sá]

Looking at the register path, there will be bigger issues if `dev` is
NULL. Hence, we can drop this check and assume there's no way that `dev`
can be NULL at this point.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/adc/adi-axi-adc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
index 86b6b65916ee..e216e37e8c8c 100644
--- a/drivers/iio/adc/adi-axi-adc.c
+++ b/drivers/iio/adc/adi-axi-adc.c
@@ -326,9 +326,6 @@ static struct adi_axi_adc_client *adi_axi_adc_attach_client(struct device *dev)
 	mutex_lock(&registered_clients_lock);
 
 	list_for_each_entry(cl, &registered_clients, entry) {
-		if (!cl->dev)
-			continue;
-
 		if (cl->dev->of_node != cln)
 			continue;
 
-- 
2.25.1

Re: [PATCH 2/2] iio: adc: adi-axi-adc: Remove unneeded check

[Ardelean, Alexandru]

On Wed, 2020-07-01 at 14:04 +0200, Nuno Sá wrote:
> [External]
> 
> Looking at the register path, there will be bigger issues if `dev` is
> NULL. Hence, we can drop this check and assume there's no way that `dev`
> can be NULL at this point.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/adi-axi-adc.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-
> adc.c
> index 86b6b65916ee..e216e37e8c8c 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -326,9 +326,6 @@ static struct adi_axi_adc_client
> *adi_axi_adc_attach_client(struct device *dev)
>  	mutex_lock(&registered_clients_lock);
>  
>  	list_for_each_entry(cl, &registered_clients, entry) {
> -		if (!cl->dev)
> -			continue;
> -

I may need to check this again.
I think this may be a left-over, of when there was un-managed
adi_axi_adc_conv_register()


>  		if (cl->dev->of_node != cln)
>  			continue;
>  

Re: [PATCH 1/2] iio: adc: adi-axi-adc: Fix object reference counting

[Ardelean, Alexandru]

On Wed, 2020-07-01 at 14:04 +0200, Nuno Sá wrote:
> [External]
> 
> When looking for a registered client to attach with, the wrong reference
> counters are being grabbed. The idea is to increment the module and
> device
> counters of the client device and not the counters of the axi device
> being
> probed.
> 

Yep.
Good catch here.

Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

> Fixes: ef04070692a2 (iio: adc: adi-axi-adc: add support for AXI ADC IP
> core)
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/adi-axi-adc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-
> adc.c
> index 63d926e86798..86b6b65916ee 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -332,12 +332,12 @@ static struct adi_axi_adc_client
> *adi_axi_adc_attach_client(struct device *dev)
>  		if (cl->dev->of_node != cln)
>  			continue;
>  
> -		if (!try_module_get(dev->driver->owner)) {
> +		if (!try_module_get(cl->dev->driver->owner)) {
>  			mutex_unlock(&registered_clients_lock);
>  			return ERR_PTR(-ENODEV);
>  		}
>  
> -		get_device(dev);
> +		get_device(cl->dev);
>  		cl->info = info;
>  		mutex_unlock(&registered_clients_lock);
>  		return cl;

Re: [PATCH 1/2] iio: adc: adi-axi-adc: Fix object reference counting

[Jonathan Cameron]

On Wed, 1 Jul 2020 14:07:05 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:

> On Wed, 2020-07-01 at 14:04 +0200, Nuno Sá wrote:
> > [External]
> > 
> > When looking for a registered client to attach with, the wrong reference
> > counters are being grabbed. The idea is to increment the module and
> > device
> > counters of the client device and not the counters of the axi device
> > being
> > probed.
> >   
> 
> Yep.
> Good catch here.
> 
> Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

> 
> > Fixes: ef04070692a2 (iio: adc: adi-axi-adc: add support for AXI ADC IP
> > core)
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/iio/adc/adi-axi-adc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-
> > adc.c
> > index 63d926e86798..86b6b65916ee 100644
> > --- a/drivers/iio/adc/adi-axi-adc.c
> > +++ b/drivers/iio/adc/adi-axi-adc.c
> > @@ -332,12 +332,12 @@ static struct adi_axi_adc_client
> > *adi_axi_adc_attach_client(struct device *dev)
> >  		if (cl->dev->of_node != cln)
> >  			continue;
> >  
> > -		if (!try_module_get(dev->driver->owner)) {
> > +		if (!try_module_get(cl->dev->driver->owner)) {
> >  			mutex_unlock(&registered_clients_lock);
> >  			return ERR_PTR(-ENODEV);
> >  		}
> >  
> > -		get_device(dev);
> > +		get_device(cl->dev);
> >  		cl->info = info;
> >  		mutex_unlock(&registered_clients_lock);
> >  		return cl;