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 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 | 261 ++++++++++++++---- 2 files changed, 274 insertions(+), 56 deletions(-) Range-diff against v3: -: ------------ > 1: 8760b368f971 iio: adc: ad7949: define and use bitfield names 1: a9243c834705 ! 2: 7b1484f2fc4c 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/regulator/consumer.h> - #include <linux/spi/spi.h> - #include <linux/bitfield.h> -+#include <asm/unaligned.h> - - #define AD7949_MASK_TOTAL GENMASK(13, 0) - @@ 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 int ad7949_spi_write_cfg(struct ad7949_adc_chip 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: static int ad7949_spi_write_cfg(struct ad7949_adc_chip + ad7949_adc->buffer = ad7949_adc->cfg; + break; + case 8: -+ /* Pack 14-bit value into 2 bytes, MSB first */ -+ 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); ++ /* Here, type is big endian as it must be sent in two transfers */ ++ ad7949_adc->buffer = (u16)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 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 + *val = ad7949_adc->buffer & GENMASK(13, 0); + break; + case 8: -+ memcpy(buf8, &ad7949_adc->buffer, 2); -+ /* Convert byte array to u16, MSB first */ -+ *val = get_unaligned_be16(buf8); ++ /* Here, type is big endian as data was sent in two transfers */ ++ *val = be16_to_cpu(ad7949_adc->buffer); + /* Shift-out padding bits */ + *val >>= 16 - ad7949_adc->resolution; + break; 2: bb25b7fcb0a3 ! 3: 41c4ab9c5e19 iio: adc: ad7949: add support for internal vref @@ drivers/iio/adc/ad7949.c: struct ad7949_adc_chip { u8 bits_per_word; u16 cfg; @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, + int ret; int i; - u8 buf8[2]; struct spi_message msg; -+ struct ad7949_channel ad7949_chan = ad7949_adc->channels[channel]; ++ struct ad7949_channel *ad7949_chan = &ad7949_adc->channels[channel]; struct spi_transfer tx[] = { { .rx_buf = &ad7949_adc->buffer, @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_channel(struct ad7949_adc_c - FIELD_PREP(AD7949_CFG_BIT_INX, channel), - AD7949_CFG_BIT_INX); + FIELD_PREP(AD7949_CFG_BIT_INX, channel) | -+ FIELD_PREP(AD7949_CFG_BIT_REF, ad7949_chan.refsel), ++ FIELD_PREP(AD7949_CFG_BIT_REF, ad7949_chan->refsel), + AD7949_CFG_BIT_INX | AD7949_CFG_BIT_REF); if (ret) return ret; @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_raw(struct iio_dev *indio_d int *val, int *val2, long mask) { struct ad7949_adc_chip *ad7949_adc = iio_priv(indio_dev); -+ struct ad7949_channel ad7949_chan = ad7949_adc->channels[chan->channel]; ++ struct ad7949_channel *ad7949_chan = &ad7949_adc->channels[chan->channel]; int ret; if (!val) @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_read_raw(struct iio_dev *indio_d - ret = regulator_get_voltage(ad7949_adc->vref); - if (ret < 0) - return ret; -+ switch (ad7949_chan.refsel) { ++ switch (ad7949_chan->refsel) { + case AD7949_CFG_VAL_REF_INT_2500: + *val = 2500; + break; @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_init(struct ad7949_adc_chip *ad7 const struct ad7949_adc_spec *spec; struct ad7949_adc_chip *ad7949_adc; + struct ad7949_channel *ad7949_chan; -+ struct device_node *child; ++ struct fwnode_handle *child; struct iio_dev *indio_dev; + int mode; + u32 tmp; @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_probe(struct spi_device *spi) - if (ret < 0) { - dev_err(dev, "fail to enable regulator\n"); - return ret; -+ if (mode > AD7949_CFG_VAL_REF_INT_4096) { ++ if (mode & AD7949_CFG_VAL_REF_EXTERNAL) { + ret = regulator_enable(ad7949_adc->vref); + if (ret < 0) { + dev_err(dev, "fail to enable regulator\n"); @@ drivers/iio/adc/ad7949.c: static int ad7949_spi_probe(struct spi_device *spi) + } + + /* Initialize all channel structures */ -+ for (i = 0; i < spec->num_channels; i++) { ++ for (i = 0; i < spec->num_channels; i++) + ad7949_adc->channels[i].refsel = mode; -+ } + + /* Read channel specific information form the devicetree */ -+ for_each_child_of_node(dev->of_node, child) { -+ ret = of_property_read_u32(child, "reg", &i); ++ device_for_each_child_node(dev, child) { ++ ret = fwnode_property_read_u32(child, "reg", &i); + if (ret) { -+ dev_err(dev, "missing reg property in child: %s\n", -+ child->full_name); -+ of_node_put(child); ++ dev_err(dev, "missing reg property in %pfw\n", child); ++ fwnode_handle_put(child); + return ret; + } + + ad7949_chan = &ad7949_adc->channels[i]; + -+ ret = of_property_read_u32(child, "adi,internal-ref-mv", &tmp); ++ ret = fwnode_property_read_u32(child, "adi,internal-ref-microvolt", &tmp); + if (ret < 0 && ret != -EINVAL) { -+ of_node_put(child); ++ dev_err(dev, "invalid internal reference in %pfw\n", child); ++ fwnode_handle_put(child); + return ret; + } + + switch (tmp) { -+ case 2500: ++ case 2500000: + ad7949_chan->refsel = AD7949_CFG_VAL_REF_INT_2500; + break; -+ case 4096: ++ case 4096000: + ad7949_chan->refsel = AD7949_CFG_VAL_REF_INT_4096; + break; + default: + dev_err(dev, "unsupported internal voltage reference\n"); -+ of_node_put(child); ++ fwnode_handle_put(child); + return -EINVAL; + } } 3: 41e3ca4f0d52 ! 4: 9cb48acbd05b dt-bindings: iio: adc: ad7949: add per channel reference @@ Commit message Add bindings documentation describing per channel reference voltage selection. - This adds the adi,internal-ref-mv property, and child nodes for each - channel. This is required to properly configure the ADC sample request - based on which reference source should be used for the calculation. + This adds the adi,internal-ref-microvolt property, and child nodes for + each channel. This is required to properly configure the ADC sample + request based on which reference source should be used for the + calculation. Signed-off-by: Liam Beguin <lvb@xxxxxxxxxx> @@ Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml: properties: + + '#size-cells': + const: 0 -+ + required: - compatible @@ Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml: properties: + minimum: 0 + maximum: 7 + -+ adi,internal-ref-mv: ++ adi,internal-ref-microvolt: + description: | -+ Internal reference voltage selection in millivolts. ++ Internal reference voltage selection in microvolts. + + If no internal reference is specified, the channel will default to the + external reference defined by vrefin-supply (or vref-supply). @@ Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml: properties: + If no supplies are defined, the reference selection will default to + 4096mV internal reference. + -+ $ref: /schemas/types.yaml#/definitions/uint32 -+ enum: [2500, 4096] -+ default: 4096 ++ enum: [2500000, 4096000] ++ default: 4096000 + + required: + - reg @@ Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml: examples: + vrefin-supply = <&vdd_supply>; + + channel@0 { -+ adi,internal-ref-mv = <4096>; ++ adi,internal-ref-microvolt = <4096000>; + reg = <0>; + }; + + channel@1 { -+ adi,internal-ref-mv = <2500>; ++ adi,internal-ref-microvolt = <2500000>; + reg = <1>; + }; + -: ------------ > 5: c48eb017058c iio: adc: ad7949: use devm managed functions base-commit: 6cbb3aa0f9d5d23221df787cf36f74d3866fdb78 -- 2.30.1.489.g328c10930387
