From: Jonathan Cameron <jic23@kernel.org>
To: Marcelo Schmitt <marcelo.schmitt@analog.com>
Cc: <apw@canonical.com>, <joe@perches.com>, <dwaipayanray1@gmail.com>,
<lukas.bulwahn@gmail.com>, <paul.cercueil@analog.com>,
<Michael.Hennerich@analog.com>, <lars@metafoo.de>,
<robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
<conor+dt@kernel.org>, <dan.carpenter@linaro.org>,
<dlechner@baylibre.com>, <marcelo.schmitt1@gmail.com>,
<linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 05/15] iio: adc: ad7091r: Move generic AD7091R code to base driver and header file
Date: Sun, 17 Dec 2023 14:59:19 +0000 [thread overview]
Message-ID: <20231217145919.0af52cb2@jic23-huawei> (raw)
In-Reply-To: <6cab8108613aec0707c02587f5f6b9396fbd9831.1702746240.git.marcelo.schmitt1@gmail.com>
On Sat, 16 Dec 2023 14:47:25 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
> Some code generic to AD7091R devices such as channel definitions and
> event spec structs were in the AD7091R-5 driver.
> There was also some generic register definitions declared in the base
> driver which would make more sense to be in the header file.
> The device state struct will be needed for the ad7091r8 driver in a
> follow up patch so that ought to be moved to the header file as well.
> Lastly, a couple of regmap callback functions are also capable of
> abstracting characteristics of different AD7091R devices and those are
> now being exported to IIO_AD7091R name space.
>
> Move AD7091R generic code either to the base driver or to the header
> file so both the ad7091r5 and the ad7091r8 driver can use those
> declaration in follow up patches.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Hi Marcelo
I'm going to stop here as can't apply next patch because of build warnings
and this one is hard to justify without the rest of the series.
So please just include this patch onwards in v5.
This looks fine to me btw. I'll just comment on patches where I have
anything to add. If not they look fine.
Jonathan
> ---
> drivers/iio/adc/ad7091r-base.c | 46 +++++++++++++++++-----------------
> drivers/iio/adc/ad7091r-base.h | 42 ++++++++++++++++++++++++++++++-
> drivers/iio/adc/ad7091r5.c | 39 +++-------------------------
> 3 files changed, 68 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index d3d287d3b953..0d1f544de07a 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -15,14 +15,6 @@
>
> #include "ad7091r-base.h"
>
> -#define AD7091R_REG_RESULT 0
> -#define AD7091R_REG_CHANNEL 1
> -#define AD7091R_REG_CONF 2
> -#define AD7091R_REG_ALERT 3
> -#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
> -#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
> -#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> -
> /* AD7091R_REG_RESULT */
> #define AD7091R_REG_RESULT_CH_ID(x) (((x) >> 13) & 0x3)
> #define AD7091R_REG_RESULT_CONV_RESULT(x) ((x) & 0xfff)
> @@ -35,20 +27,26 @@
> #define AD7091R_REG_CONF_MODE_MASK \
> (AD7091R_REG_CONF_AUTO | AD7091R_REG_CONF_CMD)
>
> -enum ad7091r_mode {
> - AD7091R_MODE_SAMPLE,
> - AD7091R_MODE_COMMAND,
> - AD7091R_MODE_AUTOCYCLE,
> -};
> -
> -struct ad7091r_state {
> - struct device *dev;
> - struct regmap *map;
> - struct regulator *vref;
> - const struct ad7091r_chip_info *chip_info;
> - enum ad7091r_mode mode;
> - struct mutex lock; /*lock to prevent concurent reads */
> +const struct iio_event_spec ad7091r_events[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_EITHER,
> + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> + },
> };
> +EXPORT_SYMBOL_NS_GPL(ad7091r_events, IIO_AD7091R);
>
> static int ad7091r_set_mode(struct ad7091r_state *st, enum ad7091r_mode mode)
> {
> @@ -269,7 +267,7 @@ int ad7091r_probe(struct device *dev, const char *name,
> }
> EXPORT_SYMBOL_NS_GPL(ad7091r_probe, IIO_AD7091R);
>
> -static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
> +bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
> {
> switch (reg) {
> case AD7091R_REG_RESULT:
> @@ -279,8 +277,9 @@ static bool ad7091r_writeable_reg(struct device *dev, unsigned int reg)
> return true;
> }
> }
> +EXPORT_SYMBOL_NS_GPL(ad7091r_writeable_reg, IIO_AD7091R);
>
> -static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
> +bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
> {
> switch (reg) {
> case AD7091R_REG_RESULT:
> @@ -290,6 +289,7 @@ static bool ad7091r_volatile_reg(struct device *dev, unsigned int reg)
> return false;
> }
> }
> +EXPORT_SYMBOL_NS_GPL(ad7091r_volatile_reg, IIO_AD7091R);
>
> const struct regmap_config ad7091r_regmap_config = {
> .reg_bits = 8,
> diff --git a/drivers/iio/adc/ad7091r-base.h b/drivers/iio/adc/ad7091r-base.h
> index 509748aef9b1..1d30eeb46bcc 100644
> --- a/drivers/iio/adc/ad7091r-base.h
> +++ b/drivers/iio/adc/ad7091r-base.h
> @@ -8,8 +8,43 @@
> #ifndef __DRIVERS_IIO_ADC_AD7091R_BASE_H__
> #define __DRIVERS_IIO_ADC_AD7091R_BASE_H__
>
> +#define AD7091R_REG_RESULT 0
> +#define AD7091R_REG_CHANNEL 1
> +#define AD7091R_REG_CONF 2
> +#define AD7091R_REG_ALERT 3
> +
> +#define AD7091R_REG_CH_LOW_LIMIT(ch) ((ch) * 3 + 4)
> +#define AD7091R_REG_CH_HIGH_LIMIT(ch) ((ch) * 3 + 5)
> +#define AD7091R_REG_CH_HYSTERESIS(ch) ((ch) * 3 + 6)
> +
> +#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
> + .type = IIO_VOLTAGE, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .indexed = 1, \
> + .channel = idx, \
> + .event_spec = ev, \
> + .num_event_specs = num_ev, \
> + .scan_type.storagebits = 16, \
> + .scan_type.realbits = bits, \
> +}
> +
> struct device;
> -struct ad7091r_state;
> +
> +enum ad7091r_mode {
> + AD7091R_MODE_SAMPLE,
> + AD7091R_MODE_COMMAND,
> + AD7091R_MODE_AUTOCYCLE,
> +};
> +
> +struct ad7091r_state {
> + struct device *dev;
> + struct regmap *map;
> + struct regulator *vref;
> + const struct ad7091r_chip_info *chip_info;
> + enum ad7091r_mode mode;
> + struct mutex lock; /*lock to prevent concurent reads */
> +};
>
> struct ad7091r_chip_info {
> unsigned int num_channels;
> @@ -17,10 +52,15 @@ struct ad7091r_chip_info {
> unsigned int vref_mV;
> };
>
> +extern const struct iio_event_spec ad7091r_events[3];
> +
> extern const struct regmap_config ad7091r_regmap_config;
>
> int ad7091r_probe(struct device *dev, const char *name,
> const struct ad7091r_chip_info *chip_info,
> struct regmap *map, int irq);
>
> +bool ad7091r_volatile_reg(struct device *dev, unsigned int reg);
> +bool ad7091r_writeable_reg(struct device *dev, unsigned int reg);
> +
> #endif /* __DRIVERS_IIO_ADC_AD7091R_BASE_H__ */
> diff --git a/drivers/iio/adc/ad7091r5.c b/drivers/iio/adc/ad7091r5.c
> index 2f048527b7b7..9d3ccfca94ec 100644
> --- a/drivers/iio/adc/ad7091r5.c
> +++ b/drivers/iio/adc/ad7091r5.c
> @@ -12,42 +12,11 @@
>
> #include "ad7091r-base.h"
>
> -static const struct iio_event_spec ad7091r5_events[] = {
> - {
> - .type = IIO_EV_TYPE_THRESH,
> - .dir = IIO_EV_DIR_RISING,
> - .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> - BIT(IIO_EV_INFO_ENABLE),
> - },
> - {
> - .type = IIO_EV_TYPE_THRESH,
> - .dir = IIO_EV_DIR_FALLING,
> - .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> - BIT(IIO_EV_INFO_ENABLE),
> - },
> - {
> - .type = IIO_EV_TYPE_THRESH,
> - .dir = IIO_EV_DIR_EITHER,
> - .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> - },
> -};
> -
> -#define AD7091R_CHANNEL(idx, bits, ev, num_ev) { \
> - .type = IIO_VOLTAGE, \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> - .indexed = 1, \
> - .channel = idx, \
> - .event_spec = ev, \
> - .num_event_specs = num_ev, \
> - .scan_type.storagebits = 16, \
> - .scan_type.realbits = bits, \
> -}
> static const struct iio_chan_spec ad7091r5_channels_irq[] = {
> - AD7091R_CHANNEL(0, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> - AD7091R_CHANNEL(1, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> - AD7091R_CHANNEL(2, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> - AD7091R_CHANNEL(3, 12, ad7091r5_events, ARRAY_SIZE(ad7091r5_events)),
> + AD7091R_CHANNEL(0, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> + AD7091R_CHANNEL(1, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> + AD7091R_CHANNEL(2, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> + AD7091R_CHANNEL(3, 12, ad7091r_events, ARRAY_SIZE(ad7091r_events)),
> };
>
> static const struct iio_chan_spec ad7091r5_channels_noirq[] = {
next prev parent reply other threads:[~2023-12-17 14:59 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-16 17:44 [PATCH v4 00/15] Add support for AD7091R-2/-4/-8 Marcelo Schmitt
2023-12-16 17:45 ` [PATCH v4 01/15] scripts: checkpatch: Add __aligned to the list of attribute notes Marcelo Schmitt
2023-12-16 18:07 ` Joe Perches
2023-12-17 14:51 ` Jonathan Cameron
2023-12-16 17:46 ` [PATCH v4 02/15] iio: adc: ad7091r: Pass iio_dev to event handler Marcelo Schmitt
2023-12-17 14:53 ` Jonathan Cameron
2023-12-16 17:46 ` [PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config register Marcelo Schmitt
2023-12-17 14:55 ` Jonathan Cameron
2023-12-18 17:22 ` Marcelo Schmitt
2023-12-16 17:47 ` [PATCH v4 04/15] iio: adc: ad7091r: Align arguments to function call parenthesis Marcelo Schmitt
2023-12-17 14:57 ` Jonathan Cameron
2023-12-16 17:47 ` [PATCH v4 05/15] iio: adc: ad7091r: Move generic AD7091R code to base driver and header file Marcelo Schmitt
2023-12-17 14:59 ` Jonathan Cameron [this message]
2023-12-16 17:47 ` [PATCH v4 06/15] iio: adc: ad7091r: Move chip init data to container struct Marcelo Schmitt
2023-12-17 13:04 ` [PATCH v4 6/15] " kernel test robot
2023-12-17 14:48 ` Jonathan Cameron
2023-12-16 17:48 ` [PATCH v4 07/15] iio: adc: ad7091r: Remove unneeded probe parameters Marcelo Schmitt
2023-12-16 17:48 ` [PATCH v4 08/15] iio: adc: ad7091r: Set device mode through chip_info callback Marcelo Schmitt
2023-12-16 17:49 ` [PATCH v4 09/15] iio: adc: ad7091r: Enable internal vref if external vref is not supplied Marcelo Schmitt
2023-12-17 15:41 ` Jonathan Cameron
2023-12-18 17:35 ` Marcelo Schmitt
2023-12-20 14:04 ` Jonathan Cameron
2023-12-18 0:36 ` David Lechner
2023-12-18 17:42 ` Marcelo Schmitt
2023-12-16 17:49 ` [PATCH v4 10/15] iio: adc: ad7091r: Add chip_info callback to get conversion result channel Marcelo Schmitt
2023-12-16 17:49 ` [PATCH v4 11/15] iio: adc: Split AD7091R-5 config symbol Marcelo Schmitt
2023-12-16 17:50 ` [PATCH v4 12/15] dt-bindings: iio: Add AD7091R-8 Marcelo Schmitt
2023-12-16 23:55 ` Conor Dooley
2023-12-16 17:50 ` [PATCH v4 13/15] iio: adc: Add support for AD7091R-8 Marcelo Schmitt
2023-12-17 15:46 ` Jonathan Cameron
2023-12-16 17:51 ` [PATCH v4 14/15] MAINTAINERS: Add MAINTAINERS entry for AD7091R Marcelo Schmitt
2023-12-16 17:51 ` [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events Marcelo Schmitt
2023-12-17 15:54 ` Jonathan Cameron
2023-12-17 23:58 ` David Lechner
2023-12-18 0:10 ` David Lechner
2023-12-18 0:30 ` David Lechner
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=20231217145919.0af52cb2@jic23-huawei \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=apw@canonical.com \
--cc=conor+dt@kernel.org \
--cc=dan.carpenter@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=dwaipayanray1@gmail.com \
--cc=joe@perches.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lukas.bulwahn@gmail.com \
--cc=marcelo.schmitt1@gmail.com \
--cc=marcelo.schmitt@analog.com \
--cc=paul.cercueil@analog.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox