* [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device
@ 2020-05-08 14:39 Alexandru Ardelean
2020-05-08 14:39 ` [PATCH 2/2] staging: iio: ad5933: convert probe init to use device managed callbacks Alexandru Ardelean
2020-05-10 10:18 ` [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device Jonathan Cameron
0 siblings, 2 replies; 3+ messages in thread
From: Alexandru Ardelean @ 2020-05-08 14:39 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
This change makes the use of devm_iio_kfifo_allocate() to attach the
life-cycle of the kfifo buffer to the parent (client->dev) object.
This removes the need to explicitly free 'indio_dev->buffer' via
iio_kfifo_free(), which is the main intent.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index af0bcf95ee8a..633adf1a08c1 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -602,11 +602,12 @@ static const struct iio_buffer_setup_ops ad5933_ring_setup_ops = {
.postdisable = ad5933_ring_postdisable,
};
-static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
+static int ad5933_register_ring_funcs_and_init(struct device *dev,
+ struct iio_dev *indio_dev)
{
struct iio_buffer *buffer;
- buffer = iio_kfifo_allocate();
+ buffer = devm_iio_kfifo_allocate(dev);
if (!buffer)
return -ENOMEM;
@@ -742,7 +743,7 @@ static int ad5933_probe(struct i2c_client *client,
indio_dev->channels = ad5933_channels;
indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
- ret = ad5933_register_ring_funcs_and_init(indio_dev);
+ ret = ad5933_register_ring_funcs_and_init(&client->dev, indio_dev);
if (ret)
goto error_disable_mclk;
@@ -756,8 +757,6 @@ static int ad5933_probe(struct i2c_client *client,
return 0;
-error_unreg_ring:
- iio_kfifo_free(indio_dev->buffer);
error_disable_mclk:
clk_disable_unprepare(st->mclk);
error_disable_reg:
@@ -772,7 +771,6 @@ static int ad5933_remove(struct i2c_client *client)
struct ad5933_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- iio_kfifo_free(indio_dev->buffer);
regulator_disable(st->reg);
clk_disable_unprepare(st->mclk);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] staging: iio: ad5933: convert probe init to use device managed callbacks
2020-05-08 14:39 [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device Alexandru Ardelean
@ 2020-05-08 14:39 ` Alexandru Ardelean
2020-05-10 10:18 ` [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Alexandru Ardelean @ 2020-05-08 14:39 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
This change moves the clock & regulator disable to use the
devm_add_action_or_reset() callback and uses the devm_iio_device_register()
to register the IIO device.
With this, it should be now possible to get rid of the remove callback.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
.../staging/iio/impedance-analyzer/ad5933.c | 67 +++++++++----------
1 file changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 633adf1a08c1..c468355b0848 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -677,6 +677,20 @@ static void ad5933_work(struct work_struct *work)
}
}
+static void ad5933_reg_disable(void *data)
+{
+ struct ad5933_state *st = data;
+
+ regulator_disable(st->reg);
+}
+
+static void ad5933_clk_disable(void *data)
+{
+ struct ad5933_state *st = data;
+
+ clk_disable_unprepare(st->mclk);
+}
+
static int ad5933_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -704,23 +718,32 @@ static int ad5933_probe(struct i2c_client *client,
dev_err(&client->dev, "Failed to enable specified VDD supply\n");
return ret;
}
- ret = regulator_get_voltage(st->reg);
+ ret = devm_add_action_or_reset(&client->dev, ad5933_reg_disable, st);
+ if (ret)
+ return ret;
+
+ ret = regulator_get_voltage(st->reg);
if (ret < 0)
- goto error_disable_reg;
+ return ret;
st->vref_mv = ret / 1000;
st->mclk = devm_clk_get(&client->dev, "mclk");
- if (IS_ERR(st->mclk) && PTR_ERR(st->mclk) != -ENOENT) {
- ret = PTR_ERR(st->mclk);
- goto error_disable_reg;
- }
+ if (IS_ERR(st->mclk) && PTR_ERR(st->mclk) != -ENOENT)
+ return PTR_ERR(st->mclk);
if (!IS_ERR(st->mclk)) {
ret = clk_prepare_enable(st->mclk);
if (ret < 0)
- goto error_disable_reg;
+ return ret;
+
+ ret = devm_add_action_or_reset(&client->dev,
+ ad5933_clk_disable,
+ st);
+ if (ret)
+ return ret;
+
ext_clk_hz = clk_get_rate(st->mclk);
}
@@ -745,36 +768,13 @@ static int ad5933_probe(struct i2c_client *client,
ret = ad5933_register_ring_funcs_and_init(&client->dev, indio_dev);
if (ret)
- goto error_disable_mclk;
+ return ret;
ret = ad5933_setup(st);
if (ret)
- goto error_unreg_ring;
-
- ret = iio_device_register(indio_dev);
- if (ret)
- goto error_unreg_ring;
-
- return 0;
-
-error_disable_mclk:
- clk_disable_unprepare(st->mclk);
-error_disable_reg:
- regulator_disable(st->reg);
-
- return ret;
-}
-
-static int ad5933_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
- struct ad5933_state *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- regulator_disable(st->reg);
- clk_disable_unprepare(st->mclk);
+ return ret;
- return 0;
+ return devm_iio_device_register(&client->dev, indio_dev);
}
static const struct i2c_device_id ad5933_id[] = {
@@ -799,7 +799,6 @@ static struct i2c_driver ad5933_driver = {
.of_match_table = ad5933_of_match,
},
.probe = ad5933_probe,
- .remove = ad5933_remove,
.id_table = ad5933_id,
};
module_i2c_driver(ad5933_driver);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device
2020-05-08 14:39 [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device Alexandru Ardelean
2020-05-08 14:39 ` [PATCH 2/2] staging: iio: ad5933: convert probe init to use device managed callbacks Alexandru Ardelean
@ 2020-05-10 10:18 ` Jonathan Cameron
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2020-05-10 10:18 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel
On Fri, 8 May 2020 17:39:35 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This change makes the use of devm_iio_kfifo_allocate() to attach the
> life-cycle of the kfifo buffer to the parent (client->dev) object.
>
> This removes the need to explicitly free 'indio_dev->buffer' via
> iio_kfifo_free(), which is the main intent.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
*mutters darkly*. Technically this patch adds a non obvious ordering
issue in remove, but then you fix it in the next one so I'll let that go.
However, you've not tried building patch 1 on it's own..
drivers/staging/iio/impedance-analyzer/ad5933.c:752:17: error: label 'error_unreg_ring' was not declared
drivers/staging/iio/impedance-analyzer/ad5933.c:756:17: error: label 'error_unreg_ring' was not declared
CC [M] drivers/staging/iio/impedance-analyzer/ad5933.o
drivers/staging/iio/impedance-analyzer/ad5933.c: In function ‘ad5933_probe’:
drivers/staging/iio/impedance-analyzer/ad5933.c:756:3: error: label ‘error_unreg_ring’ used but not defined
756 | goto error_unreg_ring;
| ^~~~
make[4]: *** [scripts/Makefile.build:267: drivers/staging/iio/impedance-analyzer/ad5933.o] Error 1
make[3]: *** [scripts/Makefile.build:488: drivers/staging/iio/impedance-analyzer] Error 2
make[2]: *** [scripts/Makefile.build:488: drivers/staging/iio] Error 2
make[1]: *** [scripts/Makefile.build:488: drivers/staging] Error 2
make: *** [Makefile:1722: drivers] Error 2
Easiest option here is just to merge the two patches into one. I've
done that and mashed the two commit messages into one semi-coherent whole.
Applied both patches as one to the togreg branch of iio.git and pushed out
as testing for the autobuilders to play with it.
thanks,
Jonathan
> ---
> drivers/staging/iio/impedance-analyzer/ad5933.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index af0bcf95ee8a..633adf1a08c1 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -602,11 +602,12 @@ static const struct iio_buffer_setup_ops ad5933_ring_setup_ops = {
> .postdisable = ad5933_ring_postdisable,
> };
>
> -static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> +static int ad5933_register_ring_funcs_and_init(struct device *dev,
> + struct iio_dev *indio_dev)
> {
> struct iio_buffer *buffer;
>
> - buffer = iio_kfifo_allocate();
> + buffer = devm_iio_kfifo_allocate(dev);
> if (!buffer)
> return -ENOMEM;
>
> @@ -742,7 +743,7 @@ static int ad5933_probe(struct i2c_client *client,
> indio_dev->channels = ad5933_channels;
> indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
>
> - ret = ad5933_register_ring_funcs_and_init(indio_dev);
> + ret = ad5933_register_ring_funcs_and_init(&client->dev, indio_dev);
> if (ret)
> goto error_disable_mclk;
>
> @@ -756,8 +757,6 @@ static int ad5933_probe(struct i2c_client *client,
>
> return 0;
>
> -error_unreg_ring:
> - iio_kfifo_free(indio_dev->buffer);
> error_disable_mclk:
> clk_disable_unprepare(st->mclk);
> error_disable_reg:
> @@ -772,7 +771,6 @@ static int ad5933_remove(struct i2c_client *client)
> struct ad5933_state *st = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> - iio_kfifo_free(indio_dev->buffer);
> regulator_disable(st->reg);
> clk_disable_unprepare(st->mclk);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-10 10:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 14:39 [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device Alexandru Ardelean
2020-05-08 14:39 ` [PATCH 2/2] staging: iio: ad5933: convert probe init to use device managed callbacks Alexandru Ardelean
2020-05-10 10:18 ` [PATCH 1/2] staging: iio: ad5933: attach life-cycle of kfifo buffer to parent device Jonathan Cameron
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.