* [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros
@ 2020-03-21 9:07 Alexandru Ardelean
2020-03-21 9:07 ` [PATCH v2 2/5] iio: adc: ad7791: " Alexandru Ardelean
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Alexandru Ardelean @ 2020-03-21 9:07 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, lars, Alexandru Ardelean
This change gets rid of the AD_SD_*_CHANNEL macros in favor of defining
it's own. The ad7780 is quite simpler than it's other Sigma-Delta brothers.
It turned out that centralizing the AD_SD_*_CHANNEL macros doesn't scale
too well, especially with some more complicated drivers. Some of the
variations in the more complicated drivers require new macros, and that way
things can become harder to maintain.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
Changelog v1 -> v2:
* re-send + change author @analog.com; GMail messed it up
drivers/iio/adc/ad7780.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c
index 291c1a898129..f47606ebbbbe 100644
--- a/drivers/iio/adc/ad7780.c
+++ b/drivers/iio/adc/ad7780.c
@@ -206,10 +206,29 @@ static const struct ad_sigma_delta_info ad7780_sigma_delta_info = {
.irq_flags = IRQF_TRIGGER_LOW,
};
-#define AD7780_CHANNEL(bits, wordsize) \
- AD_SD_CHANNEL(1, 0, 0, bits, 32, (wordsize) - (bits))
-#define AD7170_CHANNEL(bits, wordsize) \
- AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, (wordsize) - (bits))
+#define _AD7780_CHANNEL(_bits, _wordsize, _mask_all) \
+{ \
+ .type = IIO_VOLTAGE, \
+ .indexed = 1, \
+ .channel = 0, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = _mask_all, \
+ .scan_index = 1, \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (_bits), \
+ .storagebits = 32, \
+ .shift = (_wordsize) - (_bits), \
+ .endianness = IIO_BE, \
+ }, \
+}
+
+#define AD7780_CHANNEL(_bits, _wordsize) \
+ _AD7780_CHANNEL(_bits, _wordsize, BIT(IIO_CHAN_INFO_SAMP_FREQ))
+#define AD7170_CHANNEL(_bits, _wordsize) \
+ _AD7780_CHANNEL(_bits, _wordsize, 0)
static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
[ID_AD7170] = {
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/5] iio: adc: ad7791: define/use own IIO channel macros
2020-03-21 9:07 [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Alexandru Ardelean
@ 2020-03-21 9:07 ` Alexandru Ardelean
2020-03-21 16:51 ` Jonathan Cameron
2020-03-21 9:08 ` [PATCH v2 3/5] iio: adc: ad7793: " Alexandru Ardelean
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Alexandru Ardelean @ 2020-03-21 9:07 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, lars, Alexandru Ardelean
This driver seems to use most of the AD_SD_*_CHANNEL. This change will move
them in the driver. The intent is that if a new part comes along which
would require tweaks per IIO channel, these should be doable in the driver.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/ad7791.c | 62 +++++++++++++++++++++++++++++++++++-----
1 file changed, 55 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c
index abb239392631..ba22808507d0 100644
--- a/drivers/iio/adc/ad7791.c
+++ b/drivers/iio/adc/ad7791.c
@@ -64,25 +64,73 @@
#define AD7791_MODE_SEL_MASK (0x3 << 6)
#define AD7791_MODE_SEL(x) ((x) << 6)
+#define __AD7991_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift, _extend_name, _type, _mask_all) \
+ { \
+ .type = (_type), \
+ .differential = (_channel2 == -1 ? 0 : 1), \
+ .indexed = 1, \
+ .channel = (_channel1), \
+ .channel2 = (_channel2), \
+ .address = (_address), \
+ .extend_name = (_extend_name), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = _mask_all, \
+ .scan_index = (_si), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (_bits), \
+ .storagebits = (_storagebits), \
+ .shift = (_shift), \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define AD7991_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7991_CHANNEL(_si, _channel, _channel, _address, _bits, \
+ _storagebits, _shift, "shorted", IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7991_CHANNEL(_si, _channel, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7991_CHANNEL(_si, _channel, -1, _address, _bits, \
+ _storagebits, _shift, NULL, IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7991_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7991_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift, NULL, IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7991_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
+ _shift) \
+ __AD7991_CHANNEL(_si, _channel, -1, _address, _bits, \
+ _storagebits, _shift, "supply", IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
#define DECLARE_AD7787_CHANNELS(name, bits, storagebits) \
const struct iio_chan_spec name[] = { \
- AD_SD_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
+ AD7991_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
(bits), (storagebits), 0), \
- AD_SD_CHANNEL(1, 1, AD7791_CH_AIN2, (bits), (storagebits), 0), \
- AD_SD_SHORTED_CHANNEL(2, 0, AD7791_CH_AIN1N_AIN1N, \
+ AD7991_CHANNEL(1, 1, AD7791_CH_AIN2, (bits), (storagebits), 0), \
+ AD7991_SHORTED_CHANNEL(2, 0, AD7791_CH_AIN1N_AIN1N, \
(bits), (storagebits), 0), \
- AD_SD_SUPPLY_CHANNEL(3, 2, AD7791_CH_AVDD_MONITOR, \
+ AD7991_SUPPLY_CHANNEL(3, 2, AD7791_CH_AVDD_MONITOR, \
(bits), (storagebits), 0), \
IIO_CHAN_SOFT_TIMESTAMP(4), \
}
#define DECLARE_AD7791_CHANNELS(name, bits, storagebits) \
const struct iio_chan_spec name[] = { \
- AD_SD_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
+ AD7991_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
(bits), (storagebits), 0), \
- AD_SD_SHORTED_CHANNEL(1, 0, AD7791_CH_AIN1N_AIN1N, \
+ AD7991_SHORTED_CHANNEL(1, 0, AD7791_CH_AIN1N_AIN1N, \
(bits), (storagebits), 0), \
- AD_SD_SUPPLY_CHANNEL(2, 1, AD7791_CH_AVDD_MONITOR, \
+ AD7991_SUPPLY_CHANNEL(2, 1, AD7791_CH_AVDD_MONITOR, \
(bits), (storagebits), 0), \
IIO_CHAN_SOFT_TIMESTAMP(3), \
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/5] iio: adc: ad7793: define/use own IIO channel macros
2020-03-21 9:07 [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Alexandru Ardelean
2020-03-21 9:07 ` [PATCH v2 2/5] iio: adc: ad7791: " Alexandru Ardelean
@ 2020-03-21 9:08 ` Alexandru Ardelean
2020-03-21 16:52 ` Jonathan Cameron
2020-03-21 9:08 ` [PATCH v2 4/5] iio: ad_sigma_delta: remove unused " Alexandru Ardelean
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Alexandru Ardelean @ 2020-03-21 9:08 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, lars, Alexandru Ardelean
This driver seems to use most of the AD_SD_*_CHANNEL. This change will move
them in the driver. The intent is that if a new part comes along which
would require tweaks per IIO channel, these should be doable in the driver.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/ad7793.c | 95 ++++++++++++++++++++++++++++++----------
1 file changed, 71 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
index b747db97f78a..5592ae573e6b 100644
--- a/drivers/iio/adc/ad7793.c
+++ b/drivers/iio/adc/ad7793.c
@@ -546,47 +546,94 @@ static const struct iio_info ad7797_info = {
.validate_trigger = ad_sd_validate_trigger,
};
+#define __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift, _extend_name, _type, _mask_all) \
+ { \
+ .type = (_type), \
+ .differential = (_channel2 == -1 ? 0 : 1), \
+ .indexed = 1, \
+ .channel = (_channel1), \
+ .channel2 = (_channel2), \
+ .address = (_address), \
+ .extend_name = (_extend_name), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = _mask_all, \
+ .scan_index = (_si), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (_bits), \
+ .storagebits = (_storagebits), \
+ .shift = (_shift), \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define AD7793_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift, NULL, IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7793_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
+ _storagebits, _shift, "shorted", IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7793_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
+ __AD7793_CHANNEL(_si, 0, -1, _address, _bits, \
+ _storagebits, _shift, NULL, IIO_TEMP, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7793_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
+ _shift) \
+ __AD7793_CHANNEL(_si, _channel, -1, _address, _bits, \
+ _storagebits, _shift, "supply", IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
#define DECLARE_AD7793_CHANNELS(_name, _b, _sb, _s) \
const struct iio_chan_spec _name##_channels[] = { \
- AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), (_s)), \
- AD_SD_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), (_s)), \
- AD_SD_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), (_s)), \
- AD_SD_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), (_s)), \
- AD_SD_TEMP_CHANNEL(4, AD7793_CH_TEMP, (_b), (_sb), (_s)), \
- AD_SD_SUPPLY_CHANNEL(5, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), (_s)), \
+ AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), (_s)), \
+ AD7793_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), (_s)), \
+ AD7793_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), (_s)), \
+ AD7793_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), (_s)), \
+ AD7793_TEMP_CHANNEL(4, AD7793_CH_TEMP, (_b), (_sb), (_s)), \
+ AD7793_SUPPLY_CHANNEL(5, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), (_s)), \
IIO_CHAN_SOFT_TIMESTAMP(6), \
}
#define DECLARE_AD7795_CHANNELS(_name, _b, _sb) \
const struct iio_chan_spec _name##_channels[] = { \
- AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(3, 3, 3, AD7795_CH_AIN4P_AIN4M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(4, 4, 4, AD7795_CH_AIN5P_AIN5M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(5, 5, 5, AD7795_CH_AIN6P_AIN6M, (_b), (_sb), 0), \
- AD_SD_SHORTED_CHANNEL(6, 0, AD7795_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
- AD_SD_TEMP_CHANNEL(7, AD7793_CH_TEMP, (_b), (_sb), 0), \
- AD_SD_SUPPLY_CHANNEL(8, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(3, 3, 3, AD7795_CH_AIN4P_AIN4M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(4, 4, 4, AD7795_CH_AIN5P_AIN5M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(5, 5, 5, AD7795_CH_AIN6P_AIN6M, (_b), (_sb), 0), \
+ AD7793_SHORTED_CHANNEL(6, 0, AD7795_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
+ AD7793_TEMP_CHANNEL(7, AD7793_CH_TEMP, (_b), (_sb), 0), \
+ AD7793_SUPPLY_CHANNEL(8, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
IIO_CHAN_SOFT_TIMESTAMP(9), \
}
#define DECLARE_AD7797_CHANNELS(_name, _b, _sb) \
const struct iio_chan_spec _name##_channels[] = { \
- AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
- AD_SD_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
- AD_SD_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
- AD_SD_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
+ AD7793_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
+ AD7793_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
+ AD7793_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
IIO_CHAN_SOFT_TIMESTAMP(4), \
}
#define DECLARE_AD7799_CHANNELS(_name, _b, _sb) \
const struct iio_chan_spec _name##_channels[] = { \
- AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
- AD_SD_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
- AD_SD_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
- AD_SD_SUPPLY_CHANNEL(4, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
+ AD7793_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
+ AD7793_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
+ AD7793_SUPPLY_CHANNEL(4, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
IIO_CHAN_SOFT_TIMESTAMP(5), \
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/5] iio: ad_sigma_delta: remove unused IIO channel macros
2020-03-21 9:07 [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Alexandru Ardelean
2020-03-21 9:07 ` [PATCH v2 2/5] iio: adc: ad7791: " Alexandru Ardelean
2020-03-21 9:08 ` [PATCH v2 3/5] iio: adc: ad7793: " Alexandru Ardelean
@ 2020-03-21 9:08 ` Alexandru Ardelean
2020-03-21 16:53 ` Jonathan Cameron
2020-03-21 9:08 ` [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available Alexandru Ardelean
2020-03-21 16:50 ` [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Jonathan Cameron
4 siblings, 1 reply; 14+ messages in thread
From: Alexandru Ardelean @ 2020-03-21 9:08 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, lars, Alexandru Ardelean
Now that all channel SigmaDelta IIO channel macros have been localized,
remove the generic ones.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/linux/iio/adc/ad_sigma_delta.h | 58 --------------------------
1 file changed, 58 deletions(-)
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 5a127c0ed200..a3a838dcf8e4 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -133,62 +133,4 @@ void ad_sd_cleanup_buffer_and_trigger(struct iio_dev *indio_dev);
int ad_sd_validate_trigger(struct iio_dev *indio_dev, struct iio_trigger *trig);
-#define __AD_SD_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
- _storagebits, _shift, _extend_name, _type, _mask_all) \
- { \
- .type = (_type), \
- .differential = (_channel2 == -1 ? 0 : 1), \
- .indexed = 1, \
- .channel = (_channel1), \
- .channel2 = (_channel2), \
- .address = (_address), \
- .extend_name = (_extend_name), \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
- BIT(IIO_CHAN_INFO_OFFSET), \
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
- .info_mask_shared_by_all = _mask_all, \
- .scan_index = (_si), \
- .scan_type = { \
- .sign = 'u', \
- .realbits = (_bits), \
- .storagebits = (_storagebits), \
- .shift = (_shift), \
- .endianness = IIO_BE, \
- }, \
- }
-
-#define AD_SD_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
- _storagebits, _shift) \
- __AD_SD_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
- _storagebits, _shift, NULL, IIO_VOLTAGE, \
- BIT(IIO_CHAN_INFO_SAMP_FREQ))
-
-#define AD_SD_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
- _storagebits, _shift) \
- __AD_SD_CHANNEL(_si, _channel, _channel, _address, _bits, \
- _storagebits, _shift, "shorted", IIO_VOLTAGE, \
- BIT(IIO_CHAN_INFO_SAMP_FREQ))
-
-#define AD_SD_CHANNEL(_si, _channel, _address, _bits, \
- _storagebits, _shift) \
- __AD_SD_CHANNEL(_si, _channel, -1, _address, _bits, \
- _storagebits, _shift, NULL, IIO_VOLTAGE, \
- BIT(IIO_CHAN_INFO_SAMP_FREQ))
-
-#define AD_SD_CHANNEL_NO_SAMP_FREQ(_si, _channel, _address, _bits, \
- _storagebits, _shift) \
- __AD_SD_CHANNEL(_si, _channel, -1, _address, _bits, \
- _storagebits, _shift, NULL, IIO_VOLTAGE, 0)
-
-#define AD_SD_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
- __AD_SD_CHANNEL(_si, 0, -1, _address, _bits, \
- _storagebits, _shift, NULL, IIO_TEMP, \
- BIT(IIO_CHAN_INFO_SAMP_FREQ))
-
-#define AD_SD_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
- _shift) \
- __AD_SD_CHANNEL(_si, _channel, -1, _address, _bits, \
- _storagebits, _shift, "supply", IIO_VOLTAGE, \
- BIT(IIO_CHAN_INFO_SAMP_FREQ))
-
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
2020-03-21 9:07 [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Alexandru Ardelean
` (2 preceding siblings ...)
2020-03-21 9:08 ` [PATCH v2 4/5] iio: ad_sigma_delta: remove unused " Alexandru Ardelean
@ 2020-03-21 9:08 ` Alexandru Ardelean
2020-03-21 16:57 ` Jonathan Cameron
2020-03-21 19:37 ` Andy Shevchenko
2020-03-21 16:50 ` [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Jonathan Cameron
4 siblings, 2 replies; 14+ messages in thread
From: Alexandru Ardelean @ 2020-03-21 9:08 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, lars, Alexandru Ardelean
This change uses the read_avail and '.info_mask_shared_by_type_available'
modifier to set the available scale.
Essentially, nothing changes to the driver's ABI.
The main idea for this patch is to remove the AD7793 driver from
checkpatch's radar. There have been about ~3 attempts to fix/break the
'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
it to be an arithmetic operation and people were trying to change that.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/adc/ad7793.c | 53 +++++++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
index 5592ae573e6b..fad98f1801db 100644
--- a/drivers/iio/adc/ad7793.c
+++ b/drivers/iio/adc/ad7793.c
@@ -354,29 +354,28 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
static IIO_CONST_ATTR_NAMED(sampling_frequency_available_ad7797,
sampling_frequency_available, "123 62 50 33 17 16 12 10 8 6 4");
-static ssize_t ad7793_show_scale_available(struct device *dev,
- struct device_attribute *attr, char *buf)
+static int ad7793_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad7793_state *st = iio_priv(indio_dev);
- int i, len = 0;
- for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++)
- len += sprintf(buf + len, "%d.%09u ", st->scale_avail[i][0],
- st->scale_avail[i][1]);
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ *vals = (int *)st->scale_avail;
+ *type = IIO_VAL_INT_PLUS_NANO;
+ /* Values are stored in a 2D matrix */
+ *length = ARRAY_SIZE(st->scale_avail) * 2;
- len += sprintf(buf + len, "\n");
+ return IIO_AVAIL_LIST;
+ }
- return len;
+ return -EINVAL;
}
-static IIO_DEVICE_ATTR_NAMED(in_m_in_scale_available,
- in_voltage-voltage_scale_available, S_IRUGO,
- ad7793_show_scale_available, NULL, 0);
-
static struct attribute *ad7793_attributes[] = {
&iio_const_attr_sampling_frequency_available.dev_attr.attr,
- &iio_dev_attr_in_m_in_scale_available.dev_attr.attr,
NULL
};
@@ -534,6 +533,7 @@ static const struct iio_info ad7793_info = {
.read_raw = &ad7793_read_raw,
.write_raw = &ad7793_write_raw,
.write_raw_get_fmt = &ad7793_write_raw_get_fmt,
+ .read_avail = ad7793_read_avail,
.attrs = &ad7793_attribute_group,
.validate_trigger = ad_sd_validate_trigger,
};
@@ -547,7 +547,7 @@ static const struct iio_info ad7797_info = {
};
#define __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
- _storagebits, _shift, _extend_name, _type, _mask_all) \
+ _storagebits, _shift, _extend_name, _type, _mask_type_av, _mask_all) \
{ \
.type = (_type), \
.differential = (_channel2 == -1 ? 0 : 1), \
@@ -559,6 +559,7 @@ static const struct iio_info ad7797_info = {
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
BIT(IIO_CHAN_INFO_OFFSET), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_type_available = (_mask_type_av), \
.info_mask_shared_by_all = _mask_all, \
.scan_index = (_si), \
.scan_type = { \
@@ -574,23 +575,41 @@ static const struct iio_info ad7797_info = {
_storagebits, _shift) \
__AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
_storagebits, _shift, NULL, IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SCALE), \
BIT(IIO_CHAN_INFO_SAMP_FREQ))
#define AD7793_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
_storagebits, _shift) \
__AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
_storagebits, _shift, "shorted", IIO_VOLTAGE, \
+ BIT(IIO_CHAN_INFO_SCALE), \
BIT(IIO_CHAN_INFO_SAMP_FREQ))
#define AD7793_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
__AD7793_CHANNEL(_si, 0, -1, _address, _bits, \
_storagebits, _shift, NULL, IIO_TEMP, \
+ 0, \
BIT(IIO_CHAN_INFO_SAMP_FREQ))
#define AD7793_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
_shift) \
__AD7793_CHANNEL(_si, _channel, -1, _address, _bits, \
_storagebits, _shift, "supply", IIO_VOLTAGE, \
+ 0, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7797_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
+ _storagebits, _shift, NULL, IIO_VOLTAGE, \
+ 0, \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ))
+
+#define AD7797_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
+ _storagebits, _shift) \
+ __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
+ _storagebits, _shift, "shorted", IIO_VOLTAGE, \
+ 0, \
BIT(IIO_CHAN_INFO_SAMP_FREQ))
#define DECLARE_AD7793_CHANNELS(_name, _b, _sb, _s) \
@@ -620,8 +639,8 @@ const struct iio_chan_spec _name##_channels[] = { \
#define DECLARE_AD7797_CHANNELS(_name, _b, _sb) \
const struct iio_chan_spec _name##_channels[] = { \
- AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
- AD7793_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
+ AD7797_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
+ AD7797_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
AD7793_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
AD7793_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
IIO_CHAN_SOFT_TIMESTAMP(4), \
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros
2020-03-21 9:07 [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Alexandru Ardelean
` (3 preceding siblings ...)
2020-03-21 9:08 ` [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available Alexandru Ardelean
@ 2020-03-21 16:50 ` Jonathan Cameron
4 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2020-03-21 16:50 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, lars
On Sat, 21 Mar 2020 11:07:58 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This change gets rid of the AD_SD_*_CHANNEL macros in favor of defining
> it's own. The ad7780 is quite simpler than it's other Sigma-Delta brothers.
>
> It turned out that centralizing the AD_SD_*_CHANNEL macros doesn't scale
> too well, especially with some more complicated drivers. Some of the
> variations in the more complicated drivers require new macros, and that way
> things can become harder to maintain.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> ---
>
> Changelog v1 -> v2:
> * re-send + change author @analog.com; GMail messed it up
>
> drivers/iio/adc/ad7780.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7780.c b/drivers/iio/adc/ad7780.c
> index 291c1a898129..f47606ebbbbe 100644
> --- a/drivers/iio/adc/ad7780.c
> +++ b/drivers/iio/adc/ad7780.c
> @@ -206,10 +206,29 @@ static const struct ad_sigma_delta_info ad7780_sigma_delta_info = {
> .irq_flags = IRQF_TRIGGER_LOW,
> };
>
> -#define AD7780_CHANNEL(bits, wordsize) \
> - AD_SD_CHANNEL(1, 0, 0, bits, 32, (wordsize) - (bits))
> -#define AD7170_CHANNEL(bits, wordsize) \
> - AD_SD_CHANNEL_NO_SAMP_FREQ(1, 0, 0, bits, 32, (wordsize) - (bits))
> +#define _AD7780_CHANNEL(_bits, _wordsize, _mask_all) \
> +{ \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .channel = 0, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_all = _mask_all, \
> + .scan_index = 1, \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = (_bits), \
> + .storagebits = 32, \
> + .shift = (_wordsize) - (_bits), \
> + .endianness = IIO_BE, \
> + }, \
> +}
> +
> +#define AD7780_CHANNEL(_bits, _wordsize) \
> + _AD7780_CHANNEL(_bits, _wordsize, BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +#define AD7170_CHANNEL(_bits, _wordsize) \
> + _AD7780_CHANNEL(_bits, _wordsize, 0)
>
> static const struct ad7780_chip_info ad7780_chip_info_tbl[] = {
> [ID_AD7170] = {
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/5] iio: adc: ad7791: define/use own IIO channel macros
2020-03-21 9:07 ` [PATCH v2 2/5] iio: adc: ad7791: " Alexandru Ardelean
@ 2020-03-21 16:51 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2020-03-21 16:51 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, lars
On Sat, 21 Mar 2020 11:07:59 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This driver seems to use most of the AD_SD_*_CHANNEL. This change will move
> them in the driver. The intent is that if a new part comes along which
> would require tweaks per IIO channel, these should be doable in the driver.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied, thanks
Jonathan
> ---
> drivers/iio/adc/ad7791.c | 62 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 55 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c
> index abb239392631..ba22808507d0 100644
> --- a/drivers/iio/adc/ad7791.c
> +++ b/drivers/iio/adc/ad7791.c
> @@ -64,25 +64,73 @@
> #define AD7791_MODE_SEL_MASK (0x3 << 6)
> #define AD7791_MODE_SEL(x) ((x) << 6)
>
> +#define __AD7991_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift, _extend_name, _type, _mask_all) \
> + { \
> + .type = (_type), \
> + .differential = (_channel2 == -1 ? 0 : 1), \
> + .indexed = 1, \
> + .channel = (_channel1), \
> + .channel2 = (_channel2), \
> + .address = (_address), \
> + .extend_name = (_extend_name), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_all = _mask_all, \
> + .scan_index = (_si), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = (_bits), \
> + .storagebits = (_storagebits), \
> + .shift = (_shift), \
> + .endianness = IIO_BE, \
> + }, \
> + }
> +
> +#define AD7991_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7991_CHANNEL(_si, _channel, _channel, _address, _bits, \
> + _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7991_CHANNEL(_si, _channel, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7991_CHANNEL(_si, _channel, -1, _address, _bits, \
> + _storagebits, _shift, NULL, IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7991_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7991_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift, NULL, IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7991_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
> + _shift) \
> + __AD7991_CHANNEL(_si, _channel, -1, _address, _bits, \
> + _storagebits, _shift, "supply", IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> #define DECLARE_AD7787_CHANNELS(name, bits, storagebits) \
> const struct iio_chan_spec name[] = { \
> - AD_SD_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
> + AD7991_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
> (bits), (storagebits), 0), \
> - AD_SD_CHANNEL(1, 1, AD7791_CH_AIN2, (bits), (storagebits), 0), \
> - AD_SD_SHORTED_CHANNEL(2, 0, AD7791_CH_AIN1N_AIN1N, \
> + AD7991_CHANNEL(1, 1, AD7791_CH_AIN2, (bits), (storagebits), 0), \
> + AD7991_SHORTED_CHANNEL(2, 0, AD7791_CH_AIN1N_AIN1N, \
> (bits), (storagebits), 0), \
> - AD_SD_SUPPLY_CHANNEL(3, 2, AD7791_CH_AVDD_MONITOR, \
> + AD7991_SUPPLY_CHANNEL(3, 2, AD7791_CH_AVDD_MONITOR, \
> (bits), (storagebits), 0), \
> IIO_CHAN_SOFT_TIMESTAMP(4), \
> }
>
> #define DECLARE_AD7791_CHANNELS(name, bits, storagebits) \
> const struct iio_chan_spec name[] = { \
> - AD_SD_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
> + AD7991_DIFF_CHANNEL(0, 0, 0, AD7791_CH_AIN1P_AIN1N, \
> (bits), (storagebits), 0), \
> - AD_SD_SHORTED_CHANNEL(1, 0, AD7791_CH_AIN1N_AIN1N, \
> + AD7991_SHORTED_CHANNEL(1, 0, AD7791_CH_AIN1N_AIN1N, \
> (bits), (storagebits), 0), \
> - AD_SD_SUPPLY_CHANNEL(2, 1, AD7791_CH_AVDD_MONITOR, \
> + AD7991_SUPPLY_CHANNEL(2, 1, AD7791_CH_AVDD_MONITOR, \
> (bits), (storagebits), 0), \
> IIO_CHAN_SOFT_TIMESTAMP(3), \
> }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/5] iio: adc: ad7793: define/use own IIO channel macros
2020-03-21 9:08 ` [PATCH v2 3/5] iio: adc: ad7793: " Alexandru Ardelean
@ 2020-03-21 16:52 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2020-03-21 16:52 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, lars
On Sat, 21 Mar 2020 11:08:00 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This driver seems to use most of the AD_SD_*_CHANNEL. This change will move
> them in the driver. The intent is that if a new part comes along which
> would require tweaks per IIO channel, these should be doable in the driver.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7793.c | 95 ++++++++++++++++++++++++++++++----------
> 1 file changed, 71 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
> index b747db97f78a..5592ae573e6b 100644
> --- a/drivers/iio/adc/ad7793.c
> +++ b/drivers/iio/adc/ad7793.c
> @@ -546,47 +546,94 @@ static const struct iio_info ad7797_info = {
> .validate_trigger = ad_sd_validate_trigger,
> };
>
> +#define __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift, _extend_name, _type, _mask_all) \
> + { \
> + .type = (_type), \
> + .differential = (_channel2 == -1 ? 0 : 1), \
> + .indexed = 1, \
> + .channel = (_channel1), \
> + .channel2 = (_channel2), \
> + .address = (_address), \
> + .extend_name = (_extend_name), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_all = _mask_all, \
> + .scan_index = (_si), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = (_bits), \
> + .storagebits = (_storagebits), \
> + .shift = (_shift), \
> + .endianness = IIO_BE, \
> + }, \
> + }
> +
> +#define AD7793_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift, NULL, IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7793_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
> + _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7793_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
> + __AD7793_CHANNEL(_si, 0, -1, _address, _bits, \
> + _storagebits, _shift, NULL, IIO_TEMP, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7793_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
> + _shift) \
> + __AD7793_CHANNEL(_si, _channel, -1, _address, _bits, \
> + _storagebits, _shift, "supply", IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> #define DECLARE_AD7793_CHANNELS(_name, _b, _sb, _s) \
> const struct iio_chan_spec _name##_channels[] = { \
> - AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), (_s)), \
> - AD_SD_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), (_s)), \
> - AD_SD_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), (_s)), \
> - AD_SD_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), (_s)), \
> - AD_SD_TEMP_CHANNEL(4, AD7793_CH_TEMP, (_b), (_sb), (_s)), \
> - AD_SD_SUPPLY_CHANNEL(5, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), (_s)), \
> + AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), (_s)), \
> + AD7793_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), (_s)), \
> + AD7793_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), (_s)), \
> + AD7793_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), (_s)), \
> + AD7793_TEMP_CHANNEL(4, AD7793_CH_TEMP, (_b), (_sb), (_s)), \
> + AD7793_SUPPLY_CHANNEL(5, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), (_s)), \
> IIO_CHAN_SOFT_TIMESTAMP(6), \
> }
>
> #define DECLARE_AD7795_CHANNELS(_name, _b, _sb) \
> const struct iio_chan_spec _name##_channels[] = { \
> - AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(3, 3, 3, AD7795_CH_AIN4P_AIN4M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(4, 4, 4, AD7795_CH_AIN5P_AIN5M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(5, 5, 5, AD7795_CH_AIN6P_AIN6M, (_b), (_sb), 0), \
> - AD_SD_SHORTED_CHANNEL(6, 0, AD7795_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> - AD_SD_TEMP_CHANNEL(7, AD7793_CH_TEMP, (_b), (_sb), 0), \
> - AD_SD_SUPPLY_CHANNEL(8, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(3, 3, 3, AD7795_CH_AIN4P_AIN4M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(4, 4, 4, AD7795_CH_AIN5P_AIN5M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(5, 5, 5, AD7795_CH_AIN6P_AIN6M, (_b), (_sb), 0), \
> + AD7793_SHORTED_CHANNEL(6, 0, AD7795_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> + AD7793_TEMP_CHANNEL(7, AD7793_CH_TEMP, (_b), (_sb), 0), \
> + AD7793_SUPPLY_CHANNEL(8, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> IIO_CHAN_SOFT_TIMESTAMP(9), \
> }
>
> #define DECLARE_AD7797_CHANNELS(_name, _b, _sb) \
> const struct iio_chan_spec _name##_channels[] = { \
> - AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> - AD_SD_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> - AD_SD_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
> - AD_SD_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> + AD7793_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> + AD7793_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
> + AD7793_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> IIO_CHAN_SOFT_TIMESTAMP(4), \
> }
>
> #define DECLARE_AD7799_CHANNELS(_name, _b, _sb) \
> const struct iio_chan_spec _name##_channels[] = { \
> - AD_SD_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
> - AD_SD_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
> - AD_SD_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> - AD_SD_SUPPLY_CHANNEL(4, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(1, 1, 1, AD7793_CH_AIN2P_AIN2M, (_b), (_sb), 0), \
> + AD7793_DIFF_CHANNEL(2, 2, 2, AD7793_CH_AIN3P_AIN3M, (_b), (_sb), 0), \
> + AD7793_SHORTED_CHANNEL(3, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> + AD7793_SUPPLY_CHANNEL(4, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> IIO_CHAN_SOFT_TIMESTAMP(5), \
> }
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/5] iio: ad_sigma_delta: remove unused IIO channel macros
2020-03-21 9:08 ` [PATCH v2 4/5] iio: ad_sigma_delta: remove unused " Alexandru Ardelean
@ 2020-03-21 16:53 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2020-03-21 16:53 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, lars
On Sat, 21 Mar 2020 11:08:01 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> Now that all channel SigmaDelta IIO channel macros have been localized,
> remove the generic ones.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.
Thanks,
Jonathan
> ---
> include/linux/iio/adc/ad_sigma_delta.h | 58 --------------------------
> 1 file changed, 58 deletions(-)
>
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
> index 5a127c0ed200..a3a838dcf8e4 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -133,62 +133,4 @@ void ad_sd_cleanup_buffer_and_trigger(struct iio_dev *indio_dev);
>
> int ad_sd_validate_trigger(struct iio_dev *indio_dev, struct iio_trigger *trig);
>
> -#define __AD_SD_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> - _storagebits, _shift, _extend_name, _type, _mask_all) \
> - { \
> - .type = (_type), \
> - .differential = (_channel2 == -1 ? 0 : 1), \
> - .indexed = 1, \
> - .channel = (_channel1), \
> - .channel2 = (_channel2), \
> - .address = (_address), \
> - .extend_name = (_extend_name), \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> - BIT(IIO_CHAN_INFO_OFFSET), \
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> - .info_mask_shared_by_all = _mask_all, \
> - .scan_index = (_si), \
> - .scan_type = { \
> - .sign = 'u', \
> - .realbits = (_bits), \
> - .storagebits = (_storagebits), \
> - .shift = (_shift), \
> - .endianness = IIO_BE, \
> - }, \
> - }
> -
> -#define AD_SD_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> - _storagebits, _shift) \
> - __AD_SD_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> - _storagebits, _shift, NULL, IIO_VOLTAGE, \
> - BIT(IIO_CHAN_INFO_SAMP_FREQ))
> -
> -#define AD_SD_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> - _storagebits, _shift) \
> - __AD_SD_CHANNEL(_si, _channel, _channel, _address, _bits, \
> - _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> - BIT(IIO_CHAN_INFO_SAMP_FREQ))
> -
> -#define AD_SD_CHANNEL(_si, _channel, _address, _bits, \
> - _storagebits, _shift) \
> - __AD_SD_CHANNEL(_si, _channel, -1, _address, _bits, \
> - _storagebits, _shift, NULL, IIO_VOLTAGE, \
> - BIT(IIO_CHAN_INFO_SAMP_FREQ))
> -
> -#define AD_SD_CHANNEL_NO_SAMP_FREQ(_si, _channel, _address, _bits, \
> - _storagebits, _shift) \
> - __AD_SD_CHANNEL(_si, _channel, -1, _address, _bits, \
> - _storagebits, _shift, NULL, IIO_VOLTAGE, 0)
> -
> -#define AD_SD_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
> - __AD_SD_CHANNEL(_si, 0, -1, _address, _bits, \
> - _storagebits, _shift, NULL, IIO_TEMP, \
> - BIT(IIO_CHAN_INFO_SAMP_FREQ))
> -
> -#define AD_SD_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
> - _shift) \
> - __AD_SD_CHANNEL(_si, _channel, -1, _address, _bits, \
> - _storagebits, _shift, "supply", IIO_VOLTAGE, \
> - BIT(IIO_CHAN_INFO_SAMP_FREQ))
> -
> #endif
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
2020-03-21 9:08 ` [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available Alexandru Ardelean
@ 2020-03-21 16:57 ` Jonathan Cameron
2020-03-22 9:38 ` Ardelean, Alexandru
2020-03-21 19:37 ` Andy Shevchenko
1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2020-03-21 16:57 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, lars
On Sat, 21 Mar 2020 11:08:02 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> This change uses the read_avail and '.info_mask_shared_by_type_available'
> modifier to set the available scale.
> Essentially, nothing changes to the driver's ABI.
>
> The main idea for this patch is to remove the AD7793 driver from
> checkpatch's radar. There have been about ~3 attempts to fix/break the
> 'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
> it to be an arithmetic operation and people were trying to change that.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
At some point it might be nice to move the sampling_frequency over as well
but clearly not remotely urgent!
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to poke at it.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7793.c | 53 +++++++++++++++++++++++++++-------------
> 1 file changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
> index 5592ae573e6b..fad98f1801db 100644
> --- a/drivers/iio/adc/ad7793.c
> +++ b/drivers/iio/adc/ad7793.c
> @@ -354,29 +354,28 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> static IIO_CONST_ATTR_NAMED(sampling_frequency_available_ad7797,
> sampling_frequency_available, "123 62 50 33 17 16 12 10 8 6 4");
>
> -static ssize_t ad7793_show_scale_available(struct device *dev,
> - struct device_attribute *attr, char *buf)
> +static int ad7793_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> {
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct ad7793_state *st = iio_priv(indio_dev);
> - int i, len = 0;
>
> - for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++)
> - len += sprintf(buf + len, "%d.%09u ", st->scale_avail[i][0],
> - st->scale_avail[i][1]);
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (int *)st->scale_avail;
> + *type = IIO_VAL_INT_PLUS_NANO;
> + /* Values are stored in a 2D matrix */
> + *length = ARRAY_SIZE(st->scale_avail) * 2;
>
> - len += sprintf(buf + len, "\n");
> + return IIO_AVAIL_LIST;
> + }
>
> - return len;
> + return -EINVAL;
> }
>
> -static IIO_DEVICE_ATTR_NAMED(in_m_in_scale_available,
> - in_voltage-voltage_scale_available, S_IRUGO,
> - ad7793_show_scale_available, NULL, 0);
> -
> static struct attribute *ad7793_attributes[] = {
> &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> - &iio_dev_attr_in_m_in_scale_available.dev_attr.attr,
> NULL
> };
>
> @@ -534,6 +533,7 @@ static const struct iio_info ad7793_info = {
> .read_raw = &ad7793_read_raw,
> .write_raw = &ad7793_write_raw,
> .write_raw_get_fmt = &ad7793_write_raw_get_fmt,
> + .read_avail = ad7793_read_avail,
> .attrs = &ad7793_attribute_group,
> .validate_trigger = ad_sd_validate_trigger,
> };
> @@ -547,7 +547,7 @@ static const struct iio_info ad7797_info = {
> };
>
> #define __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> - _storagebits, _shift, _extend_name, _type, _mask_all) \
> + _storagebits, _shift, _extend_name, _type, _mask_type_av, _mask_all) \
> { \
> .type = (_type), \
> .differential = (_channel2 == -1 ? 0 : 1), \
> @@ -559,6 +559,7 @@ static const struct iio_info ad7797_info = {
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> BIT(IIO_CHAN_INFO_OFFSET), \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_type_available = (_mask_type_av), \
> .info_mask_shared_by_all = _mask_all, \
> .scan_index = (_si), \
> .scan_type = { \
> @@ -574,23 +575,41 @@ static const struct iio_info ad7797_info = {
> _storagebits, _shift) \
> __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> _storagebits, _shift, NULL, IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SCALE), \
> BIT(IIO_CHAN_INFO_SAMP_FREQ))
>
> #define AD7793_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> _storagebits, _shift) \
> __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
> _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> + BIT(IIO_CHAN_INFO_SCALE), \
> BIT(IIO_CHAN_INFO_SAMP_FREQ))
>
> #define AD7793_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
> __AD7793_CHANNEL(_si, 0, -1, _address, _bits, \
> _storagebits, _shift, NULL, IIO_TEMP, \
> + 0, \
> BIT(IIO_CHAN_INFO_SAMP_FREQ))
>
> #define AD7793_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits, \
> _shift) \
> __AD7793_CHANNEL(_si, _channel, -1, _address, _bits, \
> _storagebits, _shift, "supply", IIO_VOLTAGE, \
> + 0, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7797_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> + _storagebits, _shift, NULL, IIO_VOLTAGE, \
> + 0, \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> +
> +#define AD7797_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> + _storagebits, _shift) \
> + __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
> + _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> + 0, \
> BIT(IIO_CHAN_INFO_SAMP_FREQ))
>
> #define DECLARE_AD7793_CHANNELS(_name, _b, _sb, _s) \
> @@ -620,8 +639,8 @@ const struct iio_chan_spec _name##_channels[] = { \
>
> #define DECLARE_AD7797_CHANNELS(_name, _b, _sb) \
> const struct iio_chan_spec _name##_channels[] = { \
> - AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> - AD7793_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> + AD7797_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> + AD7797_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> AD7793_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
> AD7793_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> IIO_CHAN_SOFT_TIMESTAMP(4), \
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
2020-03-21 9:08 ` [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available Alexandru Ardelean
2020-03-21 16:57 ` Jonathan Cameron
@ 2020-03-21 19:37 ` Andy Shevchenko
2020-03-22 9:18 ` Ardelean, Alexandru
1 sibling, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2020-03-21 19:37 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, jic23, lars
On Sat, Mar 21, 2020 at 11:08:02AM +0200, Alexandru Ardelean wrote:
> This change uses the read_avail and '.info_mask_shared_by_type_available'
> modifier to set the available scale.
> Essentially, nothing changes to the driver's ABI.
>
> The main idea for this patch is to remove the AD7793 driver from
> checkpatch's radar. There have been about ~3 attempts to fix/break the
> 'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
> it to be an arithmetic operation and people were trying to change that.
> +static int ad7793_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> {
> struct ad7793_state *st = iio_priv(indio_dev);
>
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + *vals = (int *)st->scale_avail;
> + *type = IIO_VAL_INT_PLUS_NANO;
> + /* Values are stored in a 2D matrix */
> + *length = ARRAY_SIZE(st->scale_avail) * 2;
>
> + return IIO_AVAIL_LIST;
> + }
>
> + return -EINVAL;
Wouldn't be better move this under default case?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
2020-03-21 19:37 ` Andy Shevchenko
@ 2020-03-22 9:18 ` Ardelean, Alexandru
2020-03-22 15:15 ` Jonathan Cameron
0 siblings, 1 reply; 14+ messages in thread
From: Ardelean, Alexandru @ 2020-03-22 9:18 UTC (permalink / raw)
To: andriy.shevchenko; +Cc: jic23, linux-kernel, linux-iio, lars
On Sat, 2020-03-21 at 21:37 +0200, Andy Shevchenko wrote:
> [External]
>
> On Sat, Mar 21, 2020 at 11:08:02AM +0200, Alexandru Ardelean wrote:
> > This change uses the read_avail and '.info_mask_shared_by_type_available'
> > modifier to set the available scale.
> > Essentially, nothing changes to the driver's ABI.
> >
> > The main idea for this patch is to remove the AD7793 driver from
> > checkpatch's radar. There have been about ~3 attempts to fix/break the
> > 'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
> > it to be an arithmetic operation and people were trying to change that.
> > +static int ad7793_read_avail(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + const int **vals, int *type, int *length,
> > + long mask)
> > {
> > struct ad7793_state *st = iio_priv(indio_dev);
> >
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + *vals = (int *)st->scale_avail;
> > + *type = IIO_VAL_INT_PLUS_NANO;
> > + /* Values are stored in a 2D matrix */
> > + *length = ARRAY_SIZE(st->scale_avail) * 2;
> >
> > + return IIO_AVAIL_LIST;
> > + }
> >
> > + return -EINVAL;
>
> Wouldn't be better move this under default case?
>
Ummm, sure.
I'm a bit vague how we do from here since Jonathan accepted this and the
patchset.
I'll send a V3 of the whole set [in a few days max].
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
2020-03-21 16:57 ` Jonathan Cameron
@ 2020-03-22 9:38 ` Ardelean, Alexandru
0 siblings, 0 replies; 14+ messages in thread
From: Ardelean, Alexandru @ 2020-03-22 9:38 UTC (permalink / raw)
To: jic23; +Cc: linux-kernel, linux-iio, lars
On Sat, 2020-03-21 at 16:57 +0000, Jonathan Cameron wrote:
> [External]
>
> On Sat, 21 Mar 2020 11:08:02 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > This change uses the read_avail and '.info_mask_shared_by_type_available'
> > modifier to set the available scale.
> > Essentially, nothing changes to the driver's ABI.
> >
> > The main idea for this patch is to remove the AD7793 driver from
> > checkpatch's radar. There have been about ~3 attempts to fix/break the
> > 'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
> > it to be an arithmetic operation and people were trying to change that.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> At some point it might be nice to move the sampling_frequency over as well
> but clearly not remotely urgent!
I also thought about that now.
But I decided to defer it; the patchset to do just this seemed big enough.
I mostly want to avoid checkpatch fixes for this.
It looks bad on our part if we get hit periodically on this, and don't do
anything about it.
>
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to poke at it.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/adc/ad7793.c | 53 +++++++++++++++++++++++++++-------------
> > 1 file changed, 36 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
> > index 5592ae573e6b..fad98f1801db 100644
> > --- a/drivers/iio/adc/ad7793.c
> > +++ b/drivers/iio/adc/ad7793.c
> > @@ -354,29 +354,28 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> > static IIO_CONST_ATTR_NAMED(sampling_frequency_available_ad7797,
> > sampling_frequency_available, "123 62 50 33 17 16 12 10 8 6 4");
> >
> > -static ssize_t ad7793_show_scale_available(struct device *dev,
> > - struct device_attribute *attr, char *buf)
> > +static int ad7793_read_avail(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + const int **vals, int *type, int *length,
> > + long mask)
> > {
> > - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > struct ad7793_state *st = iio_priv(indio_dev);
> > - int i, len = 0;
> >
> > - for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++)
> > - len += sprintf(buf + len, "%d.%09u ", st->scale_avail[i][0],
> > - st->scale_avail[i][1]);
> > + switch (mask) {
> > + case IIO_CHAN_INFO_SCALE:
> > + *vals = (int *)st->scale_avail;
> > + *type = IIO_VAL_INT_PLUS_NANO;
> > + /* Values are stored in a 2D matrix */
> > + *length = ARRAY_SIZE(st->scale_avail) * 2;
> >
> > - len += sprintf(buf + len, "\n");
> > + return IIO_AVAIL_LIST;
> > + }
> >
> > - return len;
> > + return -EINVAL;
> > }
> >
> > -static IIO_DEVICE_ATTR_NAMED(in_m_in_scale_available,
> > - in_voltage-voltage_scale_available, S_IRUGO,
> > - ad7793_show_scale_available, NULL, 0);
> > -
> > static struct attribute *ad7793_attributes[] = {
> > &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> > - &iio_dev_attr_in_m_in_scale_available.dev_attr.attr,
> > NULL
> > };
> >
> > @@ -534,6 +533,7 @@ static const struct iio_info ad7793_info = {
> > .read_raw = &ad7793_read_raw,
> > .write_raw = &ad7793_write_raw,
> > .write_raw_get_fmt = &ad7793_write_raw_get_fmt,
> > + .read_avail = ad7793_read_avail,
> > .attrs = &ad7793_attribute_group,
> > .validate_trigger = ad_sd_validate_trigger,
> > };
> > @@ -547,7 +547,7 @@ static const struct iio_info ad7797_info = {
> > };
> >
> > #define __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> > - _storagebits, _shift, _extend_name, _type, _mask_all) \
> > + _storagebits, _shift, _extend_name, _type, _mask_type_av, _mask_all) \
> > { \
> > .type = (_type), \
> > .differential = (_channel2 == -1 ? 0 : 1), \
> > @@ -559,6 +559,7 @@ static const struct iio_info ad7797_info = {
> > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> > BIT(IIO_CHAN_INFO_OFFSET), \
> > .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > + .info_mask_shared_by_type_available = (_mask_type_av), \
> > .info_mask_shared_by_all = _mask_all, \
> > .scan_index = (_si), \
> > .scan_type = { \
> > @@ -574,23 +575,41 @@ static const struct iio_info ad7797_info = {
> > _storagebits, _shift) \
> > __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> > _storagebits, _shift, NULL, IIO_VOLTAGE, \
> > + BIT(IIO_CHAN_INFO_SCALE), \
> > BIT(IIO_CHAN_INFO_SAMP_FREQ))
> >
> > #define AD7793_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> > _storagebits, _shift) \
> > __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
> > _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> > + BIT(IIO_CHAN_INFO_SCALE), \
> > BIT(IIO_CHAN_INFO_SAMP_FREQ))
> >
> > #define AD7793_TEMP_CHANNEL(_si, _address, _bits, _storagebits, _shift) \
> > __AD7793_CHANNEL(_si, 0, -1, _address, _bits, \
> > _storagebits, _shift, NULL, IIO_TEMP, \
> > + 0, \
> > BIT(IIO_CHAN_INFO_SAMP_FREQ))
> >
> > #define AD7793_SUPPLY_CHANNEL(_si, _channel, _address, _bits, _storagebits,
> > \
> > _shift) \
> > __AD7793_CHANNEL(_si, _channel, -1, _address, _bits, \
> > _storagebits, _shift, "supply", IIO_VOLTAGE, \
> > + 0, \
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> > +
> > +#define AD7797_DIFF_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> > + _storagebits, _shift) \
> > + __AD7793_CHANNEL(_si, _channel1, _channel2, _address, _bits, \
> > + _storagebits, _shift, NULL, IIO_VOLTAGE, \
> > + 0, \
> > + BIT(IIO_CHAN_INFO_SAMP_FREQ))
> > +
> > +#define AD7797_SHORTED_CHANNEL(_si, _channel, _address, _bits, \
> > + _storagebits, _shift) \
> > + __AD7793_CHANNEL(_si, _channel, _channel, _address, _bits, \
> > + _storagebits, _shift, "shorted", IIO_VOLTAGE, \
> > + 0, \
> > BIT(IIO_CHAN_INFO_SAMP_FREQ))
> >
> > #define DECLARE_AD7793_CHANNELS(_name, _b, _sb, _s) \
> > @@ -620,8 +639,8 @@ const struct iio_chan_spec _name##_channels[] = { \
> >
> > #define DECLARE_AD7797_CHANNELS(_name, _b, _sb) \
> > const struct iio_chan_spec _name##_channels[] = { \
> > - AD7793_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> > - AD7793_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> > + AD7797_DIFF_CHANNEL(0, 0, 0, AD7793_CH_AIN1P_AIN1M, (_b), (_sb), 0), \
> > + AD7797_SHORTED_CHANNEL(1, 0, AD7793_CH_AIN1M_AIN1M, (_b), (_sb), 0), \
> > AD7793_TEMP_CHANNEL(2, AD7793_CH_TEMP, (_b), (_sb), 0), \
> > AD7793_SUPPLY_CHANNEL(3, 3, AD7793_CH_AVDD_MONITOR, (_b), (_sb), 0), \
> > IIO_CHAN_SOFT_TIMESTAMP(4), \
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
2020-03-22 9:18 ` Ardelean, Alexandru
@ 2020-03-22 15:15 ` Jonathan Cameron
0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2020-03-22 15:15 UTC (permalink / raw)
To: Ardelean, Alexandru; +Cc: andriy.shevchenko, linux-kernel, linux-iio, lars
On Sun, 22 Mar 2020 09:18:13 +0000
"Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote:
> On Sat, 2020-03-21 at 21:37 +0200, Andy Shevchenko wrote:
> > [External]
> >
> > On Sat, Mar 21, 2020 at 11:08:02AM +0200, Alexandru Ardelean wrote:
> > > This change uses the read_avail and '.info_mask_shared_by_type_available'
> > > modifier to set the available scale.
> > > Essentially, nothing changes to the driver's ABI.
> > >
> > > The main idea for this patch is to remove the AD7793 driver from
> > > checkpatch's radar. There have been about ~3 attempts to fix/break the
> > > 'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
> > > it to be an arithmetic operation and people were trying to change that.
> > > +static int ad7793_read_avail(struct iio_dev *indio_dev,
> > > + struct iio_chan_spec const *chan,
> > > + const int **vals, int *type, int *length,
> > > + long mask)
> > > {
> > > struct ad7793_state *st = iio_priv(indio_dev);
> > >
> > > + switch (mask) {
> > > + case IIO_CHAN_INFO_SCALE:
> > > + *vals = (int *)st->scale_avail;
> > > + *type = IIO_VAL_INT_PLUS_NANO;
> > > + /* Values are stored in a 2D matrix */
> > > + *length = ARRAY_SIZE(st->scale_avail) * 2;
> > >
> > > + return IIO_AVAIL_LIST;
> > > + }
> > >
> > > + return -EINVAL;
> >
> > Wouldn't be better move this under default case?
> >
>
> Ummm, sure.
> I'm a bit vague how we do from here since Jonathan accepted this and the
> patchset.
> I'll send a V3 of the whole set [in a few days max].
I'll drop this last patch. Just resend this one with the change.
Jonathan
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-03-22 15:16 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-21 9:07 [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Alexandru Ardelean
2020-03-21 9:07 ` [PATCH v2 2/5] iio: adc: ad7791: " Alexandru Ardelean
2020-03-21 16:51 ` Jonathan Cameron
2020-03-21 9:08 ` [PATCH v2 3/5] iio: adc: ad7793: " Alexandru Ardelean
2020-03-21 16:52 ` Jonathan Cameron
2020-03-21 9:08 ` [PATCH v2 4/5] iio: ad_sigma_delta: remove unused " Alexandru Ardelean
2020-03-21 16:53 ` Jonathan Cameron
2020-03-21 9:08 ` [PATCH v2 5/5] iio: adc: ad7793: use read_avail iio hook for scale available Alexandru Ardelean
2020-03-21 16:57 ` Jonathan Cameron
2020-03-22 9:38 ` Ardelean, Alexandru
2020-03-21 19:37 ` Andy Shevchenko
2020-03-22 9:18 ` Ardelean, Alexandru
2020-03-22 15:15 ` Jonathan Cameron
2020-03-21 16:50 ` [PATCH v2 1/5] iio: adc: ad7780: define/use own IIO channel macros Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).