Skip to contentSkip to conversation

iio: light: hid-sensor-prox: Fit assignment in one line

PATCH Applied Fabio Estevam 2 participants 5 messages May 8, 2015 – Jun 14, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

From: Fabio Estevam <[email protected]>

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

Put it in one line.

---
 drivers/iio/light/hid-sensor-prox.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/iio/light/hid-sensor-prox.c +1 -2
index 124a8f8..4ed42df 100644
@@ -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;
Signature · 1 line
1.9.1
#1 lore raw

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

From: Fabio Estevam <[email protected]>

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

---
 drivers/iio/light/hid-sensor-prox.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/iio/light/hid-sensor-prox.c +3 -6
index 4ed42df..c9f804b 100644
@@ -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);
Signature · 1 line
1.9.1
maintainer
#2 lore raw

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

Quoted 3 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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

Quoted 44 lines
---
 drivers/iio/light/hid-sensor-prox.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/iio/light/hid-sensor-prox.c +3 -6
index 4ed42df..c9f804b 100644
@@ -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);
maintainer
#3 lore raw
In reply to Fabio Estevam
Quoted 8 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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

Put it in one line.

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

Quoted 19 lines
---
 drivers/iio/light/hid-sensor-prox.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/iio/light/hid-sensor-prox.c +1 -2
index 124a8f8..4ed42df 100644
@@ -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;
maintainer Applied
#4 lore raw
In reply to Fabio Estevam
Quoted 8 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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

Put it in one line.

Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Quoted 19 lines
---
 drivers/iio/light/hid-sensor-prox.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/iio/light/hid-sensor-prox.c +1 -2
index 124a8f8..4ed42df 100644
@@ -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;