From: Alexandru Ardelean <ardeleanalex@gmail.com>
To: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>
Cc: linux-iio <linux-iio@vger.kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
"Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>,
Rob Herring <robh+dt@kernel.org>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming
Date: Tue, 16 Mar 2021 12:16:07 +0200 [thread overview]
Message-ID: <CA+U=DspgXJ9-KJ_ZpYJDgAOjcFTz7vmvMoYU0Tpm5ddADGmbTQ@mail.gmail.com> (raw)
In-Reply-To: <20210314174310.3baaa573@archlinux>
On Sun, Mar 14, 2021 at 7:44 PM Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
> On Sun, 7 Feb 2021 15:46:12 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Add _REG postfix to register addresses to avoid confusion with
> > fields. Also add additional field defines and use throughout the
> > driver in place of magic numbers.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Whilst applying this I got
> drivers/staging/iio/cdc/ad7150.c:281:24: warning: dubious: x & !y
>
> which is related to the FIELD_PREP()
>
> To avoid any potential issues there, I flipped the logic to have a postive
> 'fixed' parameter rather than adaptive.
>
> I'm going to assume that's trivial enough that Alex's tag stands and
> apply this with that change.
Yes.
It's fine from my side.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > drivers/staging/iio/cdc/ad7150.c | 123 +++++++++++++++----------------
> > 1 file changed, 58 insertions(+), 65 deletions(-)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> > index 24be97456c03..5d4783da7f98 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -21,37 +21,38 @@
> > * AD7150 registers definition
> > */
> >
> > -#define AD7150_STATUS 0
> > -#define AD7150_STATUS_OUT1 BIT(3)
> > -#define AD7150_STATUS_OUT2 BIT(5)
> > -#define AD7150_CH1_DATA_HIGH 1
> > -#define AD7150_CH2_DATA_HIGH 3
> > -#define AD7150_CH1_AVG_HIGH 5
> > -#define AD7150_CH2_AVG_HIGH 7
> > -#define AD7150_CH1_SENSITIVITY 9
> > -#define AD7150_CH1_THR_HOLD_H 9
> > -#define AD7150_CH1_TIMEOUT 10
> > -#define AD7150_CH1_SETUP 11
> > -#define AD7150_CH2_SENSITIVITY 12
> > -#define AD7150_CH2_THR_HOLD_H 12
> > -#define AD7150_CH2_TIMEOUT 13
> > -#define AD7150_CH2_SETUP 14
> > -#define AD7150_CFG 15
> > -#define AD7150_CFG_FIX BIT(7)
> > -#define AD7150_PD_TIMER 16
> > -#define AD7150_CH1_CAPDAC 17
> > -#define AD7150_CH2_CAPDAC 18
> > -#define AD7150_SN3 19
> > -#define AD7150_SN2 20
> > -#define AD7150_SN1 21
> > -#define AD7150_SN0 22
> > -#define AD7150_ID 23
> > -
> > -/* AD7150 masks */
> > -#define AD7150_THRESHTYPE_MSK GENMASK(6, 5)
> > -
> > -#define AD7150_CH_TIMEOUT_RECEDING GENMASK(3, 0)
> > -#define AD7150_CH_TIMEOUT_APPROACHING GENMASK(7, 4)
> > +#define AD7150_STATUS_REG 0
> > +#define AD7150_STATUS_OUT1 BIT(3)
> > +#define AD7150_STATUS_OUT2 BIT(5)
> > +#define AD7150_CH1_DATA_HIGH_REG 1
> > +#define AD7150_CH2_DATA_HIGH_REG 3
> > +#define AD7150_CH1_AVG_HIGH_REG 5
> > +#define AD7150_CH2_AVG_HIGH_REG 7
> > +#define AD7150_CH1_SENSITIVITY_REG 9
> > +#define AD7150_CH1_THR_HOLD_H_REG 9
> > +#define AD7150_CH1_TIMEOUT_REG 10
> > +#define AD7150_CH_TIMEOUT_RECEDING GENMASK(3, 0)
> > +#define AD7150_CH_TIMEOUT_APPROACHING GENMASK(7, 4)
> > +#define AD7150_CH1_SETUP_REG 11
> > +#define AD7150_CH2_SENSITIVITY_REG 12
> > +#define AD7150_CH2_THR_HOLD_H_REG 12
> > +#define AD7150_CH2_TIMEOUT_REG 13
> > +#define AD7150_CH2_SETUP_REG 14
> > +#define AD7150_CFG_REG 15
> > +#define AD7150_CFG_FIX BIT(7)
> > +#define AD7150_CFG_THRESHTYPE_MSK GENMASK(6, 5)
> > +#define AD7150_CFG_TT_NEG 0x0
> > +#define AD7150_CFG_TT_POS 0x1
> > +#define AD7150_CFG_TT_IN_WINDOW 0x2
> > +#define AD7150_CFG_TT_OUT_WINDOW 0x3
> > +#define AD7150_PD_TIMER_REG 16
> > +#define AD7150_CH1_CAPDAC_REG 17
> > +#define AD7150_CH2_CAPDAC_REG 18
> > +#define AD7150_SN3_REG 19
> > +#define AD7150_SN2_REG 20
> > +#define AD7150_SN1_REG 21
> > +#define AD7150_SN0_REG 22
> > +#define AD7150_ID_REG 23
> >
> > enum {
> > AD7150,
> > @@ -93,12 +94,12 @@ struct ad7150_chip_info {
> > */
> >
> > static const u8 ad7150_addresses[][6] = {
> > - { AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
> > - AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
> > - AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
> > - { AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
> > - AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
> > - AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
> > + { AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
> > + AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
> > + AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
> > + { AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
> > + AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
> > + AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
> > };
> >
> > static int ad7150_read_raw(struct iio_dev *indio_dev,
> > @@ -147,11 +148,11 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
> > bool thrfixed;
> > struct ad7150_chip_info *chip = iio_priv(indio_dev);
> >
> > - ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> > + ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> > if (ret < 0)
> > return ret;
> >
> > - threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> > + threshtype = FIELD_GET(AD7150_CFG_THRESHTYPE_MSK, ret);
> >
> > /*check if threshold mode is fixed or adaptive*/
> > thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
> > @@ -159,12 +160,12 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
> > switch (type) {
> > case IIO_EV_TYPE_THRESH_ADAPTIVE:
> > if (dir == IIO_EV_DIR_RISING)
> > - return !thrfixed && (threshtype == 0x1);
> > - return !thrfixed && (threshtype == 0x0);
> > + return !thrfixed && (threshtype == AD7150_CFG_TT_POS);
> > + return !thrfixed && (threshtype == AD7150_CFG_TT_NEG);
> > case IIO_EV_TYPE_THRESH:
> > if (dir == IIO_EV_DIR_RISING)
> > - return thrfixed && (threshtype == 0x1);
> > - return thrfixed && (threshtype == 0x0);
> > + return thrfixed && (threshtype == AD7150_CFG_TT_POS);
> > + return thrfixed && (threshtype == AD7150_CFG_TT_NEG);
> > default:
> > break;
> > }
> > @@ -261,35 +262,27 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
> > disable_irq(chip->interrupts[0]);
> > disable_irq(chip->interrupts[1]);
> >
> > - ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> > + ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> > if (ret < 0)
> > goto error_ret;
> >
> > - cfg = ret & ~((0x03 << 5) | BIT(7));
> > + cfg = ret & ~(AD7150_CFG_THRESHTYPE_MSK | AD7150_CFG_FIX);
> >
> > - switch (type) {
> > - case IIO_EV_TYPE_THRESH_ADAPTIVE:
> > + if (type == IIO_EV_TYPE_THRESH_ADAPTIVE)
> > adaptive = 1;
> > - if (rising)
> > - thresh_type = 0x1;
> > - else
> > - thresh_type = 0x0;
> > - break;
> > - case IIO_EV_TYPE_THRESH:
> > + else
> > adaptive = 0;
> > - if (rising)
> > - thresh_type = 0x1;
> > - else
> > - thresh_type = 0x0;
> > - break;
> > - default:
> > - ret = -EINVAL;
> > - goto error_ret;
> > - }
> >
> > - cfg |= (!adaptive << 7) | (thresh_type << 5);
> > + if (rising)
> > + thresh_type = AD7150_CFG_TT_POS;
> > + else
> > + thresh_type = AD7150_CFG_TT_NEG;
> > +
> > + cfg |= FIELD_PREP(AD7150_CFG_FIX, !adaptive) |
> > + FIELD_PREP(AD7150_CFG_THRESHTYPE_MSK, thresh_type);
> >
> > - ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG, cfg);
> > + ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG_REG,
> > + cfg);
> > if (ret < 0)
> > goto error_ret;
> >
> > @@ -480,7 +473,7 @@ static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
> > s64 timestamp = iio_get_time_ns(indio_dev);
> > int int_status;
> >
> > - int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS);
> > + int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS_REG);
> > if (int_status < 0)
> > return IRQ_HANDLED;
> >
>
next prev parent reply other threads:[~2021-03-16 10:16 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
2021-02-07 15:46 ` [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding Jonathan Cameron
2021-02-07 15:46 ` [PATCH 02/24] staging:iio:cdc:ad7150: Remove magnitude adaptive events Jonathan Cameron
2021-02-07 15:46 ` [PATCH 03/24] staging:iio:cdc:ad7150: Refactor event parameter update Jonathan Cameron
2021-02-07 15:46 ` [PATCH 04/24] staging:iio:cdc:ad7150: Timeout register covers both directions so both need updating Jonathan Cameron
2021-02-07 15:46 ` [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support Jonathan Cameron
2021-02-08 8:01 ` Song Bao Hua (Barry Song)
2021-02-08 11:12 ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 06/24] staging:iio:cdc:ad7150: Handle variation in chan_spec across device and irq present or not Jonathan Cameron
2021-02-07 15:46 ` [PATCH 07/24] staging:iio:cdc:ad7150: Simplify event handling by only using rising direction Jonathan Cameron
2021-02-07 15:46 ` [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe Jonathan Cameron
2021-02-08 8:02 ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 09/24] staging:iio:cdc:ad7150: Add sampling_frequency support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 10/24] iio:event: Add timeout event info type Jonathan Cameron
2021-02-07 15:46 ` [PATCH 11/24] staging:iio:cdc:ad7150: Change timeout units to seconds and use core support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 12/24] staging:iio:cdc:ad7150: Rework interrupt handling Jonathan Cameron
2021-02-07 15:46 ` [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming Jonathan Cameron
2021-03-14 17:43 ` Jonathan Cameron
2021-03-16 10:16 ` Alexandru Ardelean [this message]
2021-02-07 15:46 ` [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers Jonathan Cameron
2021-02-08 7:54 ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning Jonathan Cameron
2021-02-08 7:54 ` Song Bao Hua (Barry Song)
2021-03-14 17:46 ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments Jonathan Cameron
2021-02-08 7:52 ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 17/24] staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits Jonathan Cameron
2021-02-07 15:46 ` [PATCH 18/24] staging:iio:cdc:ad7150: Add scale and offset to info_mask_shared_by_type Jonathan Cameron
2021-02-07 15:46 ` [PATCH 19/24] staging:iio:cdc:ad7150: Really basic regulator support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
2021-02-08 7:11 ` Alexandru Ardelean
2021-02-11 15:51 ` Jonathan Cameron
2021-02-08 7:50 ` Song Bao Hua (Barry Song)
2021-02-08 8:01 ` Alexandru Ardelean
2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
2021-02-07 16:00 ` Lars-Peter Clausen
2021-02-07 16:18 ` Jonathan Cameron
2021-02-08 8:12 ` Song Bao Hua (Barry Song)
2021-02-08 11:20 ` Jonathan Cameron
2021-02-21 15:59 ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 22/24] iio:Documentation:ABI Add missing elements as used by the adi,ad7150 Jonathan Cameron
2021-02-07 15:46 ` [PATCH 23/24] staging:iio:cdc:ad7150: Add copyright notice given substantial changes Jonathan Cameron
2021-02-07 15:46 ` [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging Jonathan Cameron
2021-02-07 16:21 ` Jonathan Cameron
2021-02-07 16:12 ` [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
2021-02-08 7:20 ` Alexandru Ardelean
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='CA+U=DspgXJ9-KJ_ZpYJDgAOjcFTz7vmvMoYU0Tpm5ddADGmbTQ@mail.gmail.com' \
--to=ardeleanalex@gmail.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=Michael.Hennerich@analog.com \
--cc=jic23@jic23.retrosnub.co.uk \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=song.bao.hua@hisilicon.com \
/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.