From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <nuno.sa@analog.com>
Cc: <linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
Hartmut Knaack <knaack.h@gmx.de>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexandru Ardelean <alexandru.Ardelean@analog.com>,
Michael Hennerich <Michael.Hennerich@analog.com>
Subject: Re: [PATCH v3 1/6] iio: imu: adis: Add Managed device functions
Date: Sat, 4 Apr 2020 17:24:11 +0100 [thread overview]
Message-ID: <20200404172411.07defbab@archlinux> (raw)
In-Reply-To: <20200331114811.7978-2-nuno.sa@analog.com>
On Tue, 31 Mar 2020 13:48:06 +0200
Nuno Sá <nuno.sa@analog.com> wrote:
> This patch adds support for a managed device version of
> adis_setup_buffer_and_trigger. It works exactly as the original
> one but it calls all the devm_iio_* functions to setup an iio
> buffer and trigger. Hence we do not need to care about cleaning those
> and we do not need to support a remove() callback for every driver using
> the adis library.
>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Random thought inline. Something we could use more in IIO :)
> ---
> Changes in v2:
> * Added blank lines for readability.
>
> Changes in V3:
> * Removed unnecessary inline;
> * Free buffer resources.
>
> drivers/iio/imu/adis_buffer.c | 45 ++++++++++++++++++++++++++++++++++
> drivers/iio/imu/adis_trigger.c | 41 ++++++++++++++++++++++++++++---
> include/linux/iio/imu/adis.h | 17 +++++++++++++
> 3 files changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
> index 04e5e2a0fd6b..c2211ab80d8c 100644
> --- a/drivers/iio/imu/adis_buffer.c
> +++ b/drivers/iio/imu/adis_buffer.c
> @@ -156,6 +156,14 @@ static irqreturn_t adis_trigger_handler(int irq, void *p)
> return IRQ_HANDLED;
> }
>
> +static void adis_buffer_cleanup(void *arg)
> +{
> + struct adis *adis = arg;
> +
> + kfree(adis->buffer);
> + kfree(adis->xfer);
> +}
> +
> /**
> * adis_setup_buffer_and_trigger() - Sets up buffer and trigger for the adis device
> * @adis: The adis device.
> @@ -198,6 +206,43 @@ int adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
> }
> EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger);
>
> +/**
> + * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for
> + * the managed adis device
> + * @adis: The adis device
> + * @indio_dev: The IIO device
> + * @trigger_handler: Optional trigger handler, may be NULL.
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + *
> + * This function perfoms exactly the same as adis_setup_buffer_and_trigger()
> + */
> +int
> +devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
> + irqreturn_t (*trigger_handler)(int, void *))
It occurred to me that there must be a lot of irq handling function pointers
in the kernel and it would be odd if there wasn't a type for this...
There is :) irq_handler_t
https://elixir.bootlin.com/linux/latest/source/include/linux/interrupt.h#L92
Not sure why I never noticed that before. Hohum.
Jonathan
> +{
> + int ret;
> +
> + if (!trigger_handler)
> + trigger_handler = adis_trigger_handler;
> +
> + ret = devm_iio_triggered_buffer_setup(&adis->spi->dev, indio_dev,
> + &iio_pollfunc_store_time,
> + trigger_handler, NULL);
> + if (ret)
> + return ret;
> +
> + if (adis->spi->irq) {
> + ret = devm_adis_probe_trigger(adis, indio_dev);
> + if (ret)
> + return ret;
> + }
> +
> + return devm_add_action_or_reset(&adis->spi->dev, adis_buffer_cleanup,
> + adis);
> +}
> +EXPORT_SYMBOL_GPL(devm_adis_setup_buffer_and_trigger);
> +
> /**
> * adis_cleanup_buffer_and_trigger() - Free buffer and trigger resources
> * @adis: The adis device.
> diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> index 8b9cd02c0f9f..a36810e0b1ab 100644
> --- a/drivers/iio/imu/adis_trigger.c
> +++ b/drivers/iio/imu/adis_trigger.c
> @@ -27,6 +27,13 @@ static const struct iio_trigger_ops adis_trigger_ops = {
> .set_trigger_state = &adis_data_rdy_trigger_set_state,
> };
>
> +static void adis_trigger_setup(struct adis *adis)
> +{
> + adis->trig->dev.parent = &adis->spi->dev;
> + adis->trig->ops = &adis_trigger_ops;
> + iio_trigger_set_drvdata(adis->trig, adis);
> +}
> +
> /**
> * adis_probe_trigger() - Sets up trigger for a adis device
> * @adis: The adis device
> @@ -45,9 +52,7 @@ int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> if (adis->trig == NULL)
> return -ENOMEM;
>
> - adis->trig->dev.parent = &adis->spi->dev;
> - adis->trig->ops = &adis_trigger_ops;
> - iio_trigger_set_drvdata(adis->trig, adis);
> + adis_trigger_setup(adis);
>
> ret = request_irq(adis->spi->irq,
> &iio_trigger_generic_data_rdy_poll,
> @@ -73,6 +78,36 @@ int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> }
> EXPORT_SYMBOL_GPL(adis_probe_trigger);
>
> +/**
> + * devm_adis_probe_trigger() - Sets up trigger for a managed adis device
> + * @adis: The adis device
> + * @indio_dev: The IIO device
> + *
> + * Returns 0 on success or a negative error code
> + */
> +int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> +{
> + int ret;
> +
> + adis->trig = devm_iio_trigger_alloc(&adis->spi->dev, "%s-dev%d",
> + indio_dev->name, indio_dev->id);
> + if (!adis->trig)
> + return -ENOMEM;
> +
> + adis_trigger_setup(adis);
> +
> + ret = devm_request_irq(&adis->spi->dev, adis->spi->irq,
> + &iio_trigger_generic_data_rdy_poll,
> + IRQF_TRIGGER_RISING,
> + indio_dev->name,
> + adis->trig);
> + if (ret)
> + return ret;
> +
> + return devm_iio_trigger_register(&adis->spi->dev, adis->trig);
> +}
> +EXPORT_SYMBOL_GPL(devm_adis_probe_trigger);
> +
> /**
> * adis_remove_trigger() - Remove trigger for a adis devices
> * @adis: The adis device
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index dd8219138c2e..ac94c483bf2b 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -448,11 +448,15 @@ struct adis_burst {
> unsigned int extra_len;
> };
>
> +int
> +devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
> + irqreturn_t (*trigger_handler)(int, void *));
> int adis_setup_buffer_and_trigger(struct adis *adis,
> struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *));
> void adis_cleanup_buffer_and_trigger(struct adis *adis,
> struct iio_dev *indio_dev);
>
> +int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
> void adis_remove_trigger(struct adis *adis);
>
> @@ -461,6 +465,13 @@ int adis_update_scan_mode(struct iio_dev *indio_dev,
>
> #else /* CONFIG_IIO_BUFFER */
>
> +static inline int
> +devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
> + irqreturn_t (*trigger_handler)(int, void *))
> +{
> + return 0;
> +}
> +
> static inline int adis_setup_buffer_and_trigger(struct adis *adis,
> struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *))
> {
> @@ -472,6 +483,12 @@ static inline void adis_cleanup_buffer_and_trigger(struct adis *adis,
> {
> }
>
> +static inline int devm_adis_probe_trigger(struct adis *adis,
> + struct iio_dev *indio_dev)
> +{
> + return 0;
> +}
> +
> static inline int adis_probe_trigger(struct adis *adis,
> struct iio_dev *indio_dev)
> {
next prev parent reply other threads:[~2020-04-04 16:24 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-31 11:48 [PATCH v3 0/6] Support ADIS16475 and similar IMUs Nuno Sá
2020-03-31 11:48 ` [PATCH v3 1/6] iio: imu: adis: Add Managed device functions Nuno Sá
2020-04-04 16:24 ` Jonathan Cameron [this message]
2020-03-31 11:48 ` [PATCH v3 2/6] iio: imu: adis: Add irq mask variable Nuno Sá
2020-03-31 17:40 ` Andy Shevchenko
2020-04-01 7:22 ` Sa, Nuno
2020-04-01 10:27 ` Andy Shevchenko
2020-04-01 13:18 ` Sa, Nuno
2020-03-31 11:48 ` [PATCH v3 3/6] iio: adis: Add adis_update_bits() APIs Nuno Sá
2020-03-31 17:41 ` Andy Shevchenko
2020-04-01 7:14 ` Sa, Nuno
2020-03-31 11:48 ` [PATCH v3 4/6] iio: adis: Support different burst sizes Nuno Sá
2020-03-31 11:48 ` [PATCH v3 5/6] iio: imu: Add support for adis16475 Nuno Sá
2020-03-31 18:15 ` Andy Shevchenko
2020-04-01 7:13 ` Sa, Nuno
2020-04-01 10:22 ` Andy Shevchenko
2020-04-01 13:27 ` Sa, Nuno
2020-04-01 14:06 ` Andy Shevchenko
2020-04-01 14:18 ` Sa, Nuno
2020-04-01 16:24 ` Andy Shevchenko
2020-04-04 16:05 ` Jonathan Cameron
2020-04-04 16:01 ` Jonathan Cameron
2020-03-31 11:48 ` [PATCH v3 6/6] dt-bindings: iio: Add adis16475 documentation Nuno Sá
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=20200404172411.07defbab@archlinux \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=alexandru.Ardelean@analog.com \
--cc=devicetree@vger.kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=nuno.sa@analog.com \
--cc=pmeerw@pmeerw.net \
--cc=robh+dt@kernel.org \
/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.