From: Jonathan Cameron <jic23@kernel.org>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: Hartmut Knaack <knaack.h@gmx.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
linux-iio@vger.kernel.org
Subject: Re: [PATCH 13/13] staging:iio:ad7606: Move buffer code to main source file
Date: Sat, 22 Oct 2016 18:11:38 +0100 [thread overview]
Message-ID: <df5cf37f-79e7-c813-0f9e-7a1ad30af30b@kernel.org> (raw)
In-Reply-To: <1476896828-10544-14-git-send-email-lars@metafoo.de>
On 19/10/16 18:07, Lars-Peter Clausen wrote:
> Currently the ad7606 buffer handling code resides in its own source file.
> But this file contains only 4 small functions of which half are just
> wrappers around other functions. Buffer support is also always enabled for
> this driver, so move them over to the main source file. This reduces the
> amount of boilerplate code.
>
> Also rename the main function from ad7606_core.c to ad7606.c since there is
> only a single file now.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Applied.
A nice bit of cleanup work. It'll stop me moaning about the other
ADI drivers you made the error of taking responsibility for that
are still in staging ;) (for a week or so anyway)
Jonathan
> ---
> drivers/staging/iio/adc/Makefile | 1 -
> .../staging/iio/adc/{ad7606_core.c => ad7606.c} | 49 +++++++++++++--
> drivers/staging/iio/adc/ad7606.h | 5 --
> drivers/staging/iio/adc/ad7606_ring.c | 69 ----------------------
> 4 files changed, 44 insertions(+), 80 deletions(-)
> rename drivers/staging/iio/adc/{ad7606_core.c => ad7606.c} (88%)
> delete mode 100644 drivers/staging/iio/adc/ad7606_ring.c
>
> diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile
> index 3cdd83c..ac09485 100644
> --- a/drivers/staging/iio/adc/Makefile
> +++ b/drivers/staging/iio/adc/Makefile
> @@ -2,7 +2,6 @@
> # Makefile for industrial I/O ADC drivers
> #
>
> -ad7606-y := ad7606_core.o ad7606_ring.o
> obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o
> obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o
> obj-$(CONFIG_AD7606) += ad7606.o
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606.c
> similarity index 88%
> rename from drivers/staging/iio/adc/ad7606_core.c
> rename to drivers/staging/iio/adc/ad7606.c
> index 4ef6cb4..010c6e1 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -21,10 +21,12 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> #include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>
> #include "ad7606.h"
>
> -int ad7606_reset(struct ad7606_state *st)
> +static int ad7606_reset(struct ad7606_state *st)
> {
> if (st->gpio_reset) {
> gpiod_set_value(st->gpio_reset, 1);
> @@ -36,7 +38,7 @@ int ad7606_reset(struct ad7606_state *st)
> return -ENODEV;
> }
>
> -int ad7606_read_samples(struct ad7606_state *st)
> +static int ad7606_read_samples(struct ad7606_state *st)
> {
> unsigned int num = st->chip_info->num_channels;
> u16 *data = st->data;
> @@ -69,6 +71,41 @@ int ad7606_read_samples(struct ad7606_state *st)
> return st->bops->read_block(st->dev, num, data);
> }
>
> +static irqreturn_t ad7606_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct ad7606_state *st = iio_priv(pf->indio_dev);
> +
> + gpiod_set_value(st->gpio_convst, 1);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
> + * @work_s: the work struct through which this was scheduled
> + *
> + * Currently there is no option in this driver to disable the saving of
> + * timestamps within the ring.
> + * I think the one copy of this at a time was to avoid problems if the
> + * trigger was set far too high and the reads then locked up the computer.
> + **/
> +static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
> +{
> + struct ad7606_state *st = container_of(work_s, struct ad7606_state,
> + poll_work);
> + struct iio_dev *indio_dev = iio_priv_to_dev(st);
> + int ret;
> +
> + ret = ad7606_read_samples(st);
> + if (ret == 0)
> + iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> + iio_get_time_ns(indio_dev));
> +
> + gpiod_set_value(st->gpio_convst, 0);
> + iio_trigger_notify_done(indio_dev->trig);
> +}
> +
> static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
> {
> struct ad7606_state *st = iio_priv(indio_dev);
> @@ -385,6 +422,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
> st->base_address = base_address;
> st->range = 5000;
> st->oversampling = 1;
> + INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
>
> st->reg = devm_regulator_get(dev, "vcc");
> if (!IS_ERR(st->reg)) {
> @@ -427,7 +465,8 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
> if (ret)
> goto error_disable_reg;
>
> - ret = ad7606_register_ring_funcs_and_init(indio_dev);
> + ret = iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
> + NULL, NULL);
> if (ret)
> goto error_free_irq;
>
> @@ -439,7 +478,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>
> return 0;
> error_unregister_ring:
> - ad7606_ring_cleanup(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
>
> error_free_irq:
> free_irq(irq, indio_dev);
> @@ -457,7 +496,7 @@ int ad7606_remove(struct device *dev, int irq)
> struct ad7606_state *st = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> - ad7606_ring_cleanup(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
>
> free_irq(irq, indio_dev);
> if (!IS_ERR(st->reg))
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index f5f85fa..746f955 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -61,8 +61,6 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
> const char *name, unsigned int id,
> const struct ad7606_bus_ops *bops);
> int ad7606_remove(struct device *dev, int irq);
> -int ad7606_reset(struct ad7606_state *st);
> -int ad7606_read_samples(struct ad7606_state *st);
>
> enum ad7606_supported_device_ids {
> ID_AD7606_8,
> @@ -70,9 +68,6 @@ enum ad7606_supported_device_ids {
> ID_AD7606_4
> };
>
> -int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev);
> -void ad7606_ring_cleanup(struct iio_dev *indio_dev);
> -
> #ifdef CONFIG_PM_SLEEP
> extern const struct dev_pm_ops ad7606_pm_ops;
> #define AD7606_PM_OPS (&ad7606_pm_ops)
> diff --git a/drivers/staging/iio/adc/ad7606_ring.c b/drivers/staging/iio/adc/ad7606_ring.c
> deleted file mode 100644
> index 68d72b1..0000000
> --- a/drivers/staging/iio/adc/ad7606_ring.c
> +++ /dev/null
> @@ -1,69 +0,0 @@
> -/*
> - * Copyright 2011-2012 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
> - *
> - */
> -
> -#include <linux/interrupt.h>
> -#include <linux/gpio.h>
> -#include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -
> -#include <linux/iio/iio.h>
> -#include <linux/iio/buffer.h>
> -#include <linux/iio/trigger_consumer.h>
> -#include <linux/iio/triggered_buffer.h>
> -
> -#include "ad7606.h"
> -
> -static irqreturn_t ad7606_trigger_handler(int irq, void *p)
> -{
> - struct iio_poll_func *pf = p;
> - struct ad7606_state *st = iio_priv(pf->indio_dev);
> -
> - gpiod_set_value(st->gpio_convst, 1);
> -
> - return IRQ_HANDLED;
> -}
> -
> -/**
> - * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring buffer
> - * @work_s: the work struct through which this was scheduled
> - *
> - * Currently there is no option in this driver to disable the saving of
> - * timestamps within the ring.
> - * I think the one copy of this at a time was to avoid problems if the
> - * trigger was set far too high and the reads then locked up the computer.
> - **/
> -static void ad7606_poll_bh_to_ring(struct work_struct *work_s)
> -{
> - struct ad7606_state *st = container_of(work_s, struct ad7606_state,
> - poll_work);
> - struct iio_dev *indio_dev = iio_priv_to_dev(st);
> - int ret;
> -
> - ret = ad7606_read_samples(st);
> - if (ret == 0)
> - iio_push_to_buffers_with_timestamp(indio_dev, st->data,
> - iio_get_time_ns(indio_dev));
> -
> - gpiod_set_value(st->gpio_convst, 0);
> - iio_trigger_notify_done(indio_dev->trig);
> -}
> -
> -int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> -{
> - struct ad7606_state *st = iio_priv(indio_dev);
> -
> - INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring);
> -
> - return iio_triggered_buffer_setup(indio_dev, &ad7606_trigger_handler,
> - NULL, NULL);
> -}
> -
> -void ad7606_ring_cleanup(struct iio_dev *indio_dev)
> -{
> - iio_triggered_buffer_cleanup(indio_dev);
> -}
>
prev parent reply other threads:[~2016-10-22 17:11 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-19 17:06 [PATCH 00/13] staging:iio:ad7606: Towards moving out of staging Lars-Peter Clausen
2016-10-19 17:06 ` [PATCH 01/13] staging:iio:ad7606: Remove unused int_vref_mv field Lars-Peter Clausen
2016-10-22 15:22 ` Jonathan Cameron
2016-10-19 17:06 ` [PATCH 02/13] staging:iio:ad7606: Remove redundant name field from ad7606_chip_info Lars-Peter Clausen
2016-10-22 15:23 ` Jonathan Cameron
2016-10-19 17:06 ` [PATCH 03/13] staging:iio:ad7606: Remove default device configuration from platform data Lars-Peter Clausen
2016-10-22 15:24 ` Jonathan Cameron
2016-10-19 17:06 ` [PATCH 04/13] staging:iio:ad7606: Remove out-of-band error reporting Lars-Peter Clausen
2016-10-22 15:27 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 05/13] staging:iio:ad7606: Use oversampling ratio of 1 for no oversampling Lars-Peter Clausen
2016-10-22 15:28 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 06/13] staging:iio:ad7606: Avoid allocating buffer for each data capture Lars-Peter Clausen
2016-10-22 15:28 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 07/13] staging:iio:ad7606: Factor out common code between periodic and one-shot capture Lars-Peter Clausen
2016-10-22 17:01 ` Jonathan Cameron
2016-10-22 17:02 ` Jonathan Cameron
2016-10-22 17:20 ` Lars-Peter Clausen
2016-10-22 17:33 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 08/13] staging:iio:ad7606: Move set_drvdata() into common code Lars-Peter Clausen
2016-10-22 16:59 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 09/13] staging:iio:ad7606: Let the common probe function return int Lars-Peter Clausen
2016-10-22 17:02 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 10/13] staging:iio:ad7606: Let common remove function take a struct device * Lars-Peter Clausen
2016-10-22 17:09 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 11/13] staging:iio:ad7606: Run trigger handler only once per trigger event Lars-Peter Clausen
2016-10-22 17:04 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 12/13] staging:iio:ad7606: Use GPIO descriptor API Lars-Peter Clausen
2016-10-22 17:09 ` Jonathan Cameron
2016-10-19 17:07 ` [PATCH 13/13] staging:iio:ad7606: Move buffer code to main source file Lars-Peter Clausen
2016-10-22 17:11 ` Jonathan Cameron [this message]
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=df5cf37f-79e7-c813-0f9e-7a1ad30af30b@kernel.org \
--to=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).