From: Liam Beguin <liambeguin@gmail.com>
To: liambeguin@gmail.com, lars@metafoo.de,
Michael.Hennerich@analog.com, jic23@kernel.org,
charles-antoine.couret@essensium.com, Nuno.Sa@analog.com
Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, robh+dt@kernel.org
Subject: [PATCH v4 0/5] AD7949 Fixes
Date: Tue, 27 Jul 2021 19:29:01 -0400 [thread overview]
Message-ID: <20210727232906.980769-1-liambeguin@gmail.com> (raw)
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@xiphos.com>
## 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@xiphos.com>
@@ 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
next reply other threads:[~2021-07-27 23:29 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-27 23:29 Liam Beguin [this message]
2021-07-27 23:29 ` [PATCH v4 1/5] iio: adc: ad7949: define and use bitfield names Liam Beguin
2021-07-27 23:29 ` [PATCH v4 2/5] iio: adc: ad7949: fix spi messages on non 14-bit controllers Liam Beguin
2021-07-31 14:29 ` Jonathan Cameron
2021-07-31 14:52 ` Jonathan Cameron
2021-08-01 23:01 ` Liam Beguin
2021-08-02 10:20 ` Jonathan Cameron
2021-08-02 14:10 ` Liam Beguin
2021-08-01 20:11 ` Liam Beguin
2021-07-27 23:29 ` [PATCH v4 3/5] iio: adc: ad7949: add support for internal vref Liam Beguin
2021-07-31 14:59 ` Jonathan Cameron
2021-07-27 23:29 ` [PATCH v4 4/5] dt-bindings: iio: adc: ad7949: add per channel reference Liam Beguin
2021-07-31 15:02 ` Jonathan Cameron
2021-08-02 21:26 ` Rob Herring
2021-07-27 23:29 ` [PATCH v4 5/5] iio: adc: ad7949: use devm managed functions Liam Beguin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210727232906.980769-1-liambeguin@gmail.com \
--to=liambeguin@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=Nuno.Sa@analog.com \
--cc=charles-antoine.couret@essensium.com \
--cc=devicetree@vger.kernel.org \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.