From: Dumitru Ceclan <[email protected]>
The AD7173 family offer a complete integrated Sigma-Delta ADC solution
which can be used in high precision, low noise single channel applications
or higher speed multiplexed applications. The Sigma-Delta ADC is intended
primarily for measurement of signals close to DC but also delivers
outstanding performance with input bandwidths out to ~10kHz.
Reviewed-by: Conor Dooley <[email protected]> # except reference_select
Signed-off-by: Dumitru Ceclan <[email protected]>
---
V3 -> V4
- include supply attributes
- add channel attribute for selecting conversion reference
.../bindings/iio/adc/adi,ad7173.yaml | 166 ++++++++++++++++++
1 file changed, 166 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
new file mode 100644
index 000000000000..92aa352b6653
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
@@ -0,0 +1,166 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7173 ADC device driver
+
+maintainers:
+ - Ceclan Dumitru <[email protected]>
+
+description: |
+ Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
+ https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
+
+properties:
+ compatible:
+ enum:
+ - adi,ad7172-2
+ - adi,ad7173-8
+ - adi,ad7175-2
+ - adi,ad7176-2
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ spi-max-frequency:
+ maximum: 20000000
+
+ refin-supply:
+ description: external reference supply, can be used as reference for conversion.
+
+ refin2-supply:
+ description: external reference supply, can be used as reference for conversion.
+
+ avdd-supply:
+ description: avdd supply, can be used as reference for conversion.
+
+ dependencies:
+ refin2-supply:
+ properties:
+ compatible:
+ adi,ad7173-8
+
+ required:
+ - compatible
+ - reg
+ - interrupts
+
+patternProperties:
+ "^channel@[0-9a-f]$":
+ type: object
+ $ref: adc.yaml
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 15
+
+ diff-channels:
+ items:
+ minimum: 0
+ maximum: 31
+
+ adi,reference-select:
+ description: |
+ Select the reference source to use when converting on
+ the specific channel. Valid values are:
+ 0: REFIN(+)/REFIN(−).
+ 1: REFIN2(+)/REFIN2(−)
+ 2: REFOUT/AVSS (Internal reference)
+ 3: AVDD
+
+ External reference 2 only available on ad7173-8.
+ If not specified, internal reference used.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+ default: 2
+
+ bipolar:
+ type: boolean
+
+ required:
+ - reg
+ - diff-channels
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: adi,ad7173-8
+ then:
+ else:
+ patternProperties:
+ "^channel@[0-9a-f]$":
+ properties:
+ enum: [0, 2, 3]
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@0 {
+ compatible = "adi,ad7173-8";
+ reg = <0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio>;
+ spi-max-frequency = <5000000>;
+
+ channel@0 {
+ reg = <0>;
+ bipolar;
+ diff-channels = <0 1>;
+ };
+
+ channel@1 {
+ reg = <1>;
+ diff-channels = <2 3>;
+ };
+
+ channel@2 {
+ reg = <2>;
+ bipolar;
+ diff-channels = <4 5>;
+ };
+
+ channel@3 {
+ reg = <3>;
+ bipolar;
+ diff-channels = <6 7>;
+ };
+
+ channel@4 {
+ reg = <4>;
+ diff-channels = <8 9>;
+ };
+ };
+ };
--
2.42.0
From: Dumitru Ceclan <[email protected]>
The AD7173 family offer a complete integrated Sigma-Delta ADC solution
which can be used in high precision, low noise single channel
applications or higher speed multiplexed applications. The Sigma-Delta
ADC is intended primarily for measurement of signals close to DC but also
delivers outstanding performance with input bandwidths out to ~10kHz.
Reviewed-by: Michael Walle <[email protected]> # for gpio-regmap
Signed-off-by: Dumitru Ceclan <[email protected]>
---
V3 -> V4
- cleaned up includes
- add ad7173_gpio_disable(), include to devm actions
- remove iio_device_claim_direct_mode() from update_scan_mode()
- always set differential iio_chan attribute to true, retain bipolar info only in private struct
- store multiple regulators in state for voltage references
- compute read_raw offset depending on channel used reference
- configure channel setup using selected reference
- add ad7173_get_ref_voltage_milli()
- clean up ad7173_fw_sparse_channel_config() changing array access of channel structs to pointers
- retain chip name in st->info structure
- use spi_device_get_match_data() in probe
drivers/iio/adc/Kconfig | 13 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ad7173.c | 936 +++++++++++++++++++++++++++++++++++++++
3 files changed, 950 insertions(+)
create mode 100644 drivers/iio/adc/ad7173.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 35f9867da12c..c0d522ef1db5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -54,6 +54,19 @@ config AD7124
To compile this driver as a module, choose M here: the module will be
called ad7124.
+config AD7173
+ tristate "Analog Devices AD7173 driver"
+ depends on SPI_MASTER
+ select AD_SIGMA_DELTA
+ select GPIO_REGMAP if GPIOLIB
+ select REGMAP_SPI if GPIOLIB
+ help
+ Say yes here to build support for Analog Devices AD7173 and similar ADC
+ (currently supported: AD7172-2, AD7173-8, AD7175-2, AD7176-2).
+
+ To compile this driver as a module, choose M here: the module will be
+ called ad7173.
+
config AD7192
tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index bee11d442af4..9f1f8f263340 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
obj-$(CONFIG_AD4130) += ad4130.o
obj-$(CONFIG_AD7091R5) += ad7091r5.o ad7091r-base.o
obj-$(CONFIG_AD7124) += ad7124.o
+obj-$(CONFIG_AD7173) += ad7173.o
obj-$(CONFIG_AD7192) += ad7192.o
obj-$(CONFIG_AD7266) += ad7266.o
obj-$(CONFIG_AD7280) += ad7280a.o
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
new file mode 100644
index 000000000000..cbd3ba7ceab8
--- /dev/null
+++ b/drivers/iio/adc/ad7173.c
@@ -0,0 +1,936 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7172-2/AD7173-8/AD7175-2/AD7176-2 SPI ADC driver
+ * Copyright (C) 2015, 2023 Analog Devices, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/gpio/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#include <linux/iio/adc/ad_sigma_delta.h>
+
+#define AD7173_REG_COMMS 0x00
+#define AD7173_REG_ADC_MODE 0x01
+#define AD7173_REG_INTERFACE_MODE 0x02
+#define AD7173_REG_CRC 0x03
+#define AD7173_REG_DATA 0x04
+#define AD7173_REG_GPIO 0x06
+#define AD7173_REG_ID 0x07
+#define AD7173_REG_CH(x) (0x10 + (x))
+#define AD7173_REG_SETUP(x) (0x20 + (x))
+#define AD7173_REG_FILTER(x) (0x28 + (x))
+#define AD7173_REG_OFFSET(x) (0x30 + (x))
+#define AD7173_REG_GAIN(x) (0x38 + (x))
+
+#define AD7173_RESET_LENGTH BITS_TO_BYTES(64)
+
+#define AD7173_CH_ENABLE BIT(15)
+#define AD7173_CH_SETUP_SEL_MASK GENMASK(14, 12)
+#define AD7173_CH_SETUP_AINPOS_MASK GENMASK(9, 5)
+#define AD7173_CH_SETUP_AINNEG_MASK GENMASK(4, 0)
+
+#define AD7173_CH_ADDRESS(pos, neg) \
+ (FIELD_PREP(AD7173_CH_SETUP_AINPOS_MASK, pos) |\
+ FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg))
+#define AD7173_AIN_TEMP_POS 17
+#define AD7173_AIN_TEMP_NEG 18
+
+#define AD7172_ID 0x00d0
+#define AD7173_ID 0x30d0
+#define AD7175_ID 0x0cd0
+#define AD7176_ID 0x0c90
+#define AD7173_ID_MASK GENMASK(15, 4)
+
+#define AD7173_ADC_MODE_REF_EN BIT(15)
+#define AD7173_ADC_MODE_SING_CYC BIT(13)
+#define AD7173_ADC_MODE_MODE_MASK GENMASK(6, 4)
+#define AD7173_ADC_MODE_CLOCKSEL_MASK GENMASK(3, 2)
+
+#define AD7173_GPIO_PDSW BIT(14)
+#define AD7173_GPIO_OP_EN2_3 BIT(13)
+#define AD7173_GPIO_MUX_IO BIT(12)
+#define AD7173_GPIO_SYNC_EN BIT(11)
+#define AD7173_GPIO_ERR_EN BIT(10)
+#define AD7173_GPIO_ERR_DAT BIT(9)
+#define AD7173_GPIO_GP_DATA3 BIT(7)
+#define AD7173_GPIO_GP_DATA2 BIT(6)
+#define AD7173_GPIO_IP_EN1 BIT(5)
+#define AD7173_GPIO_IP_EN0 BIT(4)
+#define AD7173_GPIO_OP_EN1 BIT(3)
+#define AD7173_GPIO_OP_EN0 BIT(2)
+#define AD7173_GPIO_GP_DATA1 BIT(1)
+#define AD7173_GPIO_GP_DATA0 BIT(0)
+
+#define AD7173_GPO12_DATA(x) BIT(x)
+#define AD7173_GPO23_DATA(x) BIT(x + 4)
+#define AD7173_GPO_DATA(x) (x < 2 ? AD7173_GPO12_DATA(x) : AD7173_GPO23_DATA(x))
+
+#define AD7173_INTERFACE_DATA_STAT BIT(6)
+#define AD7173_INTERFACE_DATA_STAT_EN(x)\
+ FIELD_PREP(AD7173_INTERFACE_DATA_STAT, x)
+
+#define AD7173_SETUP_BIPOLAR BIT(12)
+#define AD7173_SETUP_AREF_BUF_MASK GENMASK(11, 10)
+#define AD7173_SETUP_AIN_BUF_MASK GENMASK(9, 8)
+
+#define AD7173_SETUP_REF_SEL_MASK GENMASK(5, 4)
+#define AD7173_SETUP_REF_SEL_AVDD1_AVSS 0x3
+#define AD7173_SETUP_REF_SEL_INT_REF 0x2
+#define AD7173_SETUP_REF_SEL_EXT_REF2 0x1
+#define AD7173_SETUP_REF_SEL_EXT_REF 0x0
+#define AD7173_VOLTAGE_INT_REF_MICROV 2500000
+
+#define AD7173_FILTER_ODR0_MASK GENMASK(5, 0)
+#define AD7173_MAX_CONFIGS 8
+
+enum ad7173_ids {
+ ID_AD7172_2,
+ ID_AD7173_8,
+ ID_AD7175_2,
+ ID_AD7176_2,
+};
+
+struct ad7173_device_info {
+ char *name;
+ unsigned int id;
+ unsigned int num_inputs;
+ unsigned int num_configs;
+ unsigned int num_channels;
+ unsigned char num_gpios;
+ bool has_temp;
+ unsigned int clock;
+
+ const unsigned int *sinc5_data_rates;
+ unsigned int num_sinc5_data_rates;
+};
+
+struct ad7173_channel_config {
+ bool live;
+ u8 cfg_slot;
+ /* Following fields are used to compare equality. Bipolar must be first */
+ bool bipolar;
+ bool input_buf;
+ u8 odr;
+ u8 ref_sel;
+};
+
+struct ad7173_channel {
+ unsigned int chan_reg;
+ unsigned int ain;
+ struct ad7173_channel_config cfg;
+};
+
+struct ad7173_state {
+ const struct ad7173_device_info *info;
+ struct ad_sigma_delta sd;
+ struct ad7173_channel *channels;
+ struct regulator_bulk_data regulators[3];
+ unsigned int adc_mode;
+ unsigned int interface_mode;
+ unsigned int num_channels;
+ DECLARE_BITMAP(cfg_slots_status, AD7173_MAX_CONFIGS); /* slots usage status */
+ unsigned long long config_usage_counter;
+ unsigned long long *config_cnts;
+#if IS_ENABLED(CONFIG_GPIOLIB)
+ struct regmap *reg_gpiocon_regmap;
+ struct gpio_regmap *gpio_regmap;
+#endif
+};
+
+static const unsigned int ad7173_sinc5_data_rates[] = {
+ 6211000, 6211000, 6211000, 6211000, 6211000, 6211000, 5181000, 4444000,
+ 3115000, 2597000, 1007000, 503800, 381000, 200300, 100500, 59520,
+ 49680, 20010, 16333, 10000, 5000, 2500, 1250,
+};
+
+static const unsigned int ad7175_sinc5_data_rates[] = {
+ 50000000, 41667000, 31250000, 27778000, 20833000, 17857000, 12500000,
+ 10000000, 5000000, 2500000, 1000000, 500000, 397500, 200000,
+ 100000, 59920, 49960, 20000, 16666, 10000, 5000,
+};
+
+static const struct ad7173_device_info ad7173_device_info[] = {
+ [ID_AD7172_2] = {
+ .name = "ad7172-2",
+ .id = AD7172_ID,
+ .num_inputs = 5,
+ .num_channels = 4,
+ .num_configs = 4,
+ .num_gpios = 2,
+ .has_temp = true,
+ .clock = 2 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7173_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+ },
+ [ID_AD7173_8] = {
+ .name = "ad7173-8",
+ .id = AD7173_ID,
+ .num_inputs = 17,
+ .num_channels = 16,
+ .num_configs = 8,
+ .num_gpios = 4,
+ .has_temp = true,
+ .clock = 2 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7173_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
+ },
+ [ID_AD7175_2] = {
+ .name = "ad7175-2",
+ .id = AD7175_ID,
+ .num_inputs = 5,
+ .num_channels = 4,
+ .num_configs = 4,
+ .num_gpios = 2,
+ .has_temp = true,
+ .clock = 16 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7175_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
+ },
+ [ID_AD7176_2] = {
+ .name = "ad7176-2",
+ .id = AD7176_ID,
+ .num_inputs = 5,
+ .num_channels = 4,
+ .num_configs = 4,
+ .num_gpios = 2,
+ .has_temp = false,
+ .clock = 16 * HZ_PER_MHZ,
+ .sinc5_data_rates = ad7175_sinc5_data_rates,
+ .num_sinc5_data_rates = ARRAY_SIZE(ad7175_sinc5_data_rates),
+ },
+};
+
+#if IS_ENABLED(CONFIG_GPIOLIB)
+
+static const struct regmap_range ad7173_range_gpio[] = {
+ regmap_reg_range(AD7173_REG_GPIO, AD7173_REG_GPIO),
+};
+
+static const struct regmap_access_table ad7173_access_table = {
+ .yes_ranges = ad7173_range_gpio,
+ .n_yes_ranges = ARRAY_SIZE(ad7173_range_gpio),
+};
+
+static const struct regmap_config ad7173_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 16,
+ .rd_table = &ad7173_access_table,
+ .wr_table = &ad7173_access_table,
+ .read_flag_mask = BIT(6),
+};
+
+static int ad7173_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
+ unsigned int offset, unsigned int *reg,
+ unsigned int *mask)
+{
+ *mask = AD7173_GPO_DATA(offset);
+ *reg = base;
+ return 0;
+}
+
+static void ad7173_gpio_disable(void *data)
+{
+ struct ad7173_state *st = data;
+ unsigned int mask;
+
+ mask = AD7173_GPIO_OP_EN0 | AD7173_GPIO_OP_EN1 | AD7173_GPIO_OP_EN2_3;
+ regmap_update_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, mask, ~mask);
+}
+
+static int ad7173_gpio_init(struct ad7173_state *st)
+{
+ struct gpio_regmap_config gpio_regmap = {};
+ struct device *dev = &st->sd.spi->dev;
+ unsigned int mask;
+ int ret;
+
+ st->reg_gpiocon_regmap = devm_regmap_init_spi(st->sd.spi, &ad7173_regmap_config);
+ if (IS_ERR(st->reg_gpiocon_regmap)) {
+ return dev_err_probe(dev, PTR_ERR(st->reg_gpiocon_regmap),
+ "Unable to init regmap\n");
+ }
+
+ mask = AD7173_GPIO_OP_EN0 | AD7173_GPIO_OP_EN1 | AD7173_GPIO_OP_EN2_3;
+ regmap_update_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, mask, mask);
+
+ ret = devm_add_action_or_reset(dev, ad7173_gpio_disable, st);
+ if (ret)
+ return ret;
+
+ gpio_regmap.parent = dev;
+ gpio_regmap.regmap = st->reg_gpiocon_regmap;
+ gpio_regmap.ngpio = st->info->num_gpios;
+ gpio_regmap.reg_set_base = AD7173_REG_GPIO;
+ gpio_regmap.reg_mask_xlate = ad7173_mask_xlate;
+
+ st->gpio_regmap = devm_gpio_regmap_register(dev, &gpio_regmap);
+ if (IS_ERR(st->gpio_regmap)) {
+ return dev_err_probe(dev, PTR_ERR(st->gpio_regmap),
+ "Unable to init gpio-regmap\n");
+ }
+
+ return 0;
+}
+
+#endif /* CONFIG_GPIOLIB */
+
+static struct ad7173_state *ad_sigma_delta_to_ad7173(struct ad_sigma_delta *sd)
+{
+ return container_of(sd, struct ad7173_state, sd);
+}
+
+static void ad7173_reset_usage_cnts(struct ad7173_state *st)
+{
+ memset64(st->config_cnts, 0, st->info->num_configs);
+ st->config_usage_counter = 0;
+}
+
+static struct ad7173_channel_config *ad7173_find_live_config
+ (struct ad7173_state *st, struct ad7173_channel_config *cfg)
+{
+ struct ad7173_channel_config *cfg_aux;
+ ptrdiff_t cmp_size, offset;
+ int i;
+
+ offset = offsetof(struct ad7173_channel_config, cfg_slot) +
+ sizeof(cfg->cfg_slot);
+ cmp_size = sizeof(*cfg) - offset;
+
+ for (i = 0; i < st->num_channels; i++) {
+ cfg_aux = &st->channels[i].cfg;
+
+ if (cfg_aux->live && !memcmp(&cfg->bipolar, &cfg_aux->bipolar,
+ cmp_size))
+ return cfg_aux;
+ }
+ return NULL;
+}
+
+static int ad7173_free_config_slot_lru(struct ad7173_state *st)
+{
+ int i, lru_position = 0;
+
+ for (i = 1; i < st->info->num_configs; i++)
+ if (st->config_cnts[i] < st->config_cnts[lru_position])
+ lru_position = i;
+
+ for (i = 0; i < st->num_channels; i++)
+ if (st->channels[i].cfg.cfg_slot == lru_position)
+ st->channels[i].cfg.live = false;
+
+ clear_bit(lru_position, st->cfg_slots_status);
+ return lru_position;
+}
+
+static int ad7173_load_config(struct ad7173_state *st,
+ struct ad7173_channel_config *cfg)
+{
+ unsigned int config;
+ int free_cfg_slot, ret;
+
+ free_cfg_slot = find_first_zero_bit(st->cfg_slots_status,
+ st->info->num_configs);
+ if (free_cfg_slot == st->info->num_configs)
+ free_cfg_slot = ad7173_free_config_slot_lru(st);
+
+ set_bit(free_cfg_slot, st->cfg_slots_status);
+ cfg->cfg_slot = free_cfg_slot;
+
+ config = FIELD_PREP(AD7173_SETUP_REF_SEL_MASK, cfg->ref_sel);
+
+ if (cfg->bipolar)
+ config |= AD7173_SETUP_BIPOLAR;
+
+ if (cfg->input_buf)
+ config |= AD7173_SETUP_AIN_BUF_MASK;
+
+ ret = ad_sd_write_reg(&st->sd, AD7173_REG_SETUP(free_cfg_slot), 2, config);
+ if (ret)
+ return ret;
+
+ return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
+ AD7173_FILTER_ODR0_MASK & cfg->odr);
+}
+
+static int ad7173_config_channel(struct ad7173_state *st, int addr)
+{
+ struct ad7173_channel_config *cfg = &st->channels[addr].cfg;
+ struct ad7173_channel_config *live_cfg;
+ int ret;
+
+ if (!cfg->live) {
+ live_cfg = ad7173_find_live_config(st, cfg);
+ if (live_cfg) {
+ cfg->cfg_slot = live_cfg->cfg_slot;
+ } else {
+ ret = ad7173_load_config(st, cfg);
+ if (ret)
+ return ret;
+ cfg->live = true;
+ }
+ }
+
+ if (st->config_usage_counter == U64_MAX)
+ ad7173_reset_usage_cnts(st);
+
+ st->config_usage_counter++;
+ st->config_cnts[cfg->cfg_slot] = st->config_usage_counter;
+
+ return 0;
+}
+
+static int ad7173_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
+{
+ struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+ unsigned int val;
+ int ret;
+
+ ret = ad7173_config_channel(st, channel);
+ if (ret)
+ return ret;
+
+ val = AD7173_CH_ENABLE |
+ FIELD_PREP(AD7173_CH_SETUP_SEL_MASK, st->channels[channel].cfg.cfg_slot) |
+ st->channels[channel].ain;
+
+ return ad_sd_write_reg(&st->sd, AD7173_REG_CH(channel), 2, val);
+}
+
+static int ad7173_set_mode(struct ad_sigma_delta *sd,
+ enum ad_sigma_delta_mode mode)
+{
+ struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+
+ st->adc_mode &= ~AD7173_ADC_MODE_MODE_MASK;
+ st->adc_mode |= FIELD_PREP(AD7173_ADC_MODE_MODE_MASK, mode);
+
+ return ad_sd_write_reg(&st->sd, AD7173_REG_ADC_MODE, 2, st->adc_mode);
+}
+
+static int ad7173_append_status(struct ad_sigma_delta *sd, bool append)
+{
+ struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+ unsigned int interface_mode = st->interface_mode;
+ int ret;
+
+ interface_mode |= AD7173_INTERFACE_DATA_STAT_EN(append);
+ ret = ad_sd_write_reg(&st->sd, AD7173_REG_INTERFACE_MODE, 2, interface_mode);
+ if (ret)
+ return ret;
+
+ st->interface_mode = interface_mode;
+
+ return 0;
+}
+
+static int ad7173_disable_all(struct ad_sigma_delta *sd)
+{
+ struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+ int ret;
+ int i;
+
+ for (i = 0; i < st->num_channels; i++) {
+ ret = ad_sd_write_reg(sd, AD7173_REG_CH(i), 2, 0);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static struct ad_sigma_delta_info ad7173_sigma_delta_info = {
+ .set_channel = ad7173_set_channel,
+ .append_status = ad7173_append_status,
+ .disable_all = ad7173_disable_all,
+ .set_mode = ad7173_set_mode,
+ .has_registers = true,
+ .addr_shift = 0,
+ .read_mask = BIT(6),
+ .status_ch_mask = GENMASK(3, 0),
+ .data_reg = AD7173_REG_DATA,
+ .irq_flags = IRQF_TRIGGER_FALLING,
+};
+
+static int ad7173_setup(struct iio_dev *indio_dev)
+{
+ struct ad7173_state *st = iio_priv(indio_dev);
+ unsigned int id;
+ u8 buf[AD7173_RESET_LENGTH];
+ int ret;
+
+ /* reset the serial interface */
+ memset(buf, 0xff, AD7173_RESET_LENGTH);
+ ret = spi_write_then_read(st->sd.spi, buf, sizeof(buf), NULL, 0);
+ if (ret < 0)
+ return ret;
+
+ /* datasheet recommends a delay of at least 500us after reset */
+ fsleep(500);
+
+ ret = ad_sd_read_reg(&st->sd, AD7173_REG_ID, 2, &id);
+ if (ret)
+ return ret;
+
+ id &= AD7173_ID_MASK;
+ if (id != st->info->id)
+ dev_warn(&st->sd.spi->dev,
+ "Unexpected device id: %x, expected: %x\n",
+ id, st->info->id);
+
+ st->adc_mode |= AD7173_ADC_MODE_SING_CYC;
+ st->interface_mode = 0x0;
+
+ st->config_usage_counter = 0;
+ st->config_cnts = devm_kcalloc(indio_dev->dev.parent,
+ st->info->num_configs, sizeof(u64),
+ GFP_KERNEL);
+ if (!st->config_cnts)
+ return -ENOMEM;
+
+ /* All channels are enabled by default after a reset */
+ return ad7173_disable_all(&st->sd);
+}
+
+static unsigned int ad7173_get_ref_voltage_milli(struct ad7173_state *st,
+ u8 reference_select)
+{
+ int vref;
+
+ switch (reference_select) {
+ case AD7173_SETUP_REF_SEL_EXT_REF:
+ vref = regulator_get_voltage(st->regulators[0].consumer);
+ break;
+
+ case AD7173_SETUP_REF_SEL_EXT_REF2:
+ vref = regulator_get_voltage(st->regulators[1].consumer);
+ break;
+
+ case AD7173_SETUP_REF_SEL_INT_REF:
+ vref = AD7173_VOLTAGE_INT_REF_MICROV;
+ break;
+
+ case AD7173_SETUP_REF_SEL_AVDD1_AVSS:
+ vref = regulator_get_voltage(st->regulators[2].consumer);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ if (vref < 0)
+ return vref;
+
+ return vref / (MICRO/MILLI);
+}
+
+static int ad7173_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ struct ad7173_state *st = iio_priv(indio_dev);
+ struct ad7173_channel *ch = &st->channels[chan->address];
+ unsigned int reg;
+ int ret;
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ ret = ad_sigma_delta_single_conversion(indio_dev, chan, val);
+ if (ret < 0)
+ return ret;
+
+ /* disable channel after single conversion */
+ ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(chan->address), 2, 0);
+ if (ret < 0)
+ return ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ if (chan->type == IIO_TEMP) {
+ *val = 250000000;
+ *val2 = 800273203; /* (2^24 * 477) / 10 */
+ return IIO_VAL_FRACTIONAL;
+ } else {
+ *val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
+ *val2 = chan->scan_type.realbits - !!(ch->cfg.bipolar);
+ return IIO_VAL_FRACTIONAL_LOG2;
+ }
+ case IIO_CHAN_INFO_OFFSET:
+ if (chan->type == IIO_TEMP) {
+ *val = -874379;
+ } else {
+ if (ch->cfg.bipolar)
+ /* (1<<31) is UB for a 32bit channel */
+ *val = (chan->scan_type.realbits == 32) ?
+ INT_MIN :
+ -(1 << (chan->scan_type.realbits - 1));
+ else
+ *val = 0;
+ }
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ reg = st->channels[chan->address].cfg.odr;
+
+ *val = st->info->sinc5_data_rates[reg] / (MICRO/MILLI);
+ *val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO/MILLI);
+
+ return IIO_VAL_INT_PLUS_MICRO;
+ }
+ return -EINVAL;
+}
+
+static int ad7173_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long info)
+{
+ struct ad7173_state *st = iio_priv(indio_dev);
+ struct ad7173_channel_config *cfg;
+ unsigned int freq, i, reg;
+ int ret;
+
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ return ret;
+
+ switch (info) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ freq = val * MILLI + val2 / MILLI;
+
+ for (i = 0; i < st->info->num_sinc5_data_rates - 1; i++) {
+ if (freq >= st->info->sinc5_data_rates[i])
+ break;
+ }
+
+ cfg = &st->channels[chan->address].cfg;
+ cfg->odr = i;
+
+ if (!cfg->live)
+ break;
+
+ ret = ad_sd_read_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, ®);
+ if (ret)
+ break;
+ reg &= ~AD7173_FILTER_ODR0_MASK;
+ reg |= FIELD_PREP(AD7173_FILTER_ODR0_MASK, i);
+ ret = ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, reg);
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ iio_device_release_direct_mode(indio_dev);
+ return ret;
+}
+
+static int ad7173_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct ad7173_state *st = iio_priv(indio_dev);
+ int i, ret = 0;
+
+ for (i = 0; i < indio_dev->num_channels; i++) {
+ if (test_bit(i, scan_mask))
+ ret = ad7173_set_channel(&st->sd, i);
+ else
+ ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(i), 2, 0);
+
+ if (ret < 0)
+ return ret;
+ }
+
+ return ret;
+}
+
+static int ad7173_debug(struct iio_dev *indio_dev, unsigned int reg,
+ unsigned int writeval, unsigned int *readval)
+{
+ struct ad7173_state *st = iio_priv(indio_dev);
+ u8 reg_size;
+
+ if (reg == 0)
+ reg_size = 1;
+ else if (reg == AD7173_REG_CRC || reg == AD7173_REG_DATA ||
+ reg >= AD7173_REG_OFFSET(0))
+ reg_size = 3;
+ else
+ reg_size = 2;
+
+ if (readval)
+ return ad_sd_read_reg(&st->sd, reg, reg_size, readval);
+
+ return ad_sd_write_reg(&st->sd, reg, reg_size, writeval);
+}
+
+static const struct iio_info ad7173_info = {
+ .read_raw = &ad7173_read_raw,
+ .write_raw = &ad7173_write_raw,
+ .debugfs_reg_access = &ad7173_debug,
+ .validate_trigger = ad_sd_validate_trigger,
+ .update_scan_mode = ad7173_update_scan_mode,
+};
+
+static const struct iio_chan_spec ad7173_channel_template = {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 24,
+ .storagebits = 32,
+ .endianness = IIO_BE,
+ },
+};
+
+static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
+ .type = IIO_TEMP,
+ .indexed = 1,
+ .channel = AD7173_AIN_TEMP_POS,
+ .channel2 = AD7173_AIN_TEMP_NEG,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 24,
+ .storagebits = 32,
+ .endianness = IIO_BE,
+ },
+};
+
+static void ad7173_disable_regulators(void *data)
+{
+ struct ad7173_state *st = data;
+
+ regulator_bulk_disable(ARRAY_SIZE(st->regulators), st->regulators);
+}
+
+static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
+{
+ struct ad7173_state *st = iio_priv(indio_dev);
+ struct ad7173_channel *channels_st_priv_arr, *chan_st_priv;
+ struct fwnode_handle *child;
+ struct device *dev = indio_dev->dev.parent;
+ struct iio_chan_spec *chan_arr, *chan;
+ unsigned int num_channels;
+ unsigned int ain[2], chan_index = 0;
+ u32 ref_sel;
+ int ret;
+
+ st->regulators[0].supply = "refin";
+ st->regulators[1].supply = "refin2";
+ st->regulators[2].supply = "avdd";
+
+ ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st->regulators),
+ st->regulators);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(st->regulators), st->regulators);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable regulators\n");
+
+ ret = devm_add_action_or_reset(dev, ad7173_disable_regulators, st);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to add regulators disable action\n");
+
+ num_channels = device_get_child_node_count(dev);
+
+ if (st->info->has_temp)
+ num_channels++;
+
+ if (num_channels == 0)
+ return 0;
+ st->num_channels = num_channels;
+
+ chan_arr = devm_kcalloc(dev, sizeof(*chan_arr), num_channels,
+ GFP_KERNEL);
+ if (!chan_arr)
+ return -ENOMEM;
+
+ channels_st_priv_arr = devm_kcalloc(dev, num_channels,
+ sizeof(*channels_st_priv_arr),
+ GFP_KERNEL);
+ if (!channels_st_priv_arr)
+ return -ENOMEM;
+
+ indio_dev->channels = chan_arr;
+ indio_dev->num_channels = num_channels;
+ st->channels = channels_st_priv_arr;
+
+ if (st->info->has_temp) {
+ chan_arr[chan_index] = ad7173_temp_iio_channel_template;
+ chan_st_priv = &channels_st_priv_arr[chan_index];
+ chan_st_priv->ain =
+ AD7173_CH_ADDRESS(chan_arr[chan_index].channel, chan_arr[chan_index].channel2);
+ chan_st_priv->cfg.bipolar = false;
+ chan_st_priv->cfg.input_buf = true;
+ chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
+ st->adc_mode |= AD7173_ADC_MODE_REF_EN;
+
+ chan_index++;
+ }
+
+ device_for_each_child_node(dev, child) {
+ chan = &chan_arr[chan_index];
+ chan_st_priv = &channels_st_priv_arr[chan_index];
+ ret = fwnode_property_read_u32_array(child, "diff-channels",
+ ain, ARRAY_SIZE(ain));
+ if (ret) {
+ fwnode_handle_put(child);
+ return ret;
+ }
+
+ if (ain[0] >= st->info->num_inputs ||
+ ain[1] >= st->info->num_inputs) {
+ fwnode_handle_put(child);
+ return dev_err_probe(dev, -EINVAL,
+ "Input pin number out of range for pair (%d %d).", ain[0], ain[1]);
+ }
+
+ if (fwnode_property_read_u32(child, "adi,reference-select", &ref_sel))
+ ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
+ ret = ad7173_get_ref_voltage_milli(st, (u8)ref_sel);
+ if (ret < 0)
+ return dev_err_probe(dev, ret,
+ "Cannot use reference %u", ref_sel);
+ if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 &&
+ st->info->id != AD7173_ID)
+ return dev_err_probe(dev, -EINVAL, "External reference 2 is only available on ad7173-8");
+ if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
+ st->adc_mode |= AD7173_ADC_MODE_REF_EN;
+ chan_st_priv->cfg.ref_sel = ref_sel;
+
+ *chan = ad7173_channel_template;
+ chan->address = chan_index;
+ chan->scan_index = chan_index;
+ chan->channel = ain[0];
+ chan->channel2 = ain[1];
+ chan->differential = true;
+
+ chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
+ chan_st_priv->chan_reg = chan_index;
+ chan_st_priv->cfg.input_buf = true;
+ chan_st_priv->cfg.odr = 0;
+
+ chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
+ if (chan_st_priv->cfg.bipolar)
+ chan->info_mask_separate |= BIT(IIO_CHAN_INFO_OFFSET);
+
+ chan_index++;
+ }
+
+ return 0;
+}
+
+static int ad7173_probe(struct spi_device *spi)
+{
+ struct ad7173_state *st;
+ struct iio_dev *indio_dev;
+ struct device *dev = &spi->dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ st->info = spi_get_device_match_data(spi);
+ if (!st->info)
+ return -ENODEV;
+
+ indio_dev->name = st->info->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &ad7173_info;
+
+ spi->mode = SPI_MODE_3;
+
+ ad7173_sigma_delta_info.num_slots = st->info->num_configs;
+ ret = ad_sd_init(&st->sd, indio_dev, spi, &ad7173_sigma_delta_info);
+ if (ret)
+ return ret;
+
+ spi_set_drvdata(spi, indio_dev);
+
+ ret = ad7173_fw_parse_channel_config(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = devm_ad_sd_setup_buffer_and_trigger(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ ret = ad7173_setup(indio_dev);
+ if (ret)
+ return ret;
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ if (IS_ENABLED(CONFIG_GPIOLIB))
+ return ad7173_gpio_init(st);
+
+ return 0;
+}
+
+static const struct of_device_id ad7173_of_match[] = {
+ { .compatible = "adi,ad7172-2",
+ .data = &ad7173_device_info[ID_AD7172_2], },
+ { .compatible = "adi,ad7173-8",
+ .data = &ad7173_device_info[ID_AD7173_8], },
+ { .compatible = "adi,ad7175-2",
+ .data = &ad7173_device_info[ID_AD7175_2], },
+ { .compatible = "adi,ad7176-2",
+ .data = &ad7173_device_info[ID_AD7176_2], },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad7173_of_match);
+
+static const struct spi_device_id ad7173_id_table[] = {
+ { "ad7172-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7172_2], },
+ { "ad7173-8", (kernel_ulong_t)&ad7173_device_info[ID_AD7173_8], },
+ { "ad7175-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7175_2], },
+ { "ad7176-2", (kernel_ulong_t)&ad7173_device_info[ID_AD7176_2], },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, ad7173_id_table);
+
+static struct spi_driver ad7173_driver = {
+ .driver = {
+ .name = "ad7173",
+ .of_match_table = ad7173_of_match,
+ },
+ .probe = ad7173_probe,
+ .id_table = ad7173_id_table,
+};
+module_spi_driver(ad7173_driver);
+
+MODULE_AUTHOR("Lars-Peter Clausen <[email protected]>");
+MODULE_AUTHOR("Dumitru Ceclan <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD7172/AD7173/AD7175/AD7176 ADC driver");
+MODULE_LICENSE("GPL");
--
2.42.0
On Thu, 16 Nov 2023 15:46:54 +0200, mitrutzceclan wrote:
> From: Dumitru Ceclan <[email protected]>
>
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> Reviewed-by: Conor Dooley <[email protected]> # except reference_select
> Signed-off-by: Dumitru Ceclan <[email protected]>
> ---
> V3 -> V4
> - include supply attributes
> - add channel attribute for selecting conversion reference
>
> .../bindings/iio/adc/adi,ad7173.yaml | 166 ++++++++++++++++++
> 1 file changed, 166 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml:109:10: [error] empty value in block mapping (empty-values)
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:then: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:else:patternProperties:^channel@[0-9a-f]$:properties:enum: [0, 2, 3] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:else:patternProperties:^channel@[0-9a-f]$:properties: 'enum' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:else:patternProperties:^channel@[0-9a-f]$:properties:enum: [0, 2, 3] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties: 'dependencies' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties: 'required' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:dependencies: 'anyOf' conditional failed, one must be fixed:
'refin2-supply' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
'type' was expected
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: dependencies: missing type definition
Traceback (most recent call last):
File "/usr/local/bin/dt-validate", line 8, in <module>
sys.exit(main())
^^^^^^
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 144, in main
sg.check_dtb(filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 89, in check_dtb
self.check_subtree(dt, subtree, False, "/", "/", filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 82, in check_subtree
self.check_subtree(tree, value, disabled, name, fullname + name, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 82, in check_subtree
self.check_subtree(tree, value, disabled, name, fullname + name, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 82, in check_subtree
self.check_subtree(tree, value, disabled, name, fullname + name, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 77, in check_subtree
self.check_node(tree, subtree, disabled, nodename, fullname, filename)
File "/usr/local/lib/python3.11/dist-packages/dtschema/dtb_validate.py", line 33, in check_node
for error in self.validator.iter_errors(node, filter=match_schema_file):
File "/usr/local/lib/python3.11/dist-packages/dtschema/validator.py", line 393, in iter_errors
for error in self.DtValidator(sch,
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
for error in errors:
File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 414, in if_
yield from validator.descend(instance, then, schema_path="then")
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
for error in self.evolve(schema=schema).iter_errors(instance):
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
for error in errors:
File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 362, in allOf
yield from validator.descend(instance, subschema, schema_path=index)
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
for error in self.evolve(schema=schema).iter_errors(instance):
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
for error in errors:
File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 414, in if_
yield from validator.descend(instance, then, schema_path="then")
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
for error in self.evolve(schema=schema).iter_errors(instance):
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 239, in evolve
NewValidator = validator_for(schema, default=cls)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 1148, in validator_for
if schema is True or schema is False or "$schema" not in schema:
^^^^^^^^^^^^^^^^^^^^^^^
TypeError: argument of type 'NoneType' is not iterable
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231116134655.21052-1-user@HYB-hhAwRlzzMZb
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 16/11/2023 14:46, mitrutzceclan wrote:
> From: Dumitru Ceclan <[email protected]>
>
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> Reviewed-by: Conor Dooley <[email protected]> # except reference_select
Please drop the tag. You clearly did not test it, so it must be
re-reviewed. Do not send code which was not tested.
> Signed-off-by: Dumitru Ceclan <[email protected]>
> ---
> V3 -> V4
> - include supply attributes
> - add channel attribute for selecting conversion reference
>
> .../bindings/iio/adc/adi,ad7173.yaml | 166 ++++++++++++++++++
> 1 file changed, 166 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> new file mode 100644
> index 000000000000..92aa352b6653
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -0,0 +1,166 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7173 ADC device driver
Drop: device driver
Bindings are for hardware.
> +
> +maintainers:
> + - Ceclan Dumitru <[email protected]>
> +
> +description: |
> + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad7172-2
> + - adi,ad7173-8
> + - adi,ad7175-2
> + - adi,ad7176-2
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + spi-max-frequency:
> + maximum: 20000000
> +
> + refin-supply:
> + description: external reference supply, can be used as reference for conversion.
> +
> + refin2-supply:
> + description: external reference supply, can be used as reference for conversion.
> +
> + avdd-supply:
> + description: avdd supply, can be used as reference for conversion.
> +
> + dependencies:
Nope, needs testing... See also example-schema.
> + refin2-supply:
> + properties:
> + compatible:
> + adi,ad7173-8
> +
> + required:
Please open example schema and put it in similar place.
> + - compatible
> + - reg
> + - interrupts
> +
> +patternProperties:
> + "^channel@[0-9a-f]$":
> + type: object
> + $ref: adc.yaml
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 15
> +
> + diff-channels:
> + items:
> + minimum: 0
> + maximum: 31
> +
> + adi,reference-select:
> + description: |
> + Select the reference source to use when converting on
> + the specific channel. Valid values are:
> + 0: REFIN(+)/REFIN(−).
> + 1: REFIN2(+)/REFIN2(−)
> + 2: REFOUT/AVSS (Internal reference)
> + 3: AVDD
> +
> + External reference 2 only available on ad7173-8.
> + If not specified, internal reference used.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 2
> +
> + bipolar:
> + type: boolean
> +
> + required:
> + - reg
> + - diff-channels
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: adi,ad7173-8
> + then:
??? Maybe you want to use "not"?
> + else:
> + patternProperties:
> + "^channel@[0-9a-f]$":
> + properties:
> + enum: [0, 2, 3]
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof
On Thu, Nov 16, 2023 at 03:54:13PM +0100, Krzysztof Kozlowski wrote:
> On 16/11/2023 14:46, mitrutzceclan wrote:
> > From: Dumitru Ceclan <[email protected]>
> >
> > The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> > which can be used in high precision, low noise single channel applications
> > or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> > primarily for measurement of signals close to DC but also delivers
> > outstanding performance with input bandwidths out to ~10kHz.
> >
> > Reviewed-by: Conor Dooley <[email protected]> # except reference_select
>
> Please drop the tag. You clearly did not test it, so it must be
> re-reviewed. Do not send code which was not tested.
yeah, this is vastly different from what I reviewed. I suppose if
someone finds it necessary to add a "# except foo" to the end of a tag
it is very good signifier that the tag should in fact be removed.
>
> > Signed-off-by: Dumitru Ceclan <[email protected]>
> > ---
> > V3 -> V4
> > - include supply attributes
> > - add channel attribute for selecting conversion reference
> >
> > .../bindings/iio/adc/adi,ad7173.yaml | 166 ++++++++++++++++++
> > 1 file changed, 166 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> > new file mode 100644
> > index 000000000000..92aa352b6653
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> > @@ -0,0 +1,166 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2023 Analog Devices Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD7173 ADC device driver
>
> Drop: device driver
>
> Bindings are for hardware.
>
> > +
> > +maintainers:
> > + - Ceclan Dumitru <[email protected]>
> > +
> > +description: |
> > + Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips:
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
> > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - adi,ad7172-2
> > + - adi,ad7173-8
> > + - adi,ad7175-2
> > + - adi,ad7176-2
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + spi-max-frequency:
> > + maximum: 20000000
> > +
> > + refin-supply:
> > + description: external reference supply, can be used as reference for conversion.
> > +
> > + refin2-supply:
> > + description: external reference supply, can be used as reference for conversion.
> > +
> > + avdd-supply:
> > + description: avdd supply, can be used as reference for conversion.
> > +
> > + dependencies:
>
> Nope, needs testing... See also example-schema.
>
>
> > + refin2-supply:
> > + properties:
> > + compatible:
> > + adi,ad7173-8
> > +
> > + required:
>
> Please open example schema and put it in similar place.
>
> > + - compatible
> > + - reg
> > + - interrupts
> > +
> > +patternProperties:
> > + "^channel@[0-9a-f]$":
> > + type: object
> > + $ref: adc.yaml
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + reg:
> > + minimum: 0
> > + maximum: 15
> > +
> > + diff-channels:
> > + items:
> > + minimum: 0
> > + maximum: 31
> > +
> > + adi,reference-select:
> > + description: |
> > + Select the reference source to use when converting on
> > + the specific channel. Valid values are:
> > + 0: REFIN(+)/REFIN(−).
> > + 1: REFIN2(+)/REFIN2(−)
> > + 2: REFOUT/AVSS (Internal reference)
> > + 3: AVDD
> > +
> > + External reference 2 only available on ad7173-8.
> > + If not specified, internal reference used.
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0, 1, 2, 3]
> > + default: 2
I really don't like these properties where integers are mapped to
functionalities like this. I'd much rather see a enum of strings where
the meaning for these things can be put in & there's no need to look up
the binding to figure out what "adi,reference-select = <3>" means.
For example having "refin", "refin2", "refout-avss" & "avdd" as the
options and therefore "adi,reference-select = "avdd" in a devicetree is
a lot more understandable IMO.
Cheers,
Conor.
> > +
> > + bipolar:
> > + type: boolean
> > +
> > + required:
> > + - reg
> > + - diff-channels
> > +
> > +allOf:
> > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: adi,ad7173-8
> > + then:
>
> ??? Maybe you want to use "not"?
>
> > + else:
>
> > + patternProperties:
> > + "^channel@[0-9a-f]$":
> > + properties:
> > + enum: [0, 2, 3]
> > +
> > +unevaluatedProperties: false
> > +
>
> Best regards,
> Krzysztof
>
Hi Dumitru,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.7-rc1 next-20231117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/mitrutzceclan/iio-adc-ad7173-add-AD7173-driver/20231116-214919
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20231116134655.21052-1-user%40HYB-hhAwRlzzMZb
patch subject: [PATCH v4 1/2] dt-bindings: adc: add AD7173
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231117/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/
dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml:109:10: [error] empty value in block mapping (empty-values)
--
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:then: None is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:else:patternProperties:^channel@[0-9a-f]$:properties:enum: [0, 2, 3] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:else:patternProperties:^channel@[0-9a-f]$:properties: 'enum' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: allOf:1:else:patternProperties:^channel@[0-9a-f]$:properties:enum: [0, 2, 3] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties: 'dependencies' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties: 'required' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
hint: A json-schema keyword was found instead of a DT property name.
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:dependencies: 'anyOf' conditional failed, one must be fixed:
'refin2-supply' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
'type' was expected
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: properties:required: ['compatible', 'reg', 'interrupts'] is not of type 'object', 'boolean'
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
--
/usr/local/lib/python3.11/dist-packages/dtschema/schemas/reserved-memory/framebuffer.yaml: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#'
/usr/local/lib/python3.11/dist-packages/dtschema/schemas/reserved-memory/memory-region.yaml: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/memory-region.yaml#'
/usr/local/lib/python3.11/dist-packages/dtschema/schemas/reserved-memory/reserved-memory.yaml: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/reserved-memory.yaml#'
/usr/local/lib/python3.11/dist-packages/dtschema/schemas/reserved-memory/shared-dma-pool.yaml: warning: ignoring duplicate '$id' value 'http://devicetree.org/schemas/reserved-memory/shared-dma-pool.yaml#'
Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml: i2c-alias: missing type definition
>> Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml: dependencies: missing type definition
Documentation/devicetree/bindings/sound/audio-graph.yaml: convert-sample-format: missing type definition
Documentation/devicetree/bindings/serial/8250_omap.yaml: rs485-rts-active-high: missing type definition
Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml: dual-lvds-odd-pixels: missing type definition
Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml: dual-lvds-even-pixels: missing type definition
vim +109 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 @1 # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 2 # Copyright 2023 Analog Devices Inc.
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 3 %YAML 1.2
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 4 ---
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 5 $id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 6 $schema: http://devicetree.org/meta-schemas/core.yaml#
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 7
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 8 title: Analog Devices AD7173 ADC device driver
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 9
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 10 maintainers:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 11 - Ceclan Dumitru <[email protected]>
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 12
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 13 description: |
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 14 Bindings for the Analog Devices AD717X ADC's. Datasheets for supported chips:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 15 https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 16 https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 17 https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 18 https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 19
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 20 properties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 21 compatible:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 22 enum:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 23 - adi,ad7172-2
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 24 - adi,ad7173-8
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 25 - adi,ad7175-2
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 26 - adi,ad7176-2
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 27
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 28 reg:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 29 maxItems: 1
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 30
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 31 interrupts:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 32 maxItems: 1
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 33
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 34 '#address-cells':
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 35 const: 1
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 36
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 37 '#size-cells':
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 38 const: 0
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 39
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 40 spi-max-frequency:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 41 maximum: 20000000
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 42
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 43 refin-supply:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 44 description: external reference supply, can be used as reference for conversion.
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 45
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 46 refin2-supply:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 47 description: external reference supply, can be used as reference for conversion.
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 48
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 49 avdd-supply:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 50 description: avdd supply, can be used as reference for conversion.
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 51
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 52 dependencies:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 53 refin2-supply:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 54 properties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 55 compatible:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 56 adi,ad7173-8
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 57
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 58 required:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 59 - compatible
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 60 - reg
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 61 - interrupts
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 62
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 63 patternProperties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 64 "^channel@[0-9a-f]$":
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 65 type: object
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 66 $ref: adc.yaml
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 67 unevaluatedProperties: false
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 68
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 69 properties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 70 reg:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 71 minimum: 0
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 72 maximum: 15
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 73
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 74 diff-channels:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 75 items:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 76 minimum: 0
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 77 maximum: 31
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 78
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 79 adi,reference-select:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 80 description: |
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 81 Select the reference source to use when converting on
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 82 the specific channel. Valid values are:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 83 0: REFIN(+)/REFIN(−).
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 84 1: REFIN2(+)/REFIN2(−)
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 85 2: REFOUT/AVSS (Internal reference)
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 86 3: AVDD
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 87
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 88 External reference 2 only available on ad7173-8.
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 89 If not specified, internal reference used.
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 90 $ref: /schemas/types.yaml#/definitions/uint32
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 91 enum: [0, 1, 2, 3]
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 92 default: 2
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 93
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 94 bipolar:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 95 type: boolean
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 96
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 97 required:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 98 - reg
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 99 - diff-channels
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 100
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 101 allOf:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 102 - $ref: /schemas/spi/spi-peripheral-props.yaml#
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 103
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 104 - if:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 105 properties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 106 compatible:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 107 contains:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 108 const: adi,ad7173-8
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 @109 then:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 110 else:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 111 patternProperties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 112 "^channel@[0-9a-f]$":
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 113 properties:
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 114 enum: [0, 2, 3]
6817f96dd6ee22 Dumitru Ceclan 2023-11-16 115
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Nov 16, 2023 at 03:46:55PM +0200, mitrutzceclan wrote:
> From: Dumitru Ceclan <[email protected]>
>
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel
> applications or higher speed multiplexed applications. The Sigma-Delta
> ADC is intended primarily for measurement of signals close to DC but also
> delivers outstanding performance with input bandwidths out to ~10kHz.
...
> + help
> + Say yes here to build support for Analog Devices AD7173 and similar ADC
> + (currently supported: AD7172-2, AD7173-8, AD7175-2, AD7176-2).
This is hard to maintain, list it one model per a single line.
> + To compile this driver as a module, choose M here: the module will be
> + called ad7173.
...
+ array_size.h
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/bits.h>
This is guaranteed to be included by one from the above (don't remember
by heart which one or even both).
+ container_of.h
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
How is this being used (as not a proxy)?
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/units.h>
...
> +#define AD7173_CH_ADDRESS(pos, neg) \
> + (FIELD_PREP(AD7173_CH_SETUP_AINPOS_MASK, pos) |\
Space before \ here and everywhere else in multi-line definitions.
> + FIELD_PREP(AD7173_CH_SETUP_AINNEG_MASK, neg))
...
> +#define AD7173_VOLTAGE_INT_REF_MICROV 2500000
MICROV --> uV (yes, with small letter), it's a common use for Amperes, Volts,
etc.
...
> +struct ad7173_channel_config {
> + bool live;
> + u8 cfg_slot;
> + /* Following fields are used to compare equality. Bipolar must be first */
> + bool bipolar;
> + bool input_buf;
> + u8 odr;
> + u8 ref_sel;
If you group better by types, it might save a few bytes on the architectures /
compilers where bool != byte.
> +};
...
> + st->reg_gpiocon_regmap = devm_regmap_init_spi(st->sd.spi, &ad7173_regmap_config);
> + if (IS_ERR(st->reg_gpiocon_regmap)) {
> + return dev_err_probe(dev, PTR_ERR(st->reg_gpiocon_regmap),
> + "Unable to init regmap\n");
> + }
{} are not needed, can also be written as
st->reg_gpiocon_regmap = devm_regmap_init_spi(st->sd.spi, &ad7173_regmap_config);
ret = PTR_ERR_OR_ZERO(st->reg_gpiocon_regmap);
if (ret)
return dev_err_probe(dev, ret, "Unable to init regmap\n");
...
> + st->gpio_regmap = devm_gpio_regmap_register(dev, &gpio_regmap);
> + if (IS_ERR(st->gpio_regmap)) {
> + return dev_err_probe(dev, PTR_ERR(st->gpio_regmap),
> + "Unable to init gpio-regmap\n");
> + }
Ditto.
...
> +static struct ad7173_channel_config *ad7173_find_live_config
> + (struct ad7173_state *st, struct ad7173_channel_config *cfg)
This is strange indentation.
Perhaps
static struct ad7173_channel_config *
ad7173_find_live_config(struct ad7173_state *st, struct ad7173_channel_config *cfg)
?
...
> + offset = offsetof(struct ad7173_channel_config, cfg_slot) +
> + sizeof(cfg->cfg_slot);
Isn't it a offsetofend() from stddef.h?
> + cmp_size = sizeof(*cfg) - offset;
sizeof_field() from the above mentioned header?
...
> + for (i = 0; i < st->num_channels; i++) {
> + cfg_aux = &st->channels[i].cfg;
> +
> + if (cfg_aux->live && !memcmp(&cfg->bipolar, &cfg_aux->bipolar,
> + cmp_size))
I would split this on logic operator, it will be easier to read.
> + return cfg_aux;
> + }
...
> + free_cfg_slot = find_first_zero_bit(st->cfg_slots_status,
> + st->info->num_configs);
> + if (free_cfg_slot == st->info->num_configs)
> + free_cfg_slot = ad7173_free_config_slot_lru(st);
> +
> + set_bit(free_cfg_slot, st->cfg_slots_status);
> + cfg->cfg_slot = free_cfg_slot;
Looks like reinvention of IDA.
...
> + struct ad7173_state *st = iio_priv(indio_dev);
> + unsigned int id;
> + u8 buf[AD7173_RESET_LENGTH];
> + int ret;
Reversed xmas tree order?
struct ad7173_state *st = iio_priv(indio_dev);
u8 buf[AD7173_RESET_LENGTH];
unsigned int id;
int ret;
...
> + return vref / (MICRO/MILLI);
What does the denominator mean and why you can't simply use MILL?
...
> + if (ch->cfg.bipolar)
> + /* (1<<31) is UB for a 32bit channel */
> + *val = (chan->scan_type.realbits == 32) ?
> + INT_MIN :
> + -(1 << (chan->scan_type.realbits - 1));
So, what's the issue to use BIT() which has no such issue with UB?
> + else
> + *val = 0;
...
> + *val = st->info->sinc5_data_rates[reg] / (MICRO/MILLI);
> + *val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO/MILLI);
Same Q about denominator.
...
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + freq = val * MILLI + val2 / MILLI;
> +
Unneeded blank line.
> + for (i = 0; i < st->info->num_sinc5_data_rates - 1; i++) {
> + if (freq >= st->info->sinc5_data_rates[i])
> + break;
> + }
> +
> + cfg = &st->channels[chan->address].cfg;
> + cfg->odr = i;
> +
> + if (!cfg->live)
> + break;
> +
> + ret = ad_sd_read_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, ®);
> + if (ret)
> + break;
> + reg &= ~AD7173_FILTER_ODR0_MASK;
> + reg |= FIELD_PREP(AD7173_FILTER_ODR0_MASK, i);
> + ret = ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(cfg->cfg_slot), 2, reg);
> + break;
...
> +static int ad7173_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct ad7173_state *st = iio_priv(indio_dev);
> + int i, ret = 0;
Use the 0 directly...
> +
> + for (i = 0; i < indio_dev->num_channels; i++) {
> + if (test_bit(i, scan_mask))
> + ret = ad7173_set_channel(&st->sd, i);
> + else
> + ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(i), 2, 0);
> +
> + if (ret < 0)
> + return ret;
> + }
> +
> + return ret;
...here.
> +}
> + chan_arr = devm_kcalloc(dev, sizeof(*chan_arr), num_channels,
> + GFP_KERNEL);
One line.
> + if (!chan_arr)
> + return -ENOMEM;
...
> + if (fwnode_property_read_u32(child, "adi,reference-select", &ref_sel))
> + ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
if is redundant.
ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
fwnode_property_read_u32(child, "adi,reference-select", &ref_sel);
--
With Best Regards,
Andy Shevchenko
On 11/20/23 15:00, Andy Shevchenko wrote:
> On Thu, Nov 16, 2023 at 03:46:55PM +0200, mitrutzceclan wrote:
>> +struct ad7173_channel_config {
>> + bool live;
>> + u8 cfg_slot;
>> + /* Following fields are used to compare equality. Bipolar must be first */
>> + bool bipolar;
>> + bool input_buf;
>> + u8 odr;
>> + u8 ref_sel;
>
> If you group better by types, it might save a few bytes on the architectures /
> compilers where bool != byte.
>
Grouping by type will result in not being able to use memcmp() for
comparing configs. But then there is the issue that I was under the
assumption that bool=byte. If that is not the case, the config equality
check might be comparing padding bytes.
In this case what do you suggest:
- using the packed attribute
- using only u8
- drop memcmp, manually compare fields
...
>> + cmp_size = sizeof(*cfg) - offset;
>
> sizeof_field() from the above mentioned header?
This computes the size of multiple fields, following cfg_slot. Better to
group the fields that need to be compared into another struct then use
sizeof_field()?
...
>
>> + return vref / (MICRO/MILLI);
>
> What does the denominator mean and why you can't simply use MILL?
>
Original vref values are in micro, I considered that it was adequate to
represent the conversion from MICRO to MILLI as a fraction.
>> + *val = st->info->sinc5_data_rates[reg] / (MICRO/MILLI);
>> + *val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO/MILLI);
>
> Same Q about denominator.
>
Here, a misunderstanding on my part of a suggestion from Jonathan in V2,
will be removed.
On Mon, Nov 20, 2023 at 05:55:12PM +0200, Ceclan Dumitru wrote:
> On 11/20/23 15:00, Andy Shevchenko wrote:
> > On Thu, Nov 16, 2023 at 03:46:55PM +0200, mitrutzceclan wrote:
...
> >> +struct ad7173_channel_config {
> >> + bool live;
> >> + u8 cfg_slot;
> >> + /* Following fields are used to compare equality. Bipolar must be first */
> >> + bool bipolar;
> >> + bool input_buf;
> >> + u8 odr;
> >> + u8 ref_sel;
> >
> > If you group better by types, it might save a few bytes on the architectures /
> > compilers where bool != byte.
> >
> Grouping by type will result in not being able to use memcmp() for
> comparing configs. But then there is the issue that I was under the
> assumption that bool=byte. If that is not the case, the config equality
> check might be comparing padding bytes.
It's most likely the case, BUT... it is not guaranteed by the C standard.
> In this case what do you suggest:
> - using the packed attribute
> - using only u8
> - drop memcmp, manually compare fields
Use struct_group() to show explicitly the group of members. If it's not an ABI
in any sense, then memcmp() is fine.
...
> >> + cmp_size = sizeof(*cfg) - offset;
> >
> > sizeof_field() from the above mentioned header?
>
> This computes the size of multiple fields, following cfg_slot. Better to
> group the fields that need to be compared into another struct then use
> sizeof_field()?
See above.
...
> >> + return vref / (MICRO/MILLI);
> >
> > What does the denominator mean and why you can't simply use MILL?
>
> Original vref values are in micro, I considered that it was adequate to
> represent the conversion from MICRO to MILLI as a fraction.
>
> >> + *val = st->info->sinc5_data_rates[reg] / (MICRO/MILLI);
> >> + *val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO/MILLI);
> >
> > Same Q about denominator.
> >
> Here, a misunderstanding on my part of a suggestion from Jonathan in V2,
> will be removed.
You need to clarify with him that.
--
With Best Regards,
Andy Shevchenko
On 11/20/23 18:21, Andy Shevchenko wrote:> On Mon, Nov 20, 2023 at
05:55:12PM +0200, Ceclan Dumitru wrote:
>> On 11/20/23 15:00, Andy Shevchenko wrote:
>>> On Thu, Nov 16, 2023 at 03:46:55PM +0200, mitrutzceclan wrote:
>
> Use struct_group() to show explicitly the group of members. If it's not an ABI in any sense, then memcmp() is fine.
>
Alright
>>>> + return vref / (MICRO/MILLI);
>>>
>>> What does the denominator mean and why you can't simply use MILL?
>>
>> Original vref values are in micro, I considered that it was adequate
>> to represent the conversion from MICRO to MILLI as a fraction.
>>
>>>> + *val = st->info->sinc5_data_rates[reg] / (MICRO/MILLI);
>>>> + *val2 = (st->info->sinc5_data_rates[reg] % MILLI) *
>>>> +(MICRO/MILLI);
>>>
>>> Same Q about denominator.
>>>
>> Here, a misunderstanding on my part of a suggestion from Jonathan in
>> V2, will be removed.
>
> You need to clarify with him that.
>
My misunderstanding was that he only referred to the multiplication, not
the denominator. *val has a conversion from MILLI to no metric prefix
while *val2 converts from MILLI to MICRO.