Support ROHM BD79105 ADC
Subsystems (4)
Matti Vaittinen
maintainerv3 cover Support ROHM BD79105 ADC
Add support for the ROHM BD79105 ADC
(and do some minor simplifications to the ad7476 driver while at it).
The first 2 patches were originally sent as an RFC:
https://lore.kernel.org/all/[email protected]/
Revision history:
v2 => v3:
- Drop 5/10 "Limit the scope of the chip_info"
- Add 5/10 "use *_cansleep GPIO APIs"
- Multiple fixes as suggested during v2 review. More accurate changelog included in individual patches
v1 => v2:
- Two new patches:
5/10 "Limit the scope of the chip_info" and
6/10 "Drop convstart chan_spec"
Please, let me know if you think some of the changes should be squashed.
- Multiple fixes as suggested during v1 review. More accurate changelog included in individual patches
Simplification RFC => ROHM BD79105 support series v1:
- Use spi_get_device_match_data()
- Fix uV to mV conversion
- Rewording of commit message
- Added patches 3 to 8.
Matti Vaittinen (10):
iio: adc: ad7476: Simplify chip type detection
iio: adc: ad7476: Simplify scale handling
iio: adc: ad7476: Use mV for internal reference
iio: adc: ad7476: Use correct channel for bit info
iio: adc: ad7476: use *_cansleep GPIO APIs
iio: adc: ad7476: Drop convstart chan_spec
iio: adc: ad7476: Conditionally call convstart
dt-bindings: iio: adc: ad7476: Add ROHM bd79105
iio: adc: ad7476: Support ROHM BD79105
MAINTAINERS: A driver for simple 1-channel SPI ADCs
.../bindings/iio/adc/adi,ad7476.yaml | 54 ++ MAINTAINERS | 5 + drivers/iio/adc/ad7476.c | 461 +++++++++--------- 3 files changed, 292 insertions(+), 228 deletions(-)
Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 1/10 iio: adc: ad7476: Simplify chip type detection
The ad7476 driver uses a table of structures for defining the IC variant specific data. Table is indexed using enum values, which are picked by SPI ID.
Having the table and an enum adds extra complexity and may encourage adding IC specific quircks in the code, instead of centralizing the IC differences in one place, the chip-info.
Simplify this by dropping the table and using individual structures for the IC specific data, and storing the IC specific structure's address directly in the SPI ID data. Finally, switch to the spi_get_device_match_data() and add a check for the return value.
---
Revision history:
v1 =>
- No changes
This patch was originally sent as an RFC in 2 patch series:
https://lore.kernel.org/all/[email protected]/
Changes from the RFC => v1:
- Use spi_get_device_match_data()
- Slightly reword commit message
--- drivers/iio/adc/ad7476.c | 296 +++++++++++++++++++-------------------- 1 file changed, 146 insertions(+), 150 deletions(-)
drivers/iio/adc/ad7476.c +146 -150
@@ -52,29 +52,6 @@ struct ad7476_state {unsigned char data[ALIGN (2 ,sizeof (s64)) +sizeof (s64)]__aligned(IIO_DMA_MINALIGN) ; }; -enum ad7476_supported_device_ids { -ID_AD7091 , -ID_AD7091R , -ID_AD7273 , -ID_AD7274 , -ID_AD7276 , -ID_AD7277 , -ID_AD7278 , -ID_AD7466 , -ID_AD7467 , -ID_AD7468 , -ID_AD7475 , -ID_AD7495 , -ID_AD7940 , -ID_ADC081S , -ID_ADC101S , -ID_ADC121S , -ID_ADS7866 , -ID_ADS7867 , -ID_ADS7868 , -ID_LTC2314_14 , -}; -static void ad7091_convst (struct ad7476_state *st) {if (!st->convst_gpio)
@@ -190,102 +167,119 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,#define ADS786X_CHAN (bits) _AD7476_CHAN((bits), 12 - (bits), \ BIT(IIO_CHAN_INFO_RAW)) -static const struct ad7476_chip_info ad7476_chip_info_tbl[] = { - [ID_AD7091 ] = { - .channel[0 ] =AD7091R_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), - .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .reset = ad7091_reset, - }, - [ID_AD7091R ] = { - .channel[0 ] =AD7091R_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), - .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .int_vref_uv =2500000 , - .has_vref = true, - .reset = ad7091_reset, - }, - [ID_AD7273 ] = { - .channel[0 ] =AD7940_CHAN (10 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .has_vref = true, - }, - [ID_AD7274 ] = { - .channel[0 ] =AD7940_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .has_vref = true, - }, - [ID_AD7276 ] = { - .channel[0 ] =AD7940_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_AD7277 ] = { - .channel[0 ] =AD7940_CHAN (10 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_AD7278 ] = { - .channel[0 ] =AD7940_CHAN (8 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_AD7466 ] = { - .channel[0 ] =AD7476_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_AD7467 ] = { - .channel[0 ] =AD7476_CHAN (10 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_AD7468 ] = { - .channel[0 ] =AD7476_CHAN (8 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_AD7475 ] = { - .channel[0 ] =AD7476_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .has_vref = true, - .has_vdrive = true, - }, - [ID_AD7495 ] = { - .channel[0 ] =AD7476_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .int_vref_uv =2500000 , - .has_vdrive = true, - }, - [ID_AD7940 ] = { - .channel[0 ] =AD7940_CHAN (14 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_ADC081S ] = { - .channel[0 ] =ADC081S_CHAN (8 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_ADC101S ] = { - .channel[0 ] =ADC081S_CHAN (10 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_ADC121S ] = { - .channel[0 ] =ADC081S_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_ADS7866 ] = { - .channel[0 ] =ADS786X_CHAN (12 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_ADS7867 ] = { - .channel[0 ] =ADS786X_CHAN (10 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_ADS7868 ] = { - .channel[0 ] =ADS786X_CHAN (8 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - }, - [ID_LTC2314_14 ] = { - .channel[0 ] =AD7940_CHAN (14 ), - .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .has_vref = true, - }, +static const struct ad7476_chip_info ad7091_chip_info = { + .channel[0 ] =AD7091R_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), + .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .reset = ad7091_reset, +}; + +static const struct ad7476_chip_info ad7091r_chip_info = { + .channel[0 ] =AD7091R_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), + .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .int_vref_uv =2500000 , + .has_vref = true, + .reset = ad7091_reset, +}; + +static const struct ad7476_chip_info ad7273_chip_info = { + .channel[0 ] =AD7940_CHAN (10 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .has_vref = true, +}; + +static const struct ad7476_chip_info ad7274_chip_info = { + .channel[0 ] =AD7940_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .has_vref = true, +}; + +static const struct ad7476_chip_info ad7276_chip_info = { + .channel[0 ] =AD7940_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ad7277_chip_info = { + .channel[0 ] =AD7940_CHAN (10 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ad7278_chip_info = { + .channel[0 ] =AD7940_CHAN (8 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ad7466_chip_info = { + .channel[0 ] =AD7476_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ad7467_chip_info = { + .channel[0 ] =AD7476_CHAN (10 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ad7468_chip_info = { + .channel[0 ] =AD7476_CHAN (8 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ad7475_chip_info = { + .channel[0 ] =AD7476_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .has_vref = true, + .has_vdrive = true, +}; + +static const struct ad7476_chip_info ad7495_chip_info = { + .channel[0 ] =AD7476_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .int_vref_uv =2500000 , + .has_vdrive = true, +}; + +static const struct ad7476_chip_info ad7940_chip_info = { + .channel[0 ] =AD7940_CHAN (14 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info adc081s_chip_info = { + .channel[0 ] =ADC081S_CHAN (8 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info adc101s_chip_info = { + .channel[0 ] =ADC081S_CHAN (10 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info adc121s_chip_info = { + .channel[0 ] =ADC081S_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ads7866_chip_info = { + .channel[0 ] =ADS786X_CHAN (12 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ads7867_chip_info = { + .channel[0 ] =ADS786X_CHAN (10 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ads7868_chip_info = { + .channel[0 ] =ADS786X_CHAN (8 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +}; + +static const struct ad7476_chip_info ltc2314_14_chip_info = { + .channel[0 ] =AD7940_CHAN (14 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .has_vref = true, };static const struct iio_info ad7476_info = {
@@ -311,8 +305,10 @@ static int ad7476_probe(struct spi_device *spi)return -ENOMEM ; st =iio_priv (indio_dev); - st->chip_info = - &ad7476_chip_info_tbl[spi_get_device_id (spi)->driver_data]; + + st->chip_info =spi_get_device_match_data (spi); +if (!st->chip_info) +return -ENODEV ; reg =devm_regulator_get (&spi->dev,"vcc" );if (IS_ERR (reg))
@@ -408,41 +404,41 @@ static int ad7476_probe(struct spi_device *spi) }static const struct spi_device_id ad7476_id[] = { - {"ad7091" ,ID_AD7091 }, - {"ad7091r" ,ID_AD7091R }, - {"ad7273" ,ID_AD7273 }, - {"ad7274" ,ID_AD7274 }, - {"ad7276" ,ID_AD7276 }, - {"ad7277" ,ID_AD7277 }, - {"ad7278" ,ID_AD7278 }, - {"ad7466" ,ID_AD7466 }, - {"ad7467" ,ID_AD7467 }, - {"ad7468" ,ID_AD7468 }, - {"ad7475" ,ID_AD7475 }, - {"ad7476" ,ID_AD7466 }, - {"ad7476a" ,ID_AD7466 }, - {"ad7477" ,ID_AD7467 }, - {"ad7477a" ,ID_AD7467 }, - {"ad7478" ,ID_AD7468 }, - {"ad7478a" ,ID_AD7468 }, - {"ad7495" ,ID_AD7495 }, - {"ad7910" ,ID_AD7467 }, - {"ad7920" ,ID_AD7466 }, - {"ad7940" ,ID_AD7940 }, - {"adc081s" ,ID_ADC081S }, - {"adc101s" ,ID_ADC101S }, - {"adc121s" ,ID_ADC121S }, - {"ads7866" ,ID_ADS7866 }, - {"ads7867" ,ID_ADS7867 }, - {"ads7868" ,ID_ADS7868 }, + {"ad7091" , (kernel_ulong_t)&ad7091_chip_info }, + {"ad7091r" , (kernel_ulong_t)&ad7091r_chip_info }, + {"ad7273" , (kernel_ulong_t)&ad7273_chip_info }, + {"ad7274" , (kernel_ulong_t)&ad7274_chip_info }, + {"ad7276" , (kernel_ulong_t)&ad7276_chip_info }, + {"ad7277" , (kernel_ulong_t)&ad7277_chip_info }, + {"ad7278" , (kernel_ulong_t)&ad7278_chip_info }, + {"ad7466" , (kernel_ulong_t)&ad7466_chip_info }, + {"ad7467" , (kernel_ulong_t)&ad7467_chip_info }, + {"ad7468" , (kernel_ulong_t)&ad7468_chip_info }, + {"ad7475" , (kernel_ulong_t)&ad7475_chip_info }, + {"ad7476" , (kernel_ulong_t)&ad7466_chip_info }, + {"ad7476a" , (kernel_ulong_t)&ad7466_chip_info }, + {"ad7477" , (kernel_ulong_t)&ad7467_chip_info }, + {"ad7477a" , (kernel_ulong_t)&ad7467_chip_info }, + {"ad7478" , (kernel_ulong_t)&ad7468_chip_info }, + {"ad7478a" , (kernel_ulong_t)&ad7468_chip_info }, + {"ad7495" , (kernel_ulong_t)&ad7495_chip_info }, + {"ad7910" , (kernel_ulong_t)&ad7467_chip_info }, + {"ad7920" , (kernel_ulong_t)&ad7466_chip_info }, + {"ad7940" , (kernel_ulong_t)&ad7940_chip_info }, + {"adc081s" , (kernel_ulong_t)&adc081s_chip_info }, + {"adc101s" , (kernel_ulong_t)&adc101s_chip_info }, + {"adc121s" , (kernel_ulong_t)&adc121s_chip_info }, + {"ads7866" , (kernel_ulong_t)&ads7866_chip_info }, + {"ads7867" , (kernel_ulong_t)&ads7867_chip_info }, + {"ads7868" , (kernel_ulong_t)&ads7868_chip_info },/* * The ROHM BU79100G is identical to the TI's ADS7866 from the software * point of view. The binding document mandates the ADS7866 to be * marked as a fallback for the BU79100G, but we still need the SPI ID * here to make the module loading work. */ - {"bu79100g" ,ID_ADS7866 }, - {"ltc2314-14" ,ID_LTC2314_14 }, + {"bu79100g" , (kernel_ulong_t)&ads7866_chip_info }, + {"ltc2314-14" , (kernel_ulong_t)<c2314_14_chip_info }, { } };MODULE_DEVICE_TABLE (spi, ad7476_id);
Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 2/10 iio: adc: ad7476: Simplify scale handling
In reply to Matti VaittinenThe ad7476 driver supports variants with different amount of supply regulators. On some variants there is only VCC, which is used as a reference voltage. Others have separate VREF regulator, and some rely on internal VREF. Some have both internal VREF and option to connect external one.
The ad7476 driver reads the regulator voltage only when the user asks to get the scale. This means the driver needs to do some dancing while picking the correct reference regulator (or internal reference), and store it for the later use.
According to the discussion [1] variable voltage references are rare, making it hard to justify the added complexity for supporting those.
Drop the support for the variable voltage references and simplify things by using the managed regulator get and enable interfaces.
---
Revision history:
v2 => :
- No changes
v1 => v2:
- use link-tag
This patch was originally sent as an RFC in 2 patch series:
https://lore.kernel.org/all/[email protected]/
Changes from the RFC:
- Fix the faulty conversion from uV to mV as pointed out by David.
--- drivers/iio/adc/ad7476.c | 84 ++++++++++------------------------------ 1 file changed, 21 insertions(+), 63 deletions(-)
drivers/iio/adc/ad7476.c +21 -63
@@ -39,10 +39,10 @@ struct ad7476_chip_info {struct ad7476_state {struct spi_device *spi;const struct ad7476_chip_info *chip_info; -struct regulator *ref_reg;struct gpio_desc *convst_gpio;struct spi_transfer xfer;struct spi_message msg; +int scale_mv;/* * DMA (thus cache coherency maintenance) may require the * transfer buffers to live in their own cache lines.
@@ -111,7 +111,6 @@ static int ad7476_read_raw(struct iio_dev *indio_dev, {int ret;struct ad7476_state *st =iio_priv (indio_dev); -int scale_uv;switch (m) {case IIO_CHAN_INFO_RAW :
@@ -126,14 +125,7 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,GENMASK (st->chip_info->channel[0 ].scan_type.realbits -1 ,0 );return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : -if (st->ref_reg) { - scale_uv =regulator_get_voltage (st->ref_reg); -if (scale_uv <0 ) -return scale_uv; - }else { - scale_uv = st->chip_info->int_vref_uv; - } - *val = scale_uv /1000 ; + *val = st->scale_mv; *val2 = chan->scan_type.realbits;return IIO_VAL_FRACTIONAL_LOG2 ; }
@@ -286,18 +278,10 @@ static const struct iio_info ad7476_info = { .read_raw = &ad7476_read_raw, }; -static void ad7476_reg_disable (void *data) -{ -struct regulator *reg = data; - -regulator_disable (reg); -} -static int ad7476_probe (struct spi_device *spi) {struct ad7476_state *st;struct iio_dev *indio_dev; -struct regulator *reg;int ret; indio_dev =devm_iio_device_alloc (&spi->dev,sizeof (*st));
@@ -310,58 +294,32 @@ static int ad7476_probe(struct spi_device *spi)if (!st->chip_info)return -ENODEV ; - reg =devm_regulator_get (&spi->dev,"vcc" ); -if (IS_ERR (reg)) -return PTR_ERR (reg); - - ret =regulator_enable (reg); -if (ret) -return ret; - - ret =devm_add_action_or_reset (&spi->dev, ad7476_reg_disable, reg); -if (ret) -return ret; - -/* Either vcc or vref (below) as appropriate */ -if (!st->chip_info->int_vref_uv) - st->ref_reg = reg; +/* Use VCC for reference voltage if vref / internal vref aren't used */ +if (!st->chip_info->int_vref_uv && !st->chip_info->has_vref) { + ret =devm_regulator_get_enable_read_voltage (&spi->dev,"vcc" ); +if (ret <0 ) +return ret; + st->scale_mv = ret /1000 ; + }else { + ret =devm_regulator_get_enable (&spi->dev,"vcc" ); +if (ret <0 ) +return ret; + }if (st->chip_info->has_vref) { - -/* If a device has an internal reference vref is optional */ -if (st->chip_info->int_vref_uv) { - reg =devm_regulator_get_optional (&spi->dev,"vref" ); -if (IS_ERR (reg) && (PTR_ERR (reg) != -ENODEV )) -return PTR_ERR (reg); - }else { - reg =devm_regulator_get (&spi->dev,"vref" ); -if (IS_ERR (reg)) -return PTR_ERR (reg); - } - -if (!IS_ERR (reg)) { - ret =regulator_enable (reg); -if (ret) + ret =devm_regulator_get_enable_read_voltage (&spi->dev,"vref" ); +if (ret <0 ) { +/* Vref is optional if a device has an internal reference */ +if (!st->chip_info->int_vref_uv || ret != -ENODEV )return ret; - - ret =devm_add_action_or_reset (&spi->dev, - ad7476_reg_disable, - reg); -if (ret) -return ret; - st->ref_reg = reg; }else { -/* -* Can only get here if device supports both internal -* and external reference, but the regulator connected -* to the external reference is not connected. -* Set the reference regulator pointer to NULL to -* indicate this. -*/ - st->ref_reg =NULL ; + st->scale_mv = ret /1000 ; } } +if (!st->scale_mv) + st->scale_mv = st->chip_info->int_vref_uv /1000 ; +if (st->chip_info->has_vdrive) { ret =devm_regulator_get_enable (&spi->dev,"vdrive" );if (ret)
Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 3/10 iio: adc: ad7476: Use mV for internal reference
In reply to Matti VaittinenThe ad7476 supports some ICs with an internal reference voltage. For those ICs the reference voltage has been hard-coded as micro volts, but the value which is later used in code needs to be milli volts. This results the need to divide hard coded voltage by 1000 before using it.
Simplify code by changing the hard-coded voltage to millivolts and by dropping the division.
---
Revision history:
v1 => :
- No changes
--- drivers/iio/adc/ad7476.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/iio/adc/ad7476.c +6 -6
@@ -27,7 +27,7 @@struct ad7476_state ;struct ad7476_chip_info { -unsigned int int_vref_uv; +unsigned int int_vref_mv;struct iio_chan_spec channel[2 ];/* channels used when convst gpio is defined */ struct iio_chan_spec convst_channel[2 ];
@@ -172,7 +172,7 @@ static const struct ad7476_chip_info ad7091r_chip_info = { .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1), .convst_channel[0] = AD7091R_CONVST_CHAN(12), .convst_channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1), - .int_vref_uv = 2500000, + .int_vref_mv = 2500, .has_vref = true, .reset = ad7091_reset, };
@@ -229,7 +229,7 @@ static const struct ad7476_chip_info ad7475_chip_info = {static const struct ad7476_chip_info ad7495_chip_info = { .channel[0 ] =AD7476_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .int_vref_uv =2500000 , + .int_vref_mv =2500 , .has_vdrive = true, };
@@ -295,7 +295,7 @@ static int ad7476_probe(struct spi_device *spi)return -ENODEV ;/* Use VCC for reference voltage if vref / internal vref aren't used */ -if (!st->chip_info->int_vref_uv && !st->chip_info->has_vref) { +if (!st->chip_info->int_vref_mv && !st->chip_info->has_vref) { ret =devm_regulator_get_enable_read_voltage (&spi->dev,"vcc" );if (ret <0 )return ret;
@@ -310,7 +310,7 @@ static int ad7476_probe(struct spi_device *spi) ret =devm_regulator_get_enable_read_voltage (&spi->dev,"vref" );if (ret <0 ) {/* Vref is optional if a device has an internal reference */ -if (!st->chip_info->int_vref_uv || ret != -ENODEV ) +if (!st->chip_info->int_vref_mv || ret != -ENODEV )return ret; }else { st->scale_mv = ret /1000 ;
@@ -318,7 +318,7 @@ static int ad7476_probe(struct spi_device *spi) }if (!st->scale_mv) - st->scale_mv = st->chip_info->int_vref_uv /1000 ; + st->scale_mv = st->chip_info->int_vref_mv;if (st->chip_info->has_vdrive) { ret =devm_regulator_get_enable (&spi->dev,"vdrive" );
Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 4/10 iio: adc: ad7476: Use correct channel for bit info
In reply to Matti VaittinenThe ad7476 supports ADCs which use separate GPIO for starting the conversion. For such devices, the driver uses different channel information if the GPIO is found. The bit information is still always used from the original (non 'convstart') channels.
This has not been causing problems because the bit information for the 'convstart' -channel and the 'normal' -channel is identical. It, however, will cause issues if an IC has different characteristics for an 'convstart' -channel and regular channel. Furthermore, this will cause problems if a device always requires the convstart GPIO and thus only defines the convstart channel.
---
Revision history:
v1 => :
- No changes
--- drivers/iio/adc/ad7476.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/iio/adc/ad7476.c +3 -3
@@ -121,8 +121,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,if (ret <0 )return ret; - *val = (ret >> st->chip_info->channel[0 ].scan_type.shift) & -GENMASK (st->chip_info->channel[0 ].scan_type.realbits -1 ,0 ); + *val = (ret >> chan->scan_type.shift) & +GENMASK (chan->scan_type.realbits -1 ,0 );return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : *val = st->scale_mv;
@@ -345,7 +345,7 @@ static int ad7476_probe(struct spi_device *spi)/* Setup default message */ st->xfer.rx_buf = &st->data; - st->xfer.len = st->chip_info->channel[0 ].scan_type.storagebits /8 ; + st->xfer.len = indio_dev->channels[0 ].scan_type.storagebits /8 ;spi_message_init (&st->msg);spi_message_add_tail (&st->xfer, &st->msg);
Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 5/10 iio: adc: ad7476: use *_cansleep GPIO APIs
In reply to Matti VaittinenThe ad7476 driver supports ADCs which require a GPIO pin to be used to start conversion. The GPIO line status seems to be always toggled from a process context, either from the user-initiated "raw-read conversion", or from a threaded IRQ handler. Furthermore, these ICs are connected via SPI bus, which is usually implemented in a way that the access to the device can sleep. The GPIO here is toggled from the same context which is reading the results over SPI.
Thus it seems very likely these GPIOs are toggled from a context which can sleep.
Swap the gpiod_set_value() to gpiod_set_value_cansleep() accordinlgy.
---
Revision history:
v2 => v3:
- New patch
--- drivers/iio/adc/ad7476.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/adc/ad7476.c +2 -2
@@ -57,9 +57,9 @@ static void ad7091_convst(struct ad7476_state *st)if (!st->convst_gpio)return ; -gpiod_set_value (st->convst_gpio,0 ); +gpiod_set_value_cansleep (st->convst_gpio,0 );udelay (1 );/* CONVST pulse width: 10 ns min */ -gpiod_set_value (st->convst_gpio,1 ); +gpiod_set_value_cansleep (st->convst_gpio,1 );udelay (1 );/* Conversion time: 650 ns max */ }
Signature · 1 line
2.50.1
Re: [PATCH v3 05/10] iio: adc: ad7476: use *_cansleep GPIO APIs
Quoted 13 lines · Matti Vaittinen
The ad7476 driver supports ADCs which require a GPIO pin to be used to start conversion. The GPIO line status seems to be always toggled from a process context, either from the user-initiated "raw-read conversion", or from a threaded IRQ handler. Furthermore, these ICs are connected via SPI bus, which is usually implemented in a way that the access to the device can sleep. The GPIO here is toggled from the same context which is reading the results over SPI.
Thus it seems very likely these GPIOs are toggled from a context which can sleep.
Swap the
gpiod_set_value()togpiod_set_value_cansleep()accordinlgy.
Quoted 8 lines
Suggested-by Nuno Sá <[email protected]>Signed-off-by Matti Vaittinen <[email protected]>---
Revision history:
v2 => v3:
- New patch
---
Quoted 22 lines
drivers/iio/adc/ad7476.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iio/adc/ad7476.c +2
-2@@ -57,9 +57,9 @@ static void ad7091_convst(struct ad7476_state *st)if (!st->convst_gpio)return ; -gpiod_set_value (st->convst_gpio,0 ); +gpiod_set_value_cansleep (st->convst_gpio,0 );udelay (1 );/* CONVST pulse width: 10 ns min */ -gpiod_set_value (st->convst_gpio,1 ); +gpiod_set_value_cansleep (st->convst_gpio,1 );udelay (1 );/* Conversion time: 650 ns max */ }Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 6/10 iio: adc: ad7476: Drop convstart chan_spec
In reply to Matti VaittinenThe ad7476 driver defines separate chan_spec structures for operation with and without convstart GPIO. At quick glance this may seem as if the driver did provide more than 1 data-channel to users - one for the regular data, other for the data obtained with the convstart GPIO.
The only difference between the 'convstart' and 'non convstart' -channels is presence / absence of the BIT(IIO_CHAN_INFO_RAW) in channel's flags.
We can drop the convstart channel spec, and related convstart macro, by allocating a mutable per driver instance channel spec and adding the flag in probe if needed. This will simplify the driver with the cost of added memory consumption.
Assuming there aren't systems with very many ADCs and very few resources, this tradeoff seems worth making.
Simplify the driver by dropping the 'convstart' channel spec and allocating the channel spec for each driver instance.
---
Revision history:
v2 => v3:
- Use __set_bit() instead of |=
- Swicth BUILD_BUG_ON to static_assert
- Clarify a comment
- Improve commit message
v1 => v2:
- New patch
--- drivers/iio/adc/ad7476.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
drivers/iio/adc/ad7476.c +19 -12
@@ -6,6 +6,7 @@* Copyright 2010 Analog Devices Inc. */ +#include <linux/bitops.h> #include <linux/device.h> #include <linux/kernel.h> #include <linux/slab.h>
@@ -29,8 +30,6 @@ struct ad7476_state;struct ad7476_chip_info {unsigned int int_vref_mv;struct iio_chan_spec channel[2 ]; -/* channels used when convst gpio is defined */ -struct iio_chan_spec convst_channel[2 ];void (*reset)(struct ad7476_state *);bool has_vref;bool has_vdrive;
@@ -42,6 +41,7 @@ struct ad7476_state {struct gpio_desc *convst_gpio;struct spi_transfer xfer;struct spi_message msg; +struct iio_chan_spec channel[2 ];int scale_mv;/* * DMA (thus cache coherency maintenance) may require the
@@ -154,24 +154,18 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,#define AD7940_CHAN (bits) _AD7476_CHAN((bits), 15 - (bits), \ BIT(IIO_CHAN_INFO_RAW))#define AD7091R_CHAN (bits) _AD7476_CHAN((bits), 16 - (bits), 0) -#define AD7091R_CONVST_CHAN (bits) _AD7476_CHAN((bits), 16 - (bits), \ - BIT(IIO_CHAN_INFO_RAW))#define ADS786X_CHAN (bits) _AD7476_CHAN((bits), 12 - (bits), \ BIT(IIO_CHAN_INFO_RAW))static const struct ad7476_chip_info ad7091_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), - .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), .reset = ad7091_reset, };static const struct ad7476_chip_info ad7091r_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), - .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), .int_vref_mv =2500 , .has_vref = true, .reset = ad7091_reset,
@@ -282,6 +276,7 @@ static int ad7476_probe(struct spi_device *spi) {struct ad7476_state *st;struct iio_dev *indio_dev; +unsigned int i;int ret; indio_dev =devm_iio_device_alloc (&spi->dev,sizeof (*st));
@@ -332,16 +327,28 @@ static int ad7476_probe(struct spi_device *spi)if (IS_ERR (st->convst_gpio))return PTR_ERR (st->convst_gpio); +/* +* This will never happen. Unless someone changes the channel specs +* in this driver. And if someone does, without changing the loop +* below, then we'd better immediately produce a big fat error, before +* the change proceeds from that developer's table. +*/ +static_assert (ARRAY_SIZE (st->channel) ==ARRAY_SIZE (st->chip_info->channel)); +for (i =0 ; i <ARRAY_SIZE (st->channel); i++) { + st->channel[i] = st->chip_info->channel[i]; +if (st->convst_gpio) +__set_bit (IIO_CHAN_INFO_RAW , + &st->channel[i].info_mask_separate); + } + st->spi = spi; indio_dev->name =spi_get_device_id (spi)->name; indio_dev->modes =INDIO_DIRECT_MODE ; - indio_dev->channels = st->chip_info->channel; - indio_dev->num_channels =2 ; + indio_dev->channels = st->channel; + indio_dev->num_channels =ARRAY_SIZE (st->channel); indio_dev->info = &ad7476_info; -if (st->convst_gpio) - indio_dev->channels = st->chip_info->convst_channel;/* Setup default message */ st->xfer.rx_buf = &st->data;
Signature · 1 line
2.50.1
Re: [PATCH v3 06/10] iio: adc: ad7476: Drop convstart chan_spec
Quoted 30 lines · Matti Vaittinen
The ad7476 driver defines separate
chan_specstructures for operation with and without convstart GPIO. At quick glance this may seem as if the driver did provide more than 1 data-channel to users - one for the regular data, other for the data obtained with the convstart GPIO.The only difference between the 'convstart' and 'non convstart' -channels is presence / absence of the
BIT(IIO_CHAN_INFO_RAW)in channel's flags.We can drop the convstart channel spec, and related convstart macro, by allocating a mutable per driver instance channel spec and adding the flag in probe if needed. This will simplify the driver with the cost of added memory consumption.
Assuming there aren't systems with very many ADCs and very few resources, this tradeoff seems worth making.
Simplify the driver by dropping the 'convstart' channel spec and allocating the channel spec for each driver instance.
Signed-off-by Matti Vaittinen <[email protected]>---
Revision history:
v2 => v3:
- Use__set_bit()instead of |=
- SwicthBUILD_BUG_ONtostatic_assert
- Clarify a comment
- Improve commit message
Quoted 3 lines
v1 => v2:
- New patch
---
Quoted 101 lines
drivers/iio/adc/ad7476.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)drivers/iio/adc/ad7476.c +19
-12@@ -6,6 +6,7 @@* Copyright 2010 Analog Devices Inc. */ +#include <linux/bitops.h> #include <linux/device.h> #include <linux/kernel.h> #include <linux/slab.h> @@ -29,8 +30,6 @@ struct ad7476_state;struct ad7476_chip_info {unsigned int int_vref_mv;struct iio_chan_spec channel[2 ]; -/* channels used when convst gpio is defined */ -struct iio_chan_spec convst_channel[2 ];void (*reset)(struct ad7476_state *);bool has_vref;bool has_vdrive;@@ -42,6 +41,7 @@ struct ad7476_state {struct gpio_desc *convst_gpio;struct spi_transfer xfer;struct spi_message msg; +struct iio_chan_spec channel[2 ];int scale_mv;/* * DMA (thus cache coherency maintenance) may require the @@ -154,24 +154,18 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,#define AD7940_CHAN (bits) _AD7476_CHAN((bits), 15 - (bits), \ BIT(IIO_CHAN_INFO_RAW))#define AD7091R_CHAN (bits) _AD7476_CHAN((bits), 16 - (bits), 0) -#define AD7091R_CONVST_CHAN (bits) _AD7476_CHAN((bits), 16 - (bits), \ - BIT(IIO_CHAN_INFO_RAW))#define ADS786X_CHAN (bits) _AD7476_CHAN((bits), 12 - (bits), \ BIT(IIO_CHAN_INFO_RAW))static const struct ad7476_chip_info ad7091_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), - .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), .reset = ad7091_reset, };static const struct ad7476_chip_info ad7091r_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), - .convst_channel[0 ] =AD7091R_CONVST_CHAN (12 ), - .convst_channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), .int_vref_mv =2500 , .has_vref = true, .reset = ad7091_reset,@@ -282,6 +276,7 @@ static int ad7476_probe(struct spi_device *spi) {struct ad7476_state *st;struct iio_dev *indio_dev; +unsigned int i;int ret; indio_dev =devm_iio_device_alloc (&spi->dev,sizeof (*st));@@ -332,16 +327,28 @@ static int ad7476_probe(struct spi_device *spi)if (IS_ERR (st->convst_gpio))return PTR_ERR (st->convst_gpio); +/* +* This will never happen. Unless someone changes the channel specs +* in this driver. And if someone does, without changing the loop +* below, then we'd better immediately produce a big fat error, before +* the change proceeds from that developer's table. +*/ +static_assert (ARRAY_SIZE (st->channel) ==ARRAY_SIZE (st->chip_info->channel)); +for (i =0 ; i <ARRAY_SIZE (st->channel); i++) { + st->channel[i] = st->chip_info->channel[i]; +if (st->convst_gpio) +__set_bit (IIO_CHAN_INFO_RAW , + &st->channel[i].info_mask_separate); + } + st->spi = spi; indio_dev->name =spi_get_device_id (spi)->name; indio_dev->modes =INDIO_DIRECT_MODE ; - indio_dev->channels = st->chip_info->channel; - indio_dev->num_channels =2 ; + indio_dev->channels = st->channel; + indio_dev->num_channels =ARRAY_SIZE (st->channel); indio_dev->info = &ad7476_info; -if (st->convst_gpio) - indio_dev->channels = st->chip_info->convst_channel;/* Setup default message */ st->xfer.rx_buf = &st->data;Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 7/10 iio: adc: ad7476: Conditionally call convstart
In reply to Matti VaittinenThe ad7476 supports two IC variants which may have a 'convstart' -GPIO for starting the conversion. Currently the driver calls a function which tries to access the GPIO for all of the IC variants, whether they support 'convstart' or not. This is not an error because this function returns early if GPIO information is not populated.
We can do a tad better by calling this function only for the ICs which have the 'convstart' by providing a function pointer to the convstart function from the chip_info structure, and calling this function only for the ICs which have the function pointer set.
This does also allow to support ICs which require different convstart handling than the currently supported ICs.
Call convstart function only on the ICs which can support it and allow IC-specific convstart functions for the ICs which require different handling.
---
Revision history:
v2 => v3:
- Use indirect call to convstart (via function pointer) also from the ad7476_scan_direct().
- Adapt to the change which returned the chip_info pointer back to the driver's state structure.
v1 => v2:
- Adapt to the change which removed the chip_info pointer from the driver's state structure.
The follow-up patch adding support for the ROHM BD79105 will bring different 'convstart' functions in use. The IC specific pointer will also prepare the way for this.
--- drivers/iio/adc/ad7476.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/iio/adc/ad7476.c +7 -2
@@ -31,6 +31,7 @@ struct ad7476_chip_info {unsigned int int_vref_mv;struct iio_chan_spec channel[2 ];void (*reset)(struct ad7476_state *); +void (*conversion_pre_op)(struct ad7476_state *st);bool has_vref;bool has_vdrive; };
@@ -70,7 +71,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void *p)struct ad7476_state *st =iio_priv (indio_dev);int b_sent; -ad7091_convst (st); +if (st->chip_info->conversion_pre_op) + st->chip_info->conversion_pre_op (st); b_sent =spi_sync (st->spi, &st->msg);if (b_sent <0 )
@@ -94,7 +96,8 @@ static int ad7476_scan_direct(struct ad7476_state *st) {int ret; -ad7091_convst (st); +if (st->chip_info->conversion_pre_op) + st->chip_info->conversion_pre_op (st); ret =spi_sync (st->spi, &st->msg);if (ret)
@@ -160,12 +163,14 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,static const struct ad7476_chip_info ad7091_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .conversion_pre_op = ad7091_convst, .reset = ad7091_reset, };static const struct ad7476_chip_info ad7091r_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .conversion_pre_op = ad7091_convst, .int_vref_mv =2500 , .has_vref = true, .reset = ad7091_reset,
Signature · 1 line
2.50.1
Re: [PATCH v3 07/10] iio: adc: ad7476: Conditionally call convstart
Quoted 27 lines · Matti Vaittinen
The ad7476 supports two IC variants which may have a 'convstart' -GPIO for starting the conversion. Currently the driver calls a function which tries to access the GPIO for all of the IC variants, whether they support 'convstart' or not. This is not an error because this function returns early if GPIO information is not populated.
We can do a tad better by calling this function only for the ICs which have the 'convstart' by providing a function pointer to the convstart function from the
chip_infostructure, and calling this function only for the ICs which have the function pointer set.This does also allow to support ICs which require different convstart handling than the currently supported ICs.
Call convstart function only on the ICs which can support it and allow IC-specific convstart functions for the ICs which require different handling.
Signed-off-by Matti Vaittinen <[email protected]>---
Revision history:
v2 => v3:
- Use indirect call to convstart (via function pointer) also from thead7476_scan_direct().
- Adapt to the change which returned thechip_infopointer back to the driver's state structure.
Quoted 8 lines
v1 => v2:
- Adapt to the change which removed thechip_infopointer from the driver's state structure.The follow-up patch adding support for the ROHM BD79105 will bring different 'convstart' functions in use. The IC specific pointer will also prepare the way for this.
---
Quoted 53 lines
drivers/iio/adc/ad7476.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)drivers/iio/adc/ad7476.c +7
-2@@ -31,6 +31,7 @@ struct ad7476_chip_info {unsigned int int_vref_mv;struct iio_chan_spec channel[2 ];void (*reset)(struct ad7476_state *); +void (*conversion_pre_op)(struct ad7476_state *st);bool has_vref;bool has_vdrive; };@@ -70,7 +71,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void *p)struct ad7476_state *st =iio_priv (indio_dev);int b_sent; -ad7091_convst (st); +if (st->chip_info->conversion_pre_op) + st->chip_info->conversion_pre_op (st); b_sent =spi_sync (st->spi, &st->msg);if (b_sent <0 )@@ -94,7 +96,8 @@ static int ad7476_scan_direct(struct ad7476_state *st) {int ret; -ad7091_convst (st); +if (st->chip_info->conversion_pre_op) + st->chip_info->conversion_pre_op (st); ret =spi_sync (st->spi, &st->msg);if (ret)@@ -160,12 +163,14 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,static const struct ad7476_chip_info ad7091_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .conversion_pre_op = ad7091_convst, .reset = ad7091_reset, };static const struct ad7476_chip_info ad7091r_chip_info = { .channel[0 ] =AD7091R_CHAN (12 ), .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), + .conversion_pre_op = ad7091_convst, .int_vref_mv =2500 , .has_vref = true, .reset = ad7091_reset,Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 8/10 dt-bindings: iio: adc: ad7476: Add ROHM bd79105
In reply to Matti VaittinenThe ROHM BD79105 is a simple, 16-bit, 1-channel ADC with a 'CONVSTART' pin used to start the ADC conversion. Other than the 'CONVSTART', there are 3 supply pins (one used as a reference), analog inputs, ground and communication pins. It's worth noting that the pin somewhat confusingly labeled as 'DIN', is a pin which should be used as a chip-select. The IC does not have any writable registers.
The device is designed so that the output pin can, in addition to outputting the data, be used as a 'data-ready'-IRQ. This, however, would require the IRQ to be masked from host side for the duration of the data reads - and it wouldn't also work when the SPI is shared. (As access to the other SPI devices would cause data line changes to be detected as IRQs - and the BD79105 provides no means to detect if it has generated an IRQ).
Hence the device-tree does not contain any IRQ properties.
Add a compatible for the bd79105.
---
Revision history:
v2 => :
- No changes
v1 => v2:
- BD79105 can provide data-ready IRQ (or GPIO) via DOUT-pin.
--- .../bindings/iio/adc/adi,ad7476.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+)
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +54 -0
@@ -41,6 +41,7 @@ properties: - adi,ad7910 - adi,ad7920 - adi,ad7940 + - rohm,bd79105 - ti,adc081s - ti,adc101s - ti,adc121s
@@ -55,6 +56,11 @@ properties: reg: maxItems: 1 + interrupts: + description: + The data-ready interrupt. Provided via DOUT pin. + maxItems: 1 + vcc-supply: description: Main powersupply voltage for the chips, sometimes referred to as VDD on
@@ -75,6 +81,10 @@ properties: description: A GPIO used to trigger the start of a conversion maxItems: 1 + rdy-gpios: + description: A GPIO for detecting the data-ready. + maxItems: 1 + required: - compatible - reg
@@ -82,6 +92,20 @@ required: allOf: - $ref: /schemas/spi/spi-peripheral-props.yaml# +# Devices with an IRQ + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + properties: + interrupts: true + else: + properties: + interrupts: false + # Devices where reference is vcc - if: properties:
@@ -115,6 +139,7 @@ allOf: - adi,ad7274 - adi,ad7475 - lltc,ltc2314-14 + - rohm,bd79105 then: properties: vref-supply: true
@@ -131,6 +156,7 @@ allOf: - adi,ad7274 - adi,ad7475 - lltc,ltc2314-14 + - rohm,bd79105 then: required: - vref-supply
@@ -141,12 +167,28 @@ allOf: enum: - adi,ad7475 - adi,ad7495 + - rohm,bd79105 then: properties: vdrive-supply: true else: properties: vdrive-supply: false + + # Devices which support polling the data-ready via GPIO + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + properties: + rdy-gpios: true + else: + properties: + rdy-gpios: false + - if: properties: compatible:
@@ -154,6 +196,7 @@ allOf: enum: - adi,ad7091 - adi,ad7091r + - rohm,bd79105 then: properties: adi,conversion-start-gpios: true
@@ -161,6 +204,17 @@ allOf: properties: adi,conversion-start-gpios: false + # Devices with a convstart GPIO where it is not optional + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + required: + - adi,conversion-start-gpios + unevaluatedProperties: false examples:
Signature · 1 line
2.50.1
Jonathan Cameron
maintainerRe: [PATCH v3 08/10] dt-bindings: iio: adc: ad7476: Add ROHM bd79105
Quoted 15 lines · On Fri, 8 Aug 2025 11:54:25 +0300 Matti Vaittinen <[email protected]>…
The ROHM BD79105 is a simple, 16-bit, 1-channel ADC with a 'CONVSTART' pin used to start the ADC conversion. Other than the 'CONVSTART', there are 3 supply pins (one used as a reference), analog inputs, ground and communication pins. It's worth noting that the pin somewhat confusingly labeled as 'DIN', is a pin which should be used as a chip-select. The IC does not have any writable registers.
The device is designed so that the output pin can, in addition to outputting the data, be used as a 'data-ready'-IRQ. This, however, would require the IRQ to be masked from host side for the duration of the data reads - and it wouldn't also work when the SPI is shared. (As access to the other SPI devices would cause data line changes to be detected as IRQs - and the BD79105 provides no means to detect if it has generated an IRQ).
Quoted 1 line
Hence the device-tree does not contain any IRQ properties.
Commit message needs an update as it now allows for IRQ properties.
Quoted 55 lines
Add a compatible for the bd79105.
Signed-off-by Matti Vaittinen <[email protected]>---
Revision history:
v2 => :
- No changesv1 => v2:
- BD79105 can provide data-ready IRQ (or GPIO) via DOUT-pin.--- .../bindings/iio/adc/adi,ad7476.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+)Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +10
-0@@ -41,6 +41,7 @@ properties: - adi,ad7910 - adi,ad7920 - adi,ad7940 + - rohm,bd79105 - ti,adc081s - ti,adc101s - ti,adc121s@@ -55,6 +56,11 @@ properties: reg: maxItems: 1 + interrupts: + description: + The data-ready interrupt. Provided via DOUT pin. + maxItems: 1 + vcc-supply: description: Main powersupply voltage for the chips, sometimes referred to as VDD on@@ -75,6 +81,10 @@ properties: description: A GPIO used to trigger the start of a conversion maxItems: 1 + rdy-gpios: + description: A GPIO for detecting the data-ready. + maxItems: 1 + required: - compatible - reg@@ -82,6 +92,20 @@ required: allOf: - $ref: /schemas/spi/spi-peripheral-props.yaml#
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml required: · line 104
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +10
-0+# Devices with an IRQ + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + properties: + interrupts: true
It's been a while since I wrote one of these, but do we need the true bit given we have already specified the interrupts property. I think we only need to do the setting to false on an if: not:
Quoted 35 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +8
-0+ else: + properties: + interrupts: false + # Devices where reference is vcc - if: properties:@@ -115,6 +139,7 @@ allOf: - adi,ad7274 - adi,ad7475 - lltc,ltc2314-14 + - rohm,bd79105 then: properties: vref-supply: true@@ -131,6 +156,7 @@ allOf: - adi,ad7274 - adi,ad7475 - lltc,ltc2314-14 + - rohm,bd79105 then: required: - vref-supply@@ -141,12 +167,28 @@ allOf: enum: - adi,ad7475 - adi,ad7495 + - rohm,bd79105 then: properties: vdrive-supply: true else: properties: vdrive-supply: false +
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml allOf: · line 187
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +10
-0+ # Devices which support polling the data-ready via GPIO + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + properties: + rdy-gpios: true
As above, I believe we only need the not case.
Quoted 33 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +16
-0+ else: + properties: + rdy-gpios: false + - if: properties: compatible:@@ -154,6 +196,7 @@ allOf: enum: - adi,ad7091 - adi,ad7091r + - rohm,bd79105 then: properties: adi,conversion-start-gpios: true@@ -161,6 +204,17 @@ allOf: properties: adi,conversion-start-gpios: false + # Devices with a convstart GPIO where it is not optional + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + required: + - adi,conversion-start-gpios + unevaluatedProperties: false examples:
Matti Vaittinen
maintainerRe: [PATCH v3 08/10] dt-bindings: iio: adc: ad7476: Add ROHM bd79105
Quoted 18 lines · Jonathan Cameron
Quoted 15 lines · On Fri, 8 Aug 2025 11:54:25 +0300 Matti Vaittinen <[email protected]>…
The ROHM BD79105 is a simple, 16-bit, 1-channel ADC with a 'CONVSTART' pin used to start the ADC conversion. Other than the 'CONVSTART', there are 3 supply pins (one used as a reference), analog inputs, ground and communication pins. It's worth noting that the pin somewhat confusingly labeled as 'DIN', is a pin which should be used as a chip-select. The IC does not have any writable registers.
The device is designed so that the output pin can, in addition to outputting the data, be used as a 'data-ready'-IRQ. This, however, would require the IRQ to be masked from host side for the duration of the data reads - and it wouldn't also work when the SPI is shared. (As access to the other SPI devices would cause data line changes to be detected as IRQs - and the BD79105 provides no means to detect if it has generated an IRQ).
Quoted 3 lines
Quoted 1 line
Hence the device-tree does not contain any IRQ properties.
Commit message needs an update as it now allows for IRQ properties.
Ah, true. Thanks :)
Quoted 56 lines
Quoted 56 lines
Add a compatible for the bd79105.
Signed-off-by Matti Vaittinen <[email protected]>---
Revision history:
v2 => :
- No changesv1 => v2:
- BD79105 can provide data-ready IRQ (or GPIO) via DOUT-pin.--- .../bindings/iio/adc/adi,ad7476.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+)Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +10
-0@@ -41,6 +41,7 @@ properties: - adi,ad7910 - adi,ad7920 - adi,ad7940 + - rohm,bd79105 - ti,adc081s - ti,adc101s - ti,adc121s@@ -55,6 +56,11 @@ properties: reg: maxItems: 1 + interrupts: + description: + The data-ready interrupt. Provided via DOUT pin. + maxItems: 1 + vcc-supply: description: Main powersupply voltage for the chips, sometimes referred to as VDD on@@ -75,6 +81,10 @@ properties: description: A GPIO used to trigger the start of a conversion maxItems: 1 + rdy-gpios: + description: A GPIO for detecting the data-ready. + maxItems: 1 + required: - compatible - reg@@ -82,6 +92,20 @@ required: allOf: - $ref: /schemas/spi/spi-peripheral-props.yaml#
Quoted 15 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml required: · line 104
Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/adi,ad7476.yaml +10
-0+# Devices with an IRQ + - if: + properties: + compatible: + contains: + enum: + - rohm,bd79105 + then: + properties: + interrupts: trueIt's been a while since I wrote one of these, but do we need the true bit given we have already specified the interrupts property. I think we only need to do the setting to false on an if: not:
I think you're right. I just used same approach that had already been used throughout this binding. I'll try dropping the 'true'-branches, but then also the other 'true'-branches should be dropped from the binding.
I'll just do that and send v4. Let's see what Rob/Krzk/Conor have to say :)
Yours,
-- Matti
Matti Vaittinen
maintainerPATCH v3 9/10 iio: adc: ad7476: Support ROHM BD79105
In reply to Matti VaittinenThe ROHM BD79105 is a simple 16-bit ADC accessible via SPI*.
The BD79105 has a CONVSTART pin, which must be set high to start the ADC conversion. Unlike with the ad7091 and ad7091r which also have a CONVSTART pin, the BD79105 requires that the pin must remain high also for the duration of the SPI access.
(*) Couple of words about the SPI. The BD79105 has pins named as CONVSTART, SCLK, DIN and DOUT. For the curious reader, DIN is not SPI MISO.
DIN is a signal which can be used as a chip-select. When DIN is pulled low, the ADC will output the completed measurement via DOUT as SCLK is clocked. According to the data-sheet, the DIN can also be used for daisy-chaining multiple ADCs. Furthermore, DOUT can be used also for a 'data-ready' -IRQ. These modes aren't supported by this driver.
Support reading ADC scale and data from the BD79105 using SPI, when DIN is used as a chip-select.
---
Revision history:
v2 => v3:
- Check for missing convstart GPIO at probe.
- Use indirect call to convstart (via function pointer) also from the ad7476_scan_direct().
v1 => v2:
- Fix the conversion delay for the BD79105
- Drop unnecessary GPIO check from the convstart disable
- Drop unintended whitespace change
- Fix spelling
--- drivers/iio/adc/ad7476.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
drivers/iio/adc/ad7476.c +39 -0
@@ -32,8 +32,10 @@ struct ad7476_chip_info {struct iio_chan_spec channel[2 ];void (*reset)(struct ad7476_state *);void (*conversion_pre_op)(struct ad7476_state *st); +void (*conversion_post_op)(struct ad7476_state *st);bool has_vref;bool has_vdrive; +bool convstart_required; };struct ad7476_state {
@@ -64,6 +66,18 @@ static void ad7091_convst(struct ad7476_state *st)udelay (1 );/* Conversion time: 650 ns max */ } +static void bd79105_convst_disable (struct ad7476_state *st) +{ +gpiod_set_value_cansleep (st->convst_gpio,0 ); +} + +static void bd79105_convst_enable (struct ad7476_state *st) +{ +gpiod_set_value_cansleep (st->convst_gpio,1 ); +/* Worst case, 2790 ns required for conversion */ +ndelay (2790 ); +} +static irqreturn_t ad7476_trigger_handler (int irq,void *p) {struct iio_poll_func *pf = p;
@@ -81,6 +95,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void *p)iio_push_to_buffers_with_ts (indio_dev, st->data,sizeof (st->data),iio_get_time_ns (indio_dev)); done: +if (st->chip_info->conversion_post_op) + st->chip_info->conversion_post_op (st);iio_trigger_notify_done (indio_dev->trig);return IRQ_HANDLED ;
@@ -103,6 +119,9 @@ static int ad7476_scan_direct(struct ad7476_state *st)if (ret)return ret; +if (st->chip_info->conversion_post_op) + st->chip_info->conversion_post_op (st); +return be16_to_cpup ((__be16 *)st->data); }
@@ -273,6 +292,22 @@ static const struct ad7476_chip_info ltc2314_14_chip_info = { .has_vref = true, }; +static const struct ad7476_chip_info bd79105_chip_info = { + .channel[0 ] =AD7091R_CHAN (16 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +/* +* The BD79105 starts ADC data conversion when the CONVSTART line is +* set HIGH. The CONVSTART must be kept HIGH until the data has been +* read from the ADC. +*/ + .conversion_pre_op = bd79105_convst_enable, + .conversion_post_op = bd79105_convst_disable, +/* BD79105 won't do conversion without convstart */ + .convstart_required = true, + .has_vref = true, + .has_vdrive = true, +}; +static const struct iio_info ad7476_info = { .read_raw = &ad7476_read_raw, };
@@ -332,6 +367,9 @@ static int ad7476_probe(struct spi_device *spi)if (IS_ERR (st->convst_gpio))return PTR_ERR (st->convst_gpio); +if (st->chip_info->convstart_required && !st->convst_gpio) +return dev_err_probe (&spi->dev, -EINVAL ,"No convstart GPIO\n" ); +/* * This will never happen. Unless someone changes the channel specs * in this driver. And if someone does, without changing the loop
@@ -401,6 +439,7 @@ static const struct spi_device_id ad7476_id[] = { { "ads7866", (kernel_ulong_t)&ads7866_chip_info }, { "ads7867", (kernel_ulong_t)&ads7867_chip_info }, { "ads7868", (kernel_ulong_t)&ads7868_chip_info }, + { "bd79105", (kernel_ulong_t)&bd79105_chip_info },/* * The ROHM BU79100G is identical to the TI's ADS7866 from the software * point of view. The binding document mandates the ADS7866 to be
Signature · 1 line
2.50.1
Re: [PATCH v3 09/10] iio: adc: ad7476: Support ROHM BD79105
Quoted 17 lines · Matti Vaittinen
The ROHM BD79105 is a simple 16-bit ADC accessible via SPI*.
The BD79105 has a CONVSTART pin, which must be set high to start the ADC conversion. Unlike with the ad7091 and ad7091r which also have a CONVSTART pin, the BD79105 requires that the pin must remain high also for the duration of the SPI access.
(*) Couple of words about the SPI. The BD79105 has pins named as CONVSTART, SCLK, DIN and DOUT. For the curious reader, DIN is not SPI MISO.
DIN is a signal which can be used as a chip-select. When DIN is pulled low, the ADC will output the completed measurement via DOUT as SCLK is clocked. According to the data-sheet, the DIN can also be used for daisy-chaining multiple ADCs. Furthermore, DOUT can be used also for a 'data-ready' -IRQ. These modes aren't supported by this driver.
Quoted 5 lines
Support reading ADC scale and data from the BD79105 using SPI, when DIN is used as a chip-select.
Signed-off-by Matti Vaittinen <[email protected]>---
Quoted 112 lines
Revision history:
v2 => v3:
- Check for missing convstart GPIO at probe.
- Use indirect call to convstart (via function pointer) also from thead7476_scan_direct().v1 => v2:
- Fix the conversion delay for the BD79105
- Drop unnecessary GPIO check from the convstart disable
- Drop unintended whitespace change
- Fix spelling--- drivers/iio/adc/ad7476.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)drivers/iio/adc/ad7476.c +39
-0@@ -32,8 +32,10 @@ struct ad7476_chip_info {struct iio_chan_spec channel[2 ];void (*reset)(struct ad7476_state *);void (*conversion_pre_op)(struct ad7476_state *st); +void (*conversion_post_op)(struct ad7476_state *st);bool has_vref;bool has_vdrive; +bool convstart_required; };struct ad7476_state {@@ -64,6 +66,18 @@ static void ad7091_convst(struct ad7476_state *st)udelay (1 );/* Conversion time: 650 ns max */ } +static void bd79105_convst_disable (struct ad7476_state *st) +{ +gpiod_set_value_cansleep (st->convst_gpio,0 ); +} + +static void bd79105_convst_enable (struct ad7476_state *st) +{ +gpiod_set_value_cansleep (st->convst_gpio,1 ); +/* Worst case, 2790 ns required for conversion */ +ndelay (2790 ); +} +static irqreturn_t ad7476_trigger_handler (int irq,void *p) {struct iio_poll_func *pf = p;@@ -81,6 +95,8 @@ static irqreturn_t ad7476_trigger_handler(int irq, void *p)iio_push_to_buffers_with_ts (indio_dev, st->data,sizeof (st->data),iio_get_time_ns (indio_dev)); done: +if (st->chip_info->conversion_post_op) + st->chip_info->conversion_post_op (st);iio_trigger_notify_done (indio_dev->trig);return IRQ_HANDLED ;@@ -103,6 +119,9 @@ static int ad7476_scan_direct(struct ad7476_state *st)if (ret)return ret; +if (st->chip_info->conversion_post_op) + st->chip_info->conversion_post_op (st); +return be16_to_cpup ((__be16 *)st->data); }@@ -273,6 +292,22 @@ static const struct ad7476_chip_info ltc2314_14_chip_info = { .has_vref = true, }; +static const struct ad7476_chip_info bd79105_chip_info = { + .channel[0 ] =AD7091R_CHAN (16 ), + .channel[1 ] =IIO_CHAN_SOFT_TIMESTAMP (1 ), +/* +* The BD79105 starts ADC data conversion when the CONVSTART line is +* set HIGH. The CONVSTART must be kept HIGH until the data has been +* read from the ADC. +*/ + .conversion_pre_op = bd79105_convst_enable, + .conversion_post_op = bd79105_convst_disable, +/* BD79105 won't do conversion without convstart */ + .convstart_required = true, + .has_vref = true, + .has_vdrive = true, +}; +static const struct iio_info ad7476_info = { .read_raw = &ad7476_read_raw, };@@ -332,6 +367,9 @@ static int ad7476_probe(struct spi_device *spi)if (IS_ERR (st->convst_gpio))return PTR_ERR (st->convst_gpio); +if (st->chip_info->convstart_required && !st->convst_gpio) +return dev_err_probe (&spi->dev, -EINVAL ,"No convstart GPIO\n" ); +/* * This will never happen. Unless someone changes the channel specs * in this driver. And if someone does, without changing the loop @@ -401,6 +439,7 @@ static const struct spi_device_id ad7476_id[] = { { "ads7866", (kernel_ulong_t)&ads7866_chip_info }, { "ads7867", (kernel_ulong_t)&ads7867_chip_info }, { "ads7868", (kernel_ulong_t)&ads7868_chip_info }, + { "bd79105", (kernel_ulong_t)&bd79105_chip_info },/* * The ROHM BU79100G is identical to the TI's ADS7866 from the software * point of view. The binding document mandates the ADS7866 to be Signature · 1 line
2.50.1
Matti Vaittinen
maintainerPATCH v3 10/10 MAINTAINERS: A driver for simple 1-channel SPI ADCs
In reply to Matti VaittinenAdd undersigned as a maintainer for the ad7476.c which supports a few simple 1-channel ADC connected to SPI.
---
Revision history:
v1 => :
- No changes.
I'll try to keep this on eye.
I only have access to the ROHM BD79105 and BU79100g. I would welcome anyone with access to other supported ADCs (and time, energy and the knowledge) to join me. :)
--- MAINTAINERS | 5 +++++ 1 file changed, 5 insertions(+)
MAINTAINERS +5 -0
@@ -455,6 +455,11 @@ F: Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml F: Documentation/iio/ad7380.rst F: drivers/iio/adc/ad7380.c +AD7476 ADC DRIVER FOR VARIOUS SIMPLE 1-CHANNEL SPI ADCs +M: Matti Vaittinen <[email protected]> +S: Maintained +F: drivers/iio/adc/ad7476.c + AD7877 TOUCHSCREEN DRIVER M: Michael Hennerich <[email protected]> S: Supported
Signature · 1 line
2.50.1
Quoted 6 lines · Matti Vaittinen
Add support for the ROHM BD79105 ADC
(and do some minor simplifications to the ad7476 driver while at it).The first 2 patches were originally sent as an RFC:
https://lore.kernel.org/all/[email protected]/
This version LGTM
with the exception of patches for DT and MAINTAINERS, I haven't even looked at them.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerQuoted 6 lines · On Fri, 8 Aug 2025 14:42:47 +0200 Andy Shevchenko <[email protected]>…
Quoted 4 lines · Matti Vaittinen
Add support for the ROHM BD79105 ADC
(and do some minor simplifications to the ad7476 driver while at it).
Quoted 8 lines
Quoted 2 lines
The first 2 patches were originally sent as an RFC:
https://lore.kernel.org/all/[email protected]/This version LGTM
Reviewed-by Andy Shevchenko <[email protected]>with the exception of patches for DT and
MAINTAINERS, I haven't even looked at them.
I took another quick look and just a few comments on the DT patch which also need a DT maintainer review.
Jonathan