While working on another series[1] I ran into issues where my SPI controller would fail to handle 14-bit and 16-bit SPI messages. This addresses that issue and adds support for selecting a different voltage reference source from the devicetree. V1 was base on a series[2] that seems to not have made it all the way, and was tested on an ad7689. [1] https://patchwork.kernel.org/project/linux-iio/list/?series=511545 [2] https://patchwork.kernel.org/project/linux-iio/list/?series=116971&state=%2A&archive=both Changes since v2: - add comments to ambiguous register names - fix be16 definition of the adc buffer - fix BPW logic - rework vref support - support per channel vref selection - infer reference select value based on DT parameters - update dt-binding Changes since v1: - add default case in read/write size cases - drop of_node change as the core already takes care of it - check dt user input in probe - move description at the top of dt-binding definition - drop AllOf block in dt-binding Thanks for your time, Liam Liam Beguin (4): iio: adc: ad7949: define and use bitfield names iio: adc: ad7949: fix spi messages on non 14-bit controllers iio: adc: ad7949: add support for internal vref dt-bindings: iio: adc: ad7949: add per channel reference .../bindings/iio/adc/adi,ad7949.yaml | 71 ++++- drivers/iio/adc/ad7949.c | 243 +++++++++++++++--- 2 files changed, 280 insertions(+), 34 deletions(-) Range-diff against v2: 1: b8577e93229f ! 1: 8760b368f971 iio: adc: ad7949: define and use bitfield names @@ Commit message ## drivers/iio/adc/ad7949.c ## @@ + #include <linux/module.h> #include <linux/regulator/consumer.h> #include <linux/spi/spi.h> ++#include <linux/bitfield.h> -#define AD7949_MASK_CHANNEL_SEL GENMASK(9, 7) #define AD7949_MASK_TOTAL GENMASK(13, 0) -#define AD7949_OFFSET_CHANNEL_SEL 7 -#define AD7949_CFG_READ_BACK 0x1 - #define AD7949_CFG_REG_SIZE_BITS 14 - -+#define AD7949_CFG_BIT_CFG BIT(13) -+#define AD7949_CFG_VAL_CFG_OVERWRITE 1 -+#define AD7949_CFG_VAL_CFG_KEEP 0 +-#define AD7949_CFG_REG_SIZE_BITS 14 ++ ++/* CFG: Configuration Update */ ++#define AD7949_CFG_BIT_OVERWRITE BIT(13) ++ ++/* INCC: Input Channel Configuration */ +#define AD7949_CFG_BIT_INCC GENMASK(12, 10) +#define AD7949_CFG_VAL_INCC_UNIPOLAR_GND 7 +#define AD7949_CFG_VAL_INCC_UNIPOLAR_COMM 6 @@ drivers/iio/adc/ad7949.c +#define AD7949_CFG_VAL_INCC_TEMP 3 +#define AD7949_CFG_VAL_INCC_BIPOLAR 2 +#define AD7949_CFG_VAL_INCC_BIPOLAR_DIFF 0 ++ ++/* INX: Input channel Selection in a binary fashion */ +#define AD7949_CFG_BIT_INX GENMASK(9, 7) -+#define AD7949_CFG_BIT_BW BIT(6) -+#define AD7949_CFG_VAL_BW_FULL 1 -+#define AD7949_CFG_VAL_BW_QUARTER 0 ++ ++/* BW: select bandwidth for low-pass filter. Full or Quarter */ ++#define AD7949_CFG_BIT_BW_FULL BIT(6) ++ ++/* REF: reference/buffer selection */ +#define AD7949_CFG_BIT_REF GENMASK(5, 3) ++#define AD7949_CFG_VAL_REF_EXT_BUF 7 ++ ++/* SEQ: channel sequencer. Allows for scanning channels */ +#define AD7949_CFG_BIT_SEQ GENMASK(2, 1) -+#define AD7949_CFG_BIT_RBN BIT(0) + ++/* RB: Read back the CFG register */ ++#define AD7949_CFG_BIT_RBN BIT(0) + enum { ID_AD7949 = 0, - ID_AD7682, @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, */ for (i = 0; i < 2; i++) { @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_init(struct ad7949_adc_chip *ad7 ad7949_adc->current_channel = 0; - ret = ad7949_spi_write_cfg(ad7949_adc, 0x3C79, AD7949_MASK_TOTAL); + -+ cfg = FIELD_PREP(AD7949_CFG_BIT_CFG, AD7949_CFG_VAL_CFG_OVERWRITE) | ++ cfg = FIELD_PREP(AD7949_CFG_BIT_OVERWRITE, 1) | + FIELD_PREP(AD7949_CFG_BIT_INCC, AD7949_CFG_VAL_INCC_UNIPOLAR_GND) | + FIELD_PREP(AD7949_CFG_BIT_INX, ad7949_adc->current_channel) | -+ FIELD_PREP(AD7949_CFG_BIT_BW, AD7949_CFG_VAL_BW_FULL) | -+ FIELD_PREP(AD7949_CFG_BIT_REF, AD7949_REF_EXT_BUF) | ++ FIELD_PREP(AD7949_CFG_BIT_BW_FULL, 1) | ++ FIELD_PREP(AD7949_CFG_BIT_REF, AD7949_CFG_VAL_REF_EXT_BUF) | + FIELD_PREP(AD7949_CFG_BIT_SEQ, 0x0) | + FIELD_PREP(AD7949_CFG_BIT_RBN, 1); + 2: a8468391e3d0 ! 2: a9243c834705 iio: adc: ad7949: fix spi messages on non 14-bit controllers @@ Commit message ## drivers/iio/adc/ad7949.c ## @@ - #include <linux/module.h> #include <linux/regulator/consumer.h> #include <linux/spi/spi.h> -+#include <linux/bitfield.h> + #include <linux/bitfield.h> ++#include <asm/unaligned.h> #define AD7949_MASK_TOTAL GENMASK(13, 0) - #define AD7949_CFG_REG_SIZE_BITS 14 + @@ drivers/iio/adc/ad7949.c: static const struct ad7949_adc_spec ad7949_adc_spec[] = { * @indio_dev: reference to iio structure * @spi: reference to spi structure @@ drivers/iio/adc/ad7949.c: struct ad7949_adc_chip { + u8 bits_per_word; u16 cfg; unsigned int current_channel; -- u16 buffer ____cacheline_aligned; -+ union { -+ __be16 buffer; -+ u8 buf8[2]; -+ } ____cacheline_aligned; - }; - -+static void ad7949_set_bits_per_word(struct ad7949_adc_chip *ad7949_adc) -+{ -+ u32 adc_mask = SPI_BPW_MASK(ad7949_adc->resolution); -+ u32 bpw = adc_mask & ad7949_adc->spi->controller->bits_per_word_mask; -+ -+ if (bpw == adc_mask) -+ ad7949_adc->bits_per_word = ad7949_adc->resolution; -+ else if (bpw == SPI_BPW_MASK(16)) -+ ad7949_adc->bits_per_word = 16; -+ else -+ ad7949_adc->bits_per_word = 8; -+} -+ - static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, + u16 buffer ____cacheline_aligned; +@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val, u16 mask) { int ret; - int bits_per_word = ad7949_adc->resolution; - int shift = bits_per_word - AD7949_CFG_REG_SIZE_BITS; ++ u8 buf8[2]; struct spi_message msg; struct spi_transfer tx[] = { { @@ drivers/iio/adc/ad7949.c: struct ad7949_adc_chip { + ad7949_adc->buffer = ad7949_adc->cfg; + break; + case 8: -+ default: + /* Pack 14-bit value into 2 bytes, MSB first */ -+ ad7949_adc->buf8[0] = FIELD_GET(GENMASK(13, 6), ad7949_adc->cfg); -+ ad7949_adc->buf8[1] = FIELD_GET(GENMASK(5, 0), ad7949_adc->cfg); -+ ad7949_adc->buf8[1] = ad7949_adc->buf8[1] << 2; ++ buf8[0] = FIELD_GET(GENMASK(13, 6), ad7949_adc->cfg); ++ buf8[1] = FIELD_GET(GENMASK(5, 0), ad7949_adc->cfg) << 2; ++ memcpy(&ad7949_adc->buffer, buf8, 2); + break; ++ default: ++ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n"); ++ return -EINVAL; + } + spi_message_init_with_transfers(&msg, tx, 1); @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_c int i; - int bits_per_word = ad7949_adc->resolution; - int mask = GENMASK(ad7949_adc->resolution - 1, 0); ++ u8 buf8[2]; struct spi_message msg; struct spi_transfer tx[] = { { @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_c + case 16: + *val = ad7949_adc->buffer; + /* Shift-out padding bits */ -+ if (ad7949_adc->resolution == 14) -+ *val = *val >> 2; ++ *val >>= 16 - ad7949_adc->resolution; + break; + case 14: + *val = ad7949_adc->buffer & GENMASK(13, 0); + break; + case 8: -+ default: ++ memcpy(buf8, &ad7949_adc->buffer, 2); + /* Convert byte array to u16, MSB first */ -+ *val = (ad7949_adc->buf8[0] << 8) | ad7949_adc->buf8[1]; ++ *val = get_unaligned_be16(buf8); + /* Shift-out padding bits */ -+ if (ad7949_adc->resolution == 14) -+ *val = *val >> 2; ++ *val >>= 16 - ad7949_adc->resolution; + break; ++ default: ++ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n"); ++ return -EINVAL; + } return 0; } +@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc) + + static int ad7949_spi_probe(struct spi_device *spi) + { ++ u32 spi_ctrl_mask = spi->controller->bits_per_word_mask; + struct device *dev = &spi->dev; + const struct ad7949_adc_spec *spec; + struct ad7949_adc_chip *ad7949_adc; @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_probe(struct spi_device *spi) - spec = &ad7949_adc_spec[spi_get_device_id(spi)->driver_data]; indio_dev->num_channels = spec->num_channels; ad7949_adc->resolution = spec->resolution; -+ ad7949_set_bits_per_word(ad7949_adc); ++ /* Set SPI bits per word */ ++ if (spi_ctrl_mask & SPI_BPW_MASK(ad7949_adc->resolution)) { ++ ad7949_adc->bits_per_word = ad7949_adc->resolution; ++ } else if (spi_ctrl_mask == SPI_BPW_MASK(16)) { ++ ad7949_adc->bits_per_word = 16; ++ } else if (spi_ctrl_mask == SPI_BPW_MASK(8)) { ++ ad7949_adc->bits_per_word = 8; ++ } else { ++ dev_err(dev, "unable to find common BPW with spi controller\n"); ++ return -EINVAL; ++ } ++ ad7949_adc->vref = devm_regulator_get(dev, "vref"); if (IS_ERR(ad7949_adc->vref)) { + dev_err(dev, "fail to request regulator\n"); 3: 4b0c11c9a748 < -: ------------ iio: adc: ad7949: add support for internal vref 4: a3b6a6ef15fd < -: ------------ dt-bindings: iio: adc: ad7949: add adi,reference-source -: ------------ > 3: bb25b7fcb0a3 iio: adc: ad7949: add support for internal vref -: ------------ > 4: 41e3ca4f0d52 dt-bindings: iio: adc: ad7949: add per channel reference base-commit: 6cbb3aa0f9d5d23221df787cf36f74d3866fdb78 -- 2.30.1.489.g328c10930387