[PATCH v5 0/5] AD7949 Fixes

From: Liam Beguin
Date: Sat Aug 07 2021 - 21:59:23 EST

Next message: Liam Beguin: "[PATCH v5 2/5] iio: adc: ad7949: fix spi messages on non 14-bit controllers"
Previous message: Linus Torvalds: "Re: [GIT PULL] ucount fix for v5.14-rc"
Next in thread: Liam Beguin: "[PATCH v5 2/5] iio: adc: ad7949: fix spi messages on non 14-bit controllers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 v4:
- fix premature deletion of define
- use separate be16 buffer for 8-bit transfers
- switch to devm_regulator_get_optional()
- fix vref setup
- apply Reviewed-by

Changes since v3:
- use cpu_to_be16 and be16_to_cpu instead of manual conversion
- use pointers to channel structures instead of copies
- switch to generic device firmware property API
- use standard unit suffix names (mV to microvolt)
- switch to devm_iio_device_register() for additional cleanup

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 (5):
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
iio: adc: ad7949: use devm managed functions

.../bindings/iio/adc/adi,ad7949.yaml | 69 ++++-
drivers/iio/adc/ad7949.c | 281 ++++++++++++++----
2 files changed, 291 insertions(+), 59 deletions(-)

Range-diff against v4:
1: 8760b368f971 ! 1: a5c211185661 iio: adc: ad7949: define and use bitfield names
@@ drivers/iio/adc/ad7949.c
#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_REG_SIZE_BITS 14
+
+/* CFG: Configuration Update */
+#define AD7949_CFG_BIT_OVERWRITE BIT(13)
+
@@ drivers/iio/adc/ad7949.c
+
+/* 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++) {
2: 7b1484f2fc4c ! 2: df2f6df8f3d5 iio: adc: ad7949: fix spi messages on non 14-bit controllers
@@ Commit message
Signed-off-by: Liam Beguin <lvb@xxxxxxxxxx>

## drivers/iio/adc/ad7949.c ##
+@@
+ #include <linux/bitfield.h>
+
+ #define AD7949_MASK_TOTAL GENMASK(13, 0)
+-#define AD7949_CFG_REG_SIZE_BITS 14
+
+ /* CFG: Configuration Update */
+ #define AD7949_CFG_BIT_OVERWRITE BIT(13)
@@ 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: static const struct ad7949_adc_spec ad7949_adc_spec[]
* @cfg: copy of the configuration register
* @current_channel: current channel in use
* @buffer: buffer to send / receive data to / from device
++ * @buf8b: be16 buffer to exchange data with the device in 8-bit transfers
+ */
+ struct ad7949_adc_chip {
+ struct mutex lock;
@@ drivers/iio/adc/ad7949.c: struct ad7949_adc_chip {
struct iio_dev *indio_dev;
struct spi_device *spi;
@@ drivers/iio/adc/ad7949.c: struct ad7949_adc_chip {
u16 cfg;
unsigned int current_channel;
u16 buffer ____cacheline_aligned;
-@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val,
++ __be16 buf8b;
+ };
+
+ static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val,
u16 mask)
{
int ret;
@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_write_cfg(struct ad7949_adc_chip
struct spi_message msg;
struct spi_transfer tx[] = {
{
- .tx_buf = &ad7949_adc->buffer,
+- .tx_buf = &ad7949_adc->buffer,
.len = 2,
- .bits_per_word = bits_per_word,
+ .bits_per_word = ad7949_adc->bits_per_word,
},
};

-+ ad7949_adc->buffer = 0;
ad7949_adc->cfg = (val & mask) | (ad7949_adc->cfg & ~mask);
- ad7949_adc->buffer = ad7949_adc->cfg << shift;
+
+ switch (ad7949_adc->bits_per_word) {
+ case 16:
++ tx[0].tx_buf = &ad7949_adc->buffer;
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
++ tx[0].tx_buf = &ad7949_adc->buffer;
+ ad7949_adc->buffer = ad7949_adc->cfg;
+ break;
+ case 8:
+ /* Here, type is big endian as it must be sent in two transfers */
-+ ad7949_adc->buffer = (u16)cpu_to_be16(ad7949_adc->cfg << 2);
++ tx[0].tx_buf = &ad7949_adc->buf8b;
++ ad7949_adc->buf8b = cpu_to_be16(ad7949_adc->cfg << 2);
+ break;
+ default:
+ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n");
@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_c
struct spi_message msg;
struct spi_transfer tx[] = {
{
- .rx_buf = &ad7949_adc->buffer,
+- .rx_buf = &ad7949_adc->buffer,
.len = 2,
- .bits_per_word = bits_per_word,
+ .bits_per_word = ad7949_adc->bits_per_word,
},
};

++ if (ad7949_adc->bits_per_word == 8)
++ tx[0].rx_buf = &ad7949_adc->buf8b;
++ else
++ tx[0].rx_buf = &ad7949_adc->buffer;
++
+ /*
+ * 1: write CFG for sample N and read old data (sample N-2)
+ * 2: if CFG was not changed since sample N-1 then we'll get good data
+@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,
+ }
+
+ /* 3: write something and read actual data */
+- ad7949_adc->buffer = 0;
+ spi_message_init_with_transfers(&msg, tx, 1);
+ ret = spi_sync(ad7949_adc->spi, &msg);
+ if (ret)
@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,

ad7949_adc->current_channel = channel;
@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_c
+ break;
+ case 8:
+ /* Here, type is big endian as data was sent in two transfers */
-+ *val = be16_to_cpu(ad7949_adc->buffer);
++ *val = be16_to_cpu(ad7949_adc->buf8b);
+ /* Shift-out padding bits */
+ *val >>= 16 - ad7949_adc->resolution;
+ break;
3: 41c4ab9c5e19 ! 3: 8a33618a4f90 iio: adc: ad7949: add support for internal vref
@@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_c
+ struct ad7949_channel *ad7949_chan = &ad7949_adc->channels[channel];
struct spi_transfer tx[] = {
{
- .rx_buf = &ad7949_adc->buffer,
+ .len = 2,
@@ 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_probe(struct spi_device *spi)

- ad7949_adc->vref = devm_regulator_get(dev, "vref");
+ /* Setup external voltage ref, buffered? */
-+ ad7949_adc->vref = devm_regulator_get(dev, "vrefin");
++ ad7949_adc->vref = devm_regulator_get_optional(dev, "vrefin");
if (IS_ERR(ad7949_adc->vref)) {
- dev_err(dev, "fail to request regulator\n");
- return PTR_ERR(ad7949_adc->vref);
++ ret = PTR_ERR(ad7949_adc->vref);
++ if (ret != -ENODEV)
++ return ret;
+ /* unbuffered? */
-+ ad7949_adc->vref = devm_regulator_get(dev, "vref");
++ ad7949_adc->vref = devm_regulator_get_optional(dev, "vref");
+ if (IS_ERR(ad7949_adc->vref)) {
++ ret = PTR_ERR(ad7949_adc->vref);
++ if (ret != -ENODEV)
++ return ret;
+ /* Internal then */
+ mode = AD7949_CFG_VAL_REF_INT_4096;
++ } else {
++ mode = AD7949_CFG_VAL_REF_EXT_TEMP;
+ }
-+ mode = AD7949_CFG_VAL_REF_EXT_TEMP;
++ } else {
++ mode = AD7949_CFG_VAL_REF_EXT_TEMP_BUF;
}
-+ mode = AD7949_CFG_VAL_REF_EXT_TEMP_BUF;

- ret = regulator_enable(ad7949_adc->vref);
- if (ret < 0) {
4: 9cb48acbd05b ! 4: 7612ff29db6b dt-bindings: iio: adc: ad7949: add per channel reference
@@ Commit message
calculation.

Signed-off-by: Liam Beguin <lvb@xxxxxxxxxx>
+ Reviewed-by: Rob Herring <robh@xxxxxxxxxx>

## Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml ##
@@ Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml: properties:
5: c48eb017058c = 5: 74ee82caba57 iio: adc: ad7949: use devm managed functions

base-commit: 6cbb3aa0f9d5d23221df787cf36f74d3866fdb78
--
2.32.0.452.g940fe202adcb

Next message: Liam Beguin: "[PATCH v5 2/5] iio: adc: ad7949: fix spi messages on non 14-bit controllers"
Previous message: Linus Torvalds: "Re: [GIT PULL] ucount fix for v5.14-rc"
Next in thread: Liam Beguin: "[PATCH v5 2/5] iio: adc: ad7949: fix spi messages on non 14-bit controllers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]