From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
@@ -86,19 +88,34 @@ static int ad7949_spi_write_cfg(struct ad7949_adc_chip *ad7949_adc, u16 val,u16mask){intret;-intbits_per_word=ad7949_adc->resolution;-intshift=bits_per_word-AD7949_CFG_REG_SIZE_BITS;structspi_messagemsg;structspi_transfertx[]={{.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){+case16:+ad7949_adc->buffer=ad7949_adc->cfg<<2;+break;+case14:+ad7949_adc->buffer=ad7949_adc->cfg;+break;+case8:+/* 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");+return-EINVAL;+}+spi_message_init_with_transfers(&msg,tx,1);ret=spi_sync(ad7949_adc->spi,&msg);
@@ -115,14 +132,12 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,{intret;inti;-intbits_per_word=ad7949_adc->resolution;-intmask=GENMASK(ad7949_adc->resolution-1,0);structspi_messagemsg;structspi_transfertx[]={{.rx_buf=&ad7949_adc->buffer,.len=2,-.bits_per_word=bits_per_word,+.bits_per_word=ad7949_adc->bits_per_word,},};
@@ -157,7 +172,25 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,ad7949_adc->current_channel=channel;-*val=ad7949_adc->buffer&mask;+switch(ad7949_adc->bits_per_word){+case16:+*val=ad7949_adc->buffer;+/* Shift-out padding bits */+*val>>=16-ad7949_adc->resolution;+break;+case14:+*val=ad7949_adc->buffer&GENMASK(13,0);+break;+case8:+/* 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;+default:+dev_err(&ad7949_adc->indio_dev->dev,"unsupported BPW\n");+return-EINVAL;+}return0;}
@@ -265,6 +298,7 @@ static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)staticintad7949_spi_probe(structspi_device*spi){+u32spi_ctrl_mask=spi->controller->bits_per_word_mask;structdevice*dev=&spi->dev;conststructad7949_adc_spec*spec;structad7949_adc_chip*ad7949_adc;
@@ -291,6 +325,18 @@ static int ad7949_spi_probe(struct spi_device *spi)indio_dev->num_channels=spec->num_channels;ad7949_adc->resolution=spec->resolution;+/* Set SPI bits per word */+if(spi_ctrl_mask&SPI_BPW_MASK(ad7949_adc->resolution)){+ad7949_adc->bits_per_word=ad7949_adc->resolution;+}elseif(spi_ctrl_mask==SPI_BPW_MASK(16)){+ad7949_adc->bits_per_word=16;+}elseif(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");
From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
The define for this was removed in patch 1. I'll fix that up whilst applying by
keeping it until this patch. Please check build passes on intermediate points
during a patch series as otherwise we may break bisectability and that's really
annoying if you are bisecting!
Jonathan
quoted hunk
struct spi_message msg;
struct spi_transfer tx[] = {
{
.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:
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
+ 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);
+ break;
+ default:
+ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n");
+ return -EINVAL;
+ }
+
spi_message_init_with_transfers(&msg, tx, 1);
ret = spi_sync(ad7949_adc->spi, &msg);
@@ -115,14 +132,12 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, { int ret; int i;- int bits_per_word = ad7949_adc->resolution;- int mask = GENMASK(ad7949_adc->resolution - 1, 0); struct spi_message msg; struct spi_transfer tx[] = { { .rx_buf = &ad7949_adc->buffer, .len = 2,- .bits_per_word = bits_per_word,+ .bits_per_word = ad7949_adc->bits_per_word, }, };
@@ -157,7 +172,25 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, ad7949_adc->current_channel = channel;- *val = ad7949_adc->buffer & mask;+ switch (ad7949_adc->bits_per_word) {+ case 16:+ *val = ad7949_adc->buffer;+ /* Shift-out padding bits */+ *val >>= 16 - ad7949_adc->resolution;+ break;+ case 14:+ *val = ad7949_adc->buffer & GENMASK(13, 0);+ break;+ case 8:+ /* 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;+ default:+ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n");+ return -EINVAL;+ } return 0; }
From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
The define for this was removed in patch 1. I'll fix that up whilst applying by
keeping it until this patch. Please check build passes on intermediate points
during a patch series as otherwise we may break bisectability and that's really
annoying if you are bisecting!
Jonathan
quoted
struct spi_message msg;
struct spi_transfer tx[] = {
{
.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:
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
+ 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);
Gah, I wasn't thinking clearly when I suggested this. Sparse warns on the
endian conversion
One option is to resort to ignoring the fact we know it's aligned and
use the put_unaligned_be16() and get_unaligned_be16 calls which sparse seems to be
happy with. Alternative would be to just have a be16 buffer after the existing
one in the iio_priv structure. Then you will have to change the various users
of iio_priv()->buffer to point to the new value if we are doing 8 bit transfers.
Whilst more invasive, this second option is the one I'd suggest.
Note that there will be no need to add an __cacheline_aligned marking to this
new element because it will be in a cachline that is only used for DMA simply being
after the other buffer element which is force to start on a new cacheline.
Jonathan
From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
The define for this was removed in patch 1. I'll fix that up whilst applying by
keeping it until this patch. Please check build passes on intermediate points
during a patch series as otherwise we may break bisectability and that's really
annoying if you are bisecting!
Jonathan
quoted
struct spi_message msg;
struct spi_transfer tx[] = {
{
.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:
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
+ 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);
Gah, I wasn't thinking clearly when I suggested this. Sparse warns on
the
endian conversion
One option is to resort to ignoring the fact we know it's aligned and
use the put_unaligned_be16() and get_unaligned_be16 calls which sparse
seems to be
happy with. Alternative would be to just have a be16 buffer after the
existing
one in the iio_priv structure. Then you will have to change the various
users
of iio_priv()->buffer to point to the new value if we are doing 8 bit
transfers.
Whilst more invasive, this second option is the one I'd suggest.
Understood, I'll go with your suggestion.
Out of curiosity, other that being more explicit, is there another
we'd rather not use {get,put}_unaligned_be16()?
Note that there will be no need to add an __cacheline_aligned marking to
this
new element because it will be in a cachline that is only used for DMA
simply being
after the other buffer element which is force to start on a new
cacheline.
Noted, Thanks for taking the time to explaining this.
Liam
From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
The define for this was removed in patch 1. I'll fix that up whilst applying by
keeping it until this patch. Please check build passes on intermediate points
during a patch series as otherwise we may break bisectability and that's really
annoying if you are bisecting!
Jonathan
quoted
struct spi_message msg;
struct spi_transfer tx[] = {
{
.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:
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
+ 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);
Gah, I wasn't thinking clearly when I suggested this. Sparse warns on
the
endian conversion
One option is to resort to ignoring the fact we know it's aligned and
use the put_unaligned_be16() and get_unaligned_be16 calls which sparse
seems to be
happy with. Alternative would be to just have a be16 buffer after the
existing
one in the iio_priv structure. Then you will have to change the various
users
of iio_priv()->buffer to point to the new value if we are doing 8 bit
transfers.
Whilst more invasive, this second option is the one I'd suggest.
Understood, I'll go with your suggestion.
Out of curiosity, other that being more explicit, is there another
we'd rather not use {get,put}_unaligned_be16()?
We know it is aligned (as u16) so on a big endian platform that happens
to not handle unaligned accesses we will now be doing work which wouldn't
be needed if there was a get_aligned_be16() that was more relaxed about
types than cpu_to_be16() etc.
So basically it looks odd and we will will be hiding that we are smashing
data of potentially different ordering into the same structure field.
quoted
Note that there will be no need to add an __cacheline_aligned marking to
this
new element because it will be in a cachline that is only used for DMA
simply being
after the other buffer element which is force to start on a new
cacheline.
Noted, Thanks for taking the time to explaining this.
Liam
From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
The define for this was removed in patch 1. I'll fix that up whilst applying by
keeping it until this patch. Please check build passes on intermediate points
during a patch series as otherwise we may break bisectability and that's really
annoying if you are bisecting!
Jonathan
quoted
struct spi_message msg;
struct spi_transfer tx[] = {
{
.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:
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
+ 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);
Gah, I wasn't thinking clearly when I suggested this. Sparse warns on
the
endian conversion
One option is to resort to ignoring the fact we know it's aligned and
use the put_unaligned_be16() and get_unaligned_be16 calls which sparse
seems to be
happy with. Alternative would be to just have a be16 buffer after the
existing
one in the iio_priv structure. Then you will have to change the various
users
of iio_priv()->buffer to point to the new value if we are doing 8 bit
transfers.
Whilst more invasive, this second option is the one I'd suggest.
Understood, I'll go with your suggestion.
Out of curiosity, other that being more explicit, is there another
we'd rather not use {get,put}_unaligned_be16()?
We know it is aligned (as u16) so on a big endian platform that happens
to not handle unaligned accesses we will now be doing work which
wouldn't
be needed if there was a get_aligned_be16() that was more relaxed about
types than cpu_to_be16() etc.
So basically it looks odd and we will will be hiding that we are
smashing
data of potentially different ordering into the same structure field.
Got it! Thanks for the explanation.
Liam
quoted
quoted
Note that there will be no need to add an __cacheline_aligned marking to
this
new element because it will be in a cachline that is only used for DMA
simply being
after the other buffer element which is force to start on a new
cacheline.
Noted, Thanks for taking the time to explaining this.
Liam
From: Liam Beguin <redacted>
This driver supports devices with 14-bit and 16-bit sample sizes.
This is not always handled properly by spi controllers and can fail. To
work around this limitation, pad samples to 16-bit and split the sample
into two 8-bit messages in the event that only 8-bit messages are
supported by the controller.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 62 ++++++++++++++++++++++++++++++++++------
1 file changed, 54 insertions(+), 8 deletions(-)
The define for this was removed in patch 1. I'll fix that up whilst
applying by
keeping it until this patch. Please check build passes on intermediate
points
during a patch series as otherwise we may break bisectability and that's
really
annoying if you are bisecting!
Apologies for that. I'll make sure to add at least a `git rebase -x` to
my checks.
I'll fix it before sending the next version of this series.
Thanks,
Liam
Jonathan
quoted
struct spi_message msg;
struct spi_transfer tx[] = {
{
.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:
+ ad7949_adc->buffer = ad7949_adc->cfg << 2;
+ break;
+ case 14:
+ 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);
+ break;
+ default:
+ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n");
+ return -EINVAL;
+ }
+
spi_message_init_with_transfers(&msg, tx, 1);
ret = spi_sync(ad7949_adc->spi, &msg);
@@ -115,14 +132,12 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, { int ret; int i;- int bits_per_word = ad7949_adc->resolution;- int mask = GENMASK(ad7949_adc->resolution - 1, 0); struct spi_message msg; struct spi_transfer tx[] = { { .rx_buf = &ad7949_adc->buffer, .len = 2,- .bits_per_word = bits_per_word,+ .bits_per_word = ad7949_adc->bits_per_word, }, };
@@ -157,7 +172,25 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val, ad7949_adc->current_channel = channel;- *val = ad7949_adc->buffer & mask;+ switch (ad7949_adc->bits_per_word) {+ case 16:+ *val = ad7949_adc->buffer;+ /* Shift-out padding bits */+ *val >>= 16 - ad7949_adc->resolution;+ break;+ case 14:+ *val = ad7949_adc->buffer & GENMASK(13, 0);+ break;+ case 8:+ /* 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;+ default:+ dev_err(&ad7949_adc->indio_dev->dev, "unsupported BPW\n");+ return -EINVAL;+ } return 0; }
From: Liam Beguin <redacted>
Add support for selecting a custom reference voltage from the
devicetree. If an external source is used, a vref regulator should be
defined in the devicetree.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 132 +++++++++++++++++++++++++++++++++------
1 file changed, 114 insertions(+), 18 deletions(-)
@@ -133,6 +146,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,intret;inti;structspi_messagemsg;+structad7949_channel*ad7949_chan=&ad7949_adc->channels[channel];structspi_transfertx[]={{.rx_buf=&ad7949_adc->buffer,
@@ -149,8 +163,9 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,*/for(i=0;i<2;i++){ret=ad7949_spi_write_cfg(ad7949_adc,-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),+AD7949_CFG_BIT_INX|AD7949_CFG_BIT_REF);if(ret)returnret;if(channel==ad7949_adc->current_channel)
@@ -219,6 +234,7 @@ static int ad7949_spi_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structad7949_adc_chip*ad7949_adc=iio_priv(indio_dev);+structad7949_channel*ad7949_chan=&ad7949_adc->channels[chan->channel];intret;if(!val)
@@ -236,12 +252,26 @@ static int ad7949_spi_read_raw(struct iio_dev *indio_dev,returnIIO_VAL_INT;caseIIO_CHAN_INFO_SCALE:-ret=regulator_get_voltage(ad7949_adc->vref);-if(ret<0)-returnret;+switch(ad7949_chan->refsel){+caseAD7949_CFG_VAL_REF_INT_2500:+*val=2500;+break;+caseAD7949_CFG_VAL_REF_INT_4096:+*val=4096;+break;+caseAD7949_CFG_VAL_REF_EXT_TEMP:+caseAD7949_CFG_VAL_REF_EXT_TEMP_BUF:+ret=regulator_get_voltage(ad7949_adc->vref);+if(ret<0)+returnret;++/* convert value back to mV */+*val=ret/1000;+break;+}-*val=ret/5000;-returnIIO_VAL_INT;+*val2=(1<<ad7949_adc->resolution)-1;+returnIIO_VAL_FRACTIONAL;}return-EINVAL;
@@ -280,7 +310,7 @@ static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)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_FULL,1)|-FIELD_PREP(AD7949_CFG_BIT_REF,AD7949_CFG_VAL_REF_EXT_BUF)|+FIELD_PREP(AD7949_CFG_BIT_REF,ad7949_adc->channels[0].refsel)|FIELD_PREP(AD7949_CFG_BIT_SEQ,0x0)|FIELD_PREP(AD7949_CFG_BIT_RBN,1);
@@ -296,14 +326,24 @@ static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)returnret;}+staticvoidad7949_disable_reg(void*reg)+{+regulator_disable(reg);+}+staticintad7949_spi_probe(structspi_device*spi){u32spi_ctrl_mask=spi->controller->bits_per_word_mask;structdevice*dev=&spi->dev;conststructad7949_adc_spec*spec;structad7949_adc_chip*ad7949_adc;+structad7949_channel*ad7949_chan;+structfwnode_handle*child;structiio_dev*indio_dev;+intmode;+u32tmp;intret;+inti;indio_dev=devm_iio_device_alloc(dev,sizeof(*ad7949_adc));if(!indio_dev){
@@ -337,16 +377,74 @@ static int ad7949_spi_probe(struct spi_device *spi)return-EINVAL;}-ad7949_adc->vref=devm_regulator_get(dev,"vref");+/* Setup external voltage ref, buffered? */+ad7949_adc->vref=devm_regulator_get(dev,"vrefin");if(IS_ERR(ad7949_adc->vref)){-dev_err(dev,"fail to request regulator\n");-returnPTR_ERR(ad7949_adc->vref);+/* unbuffered? */+ad7949_adc->vref=devm_regulator_get(dev,"vref");+if(IS_ERR(ad7949_adc->vref)){+/* Internal then */+mode=AD7949_CFG_VAL_REF_INT_4096;+}+mode=AD7949_CFG_VAL_REF_EXT_TEMP;}+mode=AD7949_CFG_VAL_REF_EXT_TEMP_BUF;-ret=regulator_enable(ad7949_adc->vref);-if(ret<0){-dev_err(dev,"fail to enable regulator\n");-returnret;+if(mode&AD7949_CFG_VAL_REF_EXTERNAL){+ret=regulator_enable(ad7949_adc->vref);+if(ret<0){+dev_err(dev,"fail to enable regulator\n");+returnret;+}++ret=devm_add_action_or_reset(dev,ad7949_disable_reg,+ad7949_adc->vref);+if(ret)+returnret;+}++ad7949_adc->channels=devm_kcalloc(dev,spec->num_channels,+sizeof(*ad7949_adc->channels),+GFP_KERNEL);+if(!ad7949_adc->channels){+dev_err(dev,"unable to allocate ADC channels\n");+return-ENOMEM;+}++/* Initialize all channel structures */+for(i=0;i<spec->num_channels;i++)+ad7949_adc->channels[i].refsel=mode;++/* Read channel specific information form the devicetree */+device_for_each_child_node(dev,child){+ret=fwnode_property_read_u32(child,"reg",&i);+if(ret){+dev_err(dev,"missing reg property in %pfw\n",child);+fwnode_handle_put(child);+returnret;+}++ad7949_chan=&ad7949_adc->channels[i];++ret=fwnode_property_read_u32(child,"adi,internal-ref-microvolt",&tmp);+if(ret<0&&ret!=-EINVAL){+dev_err(dev,"invalid internal reference in %pfw\n",child);+fwnode_handle_put(child);+returnret;+}++switch(tmp){+case2500000:+ad7949_chan->refsel=AD7949_CFG_VAL_REF_INT_2500;+break;+case4096000:+ad7949_chan->refsel=AD7949_CFG_VAL_REF_INT_4096;+break;+default:+dev_err(dev,"unsupported internal voltage reference\n");+fwnode_handle_put(child);+return-EINVAL;+}}mutex_init(&ad7949_adc->lock);
@@ -367,7 +465,6 @@ static int ad7949_spi_probe(struct spi_device *spi)err:mutex_destroy(&ad7949_adc->lock);-regulator_disable(ad7949_adc->vref);returnret;}
@@ -379,7 +476,6 @@ static int ad7949_spi_remove(struct spi_device *spi)iio_device_unregister(indio_dev);mutex_destroy(&ad7949_adc->lock);-regulator_disable(ad7949_adc->vref);return0;}
From: Liam Beguin <redacted>
Add support for selecting a custom reference voltage from the
devicetree. If an external source is used, a vref regulator should be
defined in the devicetree.
Signed-off-by: Liam Beguin <redacted>
I didn't do a great job reviewing earlier versions of this series. Sorry about
that. As a result some comments inline.
Jonathan
@@ -133,6 +146,7 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,intret;inti;structspi_messagemsg;+structad7949_channel*ad7949_chan=&ad7949_adc->channels[channel];structspi_transfertx[]={{.rx_buf=&ad7949_adc->buffer,
@@ -149,8 +163,9 @@ static int ad7949_spi_read_channel(struct ad7949_adc_chip *ad7949_adc, int *val,*/for(i=0;i<2;i++){ret=ad7949_spi_write_cfg(ad7949_adc,-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),+AD7949_CFG_BIT_INX|AD7949_CFG_BIT_REF);if(ret)returnret;if(channel==ad7949_adc->current_channel)
@@ -219,6 +234,7 @@ static int ad7949_spi_read_raw(struct iio_dev *indio_dev,int*val,int*val2,longmask){structad7949_adc_chip*ad7949_adc=iio_priv(indio_dev);+structad7949_channel*ad7949_chan=&ad7949_adc->channels[chan->channel];intret;if(!val)
@@ -236,12 +252,26 @@ static int ad7949_spi_read_raw(struct iio_dev *indio_dev,returnIIO_VAL_INT;caseIIO_CHAN_INFO_SCALE:-ret=regulator_get_voltage(ad7949_adc->vref);-if(ret<0)-returnret;+switch(ad7949_chan->refsel){+caseAD7949_CFG_VAL_REF_INT_2500:+*val=2500;+break;+caseAD7949_CFG_VAL_REF_INT_4096:+*val=4096;+break;+caseAD7949_CFG_VAL_REF_EXT_TEMP:+caseAD7949_CFG_VAL_REF_EXT_TEMP_BUF:+ret=regulator_get_voltage(ad7949_adc->vref);+if(ret<0)+returnret;++/* convert value back to mV */+*val=ret/1000;+break;+}-*val=ret/5000;-returnIIO_VAL_INT;+*val2=(1<<ad7949_adc->resolution)-1;+returnIIO_VAL_FRACTIONAL;}return-EINVAL;
@@ -280,7 +310,7 @@ static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)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_FULL,1)|-FIELD_PREP(AD7949_CFG_BIT_REF,AD7949_CFG_VAL_REF_EXT_BUF)|+FIELD_PREP(AD7949_CFG_BIT_REF,ad7949_adc->channels[0].refsel)|FIELD_PREP(AD7949_CFG_BIT_SEQ,0x0)|FIELD_PREP(AD7949_CFG_BIT_RBN,1);
@@ -296,14 +326,24 @@ static int ad7949_spi_init(struct ad7949_adc_chip *ad7949_adc)returnret;}+staticvoidad7949_disable_reg(void*reg)+{+regulator_disable(reg);+}+staticintad7949_spi_probe(structspi_device*spi){u32spi_ctrl_mask=spi->controller->bits_per_word_mask;structdevice*dev=&spi->dev;conststructad7949_adc_spec*spec;structad7949_adc_chip*ad7949_adc;+structad7949_channel*ad7949_chan;+structfwnode_handle*child;structiio_dev*indio_dev;+intmode;+u32tmp;intret;+inti;indio_dev=devm_iio_device_alloc(dev,sizeof(*ad7949_adc));if(!indio_dev){
@@ -337,16 +377,74 @@ static int ad7949_spi_probe(struct spi_device *spi)return-EINVAL;}-ad7949_adc->vref=devm_regulator_get(dev,"vref");+/* Setup external voltage ref, buffered? */+ad7949_adc->vref=devm_regulator_get(dev,"vrefin");
Needs to be the optional form as otherwise we may get a stub regulator
and not fail here as we should.
if (IS_ERR(ad7949_adc->vref)) {
Need to check the error code for -ENODEV (IIRC) which is the only error
to indicate it is not present. If we get -EDEFER for example then we want
to fail the probe here as we know we should have the regulator later, once its
own driver has loaded.
devm_regulator_get_optional() again to avoid stub regulators.
+ if (IS_ERR(ad7949_adc->vref)) {
As above, need to check specifically for the code that reflects it not
being specified rather than any error. Only if it is not specified
do we want to be ignore the error and use the internal regulator.
quoted hunk
+ /* Internal then */
+ mode = AD7949_CFG_VAL_REF_INT_4096;
+ }
+ mode = AD7949_CFG_VAL_REF_EXT_TEMP;
}
+ mode = AD7949_CFG_VAL_REF_EXT_TEMP_BUF;
- ret = regulator_enable(ad7949_adc->vref);
- if (ret < 0) {
- dev_err(dev, "fail to enable regulator\n");
- return ret;
+ if (mode & AD7949_CFG_VAL_REF_EXTERNAL) {
+ ret = regulator_enable(ad7949_adc->vref);
+ if (ret < 0) {
+ dev_err(dev, "fail to enable regulator\n");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, ad7949_disable_reg,
+ ad7949_adc->vref);
+ if (ret)
+ return ret;
+ }
+
+ ad7949_adc->channels = devm_kcalloc(dev, spec->num_channels,
+ sizeof(*ad7949_adc->channels),
+ GFP_KERNEL);
+ if (!ad7949_adc->channels) {
+ dev_err(dev, "unable to allocate ADC channels\n");
+ return -ENOMEM;
+ }
+
+ /* Initialize all channel structures */
+ for (i = 0; i < spec->num_channels; i++)
+ ad7949_adc->channels[i].refsel = mode;
+
+ /* Read channel specific information form the devicetree */
+ device_for_each_child_node(dev, child) {
+ ret = fwnode_property_read_u32(child, "reg", &i);
+ if (ret) {
+ dev_err(dev, "missing reg property in %pfw\n", child);
+ fwnode_handle_put(child);
+ return ret;
+ }
+
+ ad7949_chan = &ad7949_adc->channels[i];
+
+ ret = fwnode_property_read_u32(child, "adi,internal-ref-microvolt", &tmp);
+ if (ret < 0 && ret != -EINVAL) {
+ dev_err(dev, "invalid internal reference in %pfw\n", child);
+ fwnode_handle_put(child);
+ return ret;
+ }
+
+ switch (tmp) {
+ case 2500000:
+ ad7949_chan->refsel = AD7949_CFG_VAL_REF_INT_2500;
+ break;
+ case 4096000:
+ ad7949_chan->refsel = AD7949_CFG_VAL_REF_INT_4096;
+ break;
+ default:
+ dev_err(dev, "unsupported internal voltage reference\n");
+ fwnode_handle_put(child);
+ return -EINVAL;
+ }
}
mutex_init(&ad7949_adc->lock);
From: Liam Beguin <redacted>
Add bindings documentation describing per channel reference voltage
selection.
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 <redacted>
---
.../bindings/iio/adc/adi,ad7949.yaml | 69 +++++++++++++++++--
1 file changed, 65 insertions(+), 4 deletions(-)
@@ -26,19 +26,63 @@ properties:reg:maxItems:1+vrefin-supply:+description:+Buffered ADC reference voltage supply.+vref-supply:description:-ADC reference voltage supply+Unbuffered ADC reference voltage supply.spi-max-frequency:true-"#io-channel-cells":+'#io-channel-cells':const:1+'#address-cells':+const:1++'#size-cells':+const:0+required:-compatible-reg--vref-supply++patternProperties:+'^channel@([0-7])$':+type:object+description:|+Represents the external channels which are connected to the ADC.++properties:+reg:+description:|+The channel number.+Up to 4 channels, numbered from 0 to 3 for adi,ad7682.+Up to 8 channels, numbered from 0 to 7 for adi,ad7689 and adi,ad7949.+items:+minimum:0+maximum:7++adi,internal-ref-microvolt:+description:|+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).+vrefin-supply will take precedence over vref-supply if both are defined.++If no supplies are defined, the reference selection will default to+4096mV internal reference.++enum:[2500000,4096000]+default:4096000++required:+-reg++additionalProperties:falseadditionalProperties:false
From: Liam Beguin <redacted>
Add bindings documentation describing per channel reference voltage
selection.
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 <redacted>
I'm fine with this, but as it's a bit unusual, definitely want to give a
little more time for Rob and others to take a look.
Jonathan
@@ -26,19 +26,63 @@ properties:reg:maxItems:1+vrefin-supply:+description:+Buffered ADC reference voltage supply.+vref-supply:description:-ADC reference voltage supply+Unbuffered ADC reference voltage supply.spi-max-frequency:true-"#io-channel-cells":+'#io-channel-cells':const:1+'#address-cells':+const:1++'#size-cells':+const:0+required:-compatible-reg--vref-supply++patternProperties:+'^channel@([0-7])$':+type:object+description:|+Represents the external channels which are connected to the ADC.++properties:+reg:+description:|+The channel number.+Up to 4 channels, numbered from 0 to 3 for adi,ad7682.+Up to 8 channels, numbered from 0 to 7 for adi,ad7689 and adi,ad7949.+items:+minimum:0+maximum:7++adi,internal-ref-microvolt:+description:|+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).+vrefin-supply will take precedence over vref-supply if both are defined.++If no supplies are defined, the reference selection will default to+4096mV internal reference.++enum:[2500000,4096000]+default:4096000++required:+-reg++additionalProperties:falseadditionalProperties:false
From: Rob Herring <robh@kernel.org> Date: 2021-08-02 21:26:47
On Tue, 27 Jul 2021 19:29:05 -0400, Liam Beguin wrote:
From: Liam Beguin <redacted>
Add bindings documentation describing per channel reference voltage
selection.
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 <redacted>
---
.../bindings/iio/adc/adi,ad7949.yaml | 69 +++++++++++++++++--
1 file changed, 65 insertions(+), 4 deletions(-)
From: Liam Beguin <redacted>
Switch to devm_iio_device_register to finalize devm migration.
This removes the use for iio_device_unregister() and since
mutex_destroy() is not necessary here, remove it altogether.
Signed-off-by: Liam Beguin <redacted>
---
drivers/iio/adc/ad7949.c | 25 +++----------------------
1 file changed, 3 insertions(+), 22 deletions(-)
@@ -452,34 +452,16 @@ static int ad7949_spi_probe(struct spi_device *spi)ret=ad7949_spi_init(ad7949_adc);if(ret){dev_err(dev,"enable to init this device: %d\n",ret);-gotoerr;+returnret;}-ret=iio_device_register(indio_dev);-if(ret){+ret=devm_iio_device_register(dev,indio_dev);+if(ret)dev_err(dev,"fail to register iio device: %d\n",ret);-gotoerr;-}--return0;--err:-mutex_destroy(&ad7949_adc->lock);returnret;}-staticintad7949_spi_remove(structspi_device*spi)-{-structiio_dev*indio_dev=spi_get_drvdata(spi);-structad7949_adc_chip*ad7949_adc=iio_priv(indio_dev);--iio_device_unregister(indio_dev);-mutex_destroy(&ad7949_adc->lock);--return0;-}-staticconststructof_device_idad7949_spi_of_id[]={{.compatible="adi,ad7949"},{.compatible="adi,ad7682"},