[AMD Official Use Only - General] Hi, Apologies, I never thanked you for submitting the patch, so let me start this mail by saying thanks! Yes, you are correct, they are functionally the same. There is one small difference: AMS_CTRL_SEQ_BASE equates to 66, since we exclude the control channels I don’t think any overflow will actually occur (as I don’t think there are any ps or pl channels that actually have a scan index so high) but if we look at it in isolation it looks like there could still be potential for overflow. In the referenced patch PL_SEQ_MAX equates to 60 which just means that even in isolation we can see there can never be an overflow. Please see my other comment inline. Thanks &Best Regards, Conall. > -----Original Message----- > From: Sean Anderson <sean.anderson@linux.dev> > Sent: Friday, March 15, 2024 5:48 PM > To: O'Griofa, Conall <conall.ogriofa@amd.com>; Jonathan Cameron > <jic23@kernel.org> > Cc: linux-iio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; Lars-Peter Clausen <lars@metafoo.de> > Subject: Re: [PATCH] iio: xilinx-ams: Don't include ams_ctrl_channels in > scan_mask > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > Hi Conall, > > On 3/15/24 09:18, O'Griofa, Conall wrote: > > [AMD Official Use Only - General] > > > > Hi, > > > > I think there was a fix for this issue applied to the version that was running on > 5.15 that didn't seem to make it into the upstream driver. > > Please see link for reference > > https://github.com/Xilinx/linux-xlnx/commit/608426961f16ab149b1b699f1c > > 35f7ad244c0720 > > > > I think a similar fix to the above patch is may be beneficial? > > These patches look functionally identical to me. > > --Sean > > >> -----Original Message----- > >> From: Sean Anderson <sean.anderson@linux.dev> > >> Sent: Thursday, March 14, 2024 5:30 PM > >> To: Jonathan Cameron <jic23@kernel.org> > >> Cc: linux-iio@vger.kernel.org; O'Griofa, Conall > >> <conall.ogriofa@amd.com>; linux-arm-kernel@lists.infradead.org; > >> linux-kernel@vger.kernel.org; Lars-Peter Clausen <lars@metafoo.de> > >> Subject: Re: [PATCH] iio: xilinx-ams: Don't include ams_ctrl_channels > >> in scan_mask > >> > >> Caution: This message originated from an External Source. Use proper > >> caution when opening attachments, clicking links, or responding. > >> > >> > >> On 3/14/24 11:48, Jonathan Cameron wrote: > >> > On Mon, 11 Mar 2024 12:28:00 -0400 > >> > Sean Anderson <sean.anderson@linux.dev> wrote: > >> > > >> >> ams_enable_channel_sequence constructs a "scan_mask" for all the > >> >> PS and PL channels. This works out fine, since scan_index for > >> >> these channels is less than 64. However, it also includes the > >> >> ams_ctrl_channels, where scan_index is greater than 64, triggering > >> >> undefined behavior. Since we don't need these channels anyway, > >> >> just > >> exclude them. > >> >> > >> >> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") > >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > >> > > >> > Hi Sean, > >> > > >> > I'd ideally like to understand why we have channels with such large > >> > scan indexes. Those values should only be used for buffered capture. > >> > It feels like they are being abused here. Can we set them to -1 > >> > instead and check based on that? > >> > For a channel, a scan index of -1 means it can't be captured via > >> > the buffered interfaces but only accessed via sysfs reads. > >> > I think that's what we have here? > >> > >> From what I can tell, none of the channels support buffered reads. > >> And we can't naïvely convert the scan_index to -1, since that causes > >> sysfs naming conflicts (not to mention the compatibility break). > >> > >> > > >> > I just feel like if we leave these as things stand, we will get > >> > bitten by similar bugs in the future. At least with -1 it should be obvious > why! > >> > >> There are just as likely to be bugs confusing the PL/PS subdevices... > >> > >> FWIW I had no trouble identifying the channels involved with this bug. > >> > >> --Sean > >> > >> > Jonathan > >> > > >> > > >> >> --- > >> >> > >> >> drivers/iio/adc/xilinx-ams.c | 8 ++++++-- > >> >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/drivers/iio/adc/xilinx-ams.c > >> >> b/drivers/iio/adc/xilinx-ams.c index a55396c1f8b2..4de7ce598e4d > >> >> 100644 > >> >> --- a/drivers/iio/adc/xilinx-ams.c > >> >> +++ b/drivers/iio/adc/xilinx-ams.c > >> >> @@ -414,8 +414,12 @@ static void > >> >> ams_enable_channel_sequence(struct > >> >> iio_dev *indio_dev) > >> >> > >> >> /* Run calibration of PS & PL as part of the sequence */ > >> >> scan_mask = BIT(0) | BIT(AMS_PS_SEQ_MAX); > >> >> - for (i = 0; i < indio_dev->num_channels; i++) > >> >> - scan_mask |= BIT_ULL(indio_dev->channels[i].scan_index); > >> >> + for (i = 0; i < indio_dev->num_channels; i++) { > >> >> + const struct iio_chan_spec *chan = > >> >> + &indio_dev->channels[i]; [COG] I don't think there is a need for the above we can just keep using "indio_dev->channels[i].scan_index" > >> >> + > >> >> + if (chan->scan_index < AMS_CTRL_SEQ_BASE) > >> >> + scan_mask |= BIT_ULL(chan->scan_index); > >> >> + } > >> >> > >> >> if (ams->ps_base) { > >> >> /* put sysmon in a soft reset to change the sequence > >> >> */ > >> >
Add support for ad7380/1/2/3-4 parts which are 4 channels variants from ad7380/1/2/3 Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- drivers/iio/adc/ad7380.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c index 3aca41ce9a14..cf9d2ace5f20 100644 --- a/drivers/iio/adc/ad7380.c +++ b/drivers/iio/adc/ad7380.c @@ -8,6 +8,9 @@ * Datasheets of supported parts: * ad7380/1 : https://www.analog.com/media/en/technical-documentation/data-sheets/AD7380-7381.pdf * ad7383/4 : https://www.analog.com/media/en/technical-documentation/data-sheets/ad7383-7384.pdf + * ad7380-4 : https://www.analog.com/media/en/technical-documentation/data-sheets/ad7380-4.pdf + * ad7381-4 : https://www.analog.com/media/en/technical-documentation/data-sheets/ad7381-4.pdf + * ad7383/4-4 : https://www.analog.com/media/en/technical-documentation/data-sheets/ad7383-4-ad7384-4.pdf */ #include <linux/bitfield.h> @@ -29,7 +32,7 @@ #include <linux/iio/trigger_consumer.h> #include <linux/iio/triggered_buffer.h> -#define MAX_NUM_CHANNELS 2 +#define MAX_NUM_CHANNELS 4 /* 2.5V internal reference voltage */ #define AD7380_INTERNAL_REF_MV 2500 @@ -106,27 +109,53 @@ static const struct iio_chan_spec name[] = { \ IIO_CHAN_SOFT_TIMESTAMP(2), \ } +#define DEFINE_AD7380_4_CHANNEL(name, bits, diff) \ +static const struct iio_chan_spec name[] = { \ + AD7380_CHANNEL(0, bits, diff), \ + AD7380_CHANNEL(1, bits, diff), \ + AD7380_CHANNEL(2, bits, diff), \ + AD7380_CHANNEL(3, bits, diff), \ + IIO_CHAN_SOFT_TIMESTAMP(4), \ +} + /* fully differential */ DEFINE_AD7380_2_CHANNEL(ad7380_channels, 16, 1); DEFINE_AD7380_2_CHANNEL(ad7381_channels, 14, 1); +DEFINE_AD7380_4_CHANNEL(ad7380_4_channels, 16, 1); +DEFINE_AD7380_4_CHANNEL(ad7381_4_channels, 14, 1); /* pseudo differential */ DEFINE_AD7380_2_CHANNEL(ad7383_channels, 16, 0); DEFINE_AD7380_2_CHANNEL(ad7384_channels, 14, 0); +DEFINE_AD7380_4_CHANNEL(ad7383_4_channels, 16, 0); +DEFINE_AD7380_4_CHANNEL(ad7384_4_channels, 14, 0); static const char * const ad7380_2_channel_vcm_supplies[] = { "aina", "ainb", }; +static const char * const ad7380_4_channel_vcm_supplies[] = { + "aina", "ainb", "ainc", "aind", +}; + /* Since this is simultaneous sampling, we don't allow individual channels. */ static const unsigned long ad7380_2_channel_scan_masks[] = { GENMASK(1, 0), 0 }; +static const unsigned long ad7380_4_channel_scan_masks[] = { + GENMASK(3, 0), + 0 +}; + static const struct ad7380_timing_specs ad7380_timing = { .t_csh_ns = 10, }; +static const struct ad7380_timing_specs ad7380_4_timing = { + .t_csh_ns = 20, +}; + static const struct ad7380_chip_info ad7380_chip_info = { .name = "ad7380", .channels = ad7380_channels, @@ -163,6 +192,42 @@ static const struct ad7380_chip_info ad7384_chip_info = { .timing_specs = &ad7380_timing, }; +static const struct ad7380_chip_info ad7380_4_chip_info = { + .name = "ad7380-4", + .channels = ad7380_4_channels, + .num_channels = ARRAY_SIZE(ad7380_4_channels), + .available_scan_masks = ad7380_4_channel_scan_masks, + .timing_specs = &ad7380_4_timing, +}; + +static const struct ad7380_chip_info ad7381_4_chip_info = { + .name = "ad7381-4", + .channels = ad7381_4_channels, + .num_channels = ARRAY_SIZE(ad7381_4_channels), + .available_scan_masks = ad7380_4_channel_scan_masks, + .timing_specs = &ad7380_4_timing, +}; + +static const struct ad7380_chip_info ad7383_4_chip_info = { + .name = "ad7383-4", + .channels = ad7383_4_channels, + .num_channels = ARRAY_SIZE(ad7383_4_channels), + .vcm_supplies = ad7380_4_channel_vcm_supplies, + .num_vcm_supplies = ARRAY_SIZE(ad7380_4_channel_vcm_supplies), + .available_scan_masks = ad7380_4_channel_scan_masks, + .timing_specs = &ad7380_4_timing, +}; + +static const struct ad7380_chip_info ad7384_4_chip_info = { + .name = "ad7384-4", + .channels = ad7384_4_channels, + .num_channels = ARRAY_SIZE(ad7384_4_channels), + .vcm_supplies = ad7380_4_channel_vcm_supplies, + .num_vcm_supplies = ARRAY_SIZE(ad7380_4_channel_vcm_supplies), + .available_scan_masks = ad7380_4_channel_scan_masks, + .timing_specs = &ad7380_4_timing, +}; + struct ad7380_state { const struct ad7380_chip_info *chip_info; struct spi_device *spi; @@ -514,6 +579,10 @@ static const struct of_device_id ad7380_of_match_table[] = { { .compatible = "adi,ad7381", .data = &ad7381_chip_info }, { .compatible = "adi,ad7383", .data = &ad7383_chip_info }, { .compatible = "adi,ad7384", .data = &ad7384_chip_info }, + { .compatible = "adi,ad7380-4", .data = &ad7380_4_chip_info }, + { .compatible = "adi,ad7381-4", .data = &ad7381_4_chip_info }, + { .compatible = "adi,ad7383-4", .data = &ad7383_4_chip_info }, + { .compatible = "adi,ad7384-4", .data = &ad7384_4_chip_info }, { } }; @@ -522,6 +591,10 @@ static const struct spi_device_id ad7380_id_table[] = { { "ad7381", (kernel_ulong_t)&ad7381_chip_info }, { "ad7383", (kernel_ulong_t)&ad7383_chip_info }, { "ad7384", (kernel_ulong_t)&ad7384_chip_info }, + { "ad7380-4", (kernel_ulong_t)&ad7380_4_chip_info }, + { "ad7381-4", (kernel_ulong_t)&ad7381_4_chip_info }, + { "ad7383-4", (kernel_ulong_t)&ad7383_4_chip_info }, + { "ad7384-4", (kernel_ulong_t)&ad7384_4_chip_info }, { } }; MODULE_DEVICE_TABLE(spi, ad7380_id_table); -- 2.44.0
Add compatible support for ad7380/1/3/4-4 parts which are 4 channels variants from ad7380/1/3/4 Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- .../devicetree/bindings/iio/adc/adi,ad7380.yaml | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml index de3d28a021ae..899b777017ce 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml @@ -15,6 +15,10 @@ description: | * https://www.analog.com/en/products/ad7381.html * https://www.analog.com/en/products/ad7383.html * https://www.analog.com/en/products/ad7384.html + * https://www.analog.com/en/products/ad7380-4.html + * https://www.analog.com/en/products/ad7381-4.html + * https://www.analog.com/en/products/ad7383-4.html + * https://www.analog.com/en/products/ad7384-4.html $ref: /schemas/spi/spi-peripheral-props.yaml# @@ -25,6 +29,10 @@ properties: - adi,ad7381 - adi,ad7383 - adi,ad7384 + - adi,ad7380-4 + - adi,ad7381-4 + - adi,ad7383-4 + - adi,ad7384-4 reg: maxItems: 1 @@ -56,6 +64,16 @@ properties: The common mode voltage supply for the AINB- pin on pseudo-differential chips. + ainc-supply: + description: + The common mode voltage supply for the AINC- pin on pseudo-differential + chips. + + aind-supply: + description: + The common mode voltage supply for the AIND- pin on pseudo-differential + chips. + interrupts: description: When the device is using 1-wire mode, this property is used to optionally @@ -79,6 +97,8 @@ allOf: enum: - adi,ad7383 - adi,ad7384 + - adi,ad7383-4 + - adi,ad7384-4 then: required: - aina-supply @@ -87,6 +107,20 @@ allOf: properties: aina-supply: false ainb-supply: false + - if: + properties: + compatible: + enum: + - adi,ad7383-4 + - adi,ad7384-4 + then: + required: + - ainc-supply + - aind-supply + else: + properties: + ainc-supply: false + aind-supply: false examples: - | -- 2.44.0
The current driver supports only parts with 2 channels. In order to prepare the support of new compatible ADCs with more channels, this commit: - defines MAX_NUM_CHANNEL to specify the maximum number of channels currently supported by the driver - adds available_scan_mask member in ad7380_chip_info structure - fixes spi xfer struct len depending on number of channels - fixes scan_data.raw buffer size to handle more channels - adds a timing specifications structure in ad7380_chip_info structure Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- drivers/iio/adc/ad7380.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c index 996ca83feaed..3aca41ce9a14 100644 --- a/drivers/iio/adc/ad7380.c +++ b/drivers/iio/adc/ad7380.c @@ -29,6 +29,7 @@ #include <linux/iio/trigger_consumer.h> #include <linux/iio/triggered_buffer.h> +#define MAX_NUM_CHANNELS 2 /* 2.5V internal reference voltage */ #define AD7380_INTERNAL_REF_MV 2500 @@ -65,12 +66,19 @@ #define AD7380_ALERT_LOW_TH GENMASK(11, 0) #define AD7380_ALERT_HIGH_TH GENMASK(11, 0) +#define T_CONVERT_NS 190 /* conversion time */ +struct ad7380_timing_specs { + const unsigned int t_csh_ns; /* CS minimum high time */ +}; + struct ad7380_chip_info { const char *name; const struct iio_chan_spec *channels; unsigned int num_channels; const char * const *vcm_supplies; unsigned int num_vcm_supplies; + const unsigned long *available_scan_masks; + const struct ad7380_timing_specs *timing_specs; }; #define AD7380_CHANNEL(index, bits, diff) { \ @@ -115,16 +123,24 @@ static const unsigned long ad7380_2_channel_scan_masks[] = { 0 }; +static const struct ad7380_timing_specs ad7380_timing = { + .t_csh_ns = 10, +}; + static const struct ad7380_chip_info ad7380_chip_info = { .name = "ad7380", .channels = ad7380_channels, .num_channels = ARRAY_SIZE(ad7380_channels), + .available_scan_masks = ad7380_2_channel_scan_masks, + .timing_specs = &ad7380_timing, }; static const struct ad7380_chip_info ad7381_chip_info = { .name = "ad7381", .channels = ad7381_channels, .num_channels = ARRAY_SIZE(ad7381_channels), + .available_scan_masks = ad7380_2_channel_scan_masks, + .timing_specs = &ad7380_timing, }; static const struct ad7380_chip_info ad7383_chip_info = { @@ -133,6 +149,8 @@ static const struct ad7380_chip_info ad7383_chip_info = { .num_channels = ARRAY_SIZE(ad7383_channels), .vcm_supplies = ad7380_2_channel_vcm_supplies, .num_vcm_supplies = ARRAY_SIZE(ad7380_2_channel_vcm_supplies), + .available_scan_masks = ad7380_2_channel_scan_masks, + .timing_specs = &ad7380_timing, }; static const struct ad7380_chip_info ad7384_chip_info = { @@ -141,6 +159,8 @@ static const struct ad7380_chip_info ad7384_chip_info = { .num_channels = ARRAY_SIZE(ad7384_channels), .vcm_supplies = ad7380_2_channel_vcm_supplies, .num_vcm_supplies = ARRAY_SIZE(ad7380_2_channel_vcm_supplies), + .available_scan_masks = ad7380_2_channel_scan_masks, + .timing_specs = &ad7380_timing, }; struct ad7380_state { @@ -148,15 +168,15 @@ struct ad7380_state { struct spi_device *spi; struct regmap *regmap; unsigned int vref_mv; - unsigned int vcm_mv[2]; + unsigned int vcm_mv[MAX_NUM_CHANNELS]; /* * DMA (thus cache coherency maintenance) requires the * transfer buffers to live in their own cache lines. - * Make the buffer large enough for 2 16-bit samples and one 64-bit + * Make the buffer large enough for MAX_NUM_CHANNELS 16-bit samples and one 64-bit * aligned 64 bit timestamp. */ struct { - u16 raw[2]; + u16 raw[MAX_NUM_CHANNELS]; s64 ts __aligned(8); } scan_data __aligned(IIO_DMA_MINALIGN); @@ -194,7 +214,7 @@ static int ad7380_regmap_reg_read(void *context, unsigned int reg, .tx_buf = &st->tx, .cs_change = 1, .cs_change_delay = { - .value = 10, /* t[CSH] */ + .value = st->chip_info->timing_specs->t_csh_ns, .unit = SPI_DELAY_UNIT_NSECS, }, }, { @@ -255,7 +275,8 @@ static irqreturn_t ad7380_trigger_handler(int irq, void *p) struct ad7380_state *st = iio_priv(indio_dev); struct spi_transfer xfer = { .bits_per_word = st->chip_info->channels[0].scan_type.realbits, - .len = 4, + .len = (st->chip_info->num_channels - 1) * + ((st->chip_info->channels->scan_type.storagebits > 16) ? 4 : 2), .rx_buf = st->scan_data.raw, }; int ret; @@ -282,21 +303,22 @@ static int ad7380_read_direct(struct ad7380_state *st, .speed_hz = AD7380_REG_WR_SPEED_HZ, .bits_per_word = chan->scan_type.realbits, .delay = { - .value = 190, /* t[CONVERT] */ + .value = T_CONVERT_NS, .unit = SPI_DELAY_UNIT_NSECS, }, .cs_change = 1, .cs_change_delay = { - .value = 10, /* t[CSH] */ + .value = st->chip_info->timing_specs->t_csh_ns, .unit = SPI_DELAY_UNIT_NSECS, }, }, - /* then read both channels */ + /* then read all channels */ { .speed_hz = AD7380_REG_WR_SPEED_HZ, .bits_per_word = chan->scan_type.realbits, .rx_buf = st->scan_data.raw, - .len = 4, + .len = (st->chip_info->num_channels - 1) * + ((chan->scan_type.storagebits > 16) ? 4 : 2), }, }; int ret; @@ -472,7 +494,7 @@ static int ad7380_probe(struct spi_device *spi) indio_dev->name = st->chip_info->name; indio_dev->info = &ad7380_info; indio_dev->modes = INDIO_DIRECT_MODE; - indio_dev->available_scan_masks = ad7380_2_channel_scan_masks; + indio_dev->available_scan_masks = st->chip_info->available_scan_masks; ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, iio_pollfunc_store_time, -- 2.44.0
From: David Lechner <dlechner@baylibre.com> Add support for AD7383, AD7384 pseudo-differential compatible parts. Pseudo differential parts require common mode voltage supplies so add the support for them and add the support of IIO_CHAN_INFO_OFFSET to retrieve the offset Signed-off-by: David Lechner <dlechner@baylibre.com> Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- drivers/iio/adc/ad7380.c | 98 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 85 insertions(+), 13 deletions(-) diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c index caf6deb3a8b1..996ca83feaed 100644 --- a/drivers/iio/adc/ad7380.c +++ b/drivers/iio/adc/ad7380.c @@ -7,6 +7,7 @@ * * Datasheets of supported parts: * ad7380/1 : https://www.analog.com/media/en/technical-documentation/data-sheets/AD7380-7381.pdf + * ad7383/4 : https://www.analog.com/media/en/technical-documentation/data-sheets/ad7383-7384.pdf */ #include <linux/bitfield.h> @@ -68,16 +69,19 @@ struct ad7380_chip_info { const char *name; const struct iio_chan_spec *channels; unsigned int num_channels; + const char * const *vcm_supplies; + unsigned int num_vcm_supplies; }; -#define AD7380_CHANNEL(index, bits) { \ +#define AD7380_CHANNEL(index, bits, diff) { \ .type = IIO_VOLTAGE, \ - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + ((diff) ? 0 : BIT(IIO_CHAN_INFO_OFFSET)), \ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ .indexed = 1, \ - .differential = 1, \ - .channel = 2 * (index), \ - .channel2 = 2 * (index) + 1, \ + .differential = (diff), \ + .channel = (diff) ? (2 * (index)) : (index), \ + .channel2 = (diff) ? (2 * (index) + 1) : 0, \ .scan_index = (index), \ .scan_type = { \ .sign = 's', \ @@ -87,15 +91,23 @@ struct ad7380_chip_info { }, \ } -#define DEFINE_AD7380_2_CHANNEL(name, bits) \ -static const struct iio_chan_spec name[] = { \ - AD7380_CHANNEL(0, bits), \ - AD7380_CHANNEL(1, bits), \ - IIO_CHAN_SOFT_TIMESTAMP(2), \ +#define DEFINE_AD7380_2_CHANNEL(name, bits, diff) \ +static const struct iio_chan_spec name[] = { \ + AD7380_CHANNEL(0, bits, diff), \ + AD7380_CHANNEL(1, bits, diff), \ + IIO_CHAN_SOFT_TIMESTAMP(2), \ } -DEFINE_AD7380_2_CHANNEL(ad7380_channels, 16); -DEFINE_AD7380_2_CHANNEL(ad7381_channels, 14); +/* fully differential */ +DEFINE_AD7380_2_CHANNEL(ad7380_channels, 16, 1); +DEFINE_AD7380_2_CHANNEL(ad7381_channels, 14, 1); +/* pseudo differential */ +DEFINE_AD7380_2_CHANNEL(ad7383_channels, 16, 0); +DEFINE_AD7380_2_CHANNEL(ad7384_channels, 14, 0); + +static const char * const ad7380_2_channel_vcm_supplies[] = { + "aina", "ainb", +}; /* Since this is simultaneous sampling, we don't allow individual channels. */ static const unsigned long ad7380_2_channel_scan_masks[] = { @@ -115,11 +127,28 @@ static const struct ad7380_chip_info ad7381_chip_info = { .num_channels = ARRAY_SIZE(ad7381_channels), }; +static const struct ad7380_chip_info ad7383_chip_info = { + .name = "ad7383", + .channels = ad7383_channels, + .num_channels = ARRAY_SIZE(ad7383_channels), + .vcm_supplies = ad7380_2_channel_vcm_supplies, + .num_vcm_supplies = ARRAY_SIZE(ad7380_2_channel_vcm_supplies), +}; + +static const struct ad7380_chip_info ad7384_chip_info = { + .name = "ad7384", + .channels = ad7384_channels, + .num_channels = ARRAY_SIZE(ad7384_channels), + .vcm_supplies = ad7380_2_channel_vcm_supplies, + .num_vcm_supplies = ARRAY_SIZE(ad7380_2_channel_vcm_supplies), +}; + struct ad7380_state { const struct ad7380_chip_info *chip_info; struct spi_device *spi; struct regmap *regmap; unsigned int vref_mv; + unsigned int vcm_mv[2]; /* * DMA (thus cache coherency maintenance) requires the * transfer buffers to live in their own cache lines. @@ -304,6 +333,11 @@ static int ad7380_read_raw(struct iio_dev *indio_dev, *val2 = chan->scan_type.realbits; return IIO_VAL_FRACTIONAL_LOG2; + case IIO_CHAN_INFO_OFFSET: + *val = st->vcm_mv[chan->channel] * (1 << chan->scan_type.realbits) + / st->vref_mv; + + return IIO_VAL_INT; } return -EINVAL; @@ -350,7 +384,7 @@ static int ad7380_probe(struct spi_device *spi) struct iio_dev *indio_dev; struct ad7380_state *st; struct regulator *vref; - int ret; + int ret, i; indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); if (!indio_dev) @@ -394,6 +428,40 @@ static int ad7380_probe(struct spi_device *spi) st->vref_mv = AD7380_INTERNAL_REF_MV; } + if (st->chip_info->num_vcm_supplies > ARRAY_SIZE(st->vcm_mv)) + return dev_err_probe(&spi->dev, -EINVAL, + "invalid number of VCM supplies\n"); + + /* + * pseudo-differential chips have common mode supplies for the negative + * input pin. + */ + for (i = 0; i < st->chip_info->num_vcm_supplies; i++) { + struct regulator *vcm; + + vcm = devm_regulator_get_optional(&spi->dev, + st->chip_info->vcm_supplies[i]); + if (IS_ERR(vcm)) + return dev_err_probe(&spi->dev, PTR_ERR(vcm), + "Failed to get %s regulator\n", + st->chip_info->vcm_supplies[i]); + + ret = regulator_enable(vcm); + if (ret) + return ret; + + ret = devm_add_action_or_reset(&spi->dev, + ad7380_regulator_disable, vcm); + if (ret) + return ret; + + ret = regulator_get_voltage(vcm); + if (ret < 0) + return ret; + + st->vcm_mv[i] = ret / 1000; + } + st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config); if (IS_ERR(st->regmap)) return dev_err_probe(&spi->dev, PTR_ERR(st->regmap), @@ -422,12 +490,16 @@ static int ad7380_probe(struct spi_device *spi) static const struct of_device_id ad7380_of_match_table[] = { { .compatible = "adi,ad7380", .data = &ad7380_chip_info }, { .compatible = "adi,ad7381", .data = &ad7381_chip_info }, + { .compatible = "adi,ad7383", .data = &ad7383_chip_info }, + { .compatible = "adi,ad7384", .data = &ad7384_chip_info }, { } }; static const struct spi_device_id ad7380_id_table[] = { { "ad7380", (kernel_ulong_t)&ad7380_chip_info }, { "ad7381", (kernel_ulong_t)&ad7381_chip_info }, + { "ad7383", (kernel_ulong_t)&ad7383_chip_info }, + { "ad7384", (kernel_ulong_t)&ad7384_chip_info }, { } }; MODULE_DEVICE_TABLE(spi, ad7380_id_table); -- 2.44.0
From: David Lechner <dlechner@baylibre.com> Adding AD7383 and AD7384 compatible parts that are pseudo-differential. Pseudo-differential require common mode voltage supplies, so add them conditionally Signed-off-by: David Lechner <dlechner@baylibre.com> Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- .../devicetree/bindings/iio/adc/adi,ad7380.yaml | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml index 5e1ee0ebe0a2..de3d28a021ae 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml @@ -13,6 +13,8 @@ maintainers: description: | * https://www.analog.com/en/products/ad7380.html * https://www.analog.com/en/products/ad7381.html + * https://www.analog.com/en/products/ad7383.html + * https://www.analog.com/en/products/ad7384.html $ref: /schemas/spi/spi-peripheral-props.yaml# @@ -21,6 +23,8 @@ properties: enum: - adi,ad7380 - adi,ad7381 + - adi,ad7383 + - adi,ad7384 reg: maxItems: 1 @@ -42,6 +46,16 @@ properties: A 2.5V to 3.3V supply for the external reference voltage. When omitted, the internal 2.5V reference is used. + aina-supply: + description: + The common mode voltage supply for the AINA- pin on pseudo-differential + chips. + + ainb-supply: + description: + The common mode voltage supply for the AINB- pin on pseudo-differential + chips. + interrupts: description: When the device is using 1-wire mode, this property is used to optionally @@ -56,6 +70,24 @@ required: unevaluatedProperties: false +allOf: + # pseudo-differential chips require common mode voltage supplies, + # true differential chips don't use them + - if: + properties: + compatible: + enum: + - adi,ad7383 + - adi,ad7384 + then: + required: + - aina-supply + - ainb-supply + else: + properties: + aina-supply: false + ainb-supply: false + examples: - | #include <dt-bindings/interrupt-controller/irq.h> -- 2.44.0
From: David Lechner <dlechner@baylibre.com> This adds a new driver for the AD7380 family ADCs. The driver currently implements basic support for the AD7380, AD7381, 2-channel differential ADCs. Support for additional single-ended, pseudo-differential and 4-channel chips that use the same register map as well as additional features of the chip will be added in future patches. Co-developed-by: Stefan Popa <stefan.popa@analog.com> Signed-off-by: Stefan Popa <stefan.popa@analog.com> Reviewed-by: Nuno Sa <nuno.sa@analog.com> Signed-off-by: David Lechner <dlechner@baylibre.com> [Julien Stephan: add datasheet links of supported parts] [Julien Stephan: fix rx/tx buffer for regmap access] Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- MAINTAINERS | 1 + drivers/iio/adc/Kconfig | 16 ++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad7380.c | 447 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 465 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f7c512f3bbda..2277870853c7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -435,6 +435,7 @@ S: Supported W: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x W: https://ez.analog.com/linux-software-drivers F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml +F: drivers/iio/adc/ad7380.c AD7877 TOUCHSCREEN DRIVER M: Michael Hennerich <michael.hennerich@analog.com> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 8db68b80b391..631386b037ae 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -155,6 +155,22 @@ config AD7298 To compile this driver as a module, choose M here: the module will be called ad7298. +config AD7380 + tristate "Analog Devices AD7380 ADC driver" + depends on SPI_MASTER + select IIO_BUFFER + select IIO_TRIGGER + select IIO_TRIGGERED_BUFFER + help + AD7380 is a family of simultaneous sampling ADCs that share the same + SPI register map and have similar pinouts. + + Say yes here to build support for Analog Devices AD7380 ADC and + similar chips. + + To compile this driver as a module, choose M here: the module will be + called ad7380. + config AD7476 tristate "Analog Devices AD7476 1-channel ADCs driver and other similar devices from AD and TI" depends on SPI diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index edb32ce2af02..bd3cbbb178fa 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_AD7291) += ad7291.o obj-$(CONFIG_AD7292) += ad7292.o obj-$(CONFIG_AD7298) += ad7298.o obj-$(CONFIG_AD7923) += ad7923.o +obj-$(CONFIG_AD7380) += ad7380.o obj-$(CONFIG_AD7476) += ad7476.o obj-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o obj-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c new file mode 100644 index 000000000000..caf6deb3a8b1 --- /dev/null +++ b/drivers/iio/adc/ad7380.c @@ -0,0 +1,447 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Analog Devices AD738x Simultaneous Sampling SAR ADCs + * + * Copyright 2017 Analog Devices Inc. + * Copyright 2024 BayLibre, SAS + * + * Datasheets of supported parts: + * ad7380/1 : https://www.analog.com/media/en/technical-documentation/data-sheets/AD7380-7381.pdf + */ + +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/cleanup.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/slab.h> +#include <linux/spi/spi.h> +#include <linux/sysfs.h> + +#include <linux/iio/buffer.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +/* 2.5V internal reference voltage */ +#define AD7380_INTERNAL_REF_MV 2500 + +/* reading and writing registers is more reliable at lower than max speed */ +#define AD7380_REG_WR_SPEED_HZ 10000000 + +#define AD7380_REG_WR BIT(15) +#define AD7380_REG_REGADDR GENMASK(14, 12) +#define AD7380_REG_DATA GENMASK(11, 0) + +#define AD7380_REG_ADDR_NOP 0x0 +#define AD7380_REG_ADDR_CONFIG1 0x1 +#define AD7380_REG_ADDR_CONFIG2 0x2 +#define AD7380_REG_ADDR_ALERT 0x3 +#define AD7380_REG_ADDR_ALERT_LOW_TH 0x4 +#define AD7380_REG_ADDR_ALERT_HIGH_TH 0x5 + +#define AD7380_CONFIG1_OS_MODE BIT(9) +#define AD7380_CONFIG1_OSR GENMASK(8, 6) +#define AD7380_CONFIG1_CRC_W BIT(5) +#define AD7380_CONFIG1_CRC_R BIT(4) +#define AD7380_CONFIG1_ALERTEN BIT(3) +#define AD7380_CONFIG1_RES BIT(2) +#define AD7380_CONFIG1_REFSEL BIT(1) +#define AD7380_CONFIG1_PMODE BIT(0) + +#define AD7380_CONFIG2_SDO2 GENMASK(9, 8) +#define AD7380_CONFIG2_SDO BIT(8) +#define AD7380_CONFIG2_RESET GENMASK(7, 0) + +#define AD7380_CONFIG2_RESET_SOFT 0x3C +#define AD7380_CONFIG2_RESET_HARD 0xFF + +#define AD7380_ALERT_LOW_TH GENMASK(11, 0) +#define AD7380_ALERT_HIGH_TH GENMASK(11, 0) + +struct ad7380_chip_info { + const char *name; + const struct iio_chan_spec *channels; + unsigned int num_channels; +}; + +#define AD7380_CHANNEL(index, bits) { \ + .type = IIO_VOLTAGE, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .indexed = 1, \ + .differential = 1, \ + .channel = 2 * (index), \ + .channel2 = 2 * (index) + 1, \ + .scan_index = (index), \ + .scan_type = { \ + .sign = 's', \ + .realbits = (bits), \ + .storagebits = 16, \ + .endianness = IIO_CPU, \ + }, \ +} + +#define DEFINE_AD7380_2_CHANNEL(name, bits) \ +static const struct iio_chan_spec name[] = { \ + AD7380_CHANNEL(0, bits), \ + AD7380_CHANNEL(1, bits), \ + IIO_CHAN_SOFT_TIMESTAMP(2), \ +} + +DEFINE_AD7380_2_CHANNEL(ad7380_channels, 16); +DEFINE_AD7380_2_CHANNEL(ad7381_channels, 14); + +/* Since this is simultaneous sampling, we don't allow individual channels. */ +static const unsigned long ad7380_2_channel_scan_masks[] = { + GENMASK(1, 0), + 0 +}; + +static const struct ad7380_chip_info ad7380_chip_info = { + .name = "ad7380", + .channels = ad7380_channels, + .num_channels = ARRAY_SIZE(ad7380_channels), +}; + +static const struct ad7380_chip_info ad7381_chip_info = { + .name = "ad7381", + .channels = ad7381_channels, + .num_channels = ARRAY_SIZE(ad7381_channels), +}; + +struct ad7380_state { + const struct ad7380_chip_info *chip_info; + struct spi_device *spi; + struct regmap *regmap; + unsigned int vref_mv; + /* + * DMA (thus cache coherency maintenance) requires the + * transfer buffers to live in their own cache lines. + * Make the buffer large enough for 2 16-bit samples and one 64-bit + * aligned 64 bit timestamp. + */ + struct { + u16 raw[2]; + + s64 ts __aligned(8); + } scan_data __aligned(IIO_DMA_MINALIGN); + u16 tx; + u16 rx; +}; + +static int ad7380_regmap_reg_write(void *context, unsigned int reg, + unsigned int val) +{ + struct ad7380_state *st = context; + struct spi_transfer xfer = { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = 16, + .len = 2, + .tx_buf = &st->tx, + }; + + st->tx = FIELD_PREP(AD7380_REG_WR, 1) | + FIELD_PREP(AD7380_REG_REGADDR, reg) | + FIELD_PREP(AD7380_REG_DATA, val); + + return spi_sync_transfer(st->spi, &xfer, 1); +} + +static int ad7380_regmap_reg_read(void *context, unsigned int reg, + unsigned int *val) +{ + struct ad7380_state *st = context; + struct spi_transfer xfers[] = { + { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = 16, + .len = 2, + .tx_buf = &st->tx, + .cs_change = 1, + .cs_change_delay = { + .value = 10, /* t[CSH] */ + .unit = SPI_DELAY_UNIT_NSECS, + }, + }, { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = 16, + .len = 2, + .rx_buf = &st->rx, + }, + }; + int ret; + + st->tx = FIELD_PREP(AD7380_REG_WR, 0) | + FIELD_PREP(AD7380_REG_REGADDR, reg) | + FIELD_PREP(AD7380_REG_DATA, 0); + + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); + if (ret < 0) + return ret; + + *val = FIELD_GET(AD7380_REG_DATA, st->rx); + + return 0; +} + +static const struct regmap_config ad7380_regmap_config = { + .reg_bits = 3, + .val_bits = 12, + .reg_read = ad7380_regmap_reg_read, + .reg_write = ad7380_regmap_reg_write, + .max_register = AD7380_REG_ADDR_ALERT_HIGH_TH, + .can_sleep = true, +}; + +static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg, + u32 writeval, u32 *readval) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int ret; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + if (readval) + ret = regmap_read(st->regmap, reg, readval); + else + ret = regmap_write(st->regmap, reg, writeval); + + iio_device_release_direct_mode(indio_dev); + + return ret; +} + +static irqreturn_t ad7380_trigger_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct ad7380_state *st = iio_priv(indio_dev); + struct spi_transfer xfer = { + .bits_per_word = st->chip_info->channels[0].scan_type.realbits, + .len = 4, + .rx_buf = st->scan_data.raw, + }; + int ret; + + ret = spi_sync_transfer(st->spi, &xfer, 1); + if (ret) + goto out; + + iio_push_to_buffers_with_timestamp(indio_dev, &st->scan_data, + pf->timestamp); + +out: + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + +static int ad7380_read_direct(struct ad7380_state *st, + struct iio_chan_spec const *chan, int *val) +{ + struct spi_transfer xfers[] = { + /* toggle CS (no data xfer) to trigger a conversion */ + { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = chan->scan_type.realbits, + .delay = { + .value = 190, /* t[CONVERT] */ + .unit = SPI_DELAY_UNIT_NSECS, + }, + .cs_change = 1, + .cs_change_delay = { + .value = 10, /* t[CSH] */ + .unit = SPI_DELAY_UNIT_NSECS, + }, + }, + /* then read both channels */ + { + .speed_hz = AD7380_REG_WR_SPEED_HZ, + .bits_per_word = chan->scan_type.realbits, + .rx_buf = st->scan_data.raw, + .len = 4, + }, + }; + int ret; + + ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers)); + if (ret < 0) + return ret; + + *val = sign_extend32(st->scan_data.raw[chan->scan_index], + chan->scan_type.realbits - 1); + + return IIO_VAL_INT; +} + +static int ad7380_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long info) +{ + struct ad7380_state *st = iio_priv(indio_dev); + int ret; + + switch (info) { + case IIO_CHAN_INFO_RAW: + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = ad7380_read_direct(st, chan, val); + iio_device_release_direct_mode(indio_dev); + + return ret; + case IIO_CHAN_INFO_SCALE: + *val = st->vref_mv; + *val2 = chan->scan_type.realbits; + + return IIO_VAL_FRACTIONAL_LOG2; + } + + return -EINVAL; +} + +static const struct iio_info ad7380_info = { + .read_raw = &ad7380_read_raw, + .debugfs_reg_access = &ad7380_debugfs_reg_access, +}; + +static int ad7380_init(struct ad7380_state *st, struct regulator *vref) +{ + int ret; + + /* perform hard reset */ + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, + AD7380_CONFIG2_RESET, + FIELD_PREP(AD7380_CONFIG2_RESET, + AD7380_CONFIG2_RESET_HARD)); + if (ret < 0) + return ret; + + /* select internal or external reference voltage */ + ret = regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG1, + AD7380_CONFIG1_REFSEL, + FIELD_PREP(AD7380_CONFIG1_REFSEL, + vref ? 1 : 0)); + if (ret < 0) + return ret; + + /* SPI 1-wire mode */ + return regmap_update_bits(st->regmap, AD7380_REG_ADDR_CONFIG2, + AD7380_CONFIG2_SDO, + FIELD_PREP(AD7380_CONFIG2_SDO, 1)); +} + +static void ad7380_regulator_disable(void *p) +{ + regulator_disable(p); +} + +static int ad7380_probe(struct spi_device *spi) +{ + struct iio_dev *indio_dev; + struct ad7380_state *st; + struct regulator *vref; + int ret; + + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->spi = spi; + st->chip_info = spi_get_device_match_data(spi); + if (!st->chip_info) + return dev_err_probe(&spi->dev, -EINVAL, "missing match data\n"); + + vref = devm_regulator_get_optional(&spi->dev, "refio"); + if (IS_ERR(vref)) { + if (PTR_ERR(vref) != -ENODEV) + return dev_err_probe(&spi->dev, PTR_ERR(vref), + "Failed to get refio regulator\n"); + + vref = NULL; + } + + /* + * If there is no REFIO supply, then it means that we are using + * the internal 2.5V reference, otherwise REFIO is reference voltage. + */ + if (vref) { + ret = regulator_enable(vref); + if (ret) + return ret; + + ret = devm_add_action_or_reset(&spi->dev, + ad7380_regulator_disable, vref); + if (ret) + return ret; + + ret = regulator_get_voltage(vref); + if (ret < 0) + return ret; + + st->vref_mv = ret / 1000; + } else { + st->vref_mv = AD7380_INTERNAL_REF_MV; + } + + st->regmap = devm_regmap_init(&spi->dev, NULL, st, &ad7380_regmap_config); + if (IS_ERR(st->regmap)) + return dev_err_probe(&spi->dev, PTR_ERR(st->regmap), + "failed to allocate register map\n"); + + indio_dev->channels = st->chip_info->channels; + indio_dev->num_channels = st->chip_info->num_channels; + indio_dev->name = st->chip_info->name; + indio_dev->info = &ad7380_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->available_scan_masks = ad7380_2_channel_scan_masks; + + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, + iio_pollfunc_store_time, + ad7380_trigger_handler, NULL); + if (ret) + return ret; + + ret = ad7380_init(st, vref); + if (ret) + return ret; + + return devm_iio_device_register(&spi->dev, indio_dev); +} + +static const struct of_device_id ad7380_of_match_table[] = { + { .compatible = "adi,ad7380", .data = &ad7380_chip_info }, + { .compatible = "adi,ad7381", .data = &ad7381_chip_info }, + { } +}; + +static const struct spi_device_id ad7380_id_table[] = { + { "ad7380", (kernel_ulong_t)&ad7380_chip_info }, + { "ad7381", (kernel_ulong_t)&ad7381_chip_info }, + { } +}; +MODULE_DEVICE_TABLE(spi, ad7380_id_table); + +static struct spi_driver ad7380_driver = { + .driver = { + .name = "ad7380", + .of_match_table = ad7380_of_match_table, + }, + .probe = ad7380_probe, + .id_table = ad7380_id_table, +}; +module_spi_driver(ad7380_driver); + +MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>"); +MODULE_DESCRIPTION("Analog Devices AD738x ADC driver"); +MODULE_LICENSE("GPL"); -- 2.44.0
From: David Lechner <dlechner@baylibre.com> This adds a binding specification for the Analog Devices Inc. AD7380 family of ADCs. Signed-off-by: David Lechner <dlechner@baylibre.com> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> --- .../devicetree/bindings/iio/adc/adi,ad7380.yaml | 82 ++++++++++++++++++++++ MAINTAINERS | 9 +++ 2 files changed, 91 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml new file mode 100644 index 000000000000..5e1ee0ebe0a2 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml @@ -0,0 +1,82 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad7380.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices Simultaneous Sampling Analog to Digital Converters + +maintainers: + - Michael Hennerich <Michael.Hennerich@analog.com> + - Nuno Sá <nuno.sa@analog.com> + +description: | + * https://www.analog.com/en/products/ad7380.html + * https://www.analog.com/en/products/ad7381.html + +$ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + enum: + - adi,ad7380 + - adi,ad7381 + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 80000000 + spi-cpol: true + spi-cpha: true + + vcc-supply: + description: A 3V to 3.6V supply that powers the chip. + + vlogic-supply: + description: + A 1.65V to 3.6V supply for the logic pins. + + refio-supply: + description: + A 2.5V to 3.3V supply for the external reference voltage. When omitted, + the internal 2.5V reference is used. + + interrupts: + description: + When the device is using 1-wire mode, this property is used to optionally + specify the ALERT interrupt. + maxItems: 1 + +required: + - compatible + - reg + - vcc-supply + - vlogic-supply + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad7380"; + reg = <0>; + + spi-cpol; + spi-cpha; + spi-max-frequency = <80000000>; + + interrupts = <27 IRQ_TYPE_EDGE_FALLING>; + interrupt-parent = <&gpio0>; + + vcc-supply = <&supply_3_3V>; + vlogic-supply = <&supply_3_3V>; + refio-supply = <&supply_2_5V>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 7b1a6f2d0c9c..f7c512f3bbda 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -427,6 +427,15 @@ W: http://wiki.analog.com/AD7142 W: https://ez.analog.com/linux-software-drivers F: drivers/input/misc/ad714x.c +AD738X ADC DRIVER (AD7380/1/2/4) +M: Michael Hennerich <michael.hennerich@analog.com> +M: Nuno Sá <nuno.sa@analog.com> +R: David Lechner <dlechner@baylibre.com> +S: Supported +W: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad738x +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml + AD7877 TOUCHSCREEN DRIVER M: Michael Hennerich <michael.hennerich@analog.com> S: Supported -- 2.44.0
Taking over this series with David Lechner's aproval, to add some fixes, proper handling of pseudo differential parts and some extra commits adding support for 4-channel compatible parts. Here is David's cover letter: This series is adding a new driver for the Analog Devices Inc. AD7380, AD7381, AD7383, and AD7384 ADCs. These chips are part of a family of simultaneous sampling SAR ADCs. To keep things simple, the initial driver implementation only supports the 2/4-channel differential chips listed above. There are also 4-channel single-ended chips in the family that can be added later. Furthermore, the driver is just implementing basic support for capturing data. Additional features like interrupts, CRC, etc. can be added later. This work is being done by BayLibre and on behalf of Analog Devices Inc. hence the maintainers are @analog.com. To: Lars-Peter Clausen <lars@metafoo.de> To: Michael Hennerich <Michael.Hennerich@analog.com> To: Nuno Sá <nuno.sa@analog.com> To: David Lechner <dlechner@baylibre.com> To: Jonathan Cameron <jic23@kernel.org> To: Rob Herring <robh+dt@kernel.org> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> To: Conor Dooley <conor+dt@kernel.org> To: Liam Girdwood <lgirdwood@gmail.com> To: Mark Brown <broonie@kernel.org> Cc: linux-iio@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Changes in v5: - make ad7380_regmap_config static Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202401280629.5kknB57C-lkp@intel.com/ - don't use bool in FIELD_PREP Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202401280629.5kknB57C-lkp@intel.com/ - fix rx/tx buffer for regmap access - add datasheet links of supported parts - move reading reference voltage to probe function - removed DIFFERENTIAL from a few macro names Adding the following commits on top of the v4 - add supplies for pseudo-differential chips - prepare driver to add more compatible parts - add support for 4-channel compatible parts - Link to v4: https://lore.kernel.org/all/20240110-ad7380-mainline-v4-0-93a1d96b50fa@baylibre.com Changes in v4: - Dropped SPI bindings patch. - Removed `spi-rx-bus-channels` from the adi,ad7380 bindings. - Link to v3: https://lore.kernel.org/r/20231215-ad7380-mainline-v3-0-7a11ebf642b9@baylibre.com Changes in v3: - dt-bindings: - Picked up Conor's Reviewed-By on the adi,ad7380 bindings - driver: - Removed extra indent in DEFINE_AD7380_DIFFERENTIAL_2_CHANNEL macro - Removed scan mask that included timestamp channel - Removed parent device assignment - Picked up Nuno's Reviewed-by - Link to v2: https://lore.kernel.org/r/20231213-ad7380-mainline-v2-0-cd32150d84a3@baylibre.com Changes in v2: - dt-bindings: - Added new patch with generic spi-rx-bus-channels property - Added maxItems to reg property - Replaced adi,sdo-mode property with spi-rx-bus-channels - Made spi-rx-bus-channels property optional with default value of 1 (this made the if: check more complex) - Changed example to use gpio for interrupt - driver: - Fixed CONFIG_AD7380 in Makefile - rx_buf = st->scan_data.raw instead of rx_buf = &st->scan_data - Moved iio_push_to_buffers_with_timestamp() outside of if statement - Removed extra blank lines - Renamed regulator disable function - Dropped checking of adi,sdo-mode property (regardless of the actual wiring, we can always use 1-wire mode) - Added available_scan_masks - Added check for missing driver match data - Link to v1: https://lore.kernel.org/r/20231208-ad7380-mainline-v1-0-2b33fe2f44ae@baylibre.com --- Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- David Lechner (4): dt-bindings: iio: adc: Add binding for AD7380 ADCs iio: adc: ad7380: new driver for AD7380 ADCs dt-bindings: iio: adc: ad7380: add pseudo-differential parts iio: adc: ad7380: add support for pseudo-differential parts Julien Stephan (3): iio: adc: ad7380: prepare for parts with more channels dt-bindings: iio: adc: ad7380: add support for ad738x-4 4 channels variants iio: adc: ad7380: add support for ad738x-4 4 channels variants .../devicetree/bindings/iio/adc/adi,ad7380.yaml | 148 +++++ MAINTAINERS | 10 + drivers/iio/adc/Kconfig | 16 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad7380.c | 614 +++++++++++++++++++++ 5 files changed, 789 insertions(+) --- base-commit: 1446d8ef48196409f811c25071b2cc510a49fc60 change-id: 20240311-adding-new-ad738x-driver-5f9b54ad7c74 Best regards, -- Julien Stephan <jstephan@baylibre.com>
Hello everyone, I am Anshul Dalal, a pre-final year student at SRMIST (India). I am pursuing my Bachelor's in Computer Science and Engineering and wish to participate in GSoC 2024 as part of The Linux Foundation under the IIO worksgroup. Following the suggestion from the IIO GSoC page[1], I would like to work on a driver for AD7294-2. I am interested in the device since it offers a wide array of functionality that is different from my past IIO drivers[2]. I have prepared a draft proposal and would like to get early feedback: https://docs.google.com/document/d/1zf9yDq2-Ba8Vqh10w1cYI3buHzh0qIYwzf7xBkaEzDM I'm aware of interest in the same device from other contributors[3]. If required, I'm ready to work on any other part suggested by the company or the IIO community. Best regards, Anshul --- [1]: https://wiki.linuxfoundation.org/gsoc/2024-gsoc-iio-driver [2]: Microchip MCP4821 DAC: https://lore.kernel.org/lkml/20231220151954.154595-1-anshulusr@gmail.com/ LiteOn LTR390 light sensor: https://lore.kernel.org/lkml/20231208102211.413019-1-anshulusr@gmail.com/ Aosong AGS02MA air quality sensor: https://lore.kernel.org/lkml/20231215162312.143568-1-anshulusr@gmail.com/ [3]: https://lore.kernel.org/linux-iio/20240229184636.13368-1-danascape@gmail.com/ https://lore.kernel.org/linux-iio/YlXR0d7waKW9xncd@fedora/
Per filesystems/sysfs.rst, show() should only use sysfs_emit() or sysfs_emit_at() when formatting the value to be returned to user space. coccinelle complains that there are still a couple of functions that use snprintf(). Convert them to sysfs_emit(). sprintf() and scnprintf() will be converted as well if they have. Generally, this patch is generated by make coccicheck M=<path/to/file> MODE=patch \ COCCI=scripts/coccinelle/api/device_attr_show.cocci No functional change intended CC: Jiri Kosina <jikos@kernel.org> CC: Jonathan Cameron <jic23@kernel.org> CC: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> CC: Benjamin Tissoires <benjamin.tissoires@redhat.com> CC: linux-input@vger.kernel.org CC: linux-iio@vger.kernel.org Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- V3: rewrap the line as will be under 80 chars and add Reviewed-by # Jonathan This is a part of the work "Fix coccicheck device_attr_show warnings"[1] Split them per subsystem so that the maintainer can review it easily [1] https://lore.kernel.org/lkml/20240116041129.3937800-1-lizhijian@fujitsu.com/ --- drivers/hid/hid-sensor-custom.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c index d85398721659..ac214777d7d9 100644 --- a/drivers/hid/hid-sensor-custom.c +++ b/drivers/hid/hid-sensor-custom.c @@ -155,7 +155,7 @@ static ssize_t enable_sensor_show(struct device *dev, { struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev); - return sprintf(buf, "%d\n", sensor_inst->enable); + return sysfs_emit(buf, "%d\n", sensor_inst->enable); } static int set_power_report_state(struct hid_sensor_custom *sensor_inst, @@ -372,14 +372,13 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr, sizeof(struct hid_custom_usage_desc), usage_id_cmp); if (usage_desc) - return snprintf(buf, PAGE_SIZE, "%s\n", - usage_desc->desc); + return sysfs_emit(buf, "%s\n", usage_desc->desc); else - return sprintf(buf, "not-specified\n"); + return sysfs_emit(buf, "not-specified\n"); } else return -EINVAL; - return sprintf(buf, "%d\n", value); + return sysfs_emit(buf, "%d\n", value); } static ssize_t store_value(struct device *dev, struct device_attribute *attr, -- 2.29.2
BMP2xx, BMP3xx, and BMP5xx use consecutive buffers for their temperature, pressure and humidity readings. This facilitates the use of burst reads in order to acquire data much faster and in a different way from the one used in oneshot captures. BMP085 and BMP180 use a completely different measurement process that is well defined and is used in their buffer_handler(). Suggested-by: Angel Iglesias <ang.iglesiasg@gmail.com> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/Kconfig | 2 + drivers/iio/pressure/bmp280-core.c | 231 +++++++++++++++++++++++++++-- drivers/iio/pressure/bmp280-spi.c | 8 +- drivers/iio/pressure/bmp280.h | 14 +- 4 files changed, 241 insertions(+), 14 deletions(-) diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig index 79adfd059c3a..5145b94b4679 100644 --- a/drivers/iio/pressure/Kconfig +++ b/drivers/iio/pressure/Kconfig @@ -31,6 +31,8 @@ config BMP280 select REGMAP select BMP280_I2C if (I2C) select BMP280_SPI if (SPI_MASTER) + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER help Say yes here to build support for Bosch Sensortec BMP180, BMP280, BMP380 and BMP580 pressure and temperature sensors. Also supports the BME280 with diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index ddfcd23f29a0..ffcd17807cf5 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -40,7 +40,10 @@ #include <linux/regmap.h> #include <linux/regulator/consumer.h> +#include <linux/iio/buffer.h> #include <linux/iio/iio.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> #include <asm/unaligned.h> @@ -454,7 +457,7 @@ static int bmp280_read_temp(struct bmp280_data *data, s32 *comp_temp) int ret; ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB, - data->buf, sizeof(data->buf)); + data->buf, BMP280_NUM_TEMP_BYTES); if (ret < 0) { dev_err(data->dev, "failed to read temperature\n"); return ret; @@ -484,7 +487,7 @@ static int bmp280_read_press(struct bmp280_data *data, u32 *comp_press) return ret; ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB, - data->buf, sizeof(data->buf)); + data->buf, BMP280_NUM_PRESS_BYTES); if (ret < 0) { dev_err(data->dev, "failed to read pressure\n"); return ret; @@ -513,13 +516,13 @@ static int bmp280_read_humid(struct bmp280_data *data, u32 *comp_humidity) return ret; ret = regmap_bulk_read(data->regmap, BMP280_REG_HUMIDITY_MSB, - &data->be16, sizeof(data->be16)); + &data->be16, BME280_NUM_HUMIDITY_BYTES); if (ret < 0) { dev_err(data->dev, "failed to read humidity\n"); return ret; } - adc_humidity = be16_to_cpu(data->be16); + adc_humidity = get_unaligned_be16(&data->be16); if (adc_humidity == BMP280_HUMIDITY_SKIPPED) { /* reading was skipped */ dev_err(data->dev, "reading humidity skipped\n"); @@ -931,6 +934,73 @@ static int bmp280_chip_config(struct bmp280_data *data) return ret; } +static irqreturn_t bmp280_buffer_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct bmp280_data *data = iio_priv(indio_dev); + s32 adc_temp, adc_press, adc_humidity; + u8 size_of_burst_read; + int ret, chan_value; + + guard(mutex)(&data->lock); + + if (test_bit(BME280_HUMID, indio_dev->active_scan_mask)) + size_of_burst_read = 8; + else + size_of_burst_read = 6; + + /* Burst read data registers */ + ret = regmap_bulk_read(data->regmap, BMP280_REG_PRESS_MSB, + data->buf, 8); + if (ret) { + dev_err(data->dev, "failed to read sensor data\n"); + return ret; + } + + /* Temperature calculations */ + memcpy(&chan_value, &data->buf[3], 3); + + adc_temp = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&chan_value)); + if (adc_temp == BMP280_TEMP_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading temperature skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[0] = bmp280_compensate_temp(data, adc_temp); + + /* Pressure calculations */ + memcpy(&chan_value, &data->buf[0], 3); + + adc_press = FIELD_GET(BMP280_MEAS_TRIM_MASK, get_unaligned_be24(&chan_value)); + if (adc_press == BMP280_PRESS_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading pressure skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[1] = bmp280_compensate_press(data, adc_press); + + /* Humidity calculations */ + if (test_bit(BME280_HUMID, indio_dev->active_scan_mask)) { + memcpy(&chan_value, &data->buf[6], 2); + + adc_humidity = get_unaligned_be16(&chan_value); + if (adc_humidity == BMP280_HUMIDITY_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading humidity skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[2] = bmp280_compensate_humidity(data, adc_humidity); + } + + iio_push_to_buffers_with_timestamp(indio_dev, &data->iio_buf, + iio_get_time_ns(indio_dev)); + + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 }; static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID }; static const int bmp280_temp_coeffs[] = { 10, 1 }; @@ -973,6 +1043,8 @@ const struct bmp280_chip_info bmp280_chip_info = { .read_temp = bmp280_read_temp, .read_press = bmp280_read_press, .read_calib = bmp280_read_calib, + + .buffer_handler = bmp280_buffer_handler }; EXPORT_SYMBOL_NS(bmp280_chip_info, IIO_BMP280); @@ -1032,6 +1104,8 @@ const struct bmp280_chip_info bme280_chip_info = { .read_press = bmp280_read_press, .read_humid = bmp280_read_humid, .read_calib = bme280_read_calib, + + .buffer_handler = bmp280_buffer_handler }; EXPORT_SYMBOL_NS(bme280_chip_info, IIO_BMP280); @@ -1158,7 +1232,7 @@ static int bmp380_read_temp(struct bmp280_data *data, s32 *comp_temp) int ret; ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB, - data->buf, sizeof(data->buf)); + data->buf, BMP280_NUM_TEMP_BYTES); if (ret) { dev_err(data->dev, "failed to read temperature\n"); return ret; @@ -1187,7 +1261,7 @@ static int bmp380_read_press(struct bmp280_data *data, u32 *comp_press) return ret; ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB, - data->buf, sizeof(data->buf)); + data->buf, BMP280_NUM_PRESS_BYTES); if (ret) { dev_err(data->dev, "failed to read pressure\n"); return ret; @@ -1366,6 +1440,54 @@ static int bmp380_chip_config(struct bmp280_data *data) return 0; } +static irqreturn_t bmp380_buffer_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct bmp280_data *data = iio_priv(indio_dev); + s32 adc_temp, adc_press; + int ret, chan_value; + + guard(mutex)(&data->lock); + + /* Burst read data registers */ + ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB, + data->buf, 6); + if (ret) { + dev_err(data->dev, "failed to read sensor data\n"); + return ret; + } + + /* Temperature calculations */ + memcpy(&chan_value, &data->buf[3], 3); + + adc_temp = get_unaligned_le24(&chan_value); + if (adc_temp == BMP380_TEMP_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading temperature skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[0] = bmp380_compensate_temp(data, adc_temp); + + /* Pressure calculations */ + memcpy(&chan_value, &data->buf[0], 3); + + adc_press = get_unaligned_le24(&chan_value); + if (adc_press == BMP380_PRESS_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading pressure skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[1] = bmp380_compensate_press(data, adc_press); + + iio_push_to_buffers_with_timestamp(indio_dev, &data->iio_buf, + iio_get_time_ns(indio_dev)); + + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 }; static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128}; static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID }; @@ -1408,6 +1530,8 @@ const struct bmp280_chip_info bmp380_chip_info = { .read_press = bmp380_read_press, .read_calib = bmp380_read_calib, .preinit = bmp380_preinit, + + .buffer_handler = bmp380_buffer_handler }; EXPORT_SYMBOL_NS(bmp380_chip_info, IIO_BMP280); @@ -1528,7 +1652,7 @@ static int bmp580_read_temp(struct bmp280_data *data, s32 *raw_temp) int ret; ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf, - sizeof(data->buf)); + BMP280_NUM_TEMP_BYTES); if (ret) { dev_err(data->dev, "failed to read temperature\n"); return ret; @@ -1548,7 +1672,7 @@ static int bmp580_read_press(struct bmp280_data *data, u32 *raw_press) int ret; ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf, - sizeof(data->buf)); + BMP280_NUM_PRESS_BYTES); if (ret) { dev_err(data->dev, "failed to read pressure\n"); return ret; @@ -1863,6 +1987,54 @@ static int bmp580_chip_config(struct bmp280_data *data) return 0; } +static irqreturn_t bmp580_buffer_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct bmp280_data *data = iio_priv(indio_dev); + s32 adc_temp, adc_press; + int ret, chan_value; + + guard(mutex)(&data->lock); + + /* Burst read data registers */ + ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, + data->buf, 6); + if (ret) { + dev_err(data->dev, "failed to read sensor data\n"); + return ret; + } + + /* Temperature calculations */ + memcpy(&chan_value, &data->buf[3], 3); + + adc_temp = get_unaligned_le24(&chan_value); + if (adc_temp == BMP580_TEMP_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading temperature skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[0] = adc_temp; + + /* Pressure calculations */ + memcpy(&chan_value, &data->buf[0], 3); + + adc_press = get_unaligned_le24(&chan_value); + if (adc_press == BMP580_PRESS_SKIPPED) { + /* reading was skipped */ + dev_err(data->dev, "reading pressure skipped\n"); + return -EIO; + } + data->iio_buf.sensor_data[1] = adc_press; + + iio_push_to_buffers_with_timestamp(indio_dev, &data->iio_buf, + iio_get_time_ns(indio_dev)); + + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 }; static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT }; static const int bmp580_temp_coeffs[] = { 1000, 16 }; @@ -1903,6 +2075,8 @@ const struct bmp280_chip_info bmp580_chip_info = { .read_temp = bmp580_read_temp, .read_press = bmp580_read_press, .preinit = bmp580_preinit, + + .buffer_handler = bmp580_buffer_handler }; EXPORT_SYMBOL_NS(bmp580_chip_info, IIO_BMP280); @@ -2054,7 +2228,7 @@ static int bmp180_read_adc_press(struct bmp280_data *data, int *val) return ret; ret = regmap_bulk_read(data->regmap, BMP180_REG_OUT_MSB, - data->buf, sizeof(data->buf)); + data->buf, BMP280_NUM_PRESS_BYTES); if (ret) return ret; @@ -2122,6 +2296,35 @@ static int bmp180_chip_config(struct bmp280_data *data) return 0; } +static irqreturn_t bmp180_buffer_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct bmp280_data *data = iio_priv(indio_dev); + int ret, chan_value; + + guard(mutex)(&data->lock); + + ret = data->chip_info->read_temp(data, &chan_value); + if (ret) + return ret; + + data->iio_buf.sensor_data[0] = chan_value; + + ret = data->chip_info->read_press(data, &chan_value); + if (ret) + return ret; + + data->iio_buf.sensor_data[1] = chan_value; + + iio_push_to_buffers_with_timestamp(indio_dev, &data->iio_buf, + iio_get_time_ns(indio_dev)); + + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + static const int bmp180_oversampling_temp_avail[] = { 1 }; static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 }; static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID }; @@ -2157,6 +2360,8 @@ const struct bmp280_chip_info bmp180_chip_info = { .read_temp = bmp180_read_temp, .read_press = bmp180_read_press, .read_calib = bmp180_read_calib, + + .buffer_handler = bmp180_buffer_handler }; EXPORT_SYMBOL_NS(bmp180_chip_info, IIO_BMP280); @@ -2332,6 +2537,14 @@ int bmp280_common_probe(struct device *dev, "failed to read calibration coefficients\n"); } + ret = devm_iio_triggered_buffer_setup(data->dev, indio_dev, + iio_pollfunc_store_time, + data->chip_info->buffer_handler, + NULL); + if (ret) + return dev_err_probe(data->dev, ret, + "iio triggered buffer setup failed\n"); + /* * Attempt to grab an optional EOC IRQ - only the BMP085 has this * however as it happens, the BMP085 shares the chip ID of BMP180 diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c index a444d4b2978b..dc297583cac1 100644 --- a/drivers/iio/pressure/bmp280-spi.c +++ b/drivers/iio/pressure/bmp280-spi.c @@ -40,14 +40,14 @@ static int bmp380_regmap_spi_read(void *context, const void *reg, size_t reg_size, void *val, size_t val_size) { struct spi_device *spi = to_spi_device(context); - u8 rx_buf[4]; + u8 rx_buf[9]; ssize_t status; /* - * Maximum number of consecutive bytes read for a temperature or - * pressure measurement is 3. + * Maximum number of a burst read for temperature, pressure, humidity + * is 8 bytes. */ - if (val_size > 3) + if (val_size > 8) return -EINVAL; /* diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index 8cc3eed70c18..32155567faf6 100644 --- a/drivers/iio/pressure/bmp280.h +++ b/drivers/iio/pressure/bmp280.h @@ -301,6 +301,10 @@ #define BMP280_PRESS_SKIPPED 0x80000 #define BMP280_HUMIDITY_SKIPPED 0x8000 +#define BMP280_NUM_TEMP_BYTES 3 +#define BMP280_NUM_PRESS_BYTES 3 +#define BME280_NUM_HUMIDITY_BYTES 2 + /* Core exported structs */ static const char *const bmp280_supply_names[] = { @@ -400,13 +404,19 @@ struct bmp280_data { */ s32 t_fine; + /* Data to push to userspace */ + struct { + s32 sensor_data[3]; + s64 timestamp __aligned(8); + } iio_buf; + /* * DMA (thus cache coherency maintenance) may require the * transfer buffers to live in their own cache lines. */ union { /* Sensor data buffer */ - u8 buf[3]; + u8 buf[8]; /* Calibration data buffers */ __le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2]; __be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2]; @@ -462,6 +472,8 @@ struct bmp280_chip_info { int (*read_humid)(struct bmp280_data *, u32 *); int (*read_calib)(struct bmp280_data *); int (*preinit)(struct bmp280_data *); + + irqreturn_t (*buffer_handler)(int, void *); }; /* Chip infos for each variant */ -- 2.25.1
The scan mask for the BME280 supports humidity measurement and needs to be distinguished from the rest in order for the timestamp to be able to work. Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 111 +++++++++++++++++++++++++---- drivers/iio/pressure/bmp280.h | 1 + 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 312bc2617583..ddfcd23f29a0 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -134,7 +134,28 @@ enum { BMP380_P11 = 20, }; +enum bmp280_scan { + BMP280_TEMP, + BMP280_PRESS, + BME280_HUMID +}; + static const struct iio_chan_spec bmp280_channels[] = { + { + .type = IIO_TEMP, + /* PROCESSED maintained for ABI backwards compatibility */ + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, + }, { .type = IIO_PRESSURE, /* PROCESSED maintained for ABI backwards compatibility */ @@ -142,7 +163,18 @@ static const struct iio_chan_spec bmp280_channels[] = { BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, + IIO_CHAN_SOFT_TIMESTAMP(2), +}; + +static const struct iio_chan_spec bme280_channels[] = { { .type = IIO_TEMP, /* PROCESSED maintained for ABI backwards compatibility */ @@ -150,28 +182,48 @@ static const struct iio_chan_spec bmp280_channels[] = { BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, { - .type = IIO_HUMIDITYRELATIVE, + .type = IIO_PRESSURE, /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, -}; - -static const struct iio_chan_spec bmp380_channels[] = { { - .type = IIO_PRESSURE, + .type = IIO_HUMIDITYRELATIVE, /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), - .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | - BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), + .scan_index = 2, + .scan_type = { + .sign = 'u', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, + IIO_CHAN_SOFT_TIMESTAMP(3), +}; + +static const struct iio_chan_spec bmp380_channels[] = { { .type = IIO_TEMP, /* PROCESSED maintained for ABI backwards compatibility */ @@ -181,9 +233,16 @@ static const struct iio_chan_spec bmp380_channels[] = { BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, { - .type = IIO_HUMIDITYRELATIVE, + .type = IIO_PRESSURE, /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | BIT(IIO_CHAN_INFO_RAW) | @@ -191,7 +250,15 @@ static const struct iio_chan_spec bmp380_channels[] = { BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, + IIO_CHAN_SOFT_TIMESTAMP(2), }; static int bmp280_read_calib(struct bmp280_data *data) @@ -825,6 +892,16 @@ static const struct iio_info bmp280_info = { .write_raw = &bmp280_write_raw, }; +static const unsigned long bmp280_avail_scan_masks[] = { + BIT(BMP280_PRESS) | BIT(BMP280_TEMP), + 0 +}; + +static const unsigned long bme280_avail_scan_masks[] = { + BIT(BME280_HUMID) | BIT(BMP280_PRESS) | BIT(BMP280_TEMP), + 0 +}; + static int bmp280_chip_config(struct bmp280_data *data) { u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) | @@ -866,7 +943,8 @@ const struct bmp280_chip_info bmp280_chip_info = { .regmap_config = &bmp280_regmap_config, .start_up_time = 2000, .channels = bmp280_channels, - .num_channels = 2, + .num_channels = 3, + .avail_scan_masks = bmp280_avail_scan_masks, .oversampling_temp_avail = bmp280_oversampling_avail, .num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail), @@ -925,8 +1003,9 @@ const struct bmp280_chip_info bme280_chip_info = { .num_chip_id = ARRAY_SIZE(bme280_chip_ids), .regmap_config = &bmp280_regmap_config, .start_up_time = 2000, - .channels = bmp280_channels, - .num_channels = 3, + .channels = bme280_channels, + .num_channels = 4, + .avail_scan_masks = bme280_avail_scan_masks, .oversampling_temp_avail = bmp280_oversampling_avail, .num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail), @@ -1300,7 +1379,8 @@ const struct bmp280_chip_info bmp380_chip_info = { .regmap_config = &bmp380_regmap_config, .start_up_time = 2000, .channels = bmp380_channels, - .num_channels = 2, + .num_channels = 3, + .avail_scan_masks = bmp280_avail_scan_masks, .oversampling_temp_avail = bmp380_oversampling_avail, .num_oversampling_temp_avail = ARRAY_SIZE(bmp380_oversampling_avail), @@ -1795,7 +1875,8 @@ const struct bmp280_chip_info bmp580_chip_info = { .regmap_config = &bmp580_regmap_config, .start_up_time = 2000, .channels = bmp380_channels, - .num_channels = 2, + .num_channels = 3, + .avail_scan_masks = bmp280_avail_scan_masks, .oversampling_temp_avail = bmp580_oversampling_avail, .num_oversampling_temp_avail = ARRAY_SIZE(bmp580_oversampling_avail), @@ -2054,7 +2135,8 @@ const struct bmp280_chip_info bmp180_chip_info = { .regmap_config = &bmp180_regmap_config, .start_up_time = 2000, .channels = bmp280_channels, - .num_channels = 2, + .num_channels = 3, + .avail_scan_masks = bmp280_avail_scan_masks, .oversampling_temp_avail = bmp180_oversampling_temp_avail, .num_oversampling_temp_avail = @@ -2166,6 +2248,7 @@ int bmp280_common_probe(struct device *dev, /* Apply initial values from chip info structure */ indio_dev->channels = chip_info->channels; indio_dev->num_channels = chip_info->num_channels; + indio_dev->available_scan_masks = chip_info->avail_scan_masks; data->oversampling_press = chip_info->oversampling_press_default; data->oversampling_humid = chip_info->oversampling_humid_default; data->oversampling_temp = chip_info->oversampling_temp_default; diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index 6d1dca31dd52..8cc3eed70c18 100644 --- a/drivers/iio/pressure/bmp280.h +++ b/drivers/iio/pressure/bmp280.h @@ -427,6 +427,7 @@ struct bmp280_chip_info { const struct iio_chan_spec *channels; int num_channels; unsigned int start_up_time; + const unsigned long *avail_scan_masks; const int *oversampling_temp_avail; int num_oversampling_temp_avail; -- 2.25.1
Add extra IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_RAW in order to be able to calculate the processed value with standard userspace IIO tools. Can be used for triggered buffers as well. Even though it is not a good design choice to have SCALE, RAW and PROCESSED together, the PROCESSED channel is kept for ABI compatibility. Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 63 ++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 6d6173c4b744..312bc2617583 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -137,17 +137,26 @@ enum { static const struct iio_chan_spec bmp280_channels[] = { { .type = IIO_PRESSURE, + /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), }, { .type = IIO_TEMP, + /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), }, { .type = IIO_HUMIDITYRELATIVE, + /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), }, }; @@ -155,21 +164,30 @@ static const struct iio_chan_spec bmp280_channels[] = { static const struct iio_chan_spec bmp380_channels[] = { { .type = IIO_PRESSURE, + /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), }, { .type = IIO_TEMP, + /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), }, { .type = IIO_HUMIDITYRELATIVE, + /* PROCESSED maintained for ABI backwards compatibility */ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), @@ -487,6 +505,51 @@ static int bmp280_read_raw_guarded(struct iio_dev *indio_dev, return -EINVAL; } return 0; + case IIO_CHAN_INFO_RAW: + switch (chan->type) { + case IIO_HUMIDITYRELATIVE: + ret = data->chip_info->read_humid(data, &chan_value); + if (ret) + return ret; + + *val = chan_value; + return IIO_VAL_INT; + case IIO_PRESSURE: + ret = data->chip_info->read_press(data, &chan_value); + if (ret) + return ret; + + *val = chan_value; + return IIO_VAL_INT; + case IIO_TEMP: + ret = data->chip_info->read_press(data, &chan_value); + if (ret) + return ret; + + *val = chan_value; + return IIO_VAL_INT; + default: + return -EINVAL; + } + return 0; + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_HUMIDITYRELATIVE: + *val = data->chip_info->humid_coeffs[0]; + *val2 = data->chip_info->humid_coeffs[1]; + return data->chip_info->humid_coeffs_type; + case IIO_PRESSURE: + *val = data->chip_info->press_coeffs[0]; + *val2 = data->chip_info->press_coeffs[1]; + return data->chip_info->press_coeffs_type; + case IIO_TEMP: + *val = data->chip_info->temp_coeffs[0]; + *val2 = data->chip_info->temp_coeffs[1]; + return data->chip_info->temp_coeffs_type; + default: + return -EINVAL; + } + return 0; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: switch (chan->type) { case IIO_HUMIDITYRELATIVE: -- 2.25.1
Add the coefficients for the IIO standard units and the return IIO value inside the chip_info structure. Remove the calculations with the coefficients for the IIO unit compatibility from inside the read_{temp/press/humid}() functions and move it to the general read_raw() function. Execute the calculations with the coefficients inside the read_raw() oneshot capture function. In this way, all the data for the calculation of the value are located in the chip_info structure of the respective sensor. Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 189 +++++++++++++++-------------- drivers/iio/pressure/bmp280.h | 13 +- 2 files changed, 106 insertions(+), 96 deletions(-) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index f7a13ff6f26c..6d6173c4b744 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -363,10 +363,9 @@ static u32 bmp280_compensate_press(struct bmp280_data *data, return (u32)p; } -static int bmp280_read_temp(struct bmp280_data *data, - int *val, int *val2) +static int bmp280_read_temp(struct bmp280_data *data, s32 *comp_temp) { - s32 adc_temp, comp_temp; + s32 adc_temp; int ret; ret = regmap_bulk_read(data->regmap, BMP280_REG_TEMP_MSB, @@ -382,29 +381,20 @@ static int bmp280_read_temp(struct bmp280_data *data, dev_err(data->dev, "reading temperature skipped\n"); return -EIO; } - comp_temp = bmp280_compensate_temp(data, adc_temp); - /* - * val might be NULL if we're called by the read_press routine, - * who only cares about the carry over t_fine value. - */ - if (val) { - *val = comp_temp * 10; - return IIO_VAL_INT; - } + if (comp_temp) + *comp_temp = bmp280_compensate_temp(data, adc_temp); return 0; } -static int bmp280_read_press(struct bmp280_data *data, - int *val, int *val2) +static int bmp280_read_press(struct bmp280_data *data, u32 *comp_press) { - u32 comp_press; s32 adc_press; int ret; /* Read and compensate temperature so we get a reading of t_fine. */ - ret = bmp280_read_temp(data, NULL, NULL); + ret = bmp280_read_temp(data, NULL); if (ret < 0) return ret; @@ -421,22 +411,19 @@ static int bmp280_read_press(struct bmp280_data *data, dev_err(data->dev, "reading pressure skipped\n"); return -EIO; } - comp_press = bmp280_compensate_press(data, adc_press); - *val = comp_press; - *val2 = 256000; + *comp_press = bmp280_compensate_press(data, adc_press); - return IIO_VAL_FRACTIONAL; + return 0; } -static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2) +static int bmp280_read_humid(struct bmp280_data *data, u32 *comp_humidity) { - u32 comp_humidity; s32 adc_humidity; int ret; /* Read and compensate temperature so we get a reading of t_fine. */ - ret = bmp280_read_temp(data, NULL, NULL); + ret = bmp280_read_temp(data, NULL); if (ret < 0) return ret; @@ -453,11 +440,10 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2) dev_err(data->dev, "reading humidity skipped\n"); return -EIO; } - comp_humidity = bmp280_compensate_humidity(data, adc_humidity); - *val = comp_humidity * 1000 / 1024; + *comp_humidity = bmp280_compensate_humidity(data, adc_humidity); - return IIO_VAL_INT; + return 0; } static int bmp280_read_raw_guarded(struct iio_dev *indio_dev, @@ -465,6 +451,8 @@ static int bmp280_read_raw_guarded(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct bmp280_data *data = iio_priv(indio_dev); + int chan_value; + int ret; guard(mutex)(&data->lock); @@ -472,11 +460,29 @@ static int bmp280_read_raw_guarded(struct iio_dev *indio_dev, case IIO_CHAN_INFO_PROCESSED: switch (chan->type) { case IIO_HUMIDITYRELATIVE: - return data->chip_info->read_humid(data, val, val2); + ret = data->chip_info->read_humid(data, &chan_value); + if (ret) + return ret; + + *val = data->chip_info->humid_coeffs[0] * chan_value; + *val2 = data->chip_info->humid_coeffs[1]; + return data->chip_info->humid_coeffs_type; case IIO_PRESSURE: - return data->chip_info->read_press(data, val, val2); + ret = data->chip_info->read_press(data, &chan_value); + if (ret) + return ret; + + *val = data->chip_info->press_coeffs[0] * chan_value; + *val2 = data->chip_info->press_coeffs[1]; + return data->chip_info->press_coeffs_type; case IIO_TEMP: - return data->chip_info->read_temp(data, val, val2); + ret = data->chip_info->read_temp(data, &chan_value); + if (ret) + return ret; + + *val = data->chip_info->temp_coeffs[0] * chan_value; + *val2 = data->chip_info->temp_coeffs[1]; + return data->chip_info->temp_coeffs_type; default: return -EINVAL; } @@ -787,6 +793,8 @@ static int bmp280_chip_config(struct bmp280_data *data) static const int bmp280_oversampling_avail[] = { 1, 2, 4, 8, 16 }; static const u8 bmp280_chip_ids[] = { BMP280_CHIP_ID }; +static const int bmp280_temp_coeffs[] = { 10, 1 }; +static const int bmp280_press_coeffs[] = { 1, 256000 }; const struct bmp280_chip_info bmp280_chip_info = { .id_reg = BMP280_REG_ID, @@ -815,6 +823,11 @@ const struct bmp280_chip_info bmp280_chip_info = { .num_oversampling_press_avail = ARRAY_SIZE(bmp280_oversampling_avail), .oversampling_press_default = BMP280_OSRS_PRESS_16X - 1, + .temp_coeffs = bmp280_temp_coeffs, + .temp_coeffs_type = IIO_VAL_FRACTIONAL, + .press_coeffs = bmp280_press_coeffs, + .press_coeffs_type = IIO_VAL_FRACTIONAL, + .chip_config = bmp280_chip_config, .read_temp = bmp280_read_temp, .read_press = bmp280_read_press, @@ -841,6 +854,7 @@ static int bme280_chip_config(struct bmp280_data *data) } static const u8 bme280_chip_ids[] = { BME280_CHIP_ID }; +static const int bme280_humid_coeffs[] = { 1000, 1024 }; const struct bmp280_chip_info bme280_chip_info = { .id_reg = BMP280_REG_ID, @@ -863,6 +877,14 @@ const struct bmp280_chip_info bme280_chip_info = { .num_oversampling_humid_avail = ARRAY_SIZE(bmp280_oversampling_avail), .oversampling_humid_default = BMP280_OSRS_HUMIDITY_16X - 1, + .temp_coeffs = bmp280_temp_coeffs, + .temp_coeffs_type = IIO_VAL_FRACTIONAL, + .press_coeffs = bmp280_press_coeffs, + .press_coeffs_type = IIO_VAL_FRACTIONAL, + .humid_coeffs = bme280_humid_coeffs, + .humid_coeffs_type = IIO_VAL_FRACTIONAL, + + .chip_config = bme280_chip_config, .read_temp = bmp280_read_temp, .read_press = bmp280_read_press, @@ -988,9 +1010,8 @@ static u32 bmp380_compensate_press(struct bmp280_data *data, u32 adc_press) return comp_press; } -static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2) +static int bmp380_read_temp(struct bmp280_data *data, s32 *comp_temp) { - s32 comp_temp; u32 adc_temp; int ret; @@ -1006,29 +1027,20 @@ static int bmp380_read_temp(struct bmp280_data *data, int *val, int *val2) dev_err(data->dev, "reading temperature skipped\n"); return -EIO; } - comp_temp = bmp380_compensate_temp(data, adc_temp); - /* - * Val might be NULL if we're called by the read_press routine, - * who only cares about the carry over t_fine value. - */ - if (val) { - /* IIO reports temperatures in milli Celsius */ - *val = comp_temp * 10; - return IIO_VAL_INT; - } + if (comp_temp) + *comp_temp = bmp380_compensate_temp(data, adc_temp); return 0; } -static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2) +static int bmp380_read_press(struct bmp280_data *data, u32 *comp_press) { - s32 comp_press; u32 adc_press; int ret; /* Read and compensate for temperature so we get a reading of t_fine */ - ret = bmp380_read_temp(data, NULL, NULL); + ret = bmp380_read_temp(data, NULL); if (ret) return ret; @@ -1044,13 +1056,10 @@ static int bmp380_read_press(struct bmp280_data *data, int *val, int *val2) dev_err(data->dev, "reading pressure skipped\n"); return -EIO; } - comp_press = bmp380_compensate_press(data, adc_press); - *val = comp_press; - /* Compensated pressure is in cPa (centipascals) */ - *val2 = 100000; + *comp_press = bmp380_compensate_press(data, adc_press); - return IIO_VAL_FRACTIONAL; + return 0; } static int bmp380_read_calib(struct bmp280_data *data) @@ -1218,6 +1227,8 @@ static int bmp380_chip_config(struct bmp280_data *data) static const int bmp380_oversampling_avail[] = { 1, 2, 4, 8, 16, 32 }; static const int bmp380_iir_filter_coeffs_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128}; static const u8 bmp380_chip_ids[] = { BMP380_CHIP_ID, BMP390_CHIP_ID }; +static const int bmp380_temp_coeffs[] = { 10, 1 }; +static const int bmp380_press_coeffs[] = { 1, 100000 }; const struct bmp280_chip_info bmp380_chip_info = { .id_reg = BMP380_REG_ID, @@ -1244,6 +1255,11 @@ const struct bmp280_chip_info bmp380_chip_info = { .num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail), .iir_filter_coeff_default = 2, + .temp_coeffs = bmp380_temp_coeffs, + .temp_coeffs_type = IIO_VAL_FRACTIONAL, + .press_coeffs = bmp380_press_coeffs, + .press_coeffs_type = IIO_VAL_FRACTIONAL, + .chip_config = bmp380_chip_config, .read_temp = bmp380_read_temp, .read_press = bmp380_read_press, @@ -1364,9 +1380,8 @@ static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write) * for what is expected on IIO ABI. */ -static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2) +static int bmp580_read_temp(struct bmp280_data *data, s32 *raw_temp) { - s32 raw_temp; int ret; ret = regmap_bulk_read(data->regmap, BMP580_REG_TEMP_XLSB, data->buf, @@ -1376,25 +1391,17 @@ static int bmp580_read_temp(struct bmp280_data *data, int *val, int *val2) return ret; } - raw_temp = get_unaligned_le24(data->buf); - if (raw_temp == BMP580_TEMP_SKIPPED) { + *raw_temp = get_unaligned_le24(data->buf); + if (*raw_temp == BMP580_TEMP_SKIPPED) { dev_err(data->dev, "reading temperature skipped\n"); return -EIO; } - /* - * Temperature is returned in Celsius degrees in fractional - * form down 2^16. We rescale by x1000 to return milli Celsius - * to respect IIO ABI. - */ - *val = raw_temp * 1000; - *val2 = 16; - return IIO_VAL_FRACTIONAL_LOG2; + return 0; } -static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2) +static int bmp580_read_press(struct bmp280_data *data, u32 *raw_press) { - u32 raw_press; int ret; ret = regmap_bulk_read(data->regmap, BMP580_REG_PRESS_XLSB, data->buf, @@ -1404,18 +1411,13 @@ static int bmp580_read_press(struct bmp280_data *data, int *val, int *val2) return ret; } - raw_press = get_unaligned_le24(data->buf); - if (raw_press == BMP580_PRESS_SKIPPED) { + *raw_press = get_unaligned_le24(data->buf); + if (*raw_press == BMP580_PRESS_SKIPPED) { dev_err(data->dev, "reading pressure skipped\n"); return -EIO; } - /* - * Pressure is returned in Pascals in fractional form down 2^16. - * We rescale /1000 to convert to kilopascal to respect IIO ABI. - */ - *val = raw_press; - *val2 = 64000; /* 2^6 * 1000 */ - return IIO_VAL_FRACTIONAL; + + return 0; } static const int bmp580_odr_table[][2] = { @@ -1720,6 +1722,8 @@ static int bmp580_chip_config(struct bmp280_data *data) static const int bmp580_oversampling_avail[] = { 1, 2, 4, 8, 16, 32, 64, 128 }; static const u8 bmp580_chip_ids[] = { BMP580_CHIP_ID, BMP580_CHIP_ID_ALT }; +static const int bmp580_temp_coeffs[] = { 1000, 16 }; +static const int bmp580_press_coeffs[] = { 1, 64000 }; const struct bmp280_chip_info bmp580_chip_info = { .id_reg = BMP580_REG_CHIP_ID, @@ -1746,6 +1750,11 @@ const struct bmp280_chip_info bmp580_chip_info = { .num_iir_filter_coeffs_avail = ARRAY_SIZE(bmp380_iir_filter_coeffs_avail), .iir_filter_coeff_default = 2, + .temp_coeffs = bmp280_temp_coeffs, + .temp_coeffs_type = IIO_VAL_FRACTIONAL_LOG2, + .press_coeffs = bmp280_press_coeffs, + .press_coeffs_type = IIO_VAL_FRACTIONAL, + .chip_config = bmp580_chip_config, .read_temp = bmp580_read_temp, .read_press = bmp580_read_press, @@ -1873,25 +1882,17 @@ static s32 bmp180_compensate_temp(struct bmp280_data *data, s32 adc_temp) return (data->t_fine + 8) >> 4; } -static int bmp180_read_temp(struct bmp280_data *data, int *val, int *val2) +static int bmp180_read_temp(struct bmp280_data *data, s32 *comp_temp) { - s32 adc_temp, comp_temp; + s32 adc_temp; int ret; ret = bmp180_read_adc_temp(data, &adc_temp); if (ret) return ret; - comp_temp = bmp180_compensate_temp(data, adc_temp); - - /* - * val might be NULL if we're called by the read_press routine, - * who only cares about the carry over t_fine value. - */ - if (val) { - *val = comp_temp * 100; - return IIO_VAL_INT; - } + if (comp_temp) + *comp_temp = bmp180_compensate_temp(data, adc_temp); return 0; } @@ -1953,15 +1954,13 @@ static u32 bmp180_compensate_press(struct bmp280_data *data, s32 adc_press) return p + ((x1 + x2 + 3791) >> 4); } -static int bmp180_read_press(struct bmp280_data *data, - int *val, int *val2) +static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press) { - u32 comp_press; s32 adc_press; int ret; /* Read and compensate temperature so we get a reading of t_fine. */ - ret = bmp180_read_temp(data, NULL, NULL); + ret = bmp180_read_temp(data, NULL); if (ret) return ret; @@ -1969,12 +1968,9 @@ static int bmp180_read_press(struct bmp280_data *data, if (ret) return ret; - comp_press = bmp180_compensate_press(data, adc_press); - - *val = comp_press; - *val2 = 1000; + *comp_press = bmp180_compensate_press(data, adc_press); - return IIO_VAL_FRACTIONAL; + return 0; } static int bmp180_chip_config(struct bmp280_data *data) @@ -1985,6 +1981,8 @@ static int bmp180_chip_config(struct bmp280_data *data) static const int bmp180_oversampling_temp_avail[] = { 1 }; static const int bmp180_oversampling_press_avail[] = { 1, 2, 4, 8 }; static const u8 bmp180_chip_ids[] = { BMP180_CHIP_ID }; +static const int bmp180_temp_coeffs[] = { 100, 1 }; +static const int bmp180_press_coeffs[] = { 1, 1000 }; const struct bmp280_chip_info bmp180_chip_info = { .id_reg = BMP280_REG_ID, @@ -2005,6 +2003,11 @@ const struct bmp280_chip_info bmp180_chip_info = { ARRAY_SIZE(bmp180_oversampling_press_avail), .oversampling_press_default = BMP180_MEAS_PRESS_8X, + .temp_coeffs = bmp180_temp_coeffs, + .temp_coeffs_type = IIO_VAL_FRACTIONAL, + .press_coeffs = bmp180_press_coeffs, + .press_coeffs_type = IIO_VAL_FRACTIONAL, + .chip_config = bmp180_chip_config, .read_temp = bmp180_read_temp, .read_press = bmp180_read_press, diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h index 4012387d7956..6d1dca31dd52 100644 --- a/drivers/iio/pressure/bmp280.h +++ b/drivers/iio/pressure/bmp280.h @@ -448,10 +448,17 @@ struct bmp280_chip_info { int num_sampling_freq_avail; int sampling_freq_default; + const int *temp_coeffs; + const int temp_coeffs_type; + const int *press_coeffs; + const int press_coeffs_type; + const int *humid_coeffs; + const int humid_coeffs_type; + int (*chip_config)(struct bmp280_data *); - int (*read_temp)(struct bmp280_data *, int *, int *); - int (*read_press)(struct bmp280_data *, int *, int *); - int (*read_humid)(struct bmp280_data *, int *, int *); + int (*read_temp)(struct bmp280_data *, s32 *); + int (*read_press)(struct bmp280_data *, u32 *); + int (*read_humid)(struct bmp280_data *, u32 *); int (*read_calib)(struct bmp280_data *); int (*preinit)(struct bmp280_data *); }; -- 2.25.1
Introduce the new linux/cleanup.h with the guard(mutex) functionality in the {read/write}_raw() functions Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 125 +++++++++++++---------------- 1 file changed, 58 insertions(+), 67 deletions(-) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 871b2214121b..f7a13ff6f26c 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -460,77 +460,74 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2) return IIO_VAL_INT; } -static int bmp280_read_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int *val, int *val2, long mask) +static int bmp280_read_raw_guarded(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) { struct bmp280_data *data = iio_priv(indio_dev); - int ret; - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); + guard(mutex)(&data->lock); switch (mask) { case IIO_CHAN_INFO_PROCESSED: switch (chan->type) { case IIO_HUMIDITYRELATIVE: - ret = data->chip_info->read_humid(data, val, val2); - break; + return data->chip_info->read_humid(data, val, val2); case IIO_PRESSURE: - ret = data->chip_info->read_press(data, val, val2); - break; + return data->chip_info->read_press(data, val, val2); case IIO_TEMP: - ret = data->chip_info->read_temp(data, val, val2); - break; + return data->chip_info->read_temp(data, val, val2); default: - ret = -EINVAL; - break; + return -EINVAL; } - break; + return 0; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: switch (chan->type) { case IIO_HUMIDITYRELATIVE: *val = 1 << data->oversampling_humid; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; case IIO_PRESSURE: *val = 1 << data->oversampling_press; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; case IIO_TEMP: *val = 1 << data->oversampling_temp; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; default: - ret = -EINVAL; - break; + return -EINVAL; } - break; + return 0; case IIO_CHAN_INFO_SAMP_FREQ: if (!data->chip_info->sampling_freq_avail) { - ret = -EINVAL; - break; + return -EINVAL; } *val = data->chip_info->sampling_freq_avail[data->sampling_freq][0]; *val2 = data->chip_info->sampling_freq_avail[data->sampling_freq][1]; - ret = IIO_VAL_INT_PLUS_MICRO; - break; + return IIO_VAL_INT_PLUS_MICRO; case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: if (!data->chip_info->iir_filter_coeffs_avail) { - ret = -EINVAL; - break; + return -EINVAL; } *val = (1 << data->iir_filter_coeff) - 1; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; default: - ret = -EINVAL; - break; + return -EINVAL; } - mutex_unlock(&data->lock); + return 0; +} + + +static int bmp280_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct bmp280_data *data = iio_priv(indio_dev); + int ret; + + pm_runtime_get_sync(data->dev); + ret = bmp280_read_raw_guarded(indio_dev, chan, val, val2, mask); pm_runtime_mark_last_busy(data->dev); pm_runtime_put_autosuspend(data->dev); @@ -662,13 +659,13 @@ static int bmp280_write_iir_filter_coeffs(struct bmp280_data *data, int val) return -EINVAL; } -static int bmp280_write_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int val, int val2, long mask) +static int bmp280_write_raw_guarded(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) { struct bmp280_data *data = iio_priv(indio_dev); - int ret = 0; + guard(mutex)(&data->lock); /* * Helper functions to update sensor running configuration. * If an error happens applying new settings, will try restore @@ -677,46 +674,40 @@ static int bmp280_write_raw(struct iio_dev *indio_dev, */ switch (mask) { case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); switch (chan->type) { case IIO_HUMIDITYRELATIVE: - ret = bmp280_write_oversampling_ratio_humid(data, val); - break; + return bmp280_write_oversampling_ratio_humid(data, val); case IIO_PRESSURE: - ret = bmp280_write_oversampling_ratio_press(data, val); - break; + return bmp280_write_oversampling_ratio_press(data, val); case IIO_TEMP: - ret = bmp280_write_oversampling_ratio_temp(data, val); - break; + return bmp280_write_oversampling_ratio_temp(data, val); default: - ret = -EINVAL; - break; + return -EINVAL; } - mutex_unlock(&data->lock); - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); - break; + return 0; case IIO_CHAN_INFO_SAMP_FREQ: - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); - ret = bmp280_write_sampling_frequency(data, val, val2); - mutex_unlock(&data->lock); - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); - break; + return bmp280_write_sampling_frequency(data, val, val2); case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); - ret = bmp280_write_iir_filter_coeffs(data, val); - mutex_unlock(&data->lock); - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); - break; + return bmp280_write_iir_filter_coeffs(data, val); default: return -EINVAL; } + return 0; +} + +static int bmp280_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct bmp280_data *data = iio_priv(indio_dev); + int ret = 0; + + pm_runtime_get_sync(data->dev); + ret = bmp280_write_raw_guarded(indio_dev, chan, val, val2, mask); + pm_runtime_mark_last_busy(data->dev); + pm_runtime_put_autosuspend(data->dev); + return ret; } -- 2.25.1
Sort headers in alphabetical order. Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com> --- drivers/iio/pressure/bmp280-core.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index fe8734468ed3..871b2214121b 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -27,20 +27,20 @@ #include <linux/bitops.h> #include <linux/bitfield.h> -#include <linux/device.h> -#include <linux/module.h> -#include <linux/nvmem-provider.h> -#include <linux/regmap.h> +#include <linux/completion.h> #include <linux/delay.h> -#include <linux/iio/iio.h> -#include <linux/iio/sysfs.h> +#include <linux/device.h> #include <linux/gpio/consumer.h> -#include <linux/regulator/consumer.h> #include <linux/interrupt.h> #include <linux/irq.h> /* For irq_get_irq_data() */ -#include <linux/completion.h> +#include <linux/module.h> +#include <linux/nvmem-provider.h> #include <linux/pm_runtime.h> #include <linux/random.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> + +#include <linux/iio/iio.h> #include <asm/unaligned.h> -- 2.25.1
Changes in v3: Patch 2: Add guard(mutex) as per request {read/write}_raw() functions. Patch 3: Patch 2 of v2. Added IIO return value as per request. Patch 4: Patch 3 of v2. Patch 5: Patch 4 of v2. Patch 6: Completely different approach from v2. Instead of leveraging the functionality of the read_*() functions for the oneshot capture, burst reads were used in order to read all the values in one single read operation. This minimizes the number of accesses to the device to just 1 time, which all the values are being read. Different buffer_handler() functions were implemented for the different "families" of sensors because there were a lot of differences in the register configuration and read operation for different sensors. BMP085 and BMP180 have a very well defined read operation that is kept in the buffer_handler(). There is no option for a burst read in these sensors. BM(P/E)2xx, BMP3xx, and BMP5xx have their own buffer_handler() functions. Registers, endianess and compensation formulas are different in each one of those 3 categories which doesn't allow for a more generic buffer_handler(). [v2] https://lore.kernel.org/linux-iio/20240313174007.1934983-1-vassilisamir@gmail.com [v1] https://lore.kernel.org/linux-iio/20240303165300.468011-1-vassilisamir@gmail.com Vasileios Amoiridis (6): iio: pressure: BMP280 core driver headers sorting iio: pressure: Introduce new cleanup routines to BMP280 driver *_raw() functions iio: pressure: Generalize read_{temp/press/humid}() functions iio: pressure: Add SCALE and RAW values for channels iio: pressure: Add timestamp and scan_masks for BMP280 driver iio: pressure: Add triggered buffer support for BMP280 driver drivers/iio/pressure/Kconfig | 2 + drivers/iio/pressure/bmp280-core.c | 727 +++++++++++++++++++++-------- drivers/iio/pressure/bmp280-spi.c | 8 +- drivers/iio/pressure/bmp280.h | 28 +- 4 files changed, 570 insertions(+), 195 deletions(-) -- 2.25.1
On 3/18/24 5:43 PM, Andy Shevchenko wrote:
> On Mon, Mar 18, 2024 at 11:57 PM David Lechner <dlechner@baylibre.com> wrote:
>>
>> This simplifies the adi,spi-mode property parsing by using
>> device_property_match_property_string() instead of two separate
>> functions. Also, the error return value is now more informative
>> in cases where there was problem parsing the property.
>
> a problem
>
> ...
>
>> + ret = device_property_match_property_string(dev, "adi,spi-mode",
>> + ad7944_spi_modes,
>> + ARRAY_SIZE(ad7944_spi_modes));
>> + if (ret < 0) {
>> /* absence of adi,spi-mode property means default mode */
>> - adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
>> + if (ret == -EINVAL)
>> + adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
>> + else
>> + return dev_err_probe(dev, ret,
>> + "getting adi,spi-mode property failed\n");
>
> No need to have 'else'
>
> if (ret != -EINVAL)
> return dev_err_probe(dev, ret, "getting
> adi,spi-mode property failed\n");
>
> /* absence of adi,spi-mode property means default mode */
> adc->spi_mode = AD7944_SPI_MODE_DEFAULT;
>
>> + } else {
>> + adc->spi_mode = ret;
>> }
>
I agree it is better that way. Will send a v2.
On Mon, Mar 18, 2024 at 11:57 PM David Lechner <dlechner@baylibre.com> wrote: > > This simplifies the adi,spi-mode property parsing by using > device_property_match_property_string() instead of two separate > functions. Also, the error return value is now more informative > in cases where there was problem parsing the property. a problem ... > + ret = device_property_match_property_string(dev, "adi,spi-mode", > + ad7944_spi_modes, > + ARRAY_SIZE(ad7944_spi_modes)); > + if (ret < 0) { > /* absence of adi,spi-mode property means default mode */ > - adc->spi_mode = AD7944_SPI_MODE_DEFAULT; > + if (ret == -EINVAL) > + adc->spi_mode = AD7944_SPI_MODE_DEFAULT; > + else > + return dev_err_probe(dev, ret, > + "getting adi,spi-mode property failed\n"); No need to have 'else' if (ret != -EINVAL) return dev_err_probe(dev, ret, "getting adi,spi-mode property failed\n"); /* absence of adi,spi-mode property means default mode */ adc->spi_mode = AD7944_SPI_MODE_DEFAULT; > + } else { > + adc->spi_mode = ret; > } -- With Best Regards, Andy Shevchenko
This simplifies the adi,spi-mode property parsing by using device_property_match_property_string() instead of two separate functions. Also, the error return value is now more informative in cases where there was problem parsing the property. Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> Signed-off-by: David Lechner <dlechner@baylibre.com> --- drivers/iio/adc/ad7944.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/iio/adc/ad7944.c b/drivers/iio/adc/ad7944.c index d5ec6b5a41c7..356aea02920f 100644 --- a/drivers/iio/adc/ad7944.c +++ b/drivers/iio/adc/ad7944.c @@ -366,7 +366,6 @@ static int ad7944_probe(struct spi_device *spi) struct ad7944_adc *adc; bool have_refin = false; struct regulator *ref; - const char *str_val; int ret; indio_dev = devm_iio_device_alloc(dev, sizeof(*adc)); @@ -382,16 +381,18 @@ static int ad7944_probe(struct spi_device *spi) adc->timing_spec = chip_info->timing_spec; - if (device_property_read_string(dev, "adi,spi-mode", &str_val) == 0) { - ret = sysfs_match_string(ad7944_spi_modes, str_val); - if (ret < 0) - return dev_err_probe(dev, -EINVAL, - "unsupported adi,spi-mode\n"); - - adc->spi_mode = ret; - } else { + ret = device_property_match_property_string(dev, "adi,spi-mode", + ad7944_spi_modes, + ARRAY_SIZE(ad7944_spi_modes)); + if (ret < 0) { /* absence of adi,spi-mode property means default mode */ - adc->spi_mode = AD7944_SPI_MODE_DEFAULT; + if (ret == -EINVAL) + adc->spi_mode = AD7944_SPI_MODE_DEFAULT; + else + return dev_err_probe(dev, ret, + "getting adi,spi-mode property failed\n"); + } else { + adc->spi_mode = ret; } if (adc->spi_mode == AD7944_SPI_MODE_CHAIN) --- base-commit: 1446d8ef48196409f811c25071b2cc510a49fc60 change-id: 20240318-ad7944-cleanups-9f95a7c598b6
On Mon, Mar 18, 2024 at 4:17 PM David Lechner <dlechner@baylibre.com> wrote: > On Mon, Mar 18, 2024 at 8:10 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Mon, Mar 18, 2024 at 2:41 PM Jonathan Cameron > > <Jonathan.Cameron@huawei.com> wrote: ... > > > > > struct ad7944_adc { > > > > > struct spi_device *spi; > > > > > + enum ad7944_spi_mode spi_mode; > > > > > /* Chip-specific timing specifications. */ > > > > > const struct ad7944_timing_spec *timing_spec; > > > > > /* GPIO connected to CNV pin. */ > > > > > @@ -58,6 +75,9 @@ struct ad7944_adc { > > > > > } sample __aligned(IIO_DMA_MINALIGN); > > > > > }; > > > > > > > > Have you run `pahole` to see if there is a better place for a new member? > > > > > > I know this matters for structures where we see lots of them, but do we actually > > > care for one offs? Whilst it doesn't matter here I'd focus much more > > > on readability and like parameter grouping for cases like this than wasting > > > a few bytes. > > > > This is _also_ true, but think more about cache line contamination. > > Even not-so-important bytes may decrease the performance. In some > > cases it's tolerable, in some it is not (high-speed ADC). In general I > > assume that the developer has to understand many aspects of the > > software and cache line contamination may be last but definitely not > > least. > > Where could someone who doesn't know anything about cache line > contamination learn more about it? (searching the web for that phrase > doesn't turn up much) Agree that I have written a rarely used term. [1]: https://en.wikipedia.org/wiki/Cache_pollution -- With Best Regards, Andy Shevchenko
On 3/18/24 11:24, Jonathan Cameron wrote: > On Mon, 18 Mar 2024 11:18:43 -0400 > Sean Anderson <sean.anderson@linux.dev> wrote: > >> On 3/16/24 09:36, Jonathan Cameron wrote: >> > On Fri, 15 Mar 2024 13:47:40 -0400 >> > Sean Anderson <sean.anderson@linux.dev> wrote: >> > >> >> Hi Conall, >> >> >> >> On 3/15/24 09:18, O'Griofa, Conall wrote: >> >> > [AMD Official Use Only - General] >> >> > >> >> > Hi, >> >> > >> >> > I think there was a fix for this issue applied to the version that was running on 5.15 that didn't seem to make it into the upstream driver. >> >> > Please see link for reference https://github.com/Xilinx/linux-xlnx/commit/608426961f16ab149b1b699f1c35f7ad244c0720 >> >> > >> >> > I think a similar fix to the above patch is may be beneficial? >> >> >> >> These patches look functionally identical to me. >> > >> > Because there are no channels with scan index between >> > 22 * 2 + 16 (that patch) and 22 * 3 (your patch) that is >> > the effect is indeed the same. But given the issues is the >> > 64 limit on maximum scan index, 22 * 3 = 66 is an ugly value >> > to compare with. >> > >> > I'm still very against the use of scan_index for anything other >> > than scan indices (which is why partly how this bug wasn't noticed >> > in the first palce). So the check should be scan_index != -1 >> > and uses of those values elsewhere in the driver should be fixed >> > (which looks simple to do from a quick glance at the code). >> >> OK, so how do the sysfs files get named then? > > Using channel and channel2 as appropriate (+ index and modified > which change the meaning of channel2) - scan_index never had > anything to do with sysfs file names - just the value in > bufferX/in_xyz_scan_index I tried to prototype setting scan_index to -1, but when registering channels I saw [ 1.637049] iio iio:device0: tried to double register : in_voltage_raw [ 1.637245] xilinx-ams ffa50000.ams: Failed to register sysfs interfaces [ 1.637433] xilinx-ams: probe of ffa50000.ams failed with error -16 And AIUI .channel is filled in by ams_parse_firmware. --Sean >> >> --Sean >> >> >> >> >> --Sean >> >> >> >> >> -----Original Message----- >> >> >> From: Sean Anderson <sean.anderson@linux.dev> >> >> >> Sent: Thursday, March 14, 2024 5:30 PM >> >> >> To: Jonathan Cameron <jic23@kernel.org> >> >> >> Cc: linux-iio@vger.kernel.org; O'Griofa, Conall <conall.ogriofa@amd.com>; >> >> >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Lars-Peter >> >> >> Clausen <lars@metafoo.de> >> >> >> Subject: Re: [PATCH] iio: xilinx-ams: Don't include ams_ctrl_channels in >> >> >> scan_mask >> >> >> >> >> >> Caution: This message originated from an External Source. Use proper caution >> >> >> when opening attachments, clicking links, or responding. >> >> >> >> >> >> >> >> >> On 3/14/24 11:48, Jonathan Cameron wrote: >> >> >> > On Mon, 11 Mar 2024 12:28:00 -0400 >> >> >> > Sean Anderson <sean.anderson@linux.dev> wrote: >> >> >> > >> >> >> >> ams_enable_channel_sequence constructs a "scan_mask" for all the PS >> >> >> >> and PL channels. This works out fine, since scan_index for these >> >> >> >> channels is less than 64. However, it also includes the >> >> >> >> ams_ctrl_channels, where scan_index is greater than 64, triggering >> >> >> >> undefined behavior. Since we don't need these channels anyway, just >> >> >> exclude them. >> >> >> >> >> >> >> >> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") >> >> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> >> >> > >> >> >> > Hi Sean, >> >> >> > >> >> >> > I'd ideally like to understand why we have channels with such large >> >> >> > scan indexes. Those values should only be used for buffered capture. >> >> >> > It feels like they are being abused here. Can we set them to -1 >> >> >> > instead and check based on that? >> >> >> > For a channel, a scan index of -1 means it can't be captured via the >> >> >> > buffered interfaces but only accessed via sysfs reads. >> >> >> > I think that's what we have here? >> >> >> >> >> >> From what I can tell, none of the channels support buffered reads. And we can't >> >> >> naïvely convert the scan_index to -1, since that causes sysfs naming conflicts >> >> >> (not to mention the compatibility break). >> >> >> >> >> >> > >> >> >> > I just feel like if we leave these as things stand, we will get bitten >> >> >> > by similar bugs in the future. At least with -1 it should be obvious why! >> >> >> >> >> >> There are just as likely to be bugs confusing the PL/PS subdevices... >> >> >> >> >> >> FWIW I had no trouble identifying the channels involved with this bug. >> >> >> >> >> >> --Sean >> >> >> >> >> >> > Jonathan >> >> >> > >> >> >> > >> >> >> >> --- >> >> >> >> >> >> >> >> drivers/iio/adc/xilinx-ams.c | 8 ++++++-- >> >> >> >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> >> >> >> >> >> >> diff --git a/drivers/iio/adc/xilinx-ams.c >> >> >> >> b/drivers/iio/adc/xilinx-ams.c index a55396c1f8b2..4de7ce598e4d >> >> >> >> 100644 >> >> >> >> --- a/drivers/iio/adc/xilinx-ams.c >> >> >> >> +++ b/drivers/iio/adc/xilinx-ams.c >> >> >> >> @@ -414,8 +414,12 @@ static void ams_enable_channel_sequence(struct >> >> >> >> iio_dev *indio_dev) >> >> >> >> >> >> >> >> /* Run calibration of PS & PL as part of the sequence */ >> >> >> >> scan_mask = BIT(0) | BIT(AMS_PS_SEQ_MAX); >> >> >> >> - for (i = 0; i < indio_dev->num_channels; i++) >> >> >> >> - scan_mask |= BIT_ULL(indio_dev->channels[i].scan_index); >> >> >> >> + for (i = 0; i < indio_dev->num_channels; i++) { >> >> >> >> + const struct iio_chan_spec *chan = >> >> >> >> + &indio_dev->channels[i]; >> >> >> >> + >> >> >> >> + if (chan->scan_index < AMS_CTRL_SEQ_BASE) >> >> >> >> + scan_mask |= BIT_ULL(chan->scan_index); >> >> >> >> + } >> >> >> >> >> >> >> >> if (ams->ps_base) { >> >> >> >> /* put sysmon in a soft reset to change the sequence */ >> >> >> > >> >> >> > >> >> >
On Mon, 18 Mar 2024 11:18:43 -0400 Sean Anderson <sean.anderson@linux.dev> wrote: > On 3/16/24 09:36, Jonathan Cameron wrote: > > On Fri, 15 Mar 2024 13:47:40 -0400 > > Sean Anderson <sean.anderson@linux.dev> wrote: > > > >> Hi Conall, > >> > >> On 3/15/24 09:18, O'Griofa, Conall wrote: > >> > [AMD Official Use Only - General] > >> > > >> > Hi, > >> > > >> > I think there was a fix for this issue applied to the version that was running on 5.15 that didn't seem to make it into the upstream driver. > >> > Please see link for reference https://github.com/Xilinx/linux-xlnx/commit/608426961f16ab149b1b699f1c35f7ad244c0720 > >> > > >> > I think a similar fix to the above patch is may be beneficial? > >> > >> These patches look functionally identical to me. > > > > Because there are no channels with scan index between > > 22 * 2 + 16 (that patch) and 22 * 3 (your patch) that is > > the effect is indeed the same. But given the issues is the > > 64 limit on maximum scan index, 22 * 3 = 66 is an ugly value > > to compare with. > > > > I'm still very against the use of scan_index for anything other > > than scan indices (which is why partly how this bug wasn't noticed > > in the first palce). So the check should be scan_index != -1 > > and uses of those values elsewhere in the driver should be fixed > > (which looks simple to do from a quick glance at the code). > > OK, so how do the sysfs files get named then? Using channel and channel2 as appropriate (+ index and modified which change the meaning of channel2) - scan_index never had anything to do with sysfs file names - just the value in bufferX/in_xyz_scan_index > > --Sean > > >> > >> --Sean > >> > >> >> -----Original Message----- > >> >> From: Sean Anderson <sean.anderson@linux.dev> > >> >> Sent: Thursday, March 14, 2024 5:30 PM > >> >> To: Jonathan Cameron <jic23@kernel.org> > >> >> Cc: linux-iio@vger.kernel.org; O'Griofa, Conall <conall.ogriofa@amd.com>; > >> >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Lars-Peter > >> >> Clausen <lars@metafoo.de> > >> >> Subject: Re: [PATCH] iio: xilinx-ams: Don't include ams_ctrl_channels in > >> >> scan_mask > >> >> > >> >> Caution: This message originated from an External Source. Use proper caution > >> >> when opening attachments, clicking links, or responding. > >> >> > >> >> > >> >> On 3/14/24 11:48, Jonathan Cameron wrote: > >> >> > On Mon, 11 Mar 2024 12:28:00 -0400 > >> >> > Sean Anderson <sean.anderson@linux.dev> wrote: > >> >> > > >> >> >> ams_enable_channel_sequence constructs a "scan_mask" for all the PS > >> >> >> and PL channels. This works out fine, since scan_index for these > >> >> >> channels is less than 64. However, it also includes the > >> >> >> ams_ctrl_channels, where scan_index is greater than 64, triggering > >> >> >> undefined behavior. Since we don't need these channels anyway, just > >> >> exclude them. > >> >> >> > >> >> >> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") > >> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> > >> >> > > >> >> > Hi Sean, > >> >> > > >> >> > I'd ideally like to understand why we have channels with such large > >> >> > scan indexes. Those values should only be used for buffered capture. > >> >> > It feels like they are being abused here. Can we set them to -1 > >> >> > instead and check based on that? > >> >> > For a channel, a scan index of -1 means it can't be captured via the > >> >> > buffered interfaces but only accessed via sysfs reads. > >> >> > I think that's what we have here? > >> >> > >> >> From what I can tell, none of the channels support buffered reads. And we can't > >> >> naïvely convert the scan_index to -1, since that causes sysfs naming conflicts > >> >> (not to mention the compatibility break). > >> >> > >> >> > > >> >> > I just feel like if we leave these as things stand, we will get bitten > >> >> > by similar bugs in the future. At least with -1 it should be obvious why! > >> >> > >> >> There are just as likely to be bugs confusing the PL/PS subdevices... > >> >> > >> >> FWIW I had no trouble identifying the channels involved with this bug. > >> >> > >> >> --Sean > >> >> > >> >> > Jonathan > >> >> > > >> >> > > >> >> >> --- > >> >> >> > >> >> >> drivers/iio/adc/xilinx-ams.c | 8 ++++++-- > >> >> >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> >> >> > >> >> >> diff --git a/drivers/iio/adc/xilinx-ams.c > >> >> >> b/drivers/iio/adc/xilinx-ams.c index a55396c1f8b2..4de7ce598e4d > >> >> >> 100644 > >> >> >> --- a/drivers/iio/adc/xilinx-ams.c > >> >> >> +++ b/drivers/iio/adc/xilinx-ams.c > >> >> >> @@ -414,8 +414,12 @@ static void ams_enable_channel_sequence(struct > >> >> >> iio_dev *indio_dev) > >> >> >> > >> >> >> /* Run calibration of PS & PL as part of the sequence */ > >> >> >> scan_mask = BIT(0) | BIT(AMS_PS_SEQ_MAX); > >> >> >> - for (i = 0; i < indio_dev->num_channels; i++) > >> >> >> - scan_mask |= BIT_ULL(indio_dev->channels[i].scan_index); > >> >> >> + for (i = 0; i < indio_dev->num_channels; i++) { > >> >> >> + const struct iio_chan_spec *chan = > >> >> >> + &indio_dev->channels[i]; > >> >> >> + > >> >> >> + if (chan->scan_index < AMS_CTRL_SEQ_BASE) > >> >> >> + scan_mask |= BIT_ULL(chan->scan_index); > >> >> >> + } > >> >> >> > >> >> >> if (ams->ps_base) { > >> >> >> /* put sysmon in a soft reset to change the sequence */ > >> >> > > >> > > > >
On 3/16/24 09:36, Jonathan Cameron wrote: > On Fri, 15 Mar 2024 13:47:40 -0400 > Sean Anderson <sean.anderson@linux.dev> wrote: > >> Hi Conall, >> >> On 3/15/24 09:18, O'Griofa, Conall wrote: >> > [AMD Official Use Only - General] >> > >> > Hi, >> > >> > I think there was a fix for this issue applied to the version that was running on 5.15 that didn't seem to make it into the upstream driver. >> > Please see link for reference https://github.com/Xilinx/linux-xlnx/commit/608426961f16ab149b1b699f1c35f7ad244c0720 >> > >> > I think a similar fix to the above patch is may be beneficial? >> >> These patches look functionally identical to me. > > Because there are no channels with scan index between > 22 * 2 + 16 (that patch) and 22 * 3 (your patch) that is > the effect is indeed the same. But given the issues is the > 64 limit on maximum scan index, 22 * 3 = 66 is an ugly value > to compare with. > > I'm still very against the use of scan_index for anything other > than scan indices (which is why partly how this bug wasn't noticed > in the first palce). So the check should be scan_index != -1 > and uses of those values elsewhere in the driver should be fixed > (which looks simple to do from a quick glance at the code). OK, so how do the sysfs files get named then? --Sean >> >> --Sean >> >> >> -----Original Message----- >> >> From: Sean Anderson <sean.anderson@linux.dev> >> >> Sent: Thursday, March 14, 2024 5:30 PM >> >> To: Jonathan Cameron <jic23@kernel.org> >> >> Cc: linux-iio@vger.kernel.org; O'Griofa, Conall <conall.ogriofa@amd.com>; >> >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Lars-Peter >> >> Clausen <lars@metafoo.de> >> >> Subject: Re: [PATCH] iio: xilinx-ams: Don't include ams_ctrl_channels in >> >> scan_mask >> >> >> >> Caution: This message originated from an External Source. Use proper caution >> >> when opening attachments, clicking links, or responding. >> >> >> >> >> >> On 3/14/24 11:48, Jonathan Cameron wrote: >> >> > On Mon, 11 Mar 2024 12:28:00 -0400 >> >> > Sean Anderson <sean.anderson@linux.dev> wrote: >> >> > >> >> >> ams_enable_channel_sequence constructs a "scan_mask" for all the PS >> >> >> and PL channels. This works out fine, since scan_index for these >> >> >> channels is less than 64. However, it also includes the >> >> >> ams_ctrl_channels, where scan_index is greater than 64, triggering >> >> >> undefined behavior. Since we don't need these channels anyway, just >> >> exclude them. >> >> >> >> >> >> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver") >> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev> >> >> > >> >> > Hi Sean, >> >> > >> >> > I'd ideally like to understand why we have channels with such large >> >> > scan indexes. Those values should only be used for buffered capture. >> >> > It feels like they are being abused here. Can we set them to -1 >> >> > instead and check based on that? >> >> > For a channel, a scan index of -1 means it can't be captured via the >> >> > buffered interfaces but only accessed via sysfs reads. >> >> > I think that's what we have here? >> >> >> >> From what I can tell, none of the channels support buffered reads. And we can't >> >> naïvely convert the scan_index to -1, since that causes sysfs naming conflicts >> >> (not to mention the compatibility break). >> >> >> >> > >> >> > I just feel like if we leave these as things stand, we will get bitten >> >> > by similar bugs in the future. At least with -1 it should be obvious why! >> >> >> >> There are just as likely to be bugs confusing the PL/PS subdevices... >> >> >> >> FWIW I had no trouble identifying the channels involved with this bug. >> >> >> >> --Sean >> >> >> >> > Jonathan >> >> > >> >> > >> >> >> --- >> >> >> >> >> >> drivers/iio/adc/xilinx-ams.c | 8 ++++++-- >> >> >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> >> >> >> >> diff --git a/drivers/iio/adc/xilinx-ams.c >> >> >> b/drivers/iio/adc/xilinx-ams.c index a55396c1f8b2..4de7ce598e4d >> >> >> 100644 >> >> >> --- a/drivers/iio/adc/xilinx-ams.c >> >> >> +++ b/drivers/iio/adc/xilinx-ams.c >> >> >> @@ -414,8 +414,12 @@ static void ams_enable_channel_sequence(struct >> >> >> iio_dev *indio_dev) >> >> >> >> >> >> /* Run calibration of PS & PL as part of the sequence */ >> >> >> scan_mask = BIT(0) | BIT(AMS_PS_SEQ_MAX); >> >> >> - for (i = 0; i < indio_dev->num_channels; i++) >> >> >> - scan_mask |= BIT_ULL(indio_dev->channels[i].scan_index); >> >> >> + for (i = 0; i < indio_dev->num_channels; i++) { >> >> >> + const struct iio_chan_spec *chan = >> >> >> + &indio_dev->channels[i]; >> >> >> + >> >> >> + if (chan->scan_index < AMS_CTRL_SEQ_BASE) >> >> >> + scan_mask |= BIT_ULL(chan->scan_index); >> >> >> + } >> >> >> >> >> >> if (ams->ps_base) { >> >> >> /* put sysmon in a soft reset to change the sequence */ >> >> > >> >