From: Jonathan Cameron <jic23@kernel.org>
To: Gregor Boirie <gregor.boirie@parrot.com>, linux-iio@vger.kernel.org
Cc: Hartmut Knaack <knaack.h@gmx.de>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
Denis Ciocca <denis.ciocca@st.com>,
Linus Walleij <linus.walleij@linaro.org>,
Giuseppe Barba <giuseppe.barba@st.com>
Subject: Re: [RFC PATCH v1 9/9] iio:st_sensors: fix power regulator usage
Date: Sun, 29 May 2016 16:10:22 +0100 [thread overview]
Message-ID: <e26af549-bf94-c86f-918e-e297237f141e@kernel.org> (raw)
In-Reply-To: <66af1630513a8e14900a0257c6c376742a298a86.1461056711.git.gregor.boirie@parrot.com>
On 19/04/16 10:18, Gregor Boirie wrote:
> Ensure failure to enable power regulators is properly handled.
>
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
This looks good to me so I've applied it to the togreg branch of iio.git
Again, this caused a fair bit of fuzz but all looks straight forward.
If you do a series like this in future adding new part support, please put that
new stuff at the end and any fixes / core reworks like this up at the top of
the series.
Whilst better error handling is always good to have, this one doesn't strike
me as suitable material for stable or even necessary to fast track it into
the current cycle. It's more of a case of making sure we tidy up loose ends
than a 'bug' fix.
Thanks and sorry for the delay with this seris.
Jonathan
> ---
> drivers/iio/accel/st_accel_core.c | 12 ++++++----
> drivers/iio/common/st_sensors/st_sensors_core.c | 29 ++++++++++++++++++++-----
> drivers/iio/gyro/st_gyro_core.c | 12 ++++++----
> drivers/iio/magnetometer/st_magn_core.c | 12 ++++++----
> drivers/iio/pressure/st_pressure_core.c | 12 ++++++----
> include/linux/iio/common/st_sensors.h | 2 +-
> 6 files changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index dc73f2d..b8d3c3c 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -757,13 +757,15 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
> indio_dev->info = &accel_info;
> mutex_init(&adata->tb.buf_lock);
>
> - st_sensors_power_enable(indio_dev);
> + err = st_sensors_power_enable(indio_dev);
> + if (err)
> + return err;
>
> err = st_sensors_check_device_support(indio_dev,
> ARRAY_SIZE(st_accel_sensors_settings),
> st_accel_sensors_settings);
> if (err < 0)
> - return err;
> + goto st_accel_power_off;
>
> adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
> adata->multiread_bit = adata->sensor_settings->multi_read_bit;
> @@ -780,11 +782,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
>
> err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
> if (err < 0)
> - return err;
> + goto st_accel_power_off;
>
> err = st_accel_allocate_ring(indio_dev);
> if (err < 0)
> - return err;
> + goto st_accel_power_off;
>
> if (irq > 0) {
> err = st_sensors_allocate_trigger(indio_dev,
> @@ -807,6 +809,8 @@ st_accel_device_register_error:
> st_sensors_deallocate_trigger(indio_dev);
> st_accel_probe_trigger_error:
> st_accel_deallocate_ring(indio_dev);
> +st_accel_power_off:
> + st_sensors_power_disable(indio_dev);
>
> return err;
> }
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 6901c7f..286f28c 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -228,7 +228,7 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
> }
> EXPORT_SYMBOL(st_sensors_set_axis_enable);
>
> -void st_sensors_power_enable(struct iio_dev *indio_dev)
> +int st_sensors_power_enable(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *pdata = iio_priv(indio_dev);
> int err;
> @@ -237,18 +237,37 @@ void st_sensors_power_enable(struct iio_dev *indio_dev)
> pdata->vdd = devm_regulator_get_optional(indio_dev->dev.parent, "vdd");
> if (!IS_ERR(pdata->vdd)) {
> err = regulator_enable(pdata->vdd);
> - if (err != 0)
> + if (err != 0) {
> dev_warn(&indio_dev->dev,
> "Failed to enable specified Vdd supply\n");
> + return err;
> + }
> + } else {
> + err = PTR_ERR(pdata->vdd);
> + if (err != -ENODEV)
> + return err;
> }
>
> pdata->vdd_io = devm_regulator_get_optional(indio_dev->dev.parent, "vddio");
> if (!IS_ERR(pdata->vdd_io)) {
> err = regulator_enable(pdata->vdd_io);
> - if (err != 0)
> + if (err != 0) {
> dev_warn(&indio_dev->dev,
> "Failed to enable specified Vdd_IO supply\n");
> + goto st_sensors_disable_vdd;
> + }
> + } else {
> + err = PTR_ERR(pdata->vdd_io);
> + if (err != -ENODEV)
> + goto st_sensors_disable_vdd;
> }
> +
> + return 0;
> +
> +st_sensors_disable_vdd:
> + if (!IS_ERR_OR_NULL(pdata->vdd))
> + regulator_disable(pdata->vdd);
> + return err;
> }
> EXPORT_SYMBOL(st_sensors_power_enable);
>
> @@ -256,10 +275,10 @@ void st_sensors_power_disable(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *pdata = iio_priv(indio_dev);
>
> - if (!IS_ERR(pdata->vdd))
> + if (!IS_ERR_OR_NULL(pdata->vdd))
> regulator_disable(pdata->vdd);
>
> - if (!IS_ERR(pdata->vdd_io))
> + if (!IS_ERR_OR_NULL(pdata->vdd_io))
> regulator_disable(pdata->vdd_io);
> }
> EXPORT_SYMBOL(st_sensors_power_disable);
> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
> index be9057e..f07f105 100644
> --- a/drivers/iio/gyro/st_gyro_core.c
> +++ b/drivers/iio/gyro/st_gyro_core.c
> @@ -424,13 +424,15 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
> indio_dev->info = &gyro_info;
> mutex_init(&gdata->tb.buf_lock);
>
> - st_sensors_power_enable(indio_dev);
> + err = st_sensors_power_enable(indio_dev);
> + if (err)
> + return err;
>
> err = st_sensors_check_device_support(indio_dev,
> ARRAY_SIZE(st_gyro_sensors_settings),
> st_gyro_sensors_settings);
> if (err < 0)
> - return err;
> + goto st_gyro_power_off;
>
> gdata->num_data_channels = ST_GYRO_NUMBER_DATA_CHANNELS;
> gdata->multiread_bit = gdata->sensor_settings->multi_read_bit;
> @@ -444,11 +446,11 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
> err = st_sensors_init_sensor(indio_dev,
> (struct st_sensors_platform_data *)&gyro_pdata);
> if (err < 0)
> - return err;
> + goto st_gyro_power_off;
>
> err = st_gyro_allocate_ring(indio_dev);
> if (err < 0)
> - return err;
> + goto st_gyro_power_off;
>
> if (irq > 0) {
> err = st_sensors_allocate_trigger(indio_dev,
> @@ -471,6 +473,8 @@ st_gyro_device_register_error:
> st_sensors_deallocate_trigger(indio_dev);
> st_gyro_probe_trigger_error:
> st_gyro_deallocate_ring(indio_dev);
> +st_gyro_power_off:
> + st_sensors_power_disable(indio_dev);
>
> return err;
> }
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 62036d2..7c94adc 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -588,13 +588,15 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
> indio_dev->info = &magn_info;
> mutex_init(&mdata->tb.buf_lock);
>
> - st_sensors_power_enable(indio_dev);
> + err = st_sensors_power_enable(indio_dev);
> + if (err)
> + return err;
>
> err = st_sensors_check_device_support(indio_dev,
> ARRAY_SIZE(st_magn_sensors_settings),
> st_magn_sensors_settings);
> if (err < 0)
> - return err;
> + goto st_magn_power_off;
>
> mdata->num_data_channels = ST_MAGN_NUMBER_DATA_CHANNELS;
> mdata->multiread_bit = mdata->sensor_settings->multi_read_bit;
> @@ -607,11 +609,11 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
>
> err = st_sensors_init_sensor(indio_dev, NULL);
> if (err < 0)
> - return err;
> + goto st_magn_power_off;
>
> err = st_magn_allocate_ring(indio_dev);
> if (err < 0)
> - return err;
> + goto st_magn_power_off;
>
> if (irq > 0) {
> err = st_sensors_allocate_trigger(indio_dev,
> @@ -634,6 +636,8 @@ st_magn_device_register_error:
> st_sensors_deallocate_trigger(indio_dev);
> st_magn_probe_trigger_error:
> st_magn_deallocate_ring(indio_dev);
> +st_magn_power_off:
> + st_sensors_power_disable(indio_dev);
>
> return err;
> }
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index bacdf6c..4b01b19 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -588,13 +588,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
> indio_dev->info = &press_info;
> mutex_init(&press_data->tb.buf_lock);
>
> - st_sensors_power_enable(indio_dev);
> + err = st_sensors_power_enable(indio_dev);
> + if (err)
> + return err;
>
> err = st_sensors_check_device_support(indio_dev,
> ARRAY_SIZE(st_press_sensors_settings),
> st_press_sensors_settings);
> if (err < 0)
> - return err;
> + goto st_press_power_off;
>
> press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
> press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
> @@ -615,11 +617,11 @@ int st_press_common_probe(struct iio_dev *indio_dev)
>
> err = st_sensors_init_sensor(indio_dev, press_data->dev->platform_data);
> if (err < 0)
> - return err;
> + goto st_press_power_off;
>
> err = st_press_allocate_ring(indio_dev);
> if (err < 0)
> - return err;
> + goto st_press_power_off;
>
> if (irq > 0) {
> err = st_sensors_allocate_trigger(indio_dev,
> @@ -642,6 +644,8 @@ st_press_device_register_error:
> st_sensors_deallocate_trigger(indio_dev);
> st_press_probe_trigger_error:
> st_press_deallocate_ring(indio_dev);
> +st_press_power_off:
> + st_sensors_power_disable(indio_dev);
>
> return err;
> }
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 78a1934..91d5f68 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -278,7 +278,7 @@ int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
>
> int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
>
> -void st_sensors_power_enable(struct iio_dev *indio_dev);
> +int st_sensors_power_enable(struct iio_dev *indio_dev);
>
> void st_sensors_power_disable(struct iio_dev *indio_dev);
>
>
next prev parent reply other threads:[~2016-05-29 15:10 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-19 9:18 [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor Gregor Boirie
2016-04-19 9:18 ` [RFC PATCH v1 1/9] iio:st_pressure:initial lps22hb sensor support Gregor Boirie
2016-04-24 9:29 ` Jonathan Cameron
2016-05-01 19:28 ` Jonathan Cameron
2016-05-29 14:14 ` Jonathan Cameron
2016-04-19 9:18 ` [RFC PATCH v1 2/9] iio:st_pressure: fix sampling gains Gregor Boirie
2016-05-29 15:14 ` Jonathan Cameron
2016-05-30 8:17 ` Linus Walleij
2016-05-30 12:23 ` Jonathan Cameron
2016-04-19 9:18 ` [RFC PATCH v1 3/9] iio:st_pressure: lps22hb temperature support Gregor Boirie
2016-05-29 14:53 ` Jonathan Cameron
2016-04-19 9:18 ` [RFC PATCH v1 4/9] iio:st_sensors: align on storagebits boundaries Gregor Boirie
2016-04-24 9:35 ` Jonathan Cameron
2016-05-01 19:27 ` Jonathan Cameron
2016-05-02 8:19 ` Gregor Boirie
2016-05-14 17:54 ` Jonathan Cameron
2016-05-03 16:20 ` Crestez Dan Leonard
2016-04-19 9:18 ` [RFC PATCH v1 5/9] iio:st_pressure: align storagebits on power of 2 Gregor Boirie
2016-04-19 9:18 ` [RFC PATCH v1 6/9] iio:st_pressure: temperature triggered buffering Gregor Boirie
2016-04-24 10:58 ` Jonathan Cameron
2016-05-29 14:57 ` Jonathan Cameron
2016-04-19 9:18 ` [RFC PATCH v1 7/9] iio:st_sensors: unexport st_sensors_get_buffer_element Gregor Boirie
2016-05-29 14:59 ` Jonathan Cameron
2016-04-19 9:18 ` [RFC PATCH v1 8/9] iio:st_sensors: emulate SMBus block read if needed Gregor Boirie
2016-05-29 15:06 ` Jonathan Cameron
2016-04-19 9:18 ` [RFC PATCH v1 9/9] iio:st_sensors: fix power regulator usage Gregor Boirie
2016-04-24 11:01 ` Jonathan Cameron
2016-04-24 11:02 ` Jonathan Cameron
2016-05-29 15:10 ` Jonathan Cameron [this message]
2016-04-27 11:26 ` [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor Linus Walleij
2016-04-27 12:02 ` Linus Walleij
2016-04-27 13:08 ` Gregor Boirie
2016-04-28 7:47 ` Linus Walleij
2016-04-28 14:57 ` Gregor Boirie
2016-04-28 15:02 ` Gregor Boirie
2016-06-11 17:36 ` Jonathan Cameron
2016-06-15 10:58 ` Gregor Boirie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e26af549-bf94-c86f-918e-e297237f141e@kernel.org \
--to=jic23@kernel.org \
--cc=denis.ciocca@st.com \
--cc=giuseppe.barba@st.com \
--cc=gregor.boirie@parrot.com \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linus.walleij@linaro.org \
--cc=linux-iio@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.