[PATCH 1/2] iio: light: hid-sensor-prox: Fit assignment in one line

[PATCH 1/2] iio: light: hid-sensor-prox: Fit assignment in one line

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need to do the assignment to indio_dev->num_channels in two
lines code.

Put it in one line.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/iio/light/hid-sensor-prox.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index 124a8f8..4ed42df 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -286,8 +286,7 @@ static int hid_prox_probe(struct platform_device *pdev)
 		goto error_free_dev_mem;
 	}
 
-	indio_dev->num_channels =
-				ARRAY_SIZE(prox_channels);
+	indio_dev->num_channels = ARRAY_SIZE(prox_channels);
 	indio_dev->dev.parent = &pdev->dev;
 	indio_dev->info = &prox_info;
 	indio_dev->name = name;
-- 
1.9.1

[PATCH 2/2] iio: light: hid-sensor-prox: Use devm_iio_device_register()

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Using devm_iio_device_register() can make the code shorter and cleaner.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/iio/light/hid-sensor-prox.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index 4ed42df..c9f804b 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -306,7 +306,7 @@ static int hid_prox_probe(struct platform_device *pdev)
 		goto error_unreg_buffer_funcs;
 	}
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&pdev->dev, indio_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "device register failed\n");
 		goto error_remove_trigger;
@@ -319,13 +319,11 @@ static int hid_prox_probe(struct platform_device *pdev)
 					&prox_state->callbacks);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "callback reg failed\n");
-		goto error_iio_unreg;
+		return ret;
 	}
 
-	return ret;
+	return 0;
 
-error_iio_unreg:
-	iio_device_unregister(indio_dev);
 error_remove_trigger:
 	hid_sensor_remove_trigger(&prox_state->common_attributes);
 error_unreg_buffer_funcs:
@@ -343,7 +341,6 @@ static int hid_prox_remove(struct platform_device *pdev)
 	struct prox_state *prox_state = iio_priv(indio_dev);
 
 	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PROX);
-	iio_device_unregister(indio_dev);
 	hid_sensor_remove_trigger(&prox_state->common_attributes);
 	iio_triggered_buffer_cleanup(indio_dev);
 	kfree(indio_dev->channels);
-- 
1.9.1

Re: [PATCH 2/2] iio: light: hid-sensor-prox: Use devm_iio_device_register()

[Jonathan Cameron]

On 08/05/15 19:54, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Using devm_iio_device_register() can make the code shorter and cleaner.
... and totally changes the order of the remove function by making it remove
the userspace interface, after a whole load of other elements are destroyed.

This adds one heck of a race condition.  devm_iio_device_register
is pretty much only safe if it allows the remove function to be completely
empty and hence dropped.

Jonathan
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  drivers/iio/light/hid-sensor-prox.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index 4ed42df..c9f804b 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
> @@ -306,7 +306,7 @@ static int hid_prox_probe(struct platform_device *pdev)
>  		goto error_unreg_buffer_funcs;
>  	}
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_iio_device_register(&pdev->dev, indio_dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "device register failed\n");
>  		goto error_remove_trigger;
> @@ -319,13 +319,11 @@ static int hid_prox_probe(struct platform_device *pdev)
>  					&prox_state->callbacks);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> -		goto error_iio_unreg;
> +		return ret;
>  	}
>  
> -	return ret;
> +	return 0;
>  
> -error_iio_unreg:
> -	iio_device_unregister(indio_dev);
>  error_remove_trigger:
>  	hid_sensor_remove_trigger(&prox_state->common_attributes);
>  error_unreg_buffer_funcs:
> @@ -343,7 +341,6 @@ static int hid_prox_remove(struct platform_device *pdev)
>  	struct prox_state *prox_state = iio_priv(indio_dev);
>  
>  	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PROX);
> -	iio_device_unregister(indio_dev);
>  	hid_sensor_remove_trigger(&prox_state->common_attributes);
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	kfree(indio_dev->channels);
> 

Re: [PATCH 1/2] iio: light: hid-sensor-prox: Fit assignment in one line

[Jonathan Cameron]

On 08/05/15 19:54, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There is no need to do the assignment to indio_dev->num_channels in two
> lines code.
> 
> Put it in one line.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Fair enough, but I'll have to wait for the fix to the line above
this to propagate through to the staging-next tree before applying this.
Please do remind me if I seem to have forgotten about it!

Jonathan
> ---
>  drivers/iio/light/hid-sensor-prox.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index 124a8f8..4ed42df 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
> @@ -286,8 +286,7 @@ static int hid_prox_probe(struct platform_device *pdev)
>  		goto error_free_dev_mem;
>  	}
>  
> -	indio_dev->num_channels =
> -				ARRAY_SIZE(prox_channels);
> +	indio_dev->num_channels = ARRAY_SIZE(prox_channels);
>  	indio_dev->dev.parent = &pdev->dev;
>  	indio_dev->info = &prox_info;
>  	indio_dev->name = name;
> 

Re: [PATCH 1/2] iio: light: hid-sensor-prox: Fit assignment in one line

[Jonathan Cameron]

On 08/05/15 19:54, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There is no need to do the assignment to indio_dev->num_channels in two
> lines code.
> 
> Put it in one line.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Applied to the togreg branch of iio.git - initially pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/iio/light/hid-sensor-prox.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index 124a8f8..4ed42df 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
> @@ -286,8 +286,7 @@ static int hid_prox_probe(struct platform_device *pdev)
>  		goto error_free_dev_mem;
>  	}
>  
> -	indio_dev->num_channels =
> -				ARRAY_SIZE(prox_channels);
> +	indio_dev->num_channels = ARRAY_SIZE(prox_channels);
>  	indio_dev->dev.parent = &pdev->dev;
>  	indio_dev->info = &prox_info;
>  	indio_dev->name = name;
>