| Message ID | 20210816082836.67511-2-aardelean@deviqon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: st_sensors: convert probe functions to full devm | expand |
On Mon, Aug 16, 2021 at 10:30 AM Alexandru Ardelean <aardelean@deviqon.com> wrote: > Up until commit ea7e586bdd331 ("iio: st_sensors: move regulator retrieveal > to core") only the ST pressure driver seems to have had any regulator > disable. After that commit, the regulator handling was moved into the > common st_sensors logic. > > In all instances of this regulator handling, the regulators were disabled > before unregistering the IIO device. > This can cause issues where the device would be powered down and still be > available to userspace, allowing it to send invalid/garbage data. > > This change moves the st_sensors_power_disable() after the common probe > functions. These common probe functions also handle unregistering the IIO > device. > > Fixes: 774487611c949 ("iio: pressure-core: st: Provide support for the Vdd power supply") > Fixes: ea7e586bdd331 ("iio: st_sensors: move regulator retrieveal to core") > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Denis CIOCCA <denis.ciocca@st.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> That's a valid concern I suppose: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> But as it now occurs last before return 0, can't we just solve this with a devm_add_action_or_reset(dev, st_sensors_power_off_action, *); of some kind and let devres handle it? c.f drivers/input/touchscreen/cy8ctma140.c Yours, Linus Walleij
On Tue, 17 Aug 2021 at 01:50, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Mon, Aug 16, 2021 at 10:30 AM Alexandru Ardelean > <aardelean@deviqon.com> wrote: > > > Up until commit ea7e586bdd331 ("iio: st_sensors: move regulator retrieveal > > to core") only the ST pressure driver seems to have had any regulator > > disable. After that commit, the regulator handling was moved into the > > common st_sensors logic. > > > > In all instances of this regulator handling, the regulators were disabled > > before unregistering the IIO device. > > This can cause issues where the device would be powered down and still be > > available to userspace, allowing it to send invalid/garbage data. > > > > This change moves the st_sensors_power_disable() after the common probe > > functions. These common probe functions also handle unregistering the IIO > > device. > > > > Fixes: 774487611c949 ("iio: pressure-core: st: Provide support for the Vdd power supply") > > Fixes: ea7e586bdd331 ("iio: st_sensors: move regulator retrieveal to core") > > Cc: Lee Jones <lee.jones@linaro.org> > > Cc: Denis CIOCCA <denis.ciocca@st.com> > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> > > That's a valid concern I suppose: > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > But as it now occurs last before return 0, can't we just solve > this with a > devm_add_action_or_reset(dev, st_sensors_power_off_action, *); > of some kind and let devres handle it? i think this is my fault for not CC-ing the entire series to you: https://patchwork.kernel.org/project/linux-iio/list/?series=531879 this is part of a larger conversion to devm_ for ST sensors; i was hoping that if i CC you on one patch, then git send-email would CC you on the entire set; doesn't seem to be the case; i'll try to remember that on the next sets; [but maybe having this written here, will help me remember] > > c.f > drivers/input/touchscreen/cy8ctma140.c > > Yours, > Linus Walleij
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c index f711756e41e3..cba57459e90a 100644 --- a/drivers/iio/accel/st_accel_i2c.c +++ b/drivers/iio/accel/st_accel_i2c.c @@ -193,10 +193,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c index bb45d9ff95b8..5167fae1ee8e 100644 --- a/drivers/iio/accel/st_accel_spi.c +++ b/drivers/iio/accel/st_accel_spi.c @@ -143,10 +143,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c index 3ef86e16ee65..a8164fe48b85 100644 --- a/drivers/iio/gyro/st_gyro_i2c.c +++ b/drivers/iio/gyro/st_gyro_i2c.c @@ -106,10 +106,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c index 41d835493347..9d8916871b4b 100644 --- a/drivers/iio/gyro/st_gyro_spi.c +++ b/drivers/iio/gyro/st_gyro_spi.c @@ -110,10 +110,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c index 2dfe4ee99591..fa78f0a3b53e 100644 --- a/drivers/iio/magnetometer/st_magn_i2c.c +++ b/drivers/iio/magnetometer/st_magn_i2c.c @@ -102,10 +102,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c index fba978796395..ff43cbf61b05 100644 --- a/drivers/iio/magnetometer/st_magn_spi.c +++ b/drivers/iio/magnetometer/st_magn_spi.c @@ -96,10 +96,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c index 52fa98f24478..6215de677017 100644 --- a/drivers/iio/pressure/st_pressure_i2c.c +++ b/drivers/iio/pressure/st_pressure_i2c.c @@ -119,10 +119,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; } diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c index ee393df54cee..5001aae8f00b 100644 --- a/drivers/iio/pressure/st_pressure_spi.c +++ b/drivers/iio/pressure/st_pressure_spi.c @@ -102,10 +102,10 @@ 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); + st_sensors_power_disable(indio_dev); + return 0; }
Up until commit ea7e586bdd331 ("iio: st_sensors: move regulator retrieveal to core") only the ST pressure driver seems to have had any regulator disable. After that commit, the regulator handling was moved into the common st_sensors logic. In all instances of this regulator handling, the regulators were disabled before unregistering the IIO device. This can cause issues where the device would be powered down and still be available to userspace, allowing it to send invalid/garbage data. This change moves the st_sensors_power_disable() after the common probe functions. These common probe functions also handle unregistering the IIO device. Fixes: 774487611c949 ("iio: pressure-core: st: Provide support for the Vdd power supply") Fixes: ea7e586bdd331 ("iio: st_sensors: move regulator retrieveal to core") Cc: Lee Jones <lee.jones@linaro.org> Cc: Denis CIOCCA <denis.ciocca@st.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com> --- drivers/iio/accel/st_accel_i2c.c | 4 ++-- drivers/iio/accel/st_accel_spi.c | 4 ++-- drivers/iio/gyro/st_gyro_i2c.c | 4 ++-- drivers/iio/gyro/st_gyro_spi.c | 4 ++-- drivers/iio/magnetometer/st_magn_i2c.c | 4 ++-- drivers/iio/magnetometer/st_magn_spi.c | 4 ++-- drivers/iio/pressure/st_pressure_i2c.c | 4 ++-- drivers/iio/pressure/st_pressure_spi.c | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-)