* [PATCH 1/4] iio: st_sensors: remove st_sensors_deallocate_trigger() function
2021-07-26 7:14 [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Alexandru Ardelean
@ 2021-07-26 7:14 ` Alexandru Ardelean
2021-07-26 7:14 ` [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function Alexandru Ardelean
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Alexandru Ardelean @ 2021-07-26 7:14 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: denis.ciocca, jic23, Alexandru Ardelean
This change converts the st_sensors_allocate_trigger() to use
device-managed functions.
The parent device of the IIO device object is used. This is based on the
assumption that all other devm_ calls in the ST sensors use this reference.
That makes the st_sensors_deallocate_trigger() function un-needed, so it
can be removed.
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
drivers/iio/accel/st_accel_core.c | 18 +-------
.../common/st_sensors/st_sensors_trigger.c | 45 +++++++------------
drivers/iio/gyro/st_gyro_core.c | 18 +-------
drivers/iio/magnetometer/st_magn_core.c | 18 +-------
drivers/iio/pressure/st_pressure_core.c | 18 +-------
include/linux/iio/common/st_sensors.h | 5 ---
6 files changed, 19 insertions(+), 103 deletions(-)
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index f1e6ec380667..a7be1633bff1 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -1380,29 +1380,13 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
return err;
}
- err = iio_device_register(indio_dev);
- if (err)
- goto st_accel_device_register_error;
-
- dev_info(&indio_dev->dev, "registered accelerometer %s\n",
- indio_dev->name);
-
- return 0;
-
-st_accel_device_register_error:
- if (adata->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
- return err;
+ return iio_device_register(indio_dev);
}
EXPORT_SYMBOL(st_accel_common_probe);
void st_accel_common_remove(struct iio_dev *indio_dev)
{
- struct st_sensor_data *adata = iio_priv(indio_dev);
-
iio_device_unregister(indio_dev);
- if (adata->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
}
EXPORT_SYMBOL(st_accel_common_remove);
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index 64e0a748a855..d022157b66a2 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -119,11 +119,12 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
const struct iio_trigger_ops *trigger_ops)
{
struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct device *parent = indio_dev->dev.parent;
unsigned long irq_trig;
int err;
- sdata->trig = iio_trigger_alloc(sdata->dev, "%s-trigger",
- indio_dev->name);
+ sdata->trig = devm_iio_trigger_alloc(parent, "%s-trigger",
+ indio_dev->name);
if (sdata->trig == NULL) {
dev_err(&indio_dev->dev, "failed to allocate iio trigger.\n");
return -ENOMEM;
@@ -153,7 +154,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
sdata->sensor_settings->drdy_irq.addr_ihl,
sdata->sensor_settings->drdy_irq.mask_ihl, 1);
if (err < 0)
- goto iio_trigger_free;
+ return err;
dev_info(&indio_dev->dev,
"interrupts on the falling edge or active low level\n");
}
@@ -179,8 +180,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
if (!sdata->sensor_settings->drdy_irq.stat_drdy.addr) {
dev_err(&indio_dev->dev,
"edge IRQ not supported w/o stat register.\n");
- err = -EOPNOTSUPP;
- goto iio_trigger_free;
+ return -EOPNOTSUPP;
}
sdata->edge_irq = true;
} else {
@@ -205,44 +205,29 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
sdata->sensor_settings->drdy_irq.stat_drdy.addr)
irq_trig |= IRQF_SHARED;
- err = request_threaded_irq(sdata->irq,
- st_sensors_irq_handler,
- st_sensors_irq_thread,
- irq_trig,
- sdata->trig->name,
- sdata->trig);
+ err = devm_request_threaded_irq(parent,
+ sdata->irq,
+ st_sensors_irq_handler,
+ st_sensors_irq_thread,
+ irq_trig,
+ sdata->trig->name,
+ sdata->trig);
if (err) {
dev_err(&indio_dev->dev, "failed to request trigger IRQ.\n");
- goto iio_trigger_free;
+ return err;
}
- err = iio_trigger_register(sdata->trig);
+ err = devm_iio_trigger_register(parent, sdata->trig);
if (err < 0) {
dev_err(&indio_dev->dev, "failed to register iio trigger.\n");
- goto iio_trigger_register_error;
+ return err;
}
indio_dev->trig = iio_trigger_get(sdata->trig);
return 0;
-
-iio_trigger_register_error:
- free_irq(sdata->irq, sdata->trig);
-iio_trigger_free:
- iio_trigger_free(sdata->trig);
- return err;
}
EXPORT_SYMBOL(st_sensors_allocate_trigger);
-void st_sensors_deallocate_trigger(struct iio_dev *indio_dev)
-{
- struct st_sensor_data *sdata = iio_priv(indio_dev);
-
- iio_trigger_unregister(sdata->trig);
- free_irq(sdata->irq, sdata->trig);
- iio_trigger_free(sdata->trig);
-}
-EXPORT_SYMBOL(st_sensors_deallocate_trigger);
-
int st_sensors_validate_device(struct iio_trigger *trig,
struct iio_dev *indio_dev)
{
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index e8fc8af65143..cb539b47cdf4 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -515,29 +515,13 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
return err;
}
- err = iio_device_register(indio_dev);
- if (err)
- goto st_gyro_device_register_error;
-
- dev_info(&indio_dev->dev, "registered gyroscope %s\n",
- indio_dev->name);
-
- return 0;
-
-st_gyro_device_register_error:
- if (gdata->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
- return err;
+ return iio_device_register(indio_dev);
}
EXPORT_SYMBOL(st_gyro_common_probe);
void st_gyro_common_remove(struct iio_dev *indio_dev)
{
- struct st_sensor_data *gdata = iio_priv(indio_dev);
-
iio_device_unregister(indio_dev);
- if (gdata->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
}
EXPORT_SYMBOL(st_gyro_common_remove);
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 9ffd50d796bf..5be85e2405a5 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -650,29 +650,13 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
return err;
}
- err = iio_device_register(indio_dev);
- if (err)
- goto st_magn_device_register_error;
-
- dev_info(&indio_dev->dev, "registered magnetometer %s\n",
- indio_dev->name);
-
- return 0;
-
-st_magn_device_register_error:
- if (mdata->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
- return err;
+ return iio_device_register(indio_dev);
}
EXPORT_SYMBOL(st_magn_common_probe);
void st_magn_common_remove(struct iio_dev *indio_dev)
{
- struct st_sensor_data *mdata = iio_priv(indio_dev);
-
iio_device_unregister(indio_dev);
- if (mdata->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
}
EXPORT_SYMBOL(st_magn_common_remove);
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index ab1c17fac807..17ebb5171d4c 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -721,29 +721,13 @@ int st_press_common_probe(struct iio_dev *indio_dev)
return err;
}
- err = iio_device_register(indio_dev);
- if (err)
- goto st_press_device_register_error;
-
- dev_info(&indio_dev->dev, "registered pressure sensor %s\n",
- indio_dev->name);
-
- return err;
-
-st_press_device_register_error:
- if (press_data->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
- return err;
+ return iio_device_register(indio_dev);
}
EXPORT_SYMBOL(st_press_common_probe);
void st_press_common_remove(struct iio_dev *indio_dev)
{
- struct st_sensor_data *press_data = iio_priv(indio_dev);
-
iio_device_unregister(indio_dev);
- if (press_data->irq > 0)
- st_sensors_deallocate_trigger(indio_dev);
}
EXPORT_SYMBOL(st_press_common_remove);
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 8bdbaf3f3796..e74b55244f35 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -273,7 +273,6 @@ irqreturn_t st_sensors_trigger_handler(int irq, void *p);
int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
const struct iio_trigger_ops *trigger_ops);
-void st_sensors_deallocate_trigger(struct iio_dev *indio_dev);
int st_sensors_validate_device(struct iio_trigger *trig,
struct iio_dev *indio_dev);
#else
@@ -282,10 +281,6 @@ static inline int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
{
return 0;
}
-static inline void st_sensors_deallocate_trigger(struct iio_dev *indio_dev)
-{
- return;
-}
#define st_sensors_validate_device NULL
#endif
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function
2021-07-26 7:14 [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Alexandru Ardelean
2021-07-26 7:14 ` [PATCH 1/4] iio: st_sensors: remove st_sensors_deallocate_trigger() function Alexandru Ardelean
@ 2021-07-26 7:14 ` Alexandru Ardelean
2021-07-31 17:37 ` Jonathan Cameron
2021-07-26 7:14 ` [PATCH 3/4] iio: st_sensors: remove all driver remove functions Alexandru Ardelean
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Alexandru Ardelean @ 2021-07-26 7:14 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: denis.ciocca, jic23, Alexandru Ardelean
This change converts the st_sensors_power_enable() function to use
devm_add_action_or_reset() handlers to register regulator_disable hooks for
when the drivers get unloaded.
The parent device of the IIO device object is used. This is based on the
assumption that all other devm_ calls in the ST sensors use this reference.
This makes the st_sensors_power_disable() un-needed.
Removing this also changes unload order a bit, as all ST drivers would call
st_sensors_power_disable() first and iio_device_unregister() after that.
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
drivers/iio/accel/st_accel_i2c.c | 13 +------
drivers/iio/accel/st_accel_spi.c | 13 +------
.../iio/common/st_sensors/st_sensors_core.c | 34 ++++++++-----------
drivers/iio/gyro/st_gyro_i2c.c | 13 +------
drivers/iio/gyro/st_gyro_spi.c | 13 +------
drivers/iio/magnetometer/st_magn_i2c.c | 13 +------
drivers/iio/magnetometer/st_magn_spi.c | 13 +------
drivers/iio/pressure/st_pressure_i2c.c | 13 +------
drivers/iio/pressure/st_pressure_spi.c | 13 +------
include/linux/iio/common/st_sensors.h | 2 --
10 files changed, 23 insertions(+), 117 deletions(-)
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
index f711756e41e3..b377575efc41 100644
--- a/drivers/iio/accel/st_accel_i2c.c
+++ b/drivers/iio/accel/st_accel_i2c.c
@@ -177,24 +177,13 @@ static int st_accel_i2c_probe(struct i2c_client *client)
if (ret)
return ret;
- ret = st_accel_common_probe(indio_dev);
- if (ret < 0)
- goto st_accel_power_off;
-
- return 0;
-
-st_accel_power_off:
- st_sensors_power_disable(indio_dev);
-
- return ret;
+ return st_accel_common_probe(indio_dev);
}
static int st_accel_i2c_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
- st_sensors_power_disable(indio_dev);
-
st_accel_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
index bb45d9ff95b8..4ca87e73bdb3 100644
--- a/drivers/iio/accel/st_accel_spi.c
+++ b/drivers/iio/accel/st_accel_spi.c
@@ -127,24 +127,13 @@ static int st_accel_spi_probe(struct spi_device *spi)
if (err)
return err;
- err = st_accel_common_probe(indio_dev);
- if (err < 0)
- goto st_accel_power_off;
-
- return 0;
-
-st_accel_power_off:
- st_sensors_power_disable(indio_dev);
-
- return err;
+ return st_accel_common_probe(indio_dev);
}
static int st_accel_spi_remove(struct spi_device *spi)
{
struct iio_dev *indio_dev = spi_get_drvdata(spi);
- st_sensors_power_disable(indio_dev);
-
st_accel_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 0bbb090b108c..a5a140de9a23 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -215,13 +215,19 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
}
EXPORT_SYMBOL(st_sensors_set_axis_enable);
+static void st_reg_disable(void *reg)
+{
+ regulator_disable(reg);
+}
+
int st_sensors_power_enable(struct iio_dev *indio_dev)
{
struct st_sensor_data *pdata = iio_priv(indio_dev);
+ struct device *parent = indio_dev->dev.parent;
int err;
/* Regulators not mandatory, but if requested we should enable them. */
- pdata->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
+ pdata->vdd = devm_regulator_get(parent, "vdd");
if (IS_ERR(pdata->vdd)) {
dev_err(&indio_dev->dev, "unable to get Vdd supply\n");
return PTR_ERR(pdata->vdd);
@@ -233,36 +239,26 @@ int st_sensors_power_enable(struct iio_dev *indio_dev)
return err;
}
- pdata->vdd_io = devm_regulator_get(indio_dev->dev.parent, "vddio");
+ err = devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd);
+ if (err)
+ return err;
+
+ pdata->vdd_io = devm_regulator_get(parent, "vddio");
if (IS_ERR(pdata->vdd_io)) {
dev_err(&indio_dev->dev, "unable to get Vdd_IO supply\n");
- err = PTR_ERR(pdata->vdd_io);
- goto st_sensors_disable_vdd;
+ return PTR_ERR(pdata->vdd_io);
}
err = regulator_enable(pdata->vdd_io);
if (err != 0) {
dev_warn(&indio_dev->dev,
"Failed to enable specified Vdd_IO supply\n");
- goto st_sensors_disable_vdd;
+ return err;
}
- return 0;
-
-st_sensors_disable_vdd:
- regulator_disable(pdata->vdd);
- return err;
+ return devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd_io);
}
EXPORT_SYMBOL(st_sensors_power_enable);
-void st_sensors_power_disable(struct iio_dev *indio_dev)
-{
- struct st_sensor_data *pdata = iio_priv(indio_dev);
-
- regulator_disable(pdata->vdd);
- regulator_disable(pdata->vdd_io);
-}
-EXPORT_SYMBOL(st_sensors_power_disable);
-
static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
struct st_sensors_platform_data *pdata)
{
diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
index 3ef86e16ee65..0bd80dfd389f 100644
--- a/drivers/iio/gyro/st_gyro_i2c.c
+++ b/drivers/iio/gyro/st_gyro_i2c.c
@@ -90,24 +90,13 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
if (err)
return err;
- err = st_gyro_common_probe(indio_dev);
- if (err < 0)
- goto st_gyro_power_off;
-
- return 0;
-
-st_gyro_power_off:
- st_sensors_power_disable(indio_dev);
-
- return err;
+ return st_gyro_common_probe(indio_dev);
}
static int st_gyro_i2c_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
- st_sensors_power_disable(indio_dev);
-
st_gyro_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
index 41d835493347..f74b09fa5cde 100644
--- a/drivers/iio/gyro/st_gyro_spi.c
+++ b/drivers/iio/gyro/st_gyro_spi.c
@@ -94,24 +94,13 @@ static int st_gyro_spi_probe(struct spi_device *spi)
if (err)
return err;
- err = st_gyro_common_probe(indio_dev);
- if (err < 0)
- goto st_gyro_power_off;
-
- return 0;
-
-st_gyro_power_off:
- st_sensors_power_disable(indio_dev);
-
- return err;
+ return st_gyro_common_probe(indio_dev);
}
static int st_gyro_spi_remove(struct spi_device *spi)
{
struct iio_dev *indio_dev = spi_get_drvdata(spi);
- st_sensors_power_disable(indio_dev);
-
st_gyro_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
index 2dfe4ee99591..0a5117dffcf4 100644
--- a/drivers/iio/magnetometer/st_magn_i2c.c
+++ b/drivers/iio/magnetometer/st_magn_i2c.c
@@ -86,24 +86,13 @@ static int st_magn_i2c_probe(struct i2c_client *client,
if (err)
return err;
- err = st_magn_common_probe(indio_dev);
- if (err < 0)
- goto st_magn_power_off;
-
- return 0;
-
-st_magn_power_off:
- st_sensors_power_disable(indio_dev);
-
- return err;
+ return st_magn_common_probe(indio_dev);
}
static int st_magn_i2c_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
- st_sensors_power_disable(indio_dev);
-
st_magn_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index fba978796395..1f3bf02b24e0 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -80,24 +80,13 @@ static int st_magn_spi_probe(struct spi_device *spi)
if (err)
return err;
- err = st_magn_common_probe(indio_dev);
- if (err < 0)
- goto st_magn_power_off;
-
- return 0;
-
-st_magn_power_off:
- st_sensors_power_disable(indio_dev);
-
- return err;
+ return st_magn_common_probe(indio_dev);
}
static int st_magn_spi_remove(struct spi_device *spi)
{
struct iio_dev *indio_dev = spi_get_drvdata(spi);
- st_sensors_power_disable(indio_dev);
-
st_magn_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index 52fa98f24478..afeeab485c0d 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -103,24 +103,13 @@ static int st_press_i2c_probe(struct i2c_client *client,
if (ret)
return ret;
- ret = st_press_common_probe(indio_dev);
- if (ret < 0)
- goto st_press_power_off;
-
- return 0;
-
-st_press_power_off:
- st_sensors_power_disable(indio_dev);
-
- return ret;
+ return st_press_common_probe(indio_dev);
}
static int st_press_i2c_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
- st_sensors_power_disable(indio_dev);
-
st_press_common_remove(indio_dev);
return 0;
diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
index ee393df54cee..834ad6d40a70 100644
--- a/drivers/iio/pressure/st_pressure_spi.c
+++ b/drivers/iio/pressure/st_pressure_spi.c
@@ -86,24 +86,13 @@ static int st_press_spi_probe(struct spi_device *spi)
if (err)
return err;
- err = st_press_common_probe(indio_dev);
- if (err < 0)
- goto st_press_power_off;
-
- return 0;
-
-st_press_power_off:
- st_sensors_power_disable(indio_dev);
-
- return err;
+ return st_press_common_probe(indio_dev);
}
static int st_press_spi_remove(struct spi_device *spi)
{
struct iio_dev *indio_dev = spi_get_drvdata(spi);
- st_sensors_power_disable(indio_dev);
-
st_press_common_remove(indio_dev);
return 0;
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index e74b55244f35..fc90c202d15e 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -293,8 +293,6 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
int st_sensors_power_enable(struct iio_dev *indio_dev);
-void st_sensors_power_disable(struct iio_dev *indio_dev);
-
int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev,
unsigned reg, unsigned writeval,
unsigned *readval);
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function
2021-07-26 7:14 ` [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function Alexandru Ardelean
@ 2021-07-31 17:37 ` Jonathan Cameron
2021-08-02 7:30 ` Alexandru Ardelean
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2021-07-31 17:37 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, denis.ciocca
On Mon, 26 Jul 2021 10:14:02 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:
> This change converts the st_sensors_power_enable() function to use
> devm_add_action_or_reset() handlers to register regulator_disable hooks for
> when the drivers get unloaded.
>
> The parent device of the IIO device object is used. This is based on the
> assumption that all other devm_ calls in the ST sensors use this reference.
>
> This makes the st_sensors_power_disable() un-needed.
> Removing this also changes unload order a bit, as all ST drivers would call
> st_sensors_power_disable() first and iio_device_unregister() after that.
Huh. So currently the drivers turn the power off before calling
iio_device_unregister()?
That's a bad idea (turn off power when userspace is still available)
and looks like a bug fix to me. Perhaps the reorder should be pulled out as
a precursor patch in case anyone wants to backport it?
(note I initially thought you were going the other way and was just writing
a rant about breaking the order when I realised you were fixing it ;)
>
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
> drivers/iio/accel/st_accel_i2c.c | 13 +------
> drivers/iio/accel/st_accel_spi.c | 13 +------
> .../iio/common/st_sensors/st_sensors_core.c | 34 ++++++++-----------
> drivers/iio/gyro/st_gyro_i2c.c | 13 +------
> drivers/iio/gyro/st_gyro_spi.c | 13 +------
> drivers/iio/magnetometer/st_magn_i2c.c | 13 +------
> drivers/iio/magnetometer/st_magn_spi.c | 13 +------
> drivers/iio/pressure/st_pressure_i2c.c | 13 +------
> drivers/iio/pressure/st_pressure_spi.c | 13 +------
> include/linux/iio/common/st_sensors.h | 2 --
> 10 files changed, 23 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> index f711756e41e3..b377575efc41 100644
> --- a/drivers/iio/accel/st_accel_i2c.c
> +++ b/drivers/iio/accel/st_accel_i2c.c
> @@ -177,24 +177,13 @@ static int st_accel_i2c_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> - ret = st_accel_common_probe(indio_dev);
> - if (ret < 0)
> - goto st_accel_power_off;
> -
> - return 0;
> -
> -st_accel_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return ret;
> + return st_accel_common_probe(indio_dev);
> }
>
> static int st_accel_i2c_remove(struct i2c_client *client)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_accel_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> index bb45d9ff95b8..4ca87e73bdb3 100644
> --- a/drivers/iio/accel/st_accel_spi.c
> +++ b/drivers/iio/accel/st_accel_spi.c
> @@ -127,24 +127,13 @@ static int st_accel_spi_probe(struct spi_device *spi)
> if (err)
> return err;
>
> - err = st_accel_common_probe(indio_dev);
> - if (err < 0)
> - goto st_accel_power_off;
> -
> - return 0;
> -
> -st_accel_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return err;
> + return st_accel_common_probe(indio_dev);
> }
>
> static int st_accel_spi_remove(struct spi_device *spi)
> {
> struct iio_dev *indio_dev = spi_get_drvdata(spi);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_accel_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 0bbb090b108c..a5a140de9a23 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -215,13 +215,19 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
> }
> EXPORT_SYMBOL(st_sensors_set_axis_enable);
>
> +static void st_reg_disable(void *reg)
> +{
> + regulator_disable(reg);
> +}
> +
> int st_sensors_power_enable(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *pdata = iio_priv(indio_dev);
> + struct device *parent = indio_dev->dev.parent;
> int err;
>
> /* Regulators not mandatory, but if requested we should enable them. */
> - pdata->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
> + pdata->vdd = devm_regulator_get(parent, "vdd");
> if (IS_ERR(pdata->vdd)) {
> dev_err(&indio_dev->dev, "unable to get Vdd supply\n");
> return PTR_ERR(pdata->vdd);
> @@ -233,36 +239,26 @@ int st_sensors_power_enable(struct iio_dev *indio_dev)
> return err;
> }
>
> - pdata->vdd_io = devm_regulator_get(indio_dev->dev.parent, "vddio");
> + err = devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd);
> + if (err)
> + return err;
> +
> + pdata->vdd_io = devm_regulator_get(parent, "vddio");
> if (IS_ERR(pdata->vdd_io)) {
> dev_err(&indio_dev->dev, "unable to get Vdd_IO supply\n");
> - err = PTR_ERR(pdata->vdd_io);
> - goto st_sensors_disable_vdd;
> + return PTR_ERR(pdata->vdd_io);
> }
> err = regulator_enable(pdata->vdd_io);
> if (err != 0) {
> dev_warn(&indio_dev->dev,
> "Failed to enable specified Vdd_IO supply\n");
> - goto st_sensors_disable_vdd;
> + return err;
> }
>
> - return 0;
> -
> -st_sensors_disable_vdd:
> - regulator_disable(pdata->vdd);
> - return err;
> + return devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd_io);
> }
> EXPORT_SYMBOL(st_sensors_power_enable);
>
> -void st_sensors_power_disable(struct iio_dev *indio_dev)
> -{
> - struct st_sensor_data *pdata = iio_priv(indio_dev);
> -
> - regulator_disable(pdata->vdd);
> - regulator_disable(pdata->vdd_io);
> -}
> -EXPORT_SYMBOL(st_sensors_power_disable);
> -
> static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
> struct st_sensors_platform_data *pdata)
> {
> diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
> index 3ef86e16ee65..0bd80dfd389f 100644
> --- a/drivers/iio/gyro/st_gyro_i2c.c
> +++ b/drivers/iio/gyro/st_gyro_i2c.c
> @@ -90,24 +90,13 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
> if (err)
> return err;
>
> - err = st_gyro_common_probe(indio_dev);
> - if (err < 0)
> - goto st_gyro_power_off;
> -
> - return 0;
> -
> -st_gyro_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return err;
> + return st_gyro_common_probe(indio_dev);
> }
>
> static int st_gyro_i2c_remove(struct i2c_client *client)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_gyro_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
> index 41d835493347..f74b09fa5cde 100644
> --- a/drivers/iio/gyro/st_gyro_spi.c
> +++ b/drivers/iio/gyro/st_gyro_spi.c
> @@ -94,24 +94,13 @@ static int st_gyro_spi_probe(struct spi_device *spi)
> if (err)
> return err;
>
> - err = st_gyro_common_probe(indio_dev);
> - if (err < 0)
> - goto st_gyro_power_off;
> -
> - return 0;
> -
> -st_gyro_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return err;
> + return st_gyro_common_probe(indio_dev);
> }
>
> static int st_gyro_spi_remove(struct spi_device *spi)
> {
> struct iio_dev *indio_dev = spi_get_drvdata(spi);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_gyro_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> index 2dfe4ee99591..0a5117dffcf4 100644
> --- a/drivers/iio/magnetometer/st_magn_i2c.c
> +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> @@ -86,24 +86,13 @@ static int st_magn_i2c_probe(struct i2c_client *client,
> if (err)
> return err;
>
> - err = st_magn_common_probe(indio_dev);
> - if (err < 0)
> - goto st_magn_power_off;
> -
> - return 0;
> -
> -st_magn_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return err;
> + return st_magn_common_probe(indio_dev);
> }
>
> static int st_magn_i2c_remove(struct i2c_client *client)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_magn_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> index fba978796395..1f3bf02b24e0 100644
> --- a/drivers/iio/magnetometer/st_magn_spi.c
> +++ b/drivers/iio/magnetometer/st_magn_spi.c
> @@ -80,24 +80,13 @@ static int st_magn_spi_probe(struct spi_device *spi)
> if (err)
> return err;
>
> - err = st_magn_common_probe(indio_dev);
> - if (err < 0)
> - goto st_magn_power_off;
> -
> - return 0;
> -
> -st_magn_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return err;
> + return st_magn_common_probe(indio_dev);
> }
>
> static int st_magn_spi_remove(struct spi_device *spi)
> {
> struct iio_dev *indio_dev = spi_get_drvdata(spi);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_magn_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> index 52fa98f24478..afeeab485c0d 100644
> --- a/drivers/iio/pressure/st_pressure_i2c.c
> +++ b/drivers/iio/pressure/st_pressure_i2c.c
> @@ -103,24 +103,13 @@ static int st_press_i2c_probe(struct i2c_client *client,
> if (ret)
> return ret;
>
> - ret = st_press_common_probe(indio_dev);
> - if (ret < 0)
> - goto st_press_power_off;
> -
> - return 0;
> -
> -st_press_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return ret;
> + return st_press_common_probe(indio_dev);
> }
>
> static int st_press_i2c_remove(struct i2c_client *client)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_press_common_remove(indio_dev);
>
> return 0;
> diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> index ee393df54cee..834ad6d40a70 100644
> --- a/drivers/iio/pressure/st_pressure_spi.c
> +++ b/drivers/iio/pressure/st_pressure_spi.c
> @@ -86,24 +86,13 @@ static int st_press_spi_probe(struct spi_device *spi)
> if (err)
> return err;
>
> - err = st_press_common_probe(indio_dev);
> - if (err < 0)
> - goto st_press_power_off;
> -
> - return 0;
> -
> -st_press_power_off:
> - st_sensors_power_disable(indio_dev);
> -
> - return err;
> + return st_press_common_probe(indio_dev);
> }
>
> static int st_press_spi_remove(struct spi_device *spi)
> {
> struct iio_dev *indio_dev = spi_get_drvdata(spi);
>
> - st_sensors_power_disable(indio_dev);
> -
> st_press_common_remove(indio_dev);
>
> return 0;
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index e74b55244f35..fc90c202d15e 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -293,8 +293,6 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
>
> int st_sensors_power_enable(struct iio_dev *indio_dev);
>
> -void st_sensors_power_disable(struct iio_dev *indio_dev);
> -
> int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev,
> unsigned reg, unsigned writeval,
> unsigned *readval);
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function
2021-07-31 17:37 ` Jonathan Cameron
@ 2021-08-02 7:30 ` Alexandru Ardelean
2021-08-16 7:48 ` Alexandru Ardelean
0 siblings, 1 reply; 11+ messages in thread
From: Alexandru Ardelean @ 2021-08-02 7:30 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Linux Kernel Mailing List, denis.ciocca
On Sat, 31 Jul 2021 at 20:34, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 26 Jul 2021 10:14:02 +0300
> Alexandru Ardelean <aardelean@deviqon.com> wrote:
>
> > This change converts the st_sensors_power_enable() function to use
> > devm_add_action_or_reset() handlers to register regulator_disable hooks for
> > when the drivers get unloaded.
> >
> > The parent device of the IIO device object is used. This is based on the
> > assumption that all other devm_ calls in the ST sensors use this reference.
> >
> > This makes the st_sensors_power_disable() un-needed.
> > Removing this also changes unload order a bit, as all ST drivers would call
> > st_sensors_power_disable() first and iio_device_unregister() after that.
>
> Huh. So currently the drivers turn the power off before calling
> iio_device_unregister()?
>
> That's a bad idea (turn off power when userspace is still available)
> and looks like a bug fix to me. Perhaps the reorder should be pulled out as
> a precursor patch in case anyone wants to backport it?
>
> (note I initially thought you were going the other way and was just writing
> a rant about breaking the order when I realised you were fixing it ;)
Probably the best advantage of devm_ functions .
Not needing to care about un-init order.
I hated debugging driver crashes when they were unloaded, usually
because the driver gets into a weird state.
I'll send a V2 with the re-order fixes in the front.
>
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > ---
> > drivers/iio/accel/st_accel_i2c.c | 13 +------
> > drivers/iio/accel/st_accel_spi.c | 13 +------
> > .../iio/common/st_sensors/st_sensors_core.c | 34 ++++++++-----------
> > drivers/iio/gyro/st_gyro_i2c.c | 13 +------
> > drivers/iio/gyro/st_gyro_spi.c | 13 +------
> > drivers/iio/magnetometer/st_magn_i2c.c | 13 +------
> > drivers/iio/magnetometer/st_magn_spi.c | 13 +------
> > drivers/iio/pressure/st_pressure_i2c.c | 13 +------
> > drivers/iio/pressure/st_pressure_spi.c | 13 +------
> > include/linux/iio/common/st_sensors.h | 2 --
> > 10 files changed, 23 insertions(+), 117 deletions(-)
> >
> > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> > index f711756e41e3..b377575efc41 100644
> > --- a/drivers/iio/accel/st_accel_i2c.c
> > +++ b/drivers/iio/accel/st_accel_i2c.c
> > @@ -177,24 +177,13 @@ static int st_accel_i2c_probe(struct i2c_client *client)
> > if (ret)
> > return ret;
> >
> > - ret = st_accel_common_probe(indio_dev);
> > - if (ret < 0)
> > - goto st_accel_power_off;
> > -
> > - return 0;
> > -
> > -st_accel_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return ret;
> > + return st_accel_common_probe(indio_dev);
> > }
> >
> > static int st_accel_i2c_remove(struct i2c_client *client)
> > {
> > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_accel_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> > index bb45d9ff95b8..4ca87e73bdb3 100644
> > --- a/drivers/iio/accel/st_accel_spi.c
> > +++ b/drivers/iio/accel/st_accel_spi.c
> > @@ -127,24 +127,13 @@ static int st_accel_spi_probe(struct spi_device *spi)
> > if (err)
> > return err;
> >
> > - err = st_accel_common_probe(indio_dev);
> > - if (err < 0)
> > - goto st_accel_power_off;
> > -
> > - return 0;
> > -
> > -st_accel_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return err;
> > + return st_accel_common_probe(indio_dev);
> > }
> >
> > static int st_accel_spi_remove(struct spi_device *spi)
> > {
> > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_accel_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> > index 0bbb090b108c..a5a140de9a23 100644
> > --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> > @@ -215,13 +215,19 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
> > }
> > EXPORT_SYMBOL(st_sensors_set_axis_enable);
> >
> > +static void st_reg_disable(void *reg)
> > +{
> > + regulator_disable(reg);
> > +}
> > +
> > int st_sensors_power_enable(struct iio_dev *indio_dev)
> > {
> > struct st_sensor_data *pdata = iio_priv(indio_dev);
> > + struct device *parent = indio_dev->dev.parent;
> > int err;
> >
> > /* Regulators not mandatory, but if requested we should enable them. */
> > - pdata->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
> > + pdata->vdd = devm_regulator_get(parent, "vdd");
> > if (IS_ERR(pdata->vdd)) {
> > dev_err(&indio_dev->dev, "unable to get Vdd supply\n");
> > return PTR_ERR(pdata->vdd);
> > @@ -233,36 +239,26 @@ int st_sensors_power_enable(struct iio_dev *indio_dev)
> > return err;
> > }
> >
> > - pdata->vdd_io = devm_regulator_get(indio_dev->dev.parent, "vddio");
> > + err = devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd);
> > + if (err)
> > + return err;
> > +
> > + pdata->vdd_io = devm_regulator_get(parent, "vddio");
> > if (IS_ERR(pdata->vdd_io)) {
> > dev_err(&indio_dev->dev, "unable to get Vdd_IO supply\n");
> > - err = PTR_ERR(pdata->vdd_io);
> > - goto st_sensors_disable_vdd;
> > + return PTR_ERR(pdata->vdd_io);
> > }
> > err = regulator_enable(pdata->vdd_io);
> > if (err != 0) {
> > dev_warn(&indio_dev->dev,
> > "Failed to enable specified Vdd_IO supply\n");
> > - goto st_sensors_disable_vdd;
> > + return err;
> > }
> >
> > - return 0;
> > -
> > -st_sensors_disable_vdd:
> > - regulator_disable(pdata->vdd);
> > - return err;
> > + return devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd_io);
> > }
> > EXPORT_SYMBOL(st_sensors_power_enable);
> >
> > -void st_sensors_power_disable(struct iio_dev *indio_dev)
> > -{
> > - struct st_sensor_data *pdata = iio_priv(indio_dev);
> > -
> > - regulator_disable(pdata->vdd);
> > - regulator_disable(pdata->vdd_io);
> > -}
> > -EXPORT_SYMBOL(st_sensors_power_disable);
> > -
> > static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
> > struct st_sensors_platform_data *pdata)
> > {
> > diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
> > index 3ef86e16ee65..0bd80dfd389f 100644
> > --- a/drivers/iio/gyro/st_gyro_i2c.c
> > +++ b/drivers/iio/gyro/st_gyro_i2c.c
> > @@ -90,24 +90,13 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
> > if (err)
> > return err;
> >
> > - err = st_gyro_common_probe(indio_dev);
> > - if (err < 0)
> > - goto st_gyro_power_off;
> > -
> > - return 0;
> > -
> > -st_gyro_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return err;
> > + return st_gyro_common_probe(indio_dev);
> > }
> >
> > static int st_gyro_i2c_remove(struct i2c_client *client)
> > {
> > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_gyro_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
> > index 41d835493347..f74b09fa5cde 100644
> > --- a/drivers/iio/gyro/st_gyro_spi.c
> > +++ b/drivers/iio/gyro/st_gyro_spi.c
> > @@ -94,24 +94,13 @@ static int st_gyro_spi_probe(struct spi_device *spi)
> > if (err)
> > return err;
> >
> > - err = st_gyro_common_probe(indio_dev);
> > - if (err < 0)
> > - goto st_gyro_power_off;
> > -
> > - return 0;
> > -
> > -st_gyro_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return err;
> > + return st_gyro_common_probe(indio_dev);
> > }
> >
> > static int st_gyro_spi_remove(struct spi_device *spi)
> > {
> > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_gyro_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> > index 2dfe4ee99591..0a5117dffcf4 100644
> > --- a/drivers/iio/magnetometer/st_magn_i2c.c
> > +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> > @@ -86,24 +86,13 @@ static int st_magn_i2c_probe(struct i2c_client *client,
> > if (err)
> > return err;
> >
> > - err = st_magn_common_probe(indio_dev);
> > - if (err < 0)
> > - goto st_magn_power_off;
> > -
> > - return 0;
> > -
> > -st_magn_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return err;
> > + return st_magn_common_probe(indio_dev);
> > }
> >
> > static int st_magn_i2c_remove(struct i2c_client *client)
> > {
> > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_magn_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> > index fba978796395..1f3bf02b24e0 100644
> > --- a/drivers/iio/magnetometer/st_magn_spi.c
> > +++ b/drivers/iio/magnetometer/st_magn_spi.c
> > @@ -80,24 +80,13 @@ static int st_magn_spi_probe(struct spi_device *spi)
> > if (err)
> > return err;
> >
> > - err = st_magn_common_probe(indio_dev);
> > - if (err < 0)
> > - goto st_magn_power_off;
> > -
> > - return 0;
> > -
> > -st_magn_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return err;
> > + return st_magn_common_probe(indio_dev);
> > }
> >
> > static int st_magn_spi_remove(struct spi_device *spi)
> > {
> > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_magn_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> > index 52fa98f24478..afeeab485c0d 100644
> > --- a/drivers/iio/pressure/st_pressure_i2c.c
> > +++ b/drivers/iio/pressure/st_pressure_i2c.c
> > @@ -103,24 +103,13 @@ static int st_press_i2c_probe(struct i2c_client *client,
> > if (ret)
> > return ret;
> >
> > - ret = st_press_common_probe(indio_dev);
> > - if (ret < 0)
> > - goto st_press_power_off;
> > -
> > - return 0;
> > -
> > -st_press_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return ret;
> > + return st_press_common_probe(indio_dev);
> > }
> >
> > static int st_press_i2c_remove(struct i2c_client *client)
> > {
> > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_press_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> > index ee393df54cee..834ad6d40a70 100644
> > --- a/drivers/iio/pressure/st_pressure_spi.c
> > +++ b/drivers/iio/pressure/st_pressure_spi.c
> > @@ -86,24 +86,13 @@ static int st_press_spi_probe(struct spi_device *spi)
> > if (err)
> > return err;
> >
> > - err = st_press_common_probe(indio_dev);
> > - if (err < 0)
> > - goto st_press_power_off;
> > -
> > - return 0;
> > -
> > -st_press_power_off:
> > - st_sensors_power_disable(indio_dev);
> > -
> > - return err;
> > + return st_press_common_probe(indio_dev);
> > }
> >
> > static int st_press_spi_remove(struct spi_device *spi)
> > {
> > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> >
> > - st_sensors_power_disable(indio_dev);
> > -
> > st_press_common_remove(indio_dev);
> >
> > return 0;
> > diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> > index e74b55244f35..fc90c202d15e 100644
> > --- a/include/linux/iio/common/st_sensors.h
> > +++ b/include/linux/iio/common/st_sensors.h
> > @@ -293,8 +293,6 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
> >
> > int st_sensors_power_enable(struct iio_dev *indio_dev);
> >
> > -void st_sensors_power_disable(struct iio_dev *indio_dev);
> > -
> > int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev,
> > unsigned reg, unsigned writeval,
> > unsigned *readval);
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function
2021-08-02 7:30 ` Alexandru Ardelean
@ 2021-08-16 7:48 ` Alexandru Ardelean
2021-08-16 8:53 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Alexandru Ardelean @ 2021-08-16 7:48 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Linux Kernel Mailing List, denis.ciocca
On Mon, 2 Aug 2021 at 10:30, Alexandru Ardelean <aardelean@deviqon.com> wrote:
>
> On Sat, 31 Jul 2021 at 20:34, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Mon, 26 Jul 2021 10:14:02 +0300
> > Alexandru Ardelean <aardelean@deviqon.com> wrote:
> >
> > > This change converts the st_sensors_power_enable() function to use
> > > devm_add_action_or_reset() handlers to register regulator_disable hooks for
> > > when the drivers get unloaded.
> > >
> > > The parent device of the IIO device object is used. This is based on the
> > > assumption that all other devm_ calls in the ST sensors use this reference.
> > >
> > > This makes the st_sensors_power_disable() un-needed.
> > > Removing this also changes unload order a bit, as all ST drivers would call
> > > st_sensors_power_disable() first and iio_device_unregister() after that.
> >
> > Huh. So currently the drivers turn the power off before calling
> > iio_device_unregister()?
> >
> > That's a bad idea (turn off power when userspace is still available)
> > and looks like a bug fix to me. Perhaps the reorder should be pulled out as
> > a precursor patch in case anyone wants to backport it?
> >
> > (note I initially thought you were going the other way and was just writing
> > a rant about breaking the order when I realised you were fixing it ;)
I'm seeing a bit of a clash with this patch [by Andy from April 2021]:
https://lore.kernel.org/linux-devicetree/20210414195454.84183-4-andriy.shevchenko@linux.intel.com/
I can do the fix commits, but they will be short-lived, since they
will be applied on 5.13 and onwards.
We'd need maybe another set for the LTS kernels.
So, how should we proceed here?
I think I'll do the fix commits anyway for this set.
Thanks
Alex
>
> Probably the best advantage of devm_ functions .
> Not needing to care about un-init order.
>
> I hated debugging driver crashes when they were unloaded, usually
> because the driver gets into a weird state.
>
> I'll send a V2 with the re-order fixes in the front.
>
> >
> > >
> > > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > > ---
> > > drivers/iio/accel/st_accel_i2c.c | 13 +------
> > > drivers/iio/accel/st_accel_spi.c | 13 +------
> > > .../iio/common/st_sensors/st_sensors_core.c | 34 ++++++++-----------
> > > drivers/iio/gyro/st_gyro_i2c.c | 13 +------
> > > drivers/iio/gyro/st_gyro_spi.c | 13 +------
> > > drivers/iio/magnetometer/st_magn_i2c.c | 13 +------
> > > drivers/iio/magnetometer/st_magn_spi.c | 13 +------
> > > drivers/iio/pressure/st_pressure_i2c.c | 13 +------
> > > drivers/iio/pressure/st_pressure_spi.c | 13 +------
> > > include/linux/iio/common/st_sensors.h | 2 --
> > > 10 files changed, 23 insertions(+), 117 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> > > index f711756e41e3..b377575efc41 100644
> > > --- a/drivers/iio/accel/st_accel_i2c.c
> > > +++ b/drivers/iio/accel/st_accel_i2c.c
> > > @@ -177,24 +177,13 @@ static int st_accel_i2c_probe(struct i2c_client *client)
> > > if (ret)
> > > return ret;
> > >
> > > - ret = st_accel_common_probe(indio_dev);
> > > - if (ret < 0)
> > > - goto st_accel_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_accel_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return ret;
> > > + return st_accel_common_probe(indio_dev);
> > > }
> > >
> > > static int st_accel_i2c_remove(struct i2c_client *client)
> > > {
> > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_accel_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> > > index bb45d9ff95b8..4ca87e73bdb3 100644
> > > --- a/drivers/iio/accel/st_accel_spi.c
> > > +++ b/drivers/iio/accel/st_accel_spi.c
> > > @@ -127,24 +127,13 @@ static int st_accel_spi_probe(struct spi_device *spi)
> > > if (err)
> > > return err;
> > >
> > > - err = st_accel_common_probe(indio_dev);
> > > - if (err < 0)
> > > - goto st_accel_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_accel_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return err;
> > > + return st_accel_common_probe(indio_dev);
> > > }
> > >
> > > static int st_accel_spi_remove(struct spi_device *spi)
> > > {
> > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_accel_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> > > index 0bbb090b108c..a5a140de9a23 100644
> > > --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> > > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> > > @@ -215,13 +215,19 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
> > > }
> > > EXPORT_SYMBOL(st_sensors_set_axis_enable);
> > >
> > > +static void st_reg_disable(void *reg)
> > > +{
> > > + regulator_disable(reg);
> > > +}
> > > +
> > > int st_sensors_power_enable(struct iio_dev *indio_dev)
> > > {
> > > struct st_sensor_data *pdata = iio_priv(indio_dev);
> > > + struct device *parent = indio_dev->dev.parent;
> > > int err;
> > >
> > > /* Regulators not mandatory, but if requested we should enable them. */
> > > - pdata->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
> > > + pdata->vdd = devm_regulator_get(parent, "vdd");
> > > if (IS_ERR(pdata->vdd)) {
> > > dev_err(&indio_dev->dev, "unable to get Vdd supply\n");
> > > return PTR_ERR(pdata->vdd);
> > > @@ -233,36 +239,26 @@ int st_sensors_power_enable(struct iio_dev *indio_dev)
> > > return err;
> > > }
> > >
> > > - pdata->vdd_io = devm_regulator_get(indio_dev->dev.parent, "vddio");
> > > + err = devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd);
> > > + if (err)
> > > + return err;
> > > +
> > > + pdata->vdd_io = devm_regulator_get(parent, "vddio");
> > > if (IS_ERR(pdata->vdd_io)) {
> > > dev_err(&indio_dev->dev, "unable to get Vdd_IO supply\n");
> > > - err = PTR_ERR(pdata->vdd_io);
> > > - goto st_sensors_disable_vdd;
> > > + return PTR_ERR(pdata->vdd_io);
> > > }
> > > err = regulator_enable(pdata->vdd_io);
> > > if (err != 0) {
> > > dev_warn(&indio_dev->dev,
> > > "Failed to enable specified Vdd_IO supply\n");
> > > - goto st_sensors_disable_vdd;
> > > + return err;
> > > }
> > >
> > > - return 0;
> > > -
> > > -st_sensors_disable_vdd:
> > > - regulator_disable(pdata->vdd);
> > > - return err;
> > > + return devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd_io);
> > > }
> > > EXPORT_SYMBOL(st_sensors_power_enable);
> > >
> > > -void st_sensors_power_disable(struct iio_dev *indio_dev)
> > > -{
> > > - struct st_sensor_data *pdata = iio_priv(indio_dev);
> > > -
> > > - regulator_disable(pdata->vdd);
> > > - regulator_disable(pdata->vdd_io);
> > > -}
> > > -EXPORT_SYMBOL(st_sensors_power_disable);
> > > -
> > > static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
> > > struct st_sensors_platform_data *pdata)
> > > {
> > > diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
> > > index 3ef86e16ee65..0bd80dfd389f 100644
> > > --- a/drivers/iio/gyro/st_gyro_i2c.c
> > > +++ b/drivers/iio/gyro/st_gyro_i2c.c
> > > @@ -90,24 +90,13 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
> > > if (err)
> > > return err;
> > >
> > > - err = st_gyro_common_probe(indio_dev);
> > > - if (err < 0)
> > > - goto st_gyro_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_gyro_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return err;
> > > + return st_gyro_common_probe(indio_dev);
> > > }
> > >
> > > static int st_gyro_i2c_remove(struct i2c_client *client)
> > > {
> > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_gyro_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
> > > index 41d835493347..f74b09fa5cde 100644
> > > --- a/drivers/iio/gyro/st_gyro_spi.c
> > > +++ b/drivers/iio/gyro/st_gyro_spi.c
> > > @@ -94,24 +94,13 @@ static int st_gyro_spi_probe(struct spi_device *spi)
> > > if (err)
> > > return err;
> > >
> > > - err = st_gyro_common_probe(indio_dev);
> > > - if (err < 0)
> > > - goto st_gyro_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_gyro_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return err;
> > > + return st_gyro_common_probe(indio_dev);
> > > }
> > >
> > > static int st_gyro_spi_remove(struct spi_device *spi)
> > > {
> > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_gyro_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> > > index 2dfe4ee99591..0a5117dffcf4 100644
> > > --- a/drivers/iio/magnetometer/st_magn_i2c.c
> > > +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> > > @@ -86,24 +86,13 @@ static int st_magn_i2c_probe(struct i2c_client *client,
> > > if (err)
> > > return err;
> > >
> > > - err = st_magn_common_probe(indio_dev);
> > > - if (err < 0)
> > > - goto st_magn_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_magn_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return err;
> > > + return st_magn_common_probe(indio_dev);
> > > }
> > >
> > > static int st_magn_i2c_remove(struct i2c_client *client)
> > > {
> > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_magn_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> > > index fba978796395..1f3bf02b24e0 100644
> > > --- a/drivers/iio/magnetometer/st_magn_spi.c
> > > +++ b/drivers/iio/magnetometer/st_magn_spi.c
> > > @@ -80,24 +80,13 @@ static int st_magn_spi_probe(struct spi_device *spi)
> > > if (err)
> > > return err;
> > >
> > > - err = st_magn_common_probe(indio_dev);
> > > - if (err < 0)
> > > - goto st_magn_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_magn_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return err;
> > > + return st_magn_common_probe(indio_dev);
> > > }
> > >
> > > static int st_magn_spi_remove(struct spi_device *spi)
> > > {
> > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_magn_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> > > index 52fa98f24478..afeeab485c0d 100644
> > > --- a/drivers/iio/pressure/st_pressure_i2c.c
> > > +++ b/drivers/iio/pressure/st_pressure_i2c.c
> > > @@ -103,24 +103,13 @@ static int st_press_i2c_probe(struct i2c_client *client,
> > > if (ret)
> > > return ret;
> > >
> > > - ret = st_press_common_probe(indio_dev);
> > > - if (ret < 0)
> > > - goto st_press_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_press_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return ret;
> > > + return st_press_common_probe(indio_dev);
> > > }
> > >
> > > static int st_press_i2c_remove(struct i2c_client *client)
> > > {
> > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_press_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> > > index ee393df54cee..834ad6d40a70 100644
> > > --- a/drivers/iio/pressure/st_pressure_spi.c
> > > +++ b/drivers/iio/pressure/st_pressure_spi.c
> > > @@ -86,24 +86,13 @@ static int st_press_spi_probe(struct spi_device *spi)
> > > if (err)
> > > return err;
> > >
> > > - err = st_press_common_probe(indio_dev);
> > > - if (err < 0)
> > > - goto st_press_power_off;
> > > -
> > > - return 0;
> > > -
> > > -st_press_power_off:
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > - return err;
> > > + return st_press_common_probe(indio_dev);
> > > }
> > >
> > > static int st_press_spi_remove(struct spi_device *spi)
> > > {
> > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > >
> > > - st_sensors_power_disable(indio_dev);
> > > -
> > > st_press_common_remove(indio_dev);
> > >
> > > return 0;
> > > diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> > > index e74b55244f35..fc90c202d15e 100644
> > > --- a/include/linux/iio/common/st_sensors.h
> > > +++ b/include/linux/iio/common/st_sensors.h
> > > @@ -293,8 +293,6 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
> > >
> > > int st_sensors_power_enable(struct iio_dev *indio_dev);
> > >
> > > -void st_sensors_power_disable(struct iio_dev *indio_dev);
> > > -
> > > int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev,
> > > unsigned reg, unsigned writeval,
> > > unsigned *readval);
> >
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function
2021-08-16 7:48 ` Alexandru Ardelean
@ 2021-08-16 8:53 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-08-16 8:53 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, Linux Kernel Mailing List, denis.ciocca
On Mon, 16 Aug 2021 10:48:16 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:
> On Mon, 2 Aug 2021 at 10:30, Alexandru Ardelean <aardelean@deviqon.com> wrote:
> >
> > On Sat, 31 Jul 2021 at 20:34, Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Mon, 26 Jul 2021 10:14:02 +0300
> > > Alexandru Ardelean <aardelean@deviqon.com> wrote:
> > >
> > > > This change converts the st_sensors_power_enable() function to use
> > > > devm_add_action_or_reset() handlers to register regulator_disable hooks for
> > > > when the drivers get unloaded.
> > > >
> > > > The parent device of the IIO device object is used. This is based on the
> > > > assumption that all other devm_ calls in the ST sensors use this reference.
> > > >
> > > > This makes the st_sensors_power_disable() un-needed.
> > > > Removing this also changes unload order a bit, as all ST drivers would call
> > > > st_sensors_power_disable() first and iio_device_unregister() after that.
> > >
> > > Huh. So currently the drivers turn the power off before calling
> > > iio_device_unregister()?
> > >
> > > That's a bad idea (turn off power when userspace is still available)
> > > and looks like a bug fix to me. Perhaps the reorder should be pulled out as
> > > a precursor patch in case anyone wants to backport it?
> > >
> > > (note I initially thought you were going the other way and was just writing
> > > a rant about breaking the order when I realised you were fixing it ;)
>
> I'm seeing a bit of a clash with this patch [by Andy from April 2021]:
>
> https://lore.kernel.org/linux-devicetree/20210414195454.84183-4-andriy.shevchenko@linux.intel.com/
>
> I can do the fix commits, but they will be short-lived, since they
> will be applied on 5.13 and onwards.
> We'd need maybe another set for the LTS kernels.
>
> So, how should we proceed here?
> I think I'll do the fix commits anyway for this set.
Carry on with pulling this out as a fix. That at least gives
us a clear patch to point at when doing any necessary backports.
Whilst this particular ordering thing is a potential issue, it's
in a remove path that won't get heavily used and is a race
that may or may not actually cause problems.
The main issue with the current ordering is the difficulty of
being sure it is right. So I would not worry that much about
manual backports unless someone asks.
Jonathan
>
> Thanks
> Alex
>
> >
> > Probably the best advantage of devm_ functions .
> > Not needing to care about un-init order.
> >
> > I hated debugging driver crashes when they were unloaded, usually
> > because the driver gets into a weird state.
> >
> > I'll send a V2 with the re-order fixes in the front.
> >
> > >
> > > >
> > > > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > > > ---
> > > > drivers/iio/accel/st_accel_i2c.c | 13 +------
> > > > drivers/iio/accel/st_accel_spi.c | 13 +------
> > > > .../iio/common/st_sensors/st_sensors_core.c | 34 ++++++++-----------
> > > > drivers/iio/gyro/st_gyro_i2c.c | 13 +------
> > > > drivers/iio/gyro/st_gyro_spi.c | 13 +------
> > > > drivers/iio/magnetometer/st_magn_i2c.c | 13 +------
> > > > drivers/iio/magnetometer/st_magn_spi.c | 13 +------
> > > > drivers/iio/pressure/st_pressure_i2c.c | 13 +------
> > > > drivers/iio/pressure/st_pressure_spi.c | 13 +------
> > > > include/linux/iio/common/st_sensors.h | 2 --
> > > > 10 files changed, 23 insertions(+), 117 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> > > > index f711756e41e3..b377575efc41 100644
> > > > --- a/drivers/iio/accel/st_accel_i2c.c
> > > > +++ b/drivers/iio/accel/st_accel_i2c.c
> > > > @@ -177,24 +177,13 @@ static int st_accel_i2c_probe(struct i2c_client *client)
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - ret = st_accel_common_probe(indio_dev);
> > > > - if (ret < 0)
> > > > - goto st_accel_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_accel_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return ret;
> > > > + return st_accel_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_accel_i2c_remove(struct i2c_client *client)
> > > > {
> > > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_accel_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> > > > index bb45d9ff95b8..4ca87e73bdb3 100644
> > > > --- a/drivers/iio/accel/st_accel_spi.c
> > > > +++ b/drivers/iio/accel/st_accel_spi.c
> > > > @@ -127,24 +127,13 @@ static int st_accel_spi_probe(struct spi_device *spi)
> > > > if (err)
> > > > return err;
> > > >
> > > > - err = st_accel_common_probe(indio_dev);
> > > > - if (err < 0)
> > > > - goto st_accel_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_accel_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return err;
> > > > + return st_accel_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_accel_spi_remove(struct spi_device *spi)
> > > > {
> > > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_accel_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> > > > index 0bbb090b108c..a5a140de9a23 100644
> > > > --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> > > > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> > > > @@ -215,13 +215,19 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
> > > > }
> > > > EXPORT_SYMBOL(st_sensors_set_axis_enable);
> > > >
> > > > +static void st_reg_disable(void *reg)
> > > > +{
> > > > + regulator_disable(reg);
> > > > +}
> > > > +
> > > > int st_sensors_power_enable(struct iio_dev *indio_dev)
> > > > {
> > > > struct st_sensor_data *pdata = iio_priv(indio_dev);
> > > > + struct device *parent = indio_dev->dev.parent;
> > > > int err;
> > > >
> > > > /* Regulators not mandatory, but if requested we should enable them. */
> > > > - pdata->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
> > > > + pdata->vdd = devm_regulator_get(parent, "vdd");
> > > > if (IS_ERR(pdata->vdd)) {
> > > > dev_err(&indio_dev->dev, "unable to get Vdd supply\n");
> > > > return PTR_ERR(pdata->vdd);
> > > > @@ -233,36 +239,26 @@ int st_sensors_power_enable(struct iio_dev *indio_dev)
> > > > return err;
> > > > }
> > > >
> > > > - pdata->vdd_io = devm_regulator_get(indio_dev->dev.parent, "vddio");
> > > > + err = devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + pdata->vdd_io = devm_regulator_get(parent, "vddio");
> > > > if (IS_ERR(pdata->vdd_io)) {
> > > > dev_err(&indio_dev->dev, "unable to get Vdd_IO supply\n");
> > > > - err = PTR_ERR(pdata->vdd_io);
> > > > - goto st_sensors_disable_vdd;
> > > > + return PTR_ERR(pdata->vdd_io);
> > > > }
> > > > err = regulator_enable(pdata->vdd_io);
> > > > if (err != 0) {
> > > > dev_warn(&indio_dev->dev,
> > > > "Failed to enable specified Vdd_IO supply\n");
> > > > - goto st_sensors_disable_vdd;
> > > > + return err;
> > > > }
> > > >
> > > > - return 0;
> > > > -
> > > > -st_sensors_disable_vdd:
> > > > - regulator_disable(pdata->vdd);
> > > > - return err;
> > > > + return devm_add_action_or_reset(parent, st_reg_disable, pdata->vdd_io);
> > > > }
> > > > EXPORT_SYMBOL(st_sensors_power_enable);
> > > >
> > > > -void st_sensors_power_disable(struct iio_dev *indio_dev)
> > > > -{
> > > > - struct st_sensor_data *pdata = iio_priv(indio_dev);
> > > > -
> > > > - regulator_disable(pdata->vdd);
> > > > - regulator_disable(pdata->vdd_io);
> > > > -}
> > > > -EXPORT_SYMBOL(st_sensors_power_disable);
> > > > -
> > > > static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
> > > > struct st_sensors_platform_data *pdata)
> > > > {
> > > > diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
> > > > index 3ef86e16ee65..0bd80dfd389f 100644
> > > > --- a/drivers/iio/gyro/st_gyro_i2c.c
> > > > +++ b/drivers/iio/gyro/st_gyro_i2c.c
> > > > @@ -90,24 +90,13 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
> > > > if (err)
> > > > return err;
> > > >
> > > > - err = st_gyro_common_probe(indio_dev);
> > > > - if (err < 0)
> > > > - goto st_gyro_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_gyro_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return err;
> > > > + return st_gyro_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_gyro_i2c_remove(struct i2c_client *client)
> > > > {
> > > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_gyro_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
> > > > index 41d835493347..f74b09fa5cde 100644
> > > > --- a/drivers/iio/gyro/st_gyro_spi.c
> > > > +++ b/drivers/iio/gyro/st_gyro_spi.c
> > > > @@ -94,24 +94,13 @@ static int st_gyro_spi_probe(struct spi_device *spi)
> > > > if (err)
> > > > return err;
> > > >
> > > > - err = st_gyro_common_probe(indio_dev);
> > > > - if (err < 0)
> > > > - goto st_gyro_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_gyro_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return err;
> > > > + return st_gyro_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_gyro_spi_remove(struct spi_device *spi)
> > > > {
> > > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_gyro_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
> > > > index 2dfe4ee99591..0a5117dffcf4 100644
> > > > --- a/drivers/iio/magnetometer/st_magn_i2c.c
> > > > +++ b/drivers/iio/magnetometer/st_magn_i2c.c
> > > > @@ -86,24 +86,13 @@ static int st_magn_i2c_probe(struct i2c_client *client,
> > > > if (err)
> > > > return err;
> > > >
> > > > - err = st_magn_common_probe(indio_dev);
> > > > - if (err < 0)
> > > > - goto st_magn_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_magn_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return err;
> > > > + return st_magn_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_magn_i2c_remove(struct i2c_client *client)
> > > > {
> > > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_magn_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
> > > > index fba978796395..1f3bf02b24e0 100644
> > > > --- a/drivers/iio/magnetometer/st_magn_spi.c
> > > > +++ b/drivers/iio/magnetometer/st_magn_spi.c
> > > > @@ -80,24 +80,13 @@ static int st_magn_spi_probe(struct spi_device *spi)
> > > > if (err)
> > > > return err;
> > > >
> > > > - err = st_magn_common_probe(indio_dev);
> > > > - if (err < 0)
> > > > - goto st_magn_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_magn_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return err;
> > > > + return st_magn_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_magn_spi_remove(struct spi_device *spi)
> > > > {
> > > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_magn_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> > > > index 52fa98f24478..afeeab485c0d 100644
> > > > --- a/drivers/iio/pressure/st_pressure_i2c.c
> > > > +++ b/drivers/iio/pressure/st_pressure_i2c.c
> > > > @@ -103,24 +103,13 @@ static int st_press_i2c_probe(struct i2c_client *client,
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - ret = st_press_common_probe(indio_dev);
> > > > - if (ret < 0)
> > > > - goto st_press_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_press_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return ret;
> > > > + return st_press_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_press_i2c_remove(struct i2c_client *client)
> > > > {
> > > > struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_press_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> > > > index ee393df54cee..834ad6d40a70 100644
> > > > --- a/drivers/iio/pressure/st_pressure_spi.c
> > > > +++ b/drivers/iio/pressure/st_pressure_spi.c
> > > > @@ -86,24 +86,13 @@ static int st_press_spi_probe(struct spi_device *spi)
> > > > if (err)
> > > > return err;
> > > >
> > > > - err = st_press_common_probe(indio_dev);
> > > > - if (err < 0)
> > > > - goto st_press_power_off;
> > > > -
> > > > - return 0;
> > > > -
> > > > -st_press_power_off:
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > - return err;
> > > > + return st_press_common_probe(indio_dev);
> > > > }
> > > >
> > > > static int st_press_spi_remove(struct spi_device *spi)
> > > > {
> > > > struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > > >
> > > > - st_sensors_power_disable(indio_dev);
> > > > -
> > > > st_press_common_remove(indio_dev);
> > > >
> > > > return 0;
> > > > diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> > > > index e74b55244f35..fc90c202d15e 100644
> > > > --- a/include/linux/iio/common/st_sensors.h
> > > > +++ b/include/linux/iio/common/st_sensors.h
> > > > @@ -293,8 +293,6 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
> > > >
> > > > int st_sensors_power_enable(struct iio_dev *indio_dev);
> > > >
> > > > -void st_sensors_power_disable(struct iio_dev *indio_dev);
> > > > -
> > > > int st_sensors_debugfs_reg_access(struct iio_dev *indio_dev,
> > > > unsigned reg, unsigned writeval,
> > > > unsigned *readval);
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] iio: st_sensors: remove all driver remove functions
2021-07-26 7:14 [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Alexandru Ardelean
2021-07-26 7:14 ` [PATCH 1/4] iio: st_sensors: remove st_sensors_deallocate_trigger() function Alexandru Ardelean
2021-07-26 7:14 ` [PATCH 2/4] iio: st_sensors: remove st_sensors_power_disable() function Alexandru Ardelean
@ 2021-07-26 7:14 ` Alexandru Ardelean
2021-07-26 7:20 ` Andy Shevchenko
2021-07-26 7:14 ` [PATCH 4/4] iio: st_sensors: remove reference to parent device object on st_sensor_data Alexandru Ardelean
2021-07-31 17:40 ` [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Jonathan Cameron
4 siblings, 1 reply; 11+ messages in thread
From: Alexandru Ardelean @ 2021-07-26 7:14 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: denis.ciocca, jic23, Alexandru Ardelean
At this point all ST driver remove functions do iio_device_unregister().
This change removes them from them and replaces all iio_device_register()
with devm_iio_device_register().
This can be done in a single change relatively easy, since all these remove
functions are define in st_sensors.h.
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
drivers/iio/accel/st_accel_core.c | 9 ++-------
drivers/iio/accel/st_accel_i2c.c | 10 ----------
drivers/iio/accel/st_accel_spi.c | 10 ----------
drivers/iio/gyro/st_gyro_core.c | 9 ++-------
drivers/iio/gyro/st_gyro_i2c.c | 10 ----------
drivers/iio/gyro/st_gyro_spi.c | 10 ----------
drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h | 1 -
drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c | 15 +--------------
drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c | 6 ------
drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c | 6 ------
drivers/iio/magnetometer/st_magn_core.c | 9 ++-------
drivers/iio/magnetometer/st_magn_i2c.c | 10 ----------
drivers/iio/magnetometer/st_magn_spi.c | 10 ----------
drivers/iio/pressure/st_pressure_core.c | 9 ++-------
drivers/iio/pressure/st_pressure_i2c.c | 10 ----------
drivers/iio/pressure/st_pressure_spi.c | 10 ----------
include/linux/iio/common/st_sensors.h | 4 ----
17 files changed, 9 insertions(+), 139 deletions(-)
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index a7be1633bff1..01695abd9d2f 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -1335,6 +1335,7 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *adata = iio_priv(indio_dev);
struct st_sensors_platform_data *pdata = dev_get_platdata(adata->dev);
+ struct device *parent = indio_dev->dev.parent;
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -1380,16 +1381,10 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
return err;
}
- return iio_device_register(indio_dev);
+ return devm_iio_device_register(parent, indio_dev);
}
EXPORT_SYMBOL(st_accel_common_probe);
-void st_accel_common_remove(struct iio_dev *indio_dev)
-{
- iio_device_unregister(indio_dev);
-}
-EXPORT_SYMBOL(st_accel_common_remove);
-
MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
MODULE_DESCRIPTION("STMicroelectronics accelerometers driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
index b377575efc41..c0ce78eebad9 100644
--- a/drivers/iio/accel/st_accel_i2c.c
+++ b/drivers/iio/accel/st_accel_i2c.c
@@ -180,15 +180,6 @@ static int st_accel_i2c_probe(struct i2c_client *client)
return st_accel_common_probe(indio_dev);
}
-static int st_accel_i2c_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
-
- st_accel_common_remove(indio_dev);
-
- return 0;
-}
-
static struct i2c_driver st_accel_driver = {
.driver = {
.name = "st-accel-i2c",
@@ -196,7 +187,6 @@ static struct i2c_driver st_accel_driver = {
.acpi_match_table = ACPI_PTR(st_accel_acpi_match),
},
.probe_new = st_accel_i2c_probe,
- .remove = st_accel_i2c_remove,
.id_table = st_accel_id_table,
};
module_i2c_driver(st_accel_driver);
diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
index 4ca87e73bdb3..b74a1c6d03de 100644
--- a/drivers/iio/accel/st_accel_spi.c
+++ b/drivers/iio/accel/st_accel_spi.c
@@ -130,15 +130,6 @@ static int st_accel_spi_probe(struct spi_device *spi)
return st_accel_common_probe(indio_dev);
}
-static int st_accel_spi_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
- st_accel_common_remove(indio_dev);
-
- return 0;
-}
-
static const struct spi_device_id st_accel_id_table[] = {
{ LIS3DH_ACCEL_DEV_NAME },
{ LSM330D_ACCEL_DEV_NAME },
@@ -166,7 +157,6 @@ static struct spi_driver st_accel_driver = {
.of_match_table = st_accel_of_match,
},
.probe = st_accel_spi_probe,
- .remove = st_accel_spi_remove,
.id_table = st_accel_id_table,
};
module_spi_driver(st_accel_driver);
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index cb539b47cdf4..3609082a6778 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -478,6 +478,7 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *gdata = iio_priv(indio_dev);
struct st_sensors_platform_data *pdata;
+ struct device *parent = indio_dev->dev.parent;
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -515,16 +516,10 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
return err;
}
- return iio_device_register(indio_dev);
+ return devm_iio_device_register(parent, indio_dev);
}
EXPORT_SYMBOL(st_gyro_common_probe);
-void st_gyro_common_remove(struct iio_dev *indio_dev)
-{
- iio_device_unregister(indio_dev);
-}
-EXPORT_SYMBOL(st_gyro_common_remove);
-
MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
MODULE_DESCRIPTION("STMicroelectronics gyroscopes driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
index 0bd80dfd389f..163c7ba300c1 100644
--- a/drivers/iio/gyro/st_gyro_i2c.c
+++ b/drivers/iio/gyro/st_gyro_i2c.c
@@ -93,15 +93,6 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
return st_gyro_common_probe(indio_dev);
}
-static int st_gyro_i2c_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
-
- st_gyro_common_remove(indio_dev);
-
- return 0;
-}
-
static const struct i2c_device_id st_gyro_id_table[] = {
{ L3G4200D_GYRO_DEV_NAME },
{ LSM330D_GYRO_DEV_NAME },
@@ -122,7 +113,6 @@ static struct i2c_driver st_gyro_driver = {
.of_match_table = st_gyro_of_match,
},
.probe = st_gyro_i2c_probe,
- .remove = st_gyro_i2c_remove,
.id_table = st_gyro_id_table,
};
module_i2c_driver(st_gyro_driver);
diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
index f74b09fa5cde..b0023f9b9771 100644
--- a/drivers/iio/gyro/st_gyro_spi.c
+++ b/drivers/iio/gyro/st_gyro_spi.c
@@ -97,15 +97,6 @@ static int st_gyro_spi_probe(struct spi_device *spi)
return st_gyro_common_probe(indio_dev);
}
-static int st_gyro_spi_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
- st_gyro_common_remove(indio_dev);
-
- return 0;
-}
-
static const struct spi_device_id st_gyro_id_table[] = {
{ L3G4200D_GYRO_DEV_NAME },
{ LSM330D_GYRO_DEV_NAME },
@@ -126,7 +117,6 @@ static struct spi_driver st_gyro_driver = {
.of_match_table = st_gyro_of_match,
},
.probe = st_gyro_spi_probe,
- .remove = st_gyro_spi_remove,
.id_table = st_gyro_id_table,
};
module_spi_driver(st_gyro_driver);
diff --git a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h
index 146393afd9a7..76678cdefb07 100644
--- a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h
+++ b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h
@@ -18,6 +18,5 @@ struct st_lsm9ds0 {
};
int st_lsm9ds0_probe(struct st_lsm9ds0 *lsm9ds0, struct regmap *regmap);
-int st_lsm9ds0_remove(struct st_lsm9ds0 *lsm9ds0);
#endif /* ST_LSM9DS0_H */
diff --git a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c
index 5e6625140db7..d276f663fe57 100644
--- a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c
+++ b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c
@@ -142,23 +142,10 @@ int st_lsm9ds0_probe(struct st_lsm9ds0 *lsm9ds0, struct regmap *regmap)
return ret;
/* Setup magnetometer device */
- ret = st_lsm9ds0_probe_magn(lsm9ds0, regmap);
- if (ret)
- st_accel_common_remove(lsm9ds0->accel);
-
- return ret;
+ return st_lsm9ds0_probe_magn(lsm9ds0, regmap);
}
EXPORT_SYMBOL_GPL(st_lsm9ds0_probe);
-int st_lsm9ds0_remove(struct st_lsm9ds0 *lsm9ds0)
-{
- st_magn_common_remove(lsm9ds0->magn);
- st_accel_common_remove(lsm9ds0->accel);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(st_lsm9ds0_remove);
-
MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
MODULE_DESCRIPTION("STMicroelectronics LSM9DS0 IMU core driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c
index 78bede358747..8f205c477e6f 100644
--- a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c
+++ b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c
@@ -64,18 +64,12 @@ static int st_lsm9ds0_i2c_probe(struct i2c_client *client)
return st_lsm9ds0_probe(lsm9ds0, regmap);
}
-static int st_lsm9ds0_i2c_remove(struct i2c_client *client)
-{
- return st_lsm9ds0_remove(i2c_get_clientdata(client));
-}
-
static struct i2c_driver st_lsm9ds0_driver = {
.driver = {
.name = "st-lsm9ds0-i2c",
.of_match_table = st_lsm9ds0_of_match,
},
.probe_new = st_lsm9ds0_i2c_probe,
- .remove = st_lsm9ds0_i2c_remove,
.id_table = st_lsm9ds0_id_table,
};
module_i2c_driver(st_lsm9ds0_driver);
diff --git a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c
index 180b54e66438..0ddfa53166af 100644
--- a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c
+++ b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c
@@ -63,18 +63,12 @@ static int st_lsm9ds0_spi_probe(struct spi_device *spi)
return st_lsm9ds0_probe(lsm9ds0, regmap);
}
-static int st_lsm9ds0_spi_remove(struct spi_device *spi)
-{
- return st_lsm9ds0_remove(spi_get_drvdata(spi));
-}
-
static struct spi_driver st_lsm9ds0_driver = {
.driver = {
.name = "st-lsm9ds0-spi",
.of_match_table = st_lsm9ds0_of_match,
},
.probe = st_lsm9ds0_spi_probe,
- .remove = st_lsm9ds0_spi_remove,
.id_table = st_lsm9ds0_id_table,
};
module_spi_driver(st_lsm9ds0_driver);
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 5be85e2405a5..1458906a3765 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -612,6 +612,7 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *mdata = iio_priv(indio_dev);
struct st_sensors_platform_data *pdata = dev_get_platdata(mdata->dev);
+ struct device *parent = indio_dev->dev.parent;
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -650,16 +651,10 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
return err;
}
- return iio_device_register(indio_dev);
+ return devm_iio_device_register(parent, indio_dev);
}
EXPORT_SYMBOL(st_magn_common_probe);
-void st_magn_common_remove(struct iio_dev *indio_dev)
-{
- iio_device_unregister(indio_dev);
-}
-EXPORT_SYMBOL(st_magn_common_remove);
-
MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
MODULE_DESCRIPTION("STMicroelectronics magnetometers driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
index 0a5117dffcf4..7237711fc09b 100644
--- a/drivers/iio/magnetometer/st_magn_i2c.c
+++ b/drivers/iio/magnetometer/st_magn_i2c.c
@@ -89,15 +89,6 @@ static int st_magn_i2c_probe(struct i2c_client *client,
return st_magn_common_probe(indio_dev);
}
-static int st_magn_i2c_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
-
- st_magn_common_remove(indio_dev);
-
- return 0;
-}
-
static const struct i2c_device_id st_magn_id_table[] = {
{ LSM303DLH_MAGN_DEV_NAME },
{ LSM303DLHC_MAGN_DEV_NAME },
@@ -117,7 +108,6 @@ static struct i2c_driver st_magn_driver = {
.of_match_table = st_magn_of_match,
},
.probe = st_magn_i2c_probe,
- .remove = st_magn_i2c_remove,
.id_table = st_magn_id_table,
};
module_i2c_driver(st_magn_driver);
diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index 1f3bf02b24e0..489d4462862f 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -83,15 +83,6 @@ static int st_magn_spi_probe(struct spi_device *spi)
return st_magn_common_probe(indio_dev);
}
-static int st_magn_spi_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
- st_magn_common_remove(indio_dev);
-
- return 0;
-}
-
static const struct spi_device_id st_magn_id_table[] = {
{ LIS3MDL_MAGN_DEV_NAME },
{ LSM303AGR_MAGN_DEV_NAME },
@@ -108,7 +99,6 @@ static struct spi_driver st_magn_driver = {
.of_match_table = st_magn_of_match,
},
.probe = st_magn_spi_probe,
- .remove = st_magn_spi_remove,
.id_table = st_magn_id_table,
};
module_spi_driver(st_magn_driver);
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 17ebb5171d4c..cebcc1d93d0b 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -678,6 +678,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *press_data = iio_priv(indio_dev);
struct st_sensors_platform_data *pdata = dev_get_platdata(press_data->dev);
+ struct device *parent = indio_dev->dev.parent;
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -721,16 +722,10 @@ int st_press_common_probe(struct iio_dev *indio_dev)
return err;
}
- return iio_device_register(indio_dev);
+ return devm_iio_device_register(parent, indio_dev);
}
EXPORT_SYMBOL(st_press_common_probe);
-void st_press_common_remove(struct iio_dev *indio_dev)
-{
- iio_device_unregister(indio_dev);
-}
-EXPORT_SYMBOL(st_press_common_remove);
-
MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
MODULE_DESCRIPTION("STMicroelectronics pressures driver");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index afeeab485c0d..1939e999a427 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -106,15 +106,6 @@ static int st_press_i2c_probe(struct i2c_client *client,
return st_press_common_probe(indio_dev);
}
-static int st_press_i2c_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
-
- st_press_common_remove(indio_dev);
-
- return 0;
-}
-
static struct i2c_driver st_press_driver = {
.driver = {
.name = "st-press-i2c",
@@ -122,7 +113,6 @@ static struct i2c_driver st_press_driver = {
.acpi_match_table = ACPI_PTR(st_press_acpi_match),
},
.probe = st_press_i2c_probe,
- .remove = st_press_i2c_remove,
.id_table = st_press_id_table,
};
module_i2c_driver(st_press_driver);
diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
index 834ad6d40a70..9b2523c5bc94 100644
--- a/drivers/iio/pressure/st_pressure_spi.c
+++ b/drivers/iio/pressure/st_pressure_spi.c
@@ -89,15 +89,6 @@ static int st_press_spi_probe(struct spi_device *spi)
return st_press_common_probe(indio_dev);
}
-static int st_press_spi_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
- st_press_common_remove(indio_dev);
-
- return 0;
-}
-
static const struct spi_device_id st_press_id_table[] = {
{ LPS001WP_PRESS_DEV_NAME },
{ LPS25H_PRESS_DEV_NAME },
@@ -116,7 +107,6 @@ static struct spi_driver st_press_driver = {
.of_match_table = st_press_of_match,
},
.probe = st_press_spi_probe,
- .remove = st_press_spi_remove,
.id_table = st_press_id_table,
};
module_spi_driver(st_press_driver);
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index fc90c202d15e..d17ae1e5ca19 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -323,21 +323,17 @@ void st_sensors_dev_name_probe(struct device *dev, char *name, int len);
/* Accelerometer */
const struct st_sensor_settings *st_accel_get_settings(const char *name);
int st_accel_common_probe(struct iio_dev *indio_dev);
-void st_accel_common_remove(struct iio_dev *indio_dev);
/* Gyroscope */
const struct st_sensor_settings *st_gyro_get_settings(const char *name);
int st_gyro_common_probe(struct iio_dev *indio_dev);
-void st_gyro_common_remove(struct iio_dev *indio_dev);
/* Magnetometer */
const struct st_sensor_settings *st_magn_get_settings(const char *name);
int st_magn_common_probe(struct iio_dev *indio_dev);
-void st_magn_common_remove(struct iio_dev *indio_dev);
/* Pressure */
const struct st_sensor_settings *st_press_get_settings(const char *name);
int st_press_common_probe(struct iio_dev *indio_dev);
-void st_press_common_remove(struct iio_dev *indio_dev);
#endif /* ST_SENSORS_H */
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] iio: st_sensors: remove all driver remove functions
2021-07-26 7:14 ` [PATCH 3/4] iio: st_sensors: remove all driver remove functions Alexandru Ardelean
@ 2021-07-26 7:20 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2021-07-26 7:20 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-iio, Linux Kernel Mailing List, Denis Ciocca, Jonathan Cameron
On Mon, Jul 26, 2021 at 10:14 AM Alexandru Ardelean
<aardelean@deviqon.com> wrote:
>
> At this point all ST driver remove functions do iio_device_unregister().
> This change removes them from them and replaces all iio_device_register()
> with devm_iio_device_register().
>
> This can be done in a single change relatively easy, since all these remove
easily
> functions are define in st_sensors.h.
defined
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h | 1 -
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c | 15 +--------------
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c | 6 ------
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c | 6 ------
I will check these later on, but the rule of thumb about devm_*() is
that there shouldn't be gaps (non-devm) before any devm call.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] iio: st_sensors: remove reference to parent device object on st_sensor_data
2021-07-26 7:14 [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Alexandru Ardelean
` (2 preceding siblings ...)
2021-07-26 7:14 ` [PATCH 3/4] iio: st_sensors: remove all driver remove functions Alexandru Ardelean
@ 2021-07-26 7:14 ` Alexandru Ardelean
2021-07-31 17:40 ` [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Jonathan Cameron
4 siblings, 0 replies; 11+ messages in thread
From: Alexandru Ardelean @ 2021-07-26 7:14 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: denis.ciocca, jic23, Alexandru Ardelean
I'm not completely sure whether this change makes much sense.
The idea behind it, is that all devm_ calls in ST sensors are bound to the
parent device object.
However, the reference to that object is kept on both the st_sensor_data
struct and the IIO object parent (indio_dev->dev.parent).
This change only adds a bit consistency and uses the reference stored on
indio_dev->dev.parent, to enforce the assumption that all ST sensors' devm_
calls are bound to the same reference as the one store on st_sensor_data.
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
drivers/iio/accel/st_accel_core.c | 7 ++++---
drivers/iio/common/st_sensors/st_sensors_i2c.c | 1 -
drivers/iio/common/st_sensors/st_sensors_spi.c | 1 -
drivers/iio/common/st_sensors/st_sensors_trigger.c | 8 +++++---
drivers/iio/gyro/st_gyro_core.c | 2 +-
drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c | 2 --
drivers/iio/magnetometer/st_magn_core.c | 4 ++--
drivers/iio/pressure/st_pressure_core.c | 2 +-
include/linux/iio/common/st_sensors.h | 2 --
9 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 01695abd9d2f..c2d2cbe0fa3b 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -1186,6 +1186,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
*/
static int apply_acpi_orientation(struct iio_dev *indio_dev)
{
+ struct device *parent = indio_dev->dev.parent;
struct st_sensor_data *adata = iio_priv(indio_dev);
struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
struct acpi_device *adev;
@@ -1210,7 +1211,7 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev)
};
- adev = ACPI_COMPANION(adata->dev);
+ adev = ACPI_COMPANION(parent);
if (!adev)
return 0;
@@ -1334,8 +1335,8 @@ EXPORT_SYMBOL(st_accel_get_settings);
int st_accel_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *adata = iio_priv(indio_dev);
- struct st_sensors_platform_data *pdata = dev_get_platdata(adata->dev);
struct device *parent = indio_dev->dev.parent;
+ struct st_sensors_platform_data *pdata = dev_get_platdata(parent);
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -1355,7 +1356,7 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
*/
err = apply_acpi_orientation(indio_dev);
if (err) {
- err = iio_read_mount_matrix(adata->dev, &adata->mount_matrix);
+ err = iio_read_mount_matrix(parent, &adata->mount_matrix);
if (err)
return err;
}
diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
index b3ff88700866..18bd3c3d99bc 100644
--- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
+++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
@@ -57,7 +57,6 @@ int st_sensors_i2c_configure(struct iio_dev *indio_dev,
indio_dev->name = client->name;
- sdata->dev = &client->dev;
sdata->irq = client->irq;
return 0;
diff --git a/drivers/iio/common/st_sensors/st_sensors_spi.c b/drivers/iio/common/st_sensors/st_sensors_spi.c
index 0d1d66c77cd8..7c60050e90dc 100644
--- a/drivers/iio/common/st_sensors/st_sensors_spi.c
+++ b/drivers/iio/common/st_sensors/st_sensors_spi.c
@@ -109,7 +109,6 @@ int st_sensors_spi_configure(struct iio_dev *indio_dev,
indio_dev->name = spi->modalias;
- sdata->dev = &spi->dev;
sdata->irq = spi->irq;
return 0;
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index d022157b66a2..bb687d1bcbef 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -28,6 +28,7 @@
static bool st_sensors_new_samples_available(struct iio_dev *indio_dev,
struct st_sensor_data *sdata)
{
+ struct device *parent = indio_dev->dev.parent;
int ret, status;
/* How would I know if I can't check it? */
@@ -42,7 +43,7 @@ static bool st_sensors_new_samples_available(struct iio_dev *indio_dev,
sdata->sensor_settings->drdy_irq.stat_drdy.addr,
&status);
if (ret < 0) {
- dev_err(sdata->dev, "error checking samples available\n");
+ dev_err(parent, "error checking samples available\n");
return false;
}
@@ -75,6 +76,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
struct iio_trigger *trig = p;
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct device *parent = indio_dev->dev.parent;
/*
* If this trigger is backed by a hardware interrupt and we have a
@@ -87,7 +89,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
st_sensors_new_samples_available(indio_dev, sdata)) {
iio_trigger_poll_chained(p);
} else {
- dev_dbg(sdata->dev, "spurious IRQ\n");
+ dev_dbg(parent, "spurious IRQ\n");
return IRQ_NONE;
}
@@ -107,7 +109,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
*/
while (sdata->hw_irq_trigger &&
st_sensors_new_samples_available(indio_dev, sdata)) {
- dev_dbg(sdata->dev, "more samples came in during polling\n");
+ dev_dbg(parent, "more samples came in during polling\n");
sdata->hw_timestamp = iio_get_time_ns(indio_dev);
iio_trigger_poll_chained(p);
}
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index 3609082a6778..201050b76fe5 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -492,7 +492,7 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
indio_dev->channels = gdata->sensor_settings->ch;
indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS;
- err = iio_read_mount_matrix(gdata->dev, &gdata->mount_matrix);
+ err = iio_read_mount_matrix(parent, &gdata->mount_matrix);
if (err)
return err;
diff --git a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c
index d276f663fe57..b3a43a3b04ff 100644
--- a/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c
+++ b/drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c
@@ -90,7 +90,6 @@ static int st_lsm9ds0_probe_accel(struct st_lsm9ds0 *lsm9ds0, struct regmap *reg
data = iio_priv(lsm9ds0->accel);
data->sensor_settings = (struct st_sensor_settings *)settings;
- data->dev = dev;
data->irq = lsm9ds0->irq;
data->regmap = regmap;
data->vdd = lsm9ds0->vdd;
@@ -119,7 +118,6 @@ static int st_lsm9ds0_probe_magn(struct st_lsm9ds0 *lsm9ds0, struct regmap *regm
data = iio_priv(lsm9ds0->magn);
data->sensor_settings = (struct st_sensor_settings *)settings;
- data->dev = dev;
data->irq = lsm9ds0->irq;
data->regmap = regmap;
data->vdd = lsm9ds0->vdd;
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 1458906a3765..0806a1e65ce4 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -611,8 +611,8 @@ EXPORT_SYMBOL(st_magn_get_settings);
int st_magn_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *mdata = iio_priv(indio_dev);
- struct st_sensors_platform_data *pdata = dev_get_platdata(mdata->dev);
struct device *parent = indio_dev->dev.parent;
+ struct st_sensors_platform_data *pdata = dev_get_platdata(parent);
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -626,7 +626,7 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
indio_dev->channels = mdata->sensor_settings->ch;
indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS;
- err = iio_read_mount_matrix(mdata->dev, &mdata->mount_matrix);
+ err = iio_read_mount_matrix(parent, &mdata->mount_matrix);
if (err)
return err;
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index cebcc1d93d0b..26a1ee43d56e 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -677,8 +677,8 @@ EXPORT_SYMBOL(st_press_get_settings);
int st_press_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *press_data = iio_priv(indio_dev);
- struct st_sensors_platform_data *pdata = dev_get_platdata(press_data->dev);
struct device *parent = indio_dev->dev.parent;
+ struct st_sensors_platform_data *pdata = dev_get_platdata(parent);
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index d17ae1e5ca19..22f67845cdd3 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -220,7 +220,6 @@ struct st_sensor_settings {
/**
* struct st_sensor_data - ST sensor device status
- * @dev: Pointer to instance of struct device (I2C or SPI).
* @trig: The trigger in use by the core driver.
* @mount_matrix: The mounting matrix of the sensor.
* @sensor_settings: Pointer to the specific sensor settings in use.
@@ -240,7 +239,6 @@ struct st_sensor_settings {
* @buffer_data: Data used by buffer part.
*/
struct st_sensor_data {
- struct device *dev;
struct iio_trigger *trig;
struct iio_mount_matrix mount_matrix;
struct st_sensor_settings *sensor_settings;
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 0/4] iio: st_sensors: convert probe functions to full devm
2021-07-26 7:14 [PATCH 0/4] iio: st_sensors: convert probe functions to full devm Alexandru Ardelean
` (3 preceding siblings ...)
2021-07-26 7:14 ` [PATCH 4/4] iio: st_sensors: remove reference to parent device object on st_sensor_data Alexandru Ardelean
@ 2021-07-31 17:40 ` Jonathan Cameron
4 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-07-31 17:40 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, denis.ciocca
On Mon, 26 Jul 2021 10:14:00 +0300
Alexandru Ardelean <aardelean@deviqon.com> wrote:
> Continuing from series:
> https://lore.kernel.org/linux-iio/20210720074642.223293-1-aardelean@deviqon.com/
>
> This goes a little further and converts all ST drivers using the st_common
> bits to devm functions.
> As mentioned by Jonathan, the devm unwind is often attached to
> iio_dev->dev.parent, and something it's attached to some other pointer
> which references the same device object.
>
> In the last patch, that point is removed, to eliminate doubt about this
> mix-n-match.
> An alternative would be an assert/check somewhere to ensure that
> 'iio_dev->dev.parent == adata->dev'. But I [personally] don't like it that
> much.
>
> As mentioned previously, I was also thinking of sending this set with the
> previous one. But I am usually reserved to send large sets; also, because I
> don't really like to review large sets [when I'm reviewing other people's
> code].
I'm fine with this, though I would like the reorder pulled out as as
separate patch and moved to the front of the series. It's a good change
in it's own right and will also make the rest of the series more 'obviously'
correct as you'll always be stripping off the last thing done in remove.
Otherwise, needs some time for others to take a look.
thanks,
Jonathan
>
> Alexandru Ardelean (4):
> iio: st_sensors: remove st_sensors_deallocate_trigger() function
> iio: st_sensors: remove st_sensors_power_disable() function
> iio: st_sensors: remove all driver remove functions
> iio: st_sensors: remove reference to parent device object on
> st_sensor_data
>
> drivers/iio/accel/st_accel_core.c | 32 +++--------
> drivers/iio/accel/st_accel_i2c.c | 23 +-------
> drivers/iio/accel/st_accel_spi.c | 23 +-------
> .../iio/common/st_sensors/st_sensors_core.c | 34 ++++++------
> .../iio/common/st_sensors/st_sensors_i2c.c | 1 -
> .../iio/common/st_sensors/st_sensors_spi.c | 1 -
> .../common/st_sensors/st_sensors_trigger.c | 53 +++++++------------
> drivers/iio/gyro/st_gyro_core.c | 27 ++--------
> drivers/iio/gyro/st_gyro_i2c.c | 23 +-------
> drivers/iio/gyro/st_gyro_spi.c | 23 +-------
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0.h | 1 -
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_core.c | 17 +-----
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_i2c.c | 6 ---
> drivers/iio/imu/st_lsm9ds0/st_lsm9ds0_spi.c | 6 ---
> drivers/iio/magnetometer/st_magn_core.c | 29 ++--------
> drivers/iio/magnetometer/st_magn_i2c.c | 23 +-------
> drivers/iio/magnetometer/st_magn_spi.c | 23 +-------
> drivers/iio/pressure/st_pressure_core.c | 27 ++--------
> drivers/iio/pressure/st_pressure_i2c.c | 23 +-------
> drivers/iio/pressure/st_pressure_spi.c | 23 +-------
> include/linux/iio/common/st_sensors.h | 13 -----
> 21 files changed, 60 insertions(+), 371 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread