| Message ID | 20230110160124.3853593-2-hugo@hugovil.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: adc: ti-ads7924: add Texas Instruments ADS7924 driver | expand |
Le 10/01/2023 à 17:01, Hugo Villeneuve a écrit : > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > digital converter (ADC) with an I2C interface. > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- Hi, should there be a v3, a few nits below. CJ > MAINTAINERS | 7 + > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ti-ads7924.c | 492 +++++++++++++++++++++++++++++++++++ > 4 files changed, 511 insertions(+) > create mode 100644 drivers/iio/adc/ti-ads7924.c > [...] > +static int ads7924_get_channels_config(struct i2c_client *client, > + struct iio_dev *indio_dev) > +{ > + struct ads7924_data *priv = iio_priv(indio_dev); > + struct device *dev = priv->dev; > + struct fwnode_handle *node; > + int num_channels = 0; > + > + device_for_each_child_node(dev, node) { > + u32 pval; > + unsigned int channel; > + > + if (fwnode_property_read_u32(node, "reg", &pval)) { > + dev_err(dev, "invalid reg on %pfw\n", node); > + continue; > + } > + > + channel = pval; > + if (channel >= ADS7924_CHANNELS) { > + dev_err(dev, "invalid channel index %d on %pfw\n", > + channel, node); > + continue; > + } > + > + num_channels++; > + } > + > + if (num_channels > 0) { > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > + return 0; > + } else { > + return -EINVAL; > + } if (num_channels <= 0) return -EINVAL; dev_dbg(dev, "found %d ADC channels\n", num_channels); return 0; is much more usual. > +} > + [...] > +static int ads7924_reset(struct iio_dev *indio_dev) > +{ > + struct ads7924_data *data = iio_priv(indio_dev); > + > + if (data->reset_gpio) { > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > + /* Educated guess: assert time not specified in datasheet... */ > + mdelay(100); > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ > + } else { > + int ret; having 'ret' near 'struct ads7924_data *data' is more usual and saves 1 LoC. > + > + /* > + * A write of 10101010 to this register will generate a > + * software reset of the ADS7924. > + */ > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > + if (ret) > + return ret; > + } > + > + return 0; > +};
On Tue, 10 Jan 2023 19:50:42 +0100 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > Le 10/01/2023 à 17:01, Hugo Villeneuve a écrit : > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > > digital converter (ADC) with an I2C interface. > > > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > Hi, > > should there be a v3, a few nits below. Hi, there will definitely be a V3 (missing include), thanks for the input, I will incorporate these changes into V3. Hugo. > > CJ > > > MAINTAINERS | 7 + > > drivers/iio/adc/Kconfig | 11 + > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ti-ads7924.c | 492 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 511 insertions(+) > > create mode 100644 drivers/iio/adc/ti-ads7924.c > > > > [...] > > > +static int ads7924_get_channels_config(struct i2c_client *client, > > + struct iio_dev *indio_dev) > > +{ > > + struct ads7924_data *priv = iio_priv(indio_dev); > > + struct device *dev = priv->dev; > > + struct fwnode_handle *node; > > + int num_channels = 0; > > + > > + device_for_each_child_node(dev, node) { > > + u32 pval; > > + unsigned int channel; > > + > > + if (fwnode_property_read_u32(node, "reg", &pval)) { > > + dev_err(dev, "invalid reg on %pfw\n", node); > > + continue; > > + } > > + > > + channel = pval; > > + if (channel >= ADS7924_CHANNELS) { > > + dev_err(dev, "invalid channel index %d on %pfw\n", > > + channel, node); > > + continue; > > + } > > + > > + num_channels++; > > + } > > + > > + if (num_channels > 0) { > > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > > + return 0; > > + } else { > > + return -EINVAL; > > + } > > if (num_channels <= 0) > return -EINVAL; > > dev_dbg(dev, "found %d ADC channels\n", num_channels); > return 0; > > is much more usual. > > > +} > > + > > [...] > > > +static int ads7924_reset(struct iio_dev *indio_dev) > > +{ > > + struct ads7924_data *data = iio_priv(indio_dev); > > + > > + if (data->reset_gpio) { > > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > > + /* Educated guess: assert time not specified in datasheet... */ > > + mdelay(100); > > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ > > + } else { > > + int ret; > > having 'ret' near 'struct ads7924_data *data' is more usual and saves 1 LoC. > > > + > > + /* > > + * A write of 10101010 to this register will generate a > > + * software reset of the ADS7924. > > + */ > > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > > + if (ret) > > + return ret; > > + } > > + > > + return 0; > > +}; > >
On Tue, 10 Jan 2023 11:01:23 -0500 Hugo Villeneuve <hugo@hugovil.com> wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > digital converter (ADC) with an I2C interface. > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> Hi Hugo, A few minor additional comments from me as you are doing a v3 (I might just have tidied them up whilst applying otherwise to save us all time!) Thanks, Jonathan > diff --git a/drivers/iio/adc/ti-ads7924.c b/drivers/iio/adc/ti-ads7924.c > new file mode 100644 > index 000000000000..8384f3451236 > --- /dev/null > +++ b/drivers/iio/adc/ti-ads7924.c > @@ -0,0 +1,492 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * IIO driver for Texas Instruments ADS7924 ADC, 12-bit, 4-Channels, I2C > + * > + * iio/adc/ti-ads7924.c Not a huge amount of point in having a file name in the file. I don't mind if you really want it though. > + * Author: Hugo Villeneuve <hvilleneuve@dimonoff.com> > + * Copyright 2022 DimOnOff > + * > + * based on iio/adc/ti-ads1015.c > + * Copyright (c) 2016, Intel Corporation. > + * > + * Datasheet: https://www.ti.com/lit/gpn/ads7924 > + */ > + > +#define ADS7924_TOTAL_CONVTIME_US (ADS7924_PWRUPTIME_US + ADS7924_ACQTIME_US + \ > + ADS7924_CONVTIME_US) > + > +#define ADS7924_SLEEP_DELAY_MS 2000 Unused, so drop for now. > +static bool ads7924_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case ADS7924_MODECNTRL_REG: > + case ADS7924_INTCNTRL_REG: > + case ADS7924_ULR0_REG: > + case ADS7924_LLR0_REG: > + case ADS7924_ULR1_REG: > + case ADS7924_LLR1_REG: > + case ADS7924_ULR2_REG: > + case ADS7924_LLR2_REG: > + case ADS7924_ULR3_REG: > + case ADS7924_LLR3_REG: > + case ADS7924_INTCONFIG_REG: > + case ADS7924_SLPCONFIG_REG: > + case ADS7924_ACQCONFIG_REG: > + case ADS7924_PWRCONFIG_REG: > + case ADS7924_RESET_REG: > + return true; > + default: > + return false; > + } > +} > + > +static const struct regmap_config ads7924_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = ADS7924_RESET_REG, > + .writeable_reg = ads7924_is_writeable_reg, > +}; > + > +static const struct iio_chan_spec ads7924_channels[] = { > + ADS7924_V_CHAN(0, ADS7924_DATA0_U_REG), > + ADS7924_V_CHAN(1, ADS7924_DATA1_U_REG), > + ADS7924_V_CHAN(2, ADS7924_DATA2_U_REG), > + ADS7924_V_CHAN(3, ADS7924_DATA3_U_REG), > +}; > + > +static > +int ads7924_get_adc_result(struct ads7924_data *data, Drop the unusual line break as not needed (probably refactoring left over!) static int ... > + struct iio_chan_spec const *chan, int *val) > +{ > + int ret; > + __be16 be_val; > + > + if (chan->channel < 0 || chan->channel >= ADS7924_CHANNELS) > + return -EINVAL; > + > + if (data->conv_invalid) { > + int conv_time; > + > + conv_time = ADS7924_TOTAL_CONVTIME_US; > + /* Allow 10% for internal clock inaccuracy. */ > + conv_time += conv_time / 10; > + usleep_range(conv_time, conv_time + 1); > + data->conv_invalid = false; > + } > + > + ret = regmap_raw_read(data->regmap, ADS7924_AUTO_INCREMENT_BIT | > + chan->address, &be_val, sizeof(be_val)); > + if (ret) > + return ret; > + > + *val = be16_to_cpu(be_val); > + *val = *val >> ADS7924_DATA_SHIFT; *val = be16_to_cpu(be_val) >> ADS7924_DATA_SHIFT; return 0; > + > + return ret; > +} > + > +static int ads7924_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + int ret, vref_uv; > + struct ads7924_data *data = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: { > + mutex_lock(&data->lock); > + ret = ads7924_get_adc_result(data, chan, val); > + mutex_unlock(&data->lock); > + if (ret < 0) > + dev_dbg(data->dev, "%s error1\n", __func__); > + else > + ret = IIO_VAL_INT; if (ret < 0) { ... return ret; } return IIO_VAL_INT; > + break; > + } > + case IIO_CHAN_INFO_SCALE: > + vref_uv = regulator_get_voltage(data->vref_reg); > + if (vref_uv < 0) { > + /* dummy regulator "get_voltage" returns -EINVAL */ > + ret = -EINVAL; return -EINVAL; > + } else { > + *val = vref_uv / 1000; /* Convert reg voltage to mV */ > + *val2 = ADS7924_BITS; > + ret = IIO_VAL_FRACTIONAL_LOG2; return IIO_VAL_FR... > + } > + break; > + default: > + ret = -EINVAL; return -EINVAL; > + break; > + } > + > + return ret; > +} > + > +static const struct iio_info ads7924_info = { > + .read_raw = ads7924_read_raw, The tabbed indent doesn't add anything so .read_raw = ads... Aligning things is often a bad idea anyway as it just adds noise when inevitably the alignment needs increasing... Definitely not worth doing for one item. > +}; > + > +static int ads7924_get_channels_config(struct i2c_client *client, > + struct iio_dev *indio_dev) > +{ > + struct ads7924_data *priv = iio_priv(indio_dev); > + struct device *dev = priv->dev; > + struct fwnode_handle *node; > + int num_channels = 0; > + > + device_for_each_child_node(dev, node) { > + u32 pval; > + unsigned int channel; > + > + if (fwnode_property_read_u32(node, "reg", &pval)) { > + dev_err(dev, "invalid reg on %pfw\n", node); > + continue; > + } > + > + channel = pval; > + if (channel >= ADS7924_CHANNELS) { > + dev_err(dev, "invalid channel index %d on %pfw\n", > + channel, node); > + continue; > + } > + > + num_channels++; > + } > + > + if (num_channels > 0) { > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > + return 0; > + } else { As per other review. Give us what we expect which is error paths as out of line. > + return -EINVAL; > + } > +} > +static int ads7924_reset(struct iio_dev *indio_dev) > +{ > + struct ads7924_data *data = iio_priv(indio_dev); > + > + if (data->reset_gpio) { > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > + /* Educated guess: assert time not specified in datasheet... */ > + mdelay(100); > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ No real point in shared exit path. Early returns mean less code to look at when following particular flows which is always nice. return 0; > + } else { if returning above, no need to have the else { > + int ret; > + > + /* > + * A write of 10101010 to this register will generate a > + * software reset of the ADS7924. > + */ > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > + if (ret) > + return ret; return regmap_write(....) > + } > + > + return 0; and drop this. > +}; > +static int ads7924_probe(struct i2c_client *client) > +{ ... > + ret = devm_add_action_or_reset(dev, ads7924_set_idle_mode, data); The _or_reset() part of this is there so that if this function fails it will call the callback before returning an error code. As a result you will turn the device, before you've ever turned it on. Register the callback after turning it on, so ... > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to add idle mode action\n"); > + > + ret = ads7924_set_conv_mode(data, ADS7924_MODECNTRL_AUTO_SCAN); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set conversion mode\n"); ... here.
On Thu, 12 Jan 2023 19:00:30 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 10 Jan 2023 11:01:23 -0500 > Hugo Villeneuve <hugo@hugovil.com> wrote: > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > > digital converter (ADC) with an I2C interface. > > > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > Hi Hugo, > > A few minor additional comments from me as you are doing a v3 > (I might just have tidied them up whilst applying otherwise to save > us all time!) No problem, I will add them to my v3. > Thanks, > > Jonathan > > > diff --git a/drivers/iio/adc/ti-ads7924.c b/drivers/iio/adc/ti-ads7924.c > > new file mode 100644 > > index 000000000000..8384f3451236 > > --- /dev/null > > +++ b/drivers/iio/adc/ti-ads7924.c > > @@ -0,0 +1,492 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * IIO driver for Texas Instruments ADS7924 ADC, 12-bit, 4-Channels, I2C > > + * > > + * iio/adc/ti-ads7924.c > > Not a huge amount of point in having a file name in the file. I don't mind > if you really want it though. Removed. > > > + * Author: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > + * Copyright 2022 DimOnOff > > + * > > + * based on iio/adc/ti-ads1015.c > > + * Copyright (c) 2016, Intel Corporation. > > + * > > + * Datasheet: https://www.ti.com/lit/gpn/ads7924 > > + */ > > > + > > +#define ADS7924_TOTAL_CONVTIME_US (ADS7924_PWRUPTIME_US + ADS7924_ACQTIME_US + \ > > + ADS7924_CONVTIME_US) > > + > > +#define ADS7924_SLEEP_DELAY_MS 2000 > > Unused, so drop for now. Removed. > > +static bool ads7924_is_writeable_reg(struct device *dev, unsigned int reg) > > +{ > > + switch (reg) { > > + case ADS7924_MODECNTRL_REG: > > + case ADS7924_INTCNTRL_REG: > > + case ADS7924_ULR0_REG: > > + case ADS7924_LLR0_REG: > > + case ADS7924_ULR1_REG: > > + case ADS7924_LLR1_REG: > > + case ADS7924_ULR2_REG: > > + case ADS7924_LLR2_REG: > > + case ADS7924_ULR3_REG: > > + case ADS7924_LLR3_REG: > > + case ADS7924_INTCONFIG_REG: > > + case ADS7924_SLPCONFIG_REG: > > + case ADS7924_ACQCONFIG_REG: > > + case ADS7924_PWRCONFIG_REG: > > + case ADS7924_RESET_REG: > > + return true; > > + default: > > + return false; > > + } > > +} > > + > > +static const struct regmap_config ads7924_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = ADS7924_RESET_REG, > > + .writeable_reg = ads7924_is_writeable_reg, > > +}; > > + > > +static const struct iio_chan_spec ads7924_channels[] = { > > + ADS7924_V_CHAN(0, ADS7924_DATA0_U_REG), > > + ADS7924_V_CHAN(1, ADS7924_DATA1_U_REG), > > + ADS7924_V_CHAN(2, ADS7924_DATA2_U_REG), > > + ADS7924_V_CHAN(3, ADS7924_DATA3_U_REG), > > +}; > > + > > +static > > +int ads7924_get_adc_result(struct ads7924_data *data, > > Drop the unusual line break as not needed (probably refactoring left over!) > static int ... Done. > > + struct iio_chan_spec const *chan, int *val) > > +{ > > + int ret; > > + __be16 be_val; > > + > > + if (chan->channel < 0 || chan->channel >= ADS7924_CHANNELS) > > + return -EINVAL; > > + > > + if (data->conv_invalid) { > > + int conv_time; > > + > > + conv_time = ADS7924_TOTAL_CONVTIME_US; > > + /* Allow 10% for internal clock inaccuracy. */ > > + conv_time += conv_time / 10; > > + usleep_range(conv_time, conv_time + 1); > > + data->conv_invalid = false; > > + } > > + > > + ret = regmap_raw_read(data->regmap, ADS7924_AUTO_INCREMENT_BIT | > > + chan->address, &be_val, sizeof(be_val)); > > + if (ret) > > + return ret; > > + > > + *val = be16_to_cpu(be_val); > > + *val = *val >> ADS7924_DATA_SHIFT; > *val = be16_to_cpu(be_val) >> ADS7924_DATA_SHIFT; > > return 0; Yes, simpler and clearer. Done > > > + > > + return ret; > > +} > > + > > +static int ads7924_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *chan, int *val, > > + int *val2, long mask) > > +{ > > + int ret, vref_uv; > > + struct ads7924_data *data = iio_priv(indio_dev); > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: { > > + mutex_lock(&data->lock); > > + ret = ads7924_get_adc_result(data, chan, val); > > + mutex_unlock(&data->lock); > > + if (ret < 0) > > + dev_dbg(data->dev, "%s error1\n", __func__); > > + else > > + ret = IIO_VAL_INT; > if (ret < 0) { > ... > return ret; > } > > return IIO_VAL_INT; > Done. I also have removed the not-very-informative "error1" message... > > + break; > > + } > > + case IIO_CHAN_INFO_SCALE: > > + vref_uv = regulator_get_voltage(data->vref_reg); > > + if (vref_uv < 0) { > > + /* dummy regulator "get_voltage" returns -EINVAL */ > > + ret = -EINVAL; > return -EINVAL; > > + } else { > > + *val = vref_uv / 1000; /* Convert reg voltage to mV */ > > + *val2 = ADS7924_BITS; > > + ret = IIO_VAL_FRACTIONAL_LOG2; > return IIO_VAL_FR... > > > + } > > + break; > > + default: > > + ret = -EINVAL; > return -EINVAL; > > + break; > > + } > > + > > + return ret; > > +} Done. With these changes, I propose to also remove last "return ret" (like in rcar-gyroadc.c). Then, maybe also remove break statements? > > + > > +static const struct iio_info ads7924_info = { > > + .read_raw = ads7924_read_raw, > The tabbed indent doesn't add anything so > .read_raw = ads... > > Aligning things is often a bad idea anyway as it just adds noise when inevitably > the alignment needs increasing... Definitely not worth doing for one item. Ok, done. > > +}; > > + > > +static int ads7924_get_channels_config(struct i2c_client *client, > > + struct iio_dev *indio_dev) > > +{ > > + struct ads7924_data *priv = iio_priv(indio_dev); > > + struct device *dev = priv->dev; > > + struct fwnode_handle *node; > > + int num_channels = 0; > > + > > + device_for_each_child_node(dev, node) { > > + u32 pval; > > + unsigned int channel; > > + > > + if (fwnode_property_read_u32(node, "reg", &pval)) { > > + dev_err(dev, "invalid reg on %pfw\n", node); > > + continue; > > + } > > + > > + channel = pval; > > + if (channel >= ADS7924_CHANNELS) { > > + dev_err(dev, "invalid channel index %d on %pfw\n", > > + channel, node); > > + continue; > > + } > > + > > + num_channels++; > > + } > > + > > + if (num_channels > 0) { > > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > > + return 0; > > + } else { > > As per other review. Give us what we expect which is error paths > as out of line. Already done as suggested by Christophe: if (num_channels <= 0) return -EINVAL; dev_dbg(dev, "found %d ADC channels\n", num_channels); return 0; Although I do not fully understand what you mean by "...error paths as out of line"? Do you mean to drop the debug message? > > > + return -EINVAL; > > + } > > +} > > > +static int ads7924_reset(struct iio_dev *indio_dev) > > +{ > > + struct ads7924_data *data = iio_priv(indio_dev); > > + > > + if (data->reset_gpio) { > > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > > + /* Educated guess: assert time not specified in datasheet... */ > > + mdelay(100); > > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ > > No real point in shared exit path. Early returns mean less code to look at > when following particular flows which is always nice. Agreed, done. > return 0; > > > + } else { > > if returning above, no need to have the else { Done > > > + int ret; > > + > > + /* > > + * A write of 10101010 to this register will generate a > > + * software reset of the ADS7924. > > + */ > > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > > + if (ret) > > + return ret; > return regmap_write(....) > > > + } > > + > > + return 0; > and drop this. Done. also remove now unused variable "ret". > > +}; > > > +static int ads7924_probe(struct i2c_client *client) > > +{ > > ... > > > + ret = devm_add_action_or_reset(dev, ads7924_set_idle_mode, data); > > The _or_reset() part of this is there so that if this function fails > it will call the callback before returning an error code. As a result > you will turn the device, before you've ever turned it on. > > Register the callback after turning it on, so ... Makes sense, done. > > + if (ret) > > + return dev_err_probe(dev, ret, > > + "failed to add idle mode action\n"); > > + > > + ret = ads7924_set_conv_mode(data, ADS7924_MODECNTRL_AUTO_SCAN); > > + if (ret) > > + return dev_err_probe(dev, ret, > > + "failed to set conversion mode\n"); > > ... here. Thank you a lot for all your comments and meaningfull explanations!
On Tue, 10 Jan 2023 11:01:23 -0500 Hugo Villeneuve <hugo@hugovil.com> wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > digital converter (ADC) with an I2C interface. > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- > MAINTAINERS | 7 + > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ti-ads7924.c | 492 +++++++++++++++++++++++++++++++++++ > 4 files changed, 511 insertions(+) > create mode 100644 drivers/iio/adc/ti-ads7924.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ff472ca1244..94eb68076f0c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20776,6 +20776,13 @@ M: Robert Richter <rric@kernel.org> > S: Odd Fixes > F: drivers/gpio/gpio-thunderx.c > > +TI ADS7924 ADC DRIVER > +M: Hugo Villeneuve <hvilleneuve@dimonoff.com> > +L: linux-iio@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml > +F: drivers/iio/adc/ti-ads7924.c > + > TI AM437X VPFE DRIVER > M: "Lad, Prabhakar" <prabhakar.csengg@gmail.com> > L: linux-media@vger.kernel.org > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 7d6e74189a2d..99468c64daf9 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -1208,6 +1208,17 @@ config TI_ADS1015 > This driver can also be built as a module. If so, the module will be > called ti-ads1015. > > +config TI_ADS7924 > + tristate "Texas Instruments ADS7924 ADC" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for Texas Instruments ADS7924 > + 4 channels, 12-bit I2C ADC chip. > + > + This driver can also be built as a module. If so, the module will be > + called ti-ads7924. > + > config TI_ADS7950 > tristate "Texas Instruments ADS7950 ADC driver" > depends on SPI && GPIOLIB > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 6e08415c3f3a..1e5bdf47a091 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -107,6 +107,7 @@ obj-$(CONFIG_TI_ADC108S102) += ti-adc108s102.o > obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o > obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o > obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o > +obj-$(CONFIG_TI_ADS7924) += ti-ads7924.o > obj-$(CONFIG_TI_ADS7950) += ti-ads7950.o > obj-$(CONFIG_TI_ADS8344) += ti-ads8344.o > obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o > diff --git a/drivers/iio/adc/ti-ads7924.c b/drivers/iio/adc/ti-ads7924.c > new file mode 100644 > index 000000000000..8384f3451236 > --- /dev/null > +++ b/drivers/iio/adc/ti-ads7924.c > @@ -0,0 +1,492 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * IIO driver for Texas Instruments ADS7924 ADC, 12-bit, 4-Channels, I2C > + * > + * iio/adc/ti-ads7924.c > + * Author: Hugo Villeneuve <hvilleneuve@dimonoff.com> > + * Copyright 2022 DimOnOff > + * > + * based on iio/adc/ti-ads1015.c > + * Copyright (c) 2016, Intel Corporation. > + * > + * Datasheet: https://www.ti.com/lit/gpn/ads7924 > + */ > + > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/init.h> > +#include <linux/irq.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/types.h> > + > +#define ADS7924_CHANNELS 4 > +#define ADS7924_BITS 12 > +#define ADS7924_DATA_SHIFT 4 > + > +/* Registers. */ > +#define ADS7924_MODECNTRL_REG 0x00 > +#define ADS7924_INTCNTRL_REG 0x01 > +#define ADS7924_DATA0_U_REG 0x02 > +#define ADS7924_DATA0_L_REG 0x03 > +#define ADS7924_DATA1_U_REG 0x04 > +#define ADS7924_DATA1_L_REG 0x05 > +#define ADS7924_DATA2_U_REG 0x06 > +#define ADS7924_DATA2_L_REG 0x07 > +#define ADS7924_DATA3_U_REG 0x08 > +#define ADS7924_DATA3_L_REG 0x09 > +#define ADS7924_ULR0_REG 0x0A > +#define ADS7924_LLR0_REG 0x0B > +#define ADS7924_ULR1_REG 0x0C > +#define ADS7924_LLR1_REG 0x0D > +#define ADS7924_ULR2_REG 0x0E > +#define ADS7924_LLR2_REG 0x0F > +#define ADS7924_ULR3_REG 0x10 > +#define ADS7924_LLR3_REG 0x11 > +#define ADS7924_INTCONFIG_REG 0x12 > +#define ADS7924_SLPCONFIG_REG 0x13 > +#define ADS7924_ACQCONFIG_REG 0x14 > +#define ADS7924_PWRCONFIG_REG 0x15 > +#define ADS7924_RESET_REG 0x16 > + > +/* > + * Register address INC bit: when set to '1', the register address is > + * automatically incremented after every register read which allows convenient > + * reading of multiple registers. Set INC to '0' when reading a single register. > + */ > +#define ADS7924_AUTO_INCREMENT_BIT BIT(7) > + > +#define ADS7924_MODECNTRL_MODE_MASK GENMASK(7, 2) > + > +#define ADS7924_MODECNTRL_SEL_MASK GENMASK(1, 0) > + > +#define ADS7924_CFG_INTPOL_BIT 1 > +#define ADS7924_CFG_INTTRIG_BIT 0 > + > +#define ADS7924_CFG_INTPOL_MASK BIT(ADS7924_CFG_INTPOL_BIT) > +#define ADS7924_CFG_INTTRIG_MASK BIT(ADS7924_CFG_INTTRIG_BIT) > + > +/* Interrupt pin polarity */ > +#define ADS7924_CFG_INTPOL_LOW 0 > +#define ADS7924_CFG_INTPOL_HIGH 1 > + > +/* Interrupt pin signaling */ > +#define ADS7924_CFG_INTTRIG_LEVEL 0 > +#define ADS7924_CFG_INTTRIG_EDGE 1 > + > +/* Mode control values */ > +#define ADS7924_MODECNTRL_IDLE 0x00 > +#define ADS7924_MODECNTRL_AWAKE 0x20 > +#define ADS7924_MODECNTRL_MANUAL_SINGLE 0x30 > +#define ADS7924_MODECNTRL_MANUAL_SCAN 0x32 > +#define ADS7924_MODECNTRL_AUTO_SINGLE 0x31 > +#define ADS7924_MODECNTRL_AUTO_SCAN 0x33 > +#define ADS7924_MODECNTRL_AUTO_SINGLE_SLEEP 0x39 > +#define ADS7924_MODECNTRL_AUTO_SCAN_SLEEP 0x3B > +#define ADS7924_MODECNTRL_AUTO_BURST_SLEEP 0x3F > + > +#define ADS7924_ACQTIME_MASK GENMASK(4, 0) > + > +#define ADS7924_PWRUPTIME_MASK GENMASK(4, 0) > + > +/* > + * The power-up time is allowed to elapse whenever the device has been shutdown > + * in idle mode. Power-up time can allow external circuits, such as an > + * operational amplifier, between the MUXOUT and ADCIN pins to turn on. > + * The nominal time programmed by the PUTIME[4:0] register bits is given by: > + * t PU = PWRUPTIME[4:0] × 2 μs > + * If a power-up time is not required, set the bits to '0' to effectively bypass. > + */ > +#define ADS7924_PWRUPTIME_US 0 /* Bypass (0us). */ > + > +/* > + * Acquisition Time according to ACQTIME[4:0] register bits. > + * The Acquisition Time is given by: > + * t ACQ = (ACQTIME[4:0] × 2 μs) + 6 μs > + * Using default value of 0 for ACQTIME[4:0] results in a minimum acquisition > + * time of 6us. > + */ > +#define ADS7924_ACQTIME_US 6 > + > +/* The conversion time is always 4μs and cannot be programmed by the user. */ > +#define ADS7924_CONVTIME_US 4 > + > +#define ADS7924_TOTAL_CONVTIME_US (ADS7924_PWRUPTIME_US + ADS7924_ACQTIME_US + \ > + ADS7924_CONVTIME_US) > + > +#define ADS7924_SLEEP_DELAY_MS 2000 > + > +#define ADS7924_V_CHAN(_chan, _addr) { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .channel = _chan, \ > + .address = _addr, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .datasheet_name = "AIN"#_chan, \ > +} > + > +struct ads7924_data { > + struct device *dev; > + struct regmap *regmap; > + struct regulator *vref_reg; > + > + /* GPIO descriptor for device hard-reset pin. */ > + struct gpio_desc *reset_gpio; > + > + /* > + * Protects ADC ops, e.g: concurrent sysfs/buffered > + * data reads, configuration updates > + */ > + struct mutex lock; > + > + /* > + * Set to true when the ADC is switched to the continuous-conversion > + * mode and exits from a power-down state. This flag is used to avoid > + * getting the stale result from the conversion register. > + */ > + bool conv_invalid; > +}; > + > +static bool ads7924_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case ADS7924_MODECNTRL_REG: > + case ADS7924_INTCNTRL_REG: > + case ADS7924_ULR0_REG: > + case ADS7924_LLR0_REG: > + case ADS7924_ULR1_REG: > + case ADS7924_LLR1_REG: > + case ADS7924_ULR2_REG: > + case ADS7924_LLR2_REG: > + case ADS7924_ULR3_REG: > + case ADS7924_LLR3_REG: > + case ADS7924_INTCONFIG_REG: > + case ADS7924_SLPCONFIG_REG: > + case ADS7924_ACQCONFIG_REG: > + case ADS7924_PWRCONFIG_REG: > + case ADS7924_RESET_REG: > + return true; > + default: > + return false; > + } > +} > + > +static const struct regmap_config ads7924_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = ADS7924_RESET_REG, > + .writeable_reg = ads7924_is_writeable_reg, > +}; > + > +static const struct iio_chan_spec ads7924_channels[] = { > + ADS7924_V_CHAN(0, ADS7924_DATA0_U_REG), > + ADS7924_V_CHAN(1, ADS7924_DATA1_U_REG), > + ADS7924_V_CHAN(2, ADS7924_DATA2_U_REG), > + ADS7924_V_CHAN(3, ADS7924_DATA3_U_REG), > +}; > + > +static > +int ads7924_get_adc_result(struct ads7924_data *data, > + struct iio_chan_spec const *chan, int *val) > +{ > + int ret; > + __be16 be_val; > + > + if (chan->channel < 0 || chan->channel >= ADS7924_CHANNELS) > + return -EINVAL; > + > + if (data->conv_invalid) { > + int conv_time; > + > + conv_time = ADS7924_TOTAL_CONVTIME_US; > + /* Allow 10% for internal clock inaccuracy. */ > + conv_time += conv_time / 10; > + usleep_range(conv_time, conv_time + 1); > + data->conv_invalid = false; > + } > + > + ret = regmap_raw_read(data->regmap, ADS7924_AUTO_INCREMENT_BIT | > + chan->address, &be_val, sizeof(be_val)); > + if (ret) > + return ret; > + > + *val = be16_to_cpu(be_val); > + *val = *val >> ADS7924_DATA_SHIFT; > + > + return ret; > +} > + > +static int ads7924_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + int ret, vref_uv; > + struct ads7924_data *data = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: { > + mutex_lock(&data->lock); > + ret = ads7924_get_adc_result(data, chan, val); > + mutex_unlock(&data->lock); > + if (ret < 0) > + dev_dbg(data->dev, "%s error1\n", __func__); > + else > + ret = IIO_VAL_INT; > + break; > + } > + case IIO_CHAN_INFO_SCALE: > + vref_uv = regulator_get_voltage(data->vref_reg); > + if (vref_uv < 0) { > + /* dummy regulator "get_voltage" returns -EINVAL */ > + ret = -EINVAL; > + } else { > + *val = vref_uv / 1000; /* Convert reg voltage to mV */ > + *val2 = ADS7924_BITS; > + ret = IIO_VAL_FRACTIONAL_LOG2; > + } > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + return ret; > +} > + > +static const struct iio_info ads7924_info = { > + .read_raw = ads7924_read_raw, > +}; > + > +static int ads7924_get_channels_config(struct i2c_client *client, > + struct iio_dev *indio_dev) > +{ > + struct ads7924_data *priv = iio_priv(indio_dev); > + struct device *dev = priv->dev; > + struct fwnode_handle *node; > + int num_channels = 0; > + > + device_for_each_child_node(dev, node) { > + u32 pval; > + unsigned int channel; > + > + if (fwnode_property_read_u32(node, "reg", &pval)) { > + dev_err(dev, "invalid reg on %pfw\n", node); > + continue; > + } > + > + channel = pval; > + if (channel >= ADS7924_CHANNELS) { > + dev_err(dev, "invalid channel index %d on %pfw\n", > + channel, node); > + continue; > + } > + > + num_channels++; > + } > + > + if (num_channels > 0) { > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > + return 0; > + } else { > + return -EINVAL; > + } > +} > + > +static int ads7924_set_conv_mode(struct ads7924_data *data, int mode) > +{ > + int ret; > + unsigned int mode_field; > + struct device *dev = data->dev; > + > + /* > + * When switching between modes, be sure to first select the Awake mode > + * and then switch to the desired mode. This procedure ensures the > + * internal control logic is properly synchronized. > + */ > + if (mode != ADS7924_MODECNTRL_IDLE) { > + mode_field = FIELD_PREP(ADS7924_MODECNTRL_MODE_MASK, > + ADS7924_MODECNTRL_AWAKE); > + > + ret = regmap_update_bits(data->regmap, ADS7924_MODECNTRL_REG, > + ADS7924_MODECNTRL_MODE_MASK, > + mode_field); > + if (ret) { > + dev_warn(dev, "failed to set awake mode (%pe)\n", > + ERR_PTR(ret)); > + return ret; > + } > + } > + > + mode_field = FIELD_PREP(ADS7924_MODECNTRL_MODE_MASK, mode); > + > + ret = regmap_update_bits(data->regmap, ADS7924_MODECNTRL_REG, > + ADS7924_MODECNTRL_MODE_MASK, mode_field); > + if (ret) > + dev_warn(dev, "failed to set mode %d (%pe)\n", mode, > + ERR_PTR(ret)); > + > + return ret; > +} > + > +static int ads7924_reset(struct iio_dev *indio_dev) > +{ > + struct ads7924_data *data = iio_priv(indio_dev); > + > + if (data->reset_gpio) { > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > + /* Educated guess: assert time not specified in datasheet... */ > + mdelay(100); > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ > + } else { > + int ret; > + > + /* > + * A write of 10101010 to this register will generate a > + * software reset of the ADS7924. > + */ > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > + if (ret) > + return ret; > + } > + > + return 0; > +}; > + > +static void ads7924_reg_disable(void *data) > +{ > + regulator_disable(data); > +} > + > +static void ads7924_set_idle_mode(void *data) > +{ > + ads7924_set_conv_mode(data, ADS7924_MODECNTRL_IDLE); > +} > + > +static int ads7924_probe(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev; > + struct ads7924_data *data; > + struct device *dev = &client->dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; Would it be a good idea to also use dev_err_probe() here? Hugo. > + > + data = iio_priv(indio_dev); > + > + data->dev = dev; > + > + /* Initialize the reset GPIO as output with an initial value of 0. */ > + data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(data->reset_gpio)) > + return dev_err_probe(dev, PTR_ERR(data->reset_gpio), > + "failed to get request reset GPIO\n"); > + > + mutex_init(&data->lock); > + > + indio_dev->name = "ads7924"; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + indio_dev->channels = ads7924_channels; > + indio_dev->num_channels = ARRAY_SIZE(ads7924_channels); > + indio_dev->info = &ads7924_info; > + > + ret = ads7924_get_channels_config(client, indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to get channels configuration\n"); > + > + data->regmap = devm_regmap_init_i2c(client, &ads7924_regmap_config); > + if (IS_ERR(data->regmap)) > + return dev_err_probe(dev, PTR_ERR(data->regmap), > + "failed to init regmap\n"); > + > + data->vref_reg = devm_regulator_get(dev, "vref"); > + if (IS_ERR(data->vref_reg)) > + return dev_err_probe(dev, PTR_ERR(data->vref_reg), > + "failed to get vref regulator\n"); > + > + ret = regulator_enable(data->vref_reg); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to enable regulator\n"); > + > + ret = devm_add_action_or_reset(dev, ads7924_reg_disable, data->vref_reg); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to add regulator disable action\n"); > + > + ret = ads7924_reset(indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to reset device\n"); > + > + ret = devm_add_action_or_reset(dev, ads7924_set_idle_mode, data); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to add idle mode action\n"); > + > + ret = ads7924_set_conv_mode(data, ADS7924_MODECNTRL_AUTO_SCAN); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set conversion mode\n"); > + > + /* Use minimum signal acquire time. */ > + ret = regmap_update_bits(data->regmap, ADS7924_ACQCONFIG_REG, > + ADS7924_ACQTIME_MASK, > + FIELD_PREP(ADS7924_ACQTIME_MASK, 0)); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to configure signal acquire time\n"); > + > + /* Disable power-up time. */ > + ret = regmap_update_bits(data->regmap, ADS7924_PWRCONFIG_REG, > + ADS7924_PWRUPTIME_MASK, > + FIELD_PREP(ADS7924_PWRUPTIME_MASK, 0)); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to configure power-up time\n"); > + > + data->conv_invalid = true; > + > + ret = devm_iio_device_register(dev, indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to register IIO device\n"); > + > + return 0; > +} > + > +static const struct i2c_device_id ads7924_id[] = { > + { "ads7924", 0 }, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, ads7924_id); > + > +static const struct of_device_id ads7924_of_match[] = { > + { .compatible = "ti,ads7924", }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, ads7924_of_match); > + > +static struct i2c_driver ads7924_driver = { > + .driver = { > + .name = "ads7924", > + .of_match_table = ads7924_of_match, > + }, > + .probe_new = ads7924_probe, > + .id_table = ads7924_id, > +}; > + > +module_i2c_driver(ads7924_driver); > + > +MODULE_AUTHOR("Hugo Villeneuve <hvilleneuve@dimonoff.com>"); > +MODULE_DESCRIPTION("Texas Instruments ADS7924 ADC I2C driver"); > +MODULE_LICENSE("GPL"); > -- > 2.30.2 > >
On Tue, 10 Jan 2023 11:01:23 -0500 Hugo Villeneuve <hugo@hugovil.com> wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > The Texas Instruments ADS7924 is a 4 channels, 12-bit analog to > digital converter (ADC) with an I2C interface. > > Datasheet: https://www.ti.com/lit/gpn/ads7924 > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- > MAINTAINERS | 7 + > drivers/iio/adc/Kconfig | 11 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ti-ads7924.c | 492 +++++++++++++++++++++++++++++++++++ > 4 files changed, 511 insertions(+) > create mode 100644 drivers/iio/adc/ti-ads7924.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ff472ca1244..94eb68076f0c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20776,6 +20776,13 @@ M: Robert Richter <rric@kernel.org> > S: Odd Fixes > F: drivers/gpio/gpio-thunderx.c > > +TI ADS7924 ADC DRIVER > +M: Hugo Villeneuve <hvilleneuve@dimonoff.com> > +L: linux-iio@vger.kernel.org > +S: Supported > +F: Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml > +F: drivers/iio/adc/ti-ads7924.c > + > TI AM437X VPFE DRIVER > M: "Lad, Prabhakar" <prabhakar.csengg@gmail.com> > L: linux-media@vger.kernel.org > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index 7d6e74189a2d..99468c64daf9 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -1208,6 +1208,17 @@ config TI_ADS1015 > This driver can also be built as a module. If so, the module will be > called ti-ads1015. > > +config TI_ADS7924 > + tristate "Texas Instruments ADS7924 ADC" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get support for Texas Instruments ADS7924 > + 4 channels, 12-bit I2C ADC chip. > + > + This driver can also be built as a module. If so, the module will be > + called ti-ads7924. > + > config TI_ADS7950 > tristate "Texas Instruments ADS7950 ADC driver" > depends on SPI && GPIOLIB > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 6e08415c3f3a..1e5bdf47a091 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -107,6 +107,7 @@ obj-$(CONFIG_TI_ADC108S102) += ti-adc108s102.o > obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o > obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o > obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o > +obj-$(CONFIG_TI_ADS7924) += ti-ads7924.o > obj-$(CONFIG_TI_ADS7950) += ti-ads7950.o > obj-$(CONFIG_TI_ADS8344) += ti-ads8344.o > obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o > diff --git a/drivers/iio/adc/ti-ads7924.c b/drivers/iio/adc/ti-ads7924.c > new file mode 100644 > index 000000000000..8384f3451236 > --- /dev/null > +++ b/drivers/iio/adc/ti-ads7924.c > @@ -0,0 +1,492 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * IIO driver for Texas Instruments ADS7924 ADC, 12-bit, 4-Channels, I2C > + * > + * iio/adc/ti-ads7924.c > + * Author: Hugo Villeneuve <hvilleneuve@dimonoff.com> > + * Copyright 2022 DimOnOff > + * > + * based on iio/adc/ti-ads1015.c > + * Copyright (c) 2016, Intel Corporation. > + * > + * Datasheet: https://www.ti.com/lit/gpn/ads7924 > + */ > + > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/init.h> > +#include <linux/irq.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/types.h> > + > +#define ADS7924_CHANNELS 4 > +#define ADS7924_BITS 12 > +#define ADS7924_DATA_SHIFT 4 > + > +/* Registers. */ > +#define ADS7924_MODECNTRL_REG 0x00 > +#define ADS7924_INTCNTRL_REG 0x01 > +#define ADS7924_DATA0_U_REG 0x02 > +#define ADS7924_DATA0_L_REG 0x03 > +#define ADS7924_DATA1_U_REG 0x04 > +#define ADS7924_DATA1_L_REG 0x05 > +#define ADS7924_DATA2_U_REG 0x06 > +#define ADS7924_DATA2_L_REG 0x07 > +#define ADS7924_DATA3_U_REG 0x08 > +#define ADS7924_DATA3_L_REG 0x09 > +#define ADS7924_ULR0_REG 0x0A > +#define ADS7924_LLR0_REG 0x0B > +#define ADS7924_ULR1_REG 0x0C > +#define ADS7924_LLR1_REG 0x0D > +#define ADS7924_ULR2_REG 0x0E > +#define ADS7924_LLR2_REG 0x0F > +#define ADS7924_ULR3_REG 0x10 > +#define ADS7924_LLR3_REG 0x11 > +#define ADS7924_INTCONFIG_REG 0x12 > +#define ADS7924_SLPCONFIG_REG 0x13 > +#define ADS7924_ACQCONFIG_REG 0x14 > +#define ADS7924_PWRCONFIG_REG 0x15 > +#define ADS7924_RESET_REG 0x16 > + > +/* > + * Register address INC bit: when set to '1', the register address is > + * automatically incremented after every register read which allows convenient > + * reading of multiple registers. Set INC to '0' when reading a single register. > + */ > +#define ADS7924_AUTO_INCREMENT_BIT BIT(7) > + > +#define ADS7924_MODECNTRL_MODE_MASK GENMASK(7, 2) > + > +#define ADS7924_MODECNTRL_SEL_MASK GENMASK(1, 0) > + > +#define ADS7924_CFG_INTPOL_BIT 1 > +#define ADS7924_CFG_INTTRIG_BIT 0 > + > +#define ADS7924_CFG_INTPOL_MASK BIT(ADS7924_CFG_INTPOL_BIT) > +#define ADS7924_CFG_INTTRIG_MASK BIT(ADS7924_CFG_INTTRIG_BIT) > + > +/* Interrupt pin polarity */ > +#define ADS7924_CFG_INTPOL_LOW 0 > +#define ADS7924_CFG_INTPOL_HIGH 1 > + > +/* Interrupt pin signaling */ > +#define ADS7924_CFG_INTTRIG_LEVEL 0 > +#define ADS7924_CFG_INTTRIG_EDGE 1 > + > +/* Mode control values */ > +#define ADS7924_MODECNTRL_IDLE 0x00 > +#define ADS7924_MODECNTRL_AWAKE 0x20 > +#define ADS7924_MODECNTRL_MANUAL_SINGLE 0x30 > +#define ADS7924_MODECNTRL_MANUAL_SCAN 0x32 > +#define ADS7924_MODECNTRL_AUTO_SINGLE 0x31 > +#define ADS7924_MODECNTRL_AUTO_SCAN 0x33 > +#define ADS7924_MODECNTRL_AUTO_SINGLE_SLEEP 0x39 > +#define ADS7924_MODECNTRL_AUTO_SCAN_SLEEP 0x3B > +#define ADS7924_MODECNTRL_AUTO_BURST_SLEEP 0x3F > + > +#define ADS7924_ACQTIME_MASK GENMASK(4, 0) > + > +#define ADS7924_PWRUPTIME_MASK GENMASK(4, 0) > + > +/* > + * The power-up time is allowed to elapse whenever the device has been shutdown > + * in idle mode. Power-up time can allow external circuits, such as an > + * operational amplifier, between the MUXOUT and ADCIN pins to turn on. > + * The nominal time programmed by the PUTIME[4:0] register bits is given by: > + * t PU = PWRUPTIME[4:0] × 2 μs > + * If a power-up time is not required, set the bits to '0' to effectively bypass. > + */ > +#define ADS7924_PWRUPTIME_US 0 /* Bypass (0us). */ > + > +/* > + * Acquisition Time according to ACQTIME[4:0] register bits. > + * The Acquisition Time is given by: > + * t ACQ = (ACQTIME[4:0] × 2 μs) + 6 μs > + * Using default value of 0 for ACQTIME[4:0] results in a minimum acquisition > + * time of 6us. > + */ > +#define ADS7924_ACQTIME_US 6 > + > +/* The conversion time is always 4μs and cannot be programmed by the user. */ > +#define ADS7924_CONVTIME_US 4 > + > +#define ADS7924_TOTAL_CONVTIME_US (ADS7924_PWRUPTIME_US + ADS7924_ACQTIME_US + \ > + ADS7924_CONVTIME_US) > + > +#define ADS7924_SLEEP_DELAY_MS 2000 > + > +#define ADS7924_V_CHAN(_chan, _addr) { \ > + .type = IIO_VOLTAGE, \ > + .indexed = 1, \ > + .channel = _chan, \ > + .address = _addr, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .datasheet_name = "AIN"#_chan, \ > +} > + > +struct ads7924_data { > + struct device *dev; > + struct regmap *regmap; > + struct regulator *vref_reg; > + > + /* GPIO descriptor for device hard-reset pin. */ > + struct gpio_desc *reset_gpio; > + > + /* > + * Protects ADC ops, e.g: concurrent sysfs/buffered > + * data reads, configuration updates > + */ > + struct mutex lock; > + > + /* > + * Set to true when the ADC is switched to the continuous-conversion > + * mode and exits from a power-down state. This flag is used to avoid > + * getting the stale result from the conversion register. > + */ > + bool conv_invalid; > +}; > + > +static bool ads7924_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case ADS7924_MODECNTRL_REG: > + case ADS7924_INTCNTRL_REG: > + case ADS7924_ULR0_REG: > + case ADS7924_LLR0_REG: > + case ADS7924_ULR1_REG: > + case ADS7924_LLR1_REG: > + case ADS7924_ULR2_REG: > + case ADS7924_LLR2_REG: > + case ADS7924_ULR3_REG: > + case ADS7924_LLR3_REG: > + case ADS7924_INTCONFIG_REG: > + case ADS7924_SLPCONFIG_REG: > + case ADS7924_ACQCONFIG_REG: > + case ADS7924_PWRCONFIG_REG: > + case ADS7924_RESET_REG: > + return true; > + default: > + return false; > + } > +} > + > +static const struct regmap_config ads7924_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = ADS7924_RESET_REG, > + .writeable_reg = ads7924_is_writeable_reg, > +}; > + > +static const struct iio_chan_spec ads7924_channels[] = { > + ADS7924_V_CHAN(0, ADS7924_DATA0_U_REG), > + ADS7924_V_CHAN(1, ADS7924_DATA1_U_REG), > + ADS7924_V_CHAN(2, ADS7924_DATA2_U_REG), > + ADS7924_V_CHAN(3, ADS7924_DATA3_U_REG), > +}; > + > +static > +int ads7924_get_adc_result(struct ads7924_data *data, > + struct iio_chan_spec const *chan, int *val) > +{ > + int ret; > + __be16 be_val; > + > + if (chan->channel < 0 || chan->channel >= ADS7924_CHANNELS) > + return -EINVAL; > + > + if (data->conv_invalid) { > + int conv_time; > + > + conv_time = ADS7924_TOTAL_CONVTIME_US; > + /* Allow 10% for internal clock inaccuracy. */ > + conv_time += conv_time / 10; > + usleep_range(conv_time, conv_time + 1); > + data->conv_invalid = false; > + } > + > + ret = regmap_raw_read(data->regmap, ADS7924_AUTO_INCREMENT_BIT | > + chan->address, &be_val, sizeof(be_val)); > + if (ret) > + return ret; > + > + *val = be16_to_cpu(be_val); > + *val = *val >> ADS7924_DATA_SHIFT; > + > + return ret; > +} > + > +static int ads7924_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + int ret, vref_uv; > + struct ads7924_data *data = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: { > + mutex_lock(&data->lock); > + ret = ads7924_get_adc_result(data, chan, val); > + mutex_unlock(&data->lock); > + if (ret < 0) > + dev_dbg(data->dev, "%s error1\n", __func__); > + else > + ret = IIO_VAL_INT; > + break; > + } > + case IIO_CHAN_INFO_SCALE: > + vref_uv = regulator_get_voltage(data->vref_reg); > + if (vref_uv < 0) { > + /* dummy regulator "get_voltage" returns -EINVAL */ > + ret = -EINVAL; > + } else { > + *val = vref_uv / 1000; /* Convert reg voltage to mV */ > + *val2 = ADS7924_BITS; > + ret = IIO_VAL_FRACTIONAL_LOG2; > + } > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + return ret; > +} > + > +static const struct iio_info ads7924_info = { > + .read_raw = ads7924_read_raw, > +}; > + > +static int ads7924_get_channels_config(struct i2c_client *client, > + struct iio_dev *indio_dev) > +{ > + struct ads7924_data *priv = iio_priv(indio_dev); > + struct device *dev = priv->dev; > + struct fwnode_handle *node; > + int num_channels = 0; > + > + device_for_each_child_node(dev, node) { > + u32 pval; > + unsigned int channel; > + > + if (fwnode_property_read_u32(node, "reg", &pval)) { > + dev_err(dev, "invalid reg on %pfw\n", node); > + continue; > + } > + > + channel = pval; > + if (channel >= ADS7924_CHANNELS) { > + dev_err(dev, "invalid channel index %d on %pfw\n", > + channel, node); > + continue; > + } > + > + num_channels++; > + } > + > + if (num_channels > 0) { > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > + return 0; > + } else { > + return -EINVAL; > + } > +} > + > +static int ads7924_set_conv_mode(struct ads7924_data *data, int mode) > +{ > + int ret; > + unsigned int mode_field; > + struct device *dev = data->dev; > + > + /* > + * When switching between modes, be sure to first select the Awake mode > + * and then switch to the desired mode. This procedure ensures the > + * internal control logic is properly synchronized. > + */ > + if (mode != ADS7924_MODECNTRL_IDLE) { > + mode_field = FIELD_PREP(ADS7924_MODECNTRL_MODE_MASK, > + ADS7924_MODECNTRL_AWAKE); > + > + ret = regmap_update_bits(data->regmap, ADS7924_MODECNTRL_REG, > + ADS7924_MODECNTRL_MODE_MASK, > + mode_field); > + if (ret) { > + dev_warn(dev, "failed to set awake mode (%pe)\n", > + ERR_PTR(ret)); > + return ret; > + } > + } > + > + mode_field = FIELD_PREP(ADS7924_MODECNTRL_MODE_MASK, mode); > + > + ret = regmap_update_bits(data->regmap, ADS7924_MODECNTRL_REG, > + ADS7924_MODECNTRL_MODE_MASK, mode_field); > + if (ret) > + dev_warn(dev, "failed to set mode %d (%pe)\n", mode, > + ERR_PTR(ret)); > + > + return ret; > +} > + > +static int ads7924_reset(struct iio_dev *indio_dev) > +{ > + struct ads7924_data *data = iio_priv(indio_dev); > + > + if (data->reset_gpio) { > + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ > + /* Educated guess: assert time not specified in datasheet... */ > + mdelay(100); > + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ > + } else { > + int ret; > + > + /* > + * A write of 10101010 to this register will generate a > + * software reset of the ADS7924. > + */ > + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); > + if (ret) > + return ret; > + } > + > + return 0; > +}; > + > +static void ads7924_reg_disable(void *data) > +{ > + regulator_disable(data); > +} > + > +static void ads7924_set_idle_mode(void *data) > +{ > + ads7924_set_conv_mode(data, ADS7924_MODECNTRL_IDLE); > +} > + > +static int ads7924_probe(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev; > + struct ads7924_data *data; > + struct device *dev = &client->dev; > + int ret; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; Would it be a good idea to also use dev_err_probe() here? Hugo. > + > + data = iio_priv(indio_dev); > + > + data->dev = dev; > + > + /* Initialize the reset GPIO as output with an initial value of 0. */ > + data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(data->reset_gpio)) > + return dev_err_probe(dev, PTR_ERR(data->reset_gpio), > + "failed to get request reset GPIO\n"); > + > + mutex_init(&data->lock); > + > + indio_dev->name = "ads7924"; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + indio_dev->channels = ads7924_channels; > + indio_dev->num_channels = ARRAY_SIZE(ads7924_channels); > + indio_dev->info = &ads7924_info; > + > + ret = ads7924_get_channels_config(client, indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to get channels configuration\n"); > + > + data->regmap = devm_regmap_init_i2c(client, &ads7924_regmap_config); > + if (IS_ERR(data->regmap)) > + return dev_err_probe(dev, PTR_ERR(data->regmap), > + "failed to init regmap\n"); > + > + data->vref_reg = devm_regulator_get(dev, "vref"); > + if (IS_ERR(data->vref_reg)) > + return dev_err_probe(dev, PTR_ERR(data->vref_reg), > + "failed to get vref regulator\n"); > + > + ret = regulator_enable(data->vref_reg); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to enable regulator\n"); > + > + ret = devm_add_action_or_reset(dev, ads7924_reg_disable, data->vref_reg); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to add regulator disable action\n"); > + > + ret = ads7924_reset(indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to reset device\n"); > + > + ret = devm_add_action_or_reset(dev, ads7924_set_idle_mode, data); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to add idle mode action\n"); > + > + ret = ads7924_set_conv_mode(data, ADS7924_MODECNTRL_AUTO_SCAN); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set conversion mode\n"); > + > + /* Use minimum signal acquire time. */ > + ret = regmap_update_bits(data->regmap, ADS7924_ACQCONFIG_REG, > + ADS7924_ACQTIME_MASK, > + FIELD_PREP(ADS7924_ACQTIME_MASK, 0)); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to configure signal acquire time\n"); > + > + /* Disable power-up time. */ > + ret = regmap_update_bits(data->regmap, ADS7924_PWRCONFIG_REG, > + ADS7924_PWRUPTIME_MASK, > + FIELD_PREP(ADS7924_PWRUPTIME_MASK, 0)); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to configure power-up time\n"); > + > + data->conv_invalid = true; > + > + ret = devm_iio_device_register(dev, indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "failed to register IIO device\n"); > + > + return 0; > +} > + > +static const struct i2c_device_id ads7924_id[] = { > + { "ads7924", 0 }, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, ads7924_id); > + > +static const struct of_device_id ads7924_of_match[] = { > + { .compatible = "ti,ads7924", }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, ads7924_of_match); > + > +static struct i2c_driver ads7924_driver = { > + .driver = { > + .name = "ads7924", > + .of_match_table = ads7924_of_match, > + }, > + .probe_new = ads7924_probe, > + .id_table = ads7924_id, > +}; > + > +module_i2c_driver(ads7924_driver); > + > +MODULE_AUTHOR("Hugo Villeneuve <hvilleneuve@dimonoff.com>"); > +MODULE_DESCRIPTION("Texas Instruments ADS7924 ADC I2C driver"); > +MODULE_LICENSE("GPL"); > -- > 2.30.2 > >
> > > + break; > > > + } > > > + case IIO_CHAN_INFO_SCALE: > > > + vref_uv = regulator_get_voltage(data->vref_reg); > > > + if (vref_uv < 0) { > > > + /* dummy regulator "get_voltage" returns -EINVAL */ > > > + ret = -EINVAL; > > return -EINVAL; > > > + } else { > > > + *val = vref_uv / 1000; /* Convert reg voltage to mV */ > > > + *val2 = ADS7924_BITS; > > > + ret = IIO_VAL_FRACTIONAL_LOG2; > > return IIO_VAL_FR... > > > > > + } > > > + break; > > > + default: > > > + ret = -EINVAL; > > return -EINVAL; > > > + break; > > > + } > > > + > > > + return ret; > > > +} > > Done. With these changes, I propose to also remove last "return ret" (like in rcar-gyroadc.c). Then, maybe also remove break statements? Definitely to both. I was just being lazy whilst commenting ;) No breaks after returns, and as you have noted, the last return ret is unreachable. > > > + > > > + if (num_channels > 0) { > > > + dev_dbg(dev, "found %d ADC channels\n", num_channels); > > > + return 0; > > > + } else { > > > > As per other review. Give us what we expect which is error paths > > as out of line. > > Already done as suggested by Christophe: > > if (num_channels <= 0) > return -EINVAL; This is an out of line error path because it's indented more than the normal flow. > > dev_dbg(dev, "found %d ADC channels\n", num_channels); > return 0; > > Although I do not fully understand what you mean by "...error paths as out of line"? Do you mean to drop the debug message? See above. > > > > > > + return -EINVAL; > > > + } > > > +}
diff --git a/MAINTAINERS b/MAINTAINERS index 9ff472ca1244..94eb68076f0c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20776,6 +20776,13 @@ M: Robert Richter <rric@kernel.org> S: Odd Fixes F: drivers/gpio/gpio-thunderx.c +TI ADS7924 ADC DRIVER +M: Hugo Villeneuve <hvilleneuve@dimonoff.com> +L: linux-iio@vger.kernel.org +S: Supported +F: Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml +F: drivers/iio/adc/ti-ads7924.c + TI AM437X VPFE DRIVER M: "Lad, Prabhakar" <prabhakar.csengg@gmail.com> L: linux-media@vger.kernel.org diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 7d6e74189a2d..99468c64daf9 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -1208,6 +1208,17 @@ config TI_ADS1015 This driver can also be built as a module. If so, the module will be called ti-ads1015. +config TI_ADS7924 + tristate "Texas Instruments ADS7924 ADC" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get support for Texas Instruments ADS7924 + 4 channels, 12-bit I2C ADC chip. + + This driver can also be built as a module. If so, the module will be + called ti-ads7924. + config TI_ADS7950 tristate "Texas Instruments ADS7950 ADC driver" depends on SPI && GPIOLIB diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index 6e08415c3f3a..1e5bdf47a091 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -107,6 +107,7 @@ obj-$(CONFIG_TI_ADC108S102) += ti-adc108s102.o obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o +obj-$(CONFIG_TI_ADS7924) += ti-ads7924.o obj-$(CONFIG_TI_ADS7950) += ti-ads7950.o obj-$(CONFIG_TI_ADS8344) += ti-ads8344.o obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o diff --git a/drivers/iio/adc/ti-ads7924.c b/drivers/iio/adc/ti-ads7924.c new file mode 100644 index 000000000000..8384f3451236 --- /dev/null +++ b/drivers/iio/adc/ti-ads7924.c @@ -0,0 +1,492 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * IIO driver for Texas Instruments ADS7924 ADC, 12-bit, 4-Channels, I2C + * + * iio/adc/ti-ads7924.c + * Author: Hugo Villeneuve <hvilleneuve@dimonoff.com> + * Copyright 2022 DimOnOff + * + * based on iio/adc/ti-ads1015.c + * Copyright (c) 2016, Intel Corporation. + * + * Datasheet: https://www.ti.com/lit/gpn/ads7924 + */ + +#include <linux/delay.h> +#include <linux/gpio/consumer.h> +#include <linux/init.h> +#include <linux/irq.h> +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> + +#include <linux/iio/iio.h> +#include <linux/iio/types.h> + +#define ADS7924_CHANNELS 4 +#define ADS7924_BITS 12 +#define ADS7924_DATA_SHIFT 4 + +/* Registers. */ +#define ADS7924_MODECNTRL_REG 0x00 +#define ADS7924_INTCNTRL_REG 0x01 +#define ADS7924_DATA0_U_REG 0x02 +#define ADS7924_DATA0_L_REG 0x03 +#define ADS7924_DATA1_U_REG 0x04 +#define ADS7924_DATA1_L_REG 0x05 +#define ADS7924_DATA2_U_REG 0x06 +#define ADS7924_DATA2_L_REG 0x07 +#define ADS7924_DATA3_U_REG 0x08 +#define ADS7924_DATA3_L_REG 0x09 +#define ADS7924_ULR0_REG 0x0A +#define ADS7924_LLR0_REG 0x0B +#define ADS7924_ULR1_REG 0x0C +#define ADS7924_LLR1_REG 0x0D +#define ADS7924_ULR2_REG 0x0E +#define ADS7924_LLR2_REG 0x0F +#define ADS7924_ULR3_REG 0x10 +#define ADS7924_LLR3_REG 0x11 +#define ADS7924_INTCONFIG_REG 0x12 +#define ADS7924_SLPCONFIG_REG 0x13 +#define ADS7924_ACQCONFIG_REG 0x14 +#define ADS7924_PWRCONFIG_REG 0x15 +#define ADS7924_RESET_REG 0x16 + +/* + * Register address INC bit: when set to '1', the register address is + * automatically incremented after every register read which allows convenient + * reading of multiple registers. Set INC to '0' when reading a single register. + */ +#define ADS7924_AUTO_INCREMENT_BIT BIT(7) + +#define ADS7924_MODECNTRL_MODE_MASK GENMASK(7, 2) + +#define ADS7924_MODECNTRL_SEL_MASK GENMASK(1, 0) + +#define ADS7924_CFG_INTPOL_BIT 1 +#define ADS7924_CFG_INTTRIG_BIT 0 + +#define ADS7924_CFG_INTPOL_MASK BIT(ADS7924_CFG_INTPOL_BIT) +#define ADS7924_CFG_INTTRIG_MASK BIT(ADS7924_CFG_INTTRIG_BIT) + +/* Interrupt pin polarity */ +#define ADS7924_CFG_INTPOL_LOW 0 +#define ADS7924_CFG_INTPOL_HIGH 1 + +/* Interrupt pin signaling */ +#define ADS7924_CFG_INTTRIG_LEVEL 0 +#define ADS7924_CFG_INTTRIG_EDGE 1 + +/* Mode control values */ +#define ADS7924_MODECNTRL_IDLE 0x00 +#define ADS7924_MODECNTRL_AWAKE 0x20 +#define ADS7924_MODECNTRL_MANUAL_SINGLE 0x30 +#define ADS7924_MODECNTRL_MANUAL_SCAN 0x32 +#define ADS7924_MODECNTRL_AUTO_SINGLE 0x31 +#define ADS7924_MODECNTRL_AUTO_SCAN 0x33 +#define ADS7924_MODECNTRL_AUTO_SINGLE_SLEEP 0x39 +#define ADS7924_MODECNTRL_AUTO_SCAN_SLEEP 0x3B +#define ADS7924_MODECNTRL_AUTO_BURST_SLEEP 0x3F + +#define ADS7924_ACQTIME_MASK GENMASK(4, 0) + +#define ADS7924_PWRUPTIME_MASK GENMASK(4, 0) + +/* + * The power-up time is allowed to elapse whenever the device has been shutdown + * in idle mode. Power-up time can allow external circuits, such as an + * operational amplifier, between the MUXOUT and ADCIN pins to turn on. + * The nominal time programmed by the PUTIME[4:0] register bits is given by: + * t PU = PWRUPTIME[4:0] × 2 μs + * If a power-up time is not required, set the bits to '0' to effectively bypass. + */ +#define ADS7924_PWRUPTIME_US 0 /* Bypass (0us). */ + +/* + * Acquisition Time according to ACQTIME[4:0] register bits. + * The Acquisition Time is given by: + * t ACQ = (ACQTIME[4:0] × 2 μs) + 6 μs + * Using default value of 0 for ACQTIME[4:0] results in a minimum acquisition + * time of 6us. + */ +#define ADS7924_ACQTIME_US 6 + +/* The conversion time is always 4μs and cannot be programmed by the user. */ +#define ADS7924_CONVTIME_US 4 + +#define ADS7924_TOTAL_CONVTIME_US (ADS7924_PWRUPTIME_US + ADS7924_ACQTIME_US + \ + ADS7924_CONVTIME_US) + +#define ADS7924_SLEEP_DELAY_MS 2000 + +#define ADS7924_V_CHAN(_chan, _addr) { \ + .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .channel = _chan, \ + .address = _addr, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .datasheet_name = "AIN"#_chan, \ +} + +struct ads7924_data { + struct device *dev; + struct regmap *regmap; + struct regulator *vref_reg; + + /* GPIO descriptor for device hard-reset pin. */ + struct gpio_desc *reset_gpio; + + /* + * Protects ADC ops, e.g: concurrent sysfs/buffered + * data reads, configuration updates + */ + struct mutex lock; + + /* + * Set to true when the ADC is switched to the continuous-conversion + * mode and exits from a power-down state. This flag is used to avoid + * getting the stale result from the conversion register. + */ + bool conv_invalid; +}; + +static bool ads7924_is_writeable_reg(struct device *dev, unsigned int reg) +{ + switch (reg) { + case ADS7924_MODECNTRL_REG: + case ADS7924_INTCNTRL_REG: + case ADS7924_ULR0_REG: + case ADS7924_LLR0_REG: + case ADS7924_ULR1_REG: + case ADS7924_LLR1_REG: + case ADS7924_ULR2_REG: + case ADS7924_LLR2_REG: + case ADS7924_ULR3_REG: + case ADS7924_LLR3_REG: + case ADS7924_INTCONFIG_REG: + case ADS7924_SLPCONFIG_REG: + case ADS7924_ACQCONFIG_REG: + case ADS7924_PWRCONFIG_REG: + case ADS7924_RESET_REG: + return true; + default: + return false; + } +} + +static const struct regmap_config ads7924_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = ADS7924_RESET_REG, + .writeable_reg = ads7924_is_writeable_reg, +}; + +static const struct iio_chan_spec ads7924_channels[] = { + ADS7924_V_CHAN(0, ADS7924_DATA0_U_REG), + ADS7924_V_CHAN(1, ADS7924_DATA1_U_REG), + ADS7924_V_CHAN(2, ADS7924_DATA2_U_REG), + ADS7924_V_CHAN(3, ADS7924_DATA3_U_REG), +}; + +static +int ads7924_get_adc_result(struct ads7924_data *data, + struct iio_chan_spec const *chan, int *val) +{ + int ret; + __be16 be_val; + + if (chan->channel < 0 || chan->channel >= ADS7924_CHANNELS) + return -EINVAL; + + if (data->conv_invalid) { + int conv_time; + + conv_time = ADS7924_TOTAL_CONVTIME_US; + /* Allow 10% for internal clock inaccuracy. */ + conv_time += conv_time / 10; + usleep_range(conv_time, conv_time + 1); + data->conv_invalid = false; + } + + ret = regmap_raw_read(data->regmap, ADS7924_AUTO_INCREMENT_BIT | + chan->address, &be_val, sizeof(be_val)); + if (ret) + return ret; + + *val = be16_to_cpu(be_val); + *val = *val >> ADS7924_DATA_SHIFT; + + return ret; +} + +static int ads7924_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + int ret, vref_uv; + struct ads7924_data *data = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_RAW: { + mutex_lock(&data->lock); + ret = ads7924_get_adc_result(data, chan, val); + mutex_unlock(&data->lock); + if (ret < 0) + dev_dbg(data->dev, "%s error1\n", __func__); + else + ret = IIO_VAL_INT; + break; + } + case IIO_CHAN_INFO_SCALE: + vref_uv = regulator_get_voltage(data->vref_reg); + if (vref_uv < 0) { + /* dummy regulator "get_voltage" returns -EINVAL */ + ret = -EINVAL; + } else { + *val = vref_uv / 1000; /* Convert reg voltage to mV */ + *val2 = ADS7924_BITS; + ret = IIO_VAL_FRACTIONAL_LOG2; + } + break; + default: + ret = -EINVAL; + break; + } + + return ret; +} + +static const struct iio_info ads7924_info = { + .read_raw = ads7924_read_raw, +}; + +static int ads7924_get_channels_config(struct i2c_client *client, + struct iio_dev *indio_dev) +{ + struct ads7924_data *priv = iio_priv(indio_dev); + struct device *dev = priv->dev; + struct fwnode_handle *node; + int num_channels = 0; + + device_for_each_child_node(dev, node) { + u32 pval; + unsigned int channel; + + if (fwnode_property_read_u32(node, "reg", &pval)) { + dev_err(dev, "invalid reg on %pfw\n", node); + continue; + } + + channel = pval; + if (channel >= ADS7924_CHANNELS) { + dev_err(dev, "invalid channel index %d on %pfw\n", + channel, node); + continue; + } + + num_channels++; + } + + if (num_channels > 0) { + dev_dbg(dev, "found %d ADC channels\n", num_channels); + return 0; + } else { + return -EINVAL; + } +} + +static int ads7924_set_conv_mode(struct ads7924_data *data, int mode) +{ + int ret; + unsigned int mode_field; + struct device *dev = data->dev; + + /* + * When switching between modes, be sure to first select the Awake mode + * and then switch to the desired mode. This procedure ensures the + * internal control logic is properly synchronized. + */ + if (mode != ADS7924_MODECNTRL_IDLE) { + mode_field = FIELD_PREP(ADS7924_MODECNTRL_MODE_MASK, + ADS7924_MODECNTRL_AWAKE); + + ret = regmap_update_bits(data->regmap, ADS7924_MODECNTRL_REG, + ADS7924_MODECNTRL_MODE_MASK, + mode_field); + if (ret) { + dev_warn(dev, "failed to set awake mode (%pe)\n", + ERR_PTR(ret)); + return ret; + } + } + + mode_field = FIELD_PREP(ADS7924_MODECNTRL_MODE_MASK, mode); + + ret = regmap_update_bits(data->regmap, ADS7924_MODECNTRL_REG, + ADS7924_MODECNTRL_MODE_MASK, mode_field); + if (ret) + dev_warn(dev, "failed to set mode %d (%pe)\n", mode, + ERR_PTR(ret)); + + return ret; +} + +static int ads7924_reset(struct iio_dev *indio_dev) +{ + struct ads7924_data *data = iio_priv(indio_dev); + + if (data->reset_gpio) { + gpiod_set_value(data->reset_gpio, 1); /* Assert. */ + /* Educated guess: assert time not specified in datasheet... */ + mdelay(100); + gpiod_set_value(data->reset_gpio, 0); /* Deassert. */ + } else { + int ret; + + /* + * A write of 10101010 to this register will generate a + * software reset of the ADS7924. + */ + ret = regmap_write(data->regmap, ADS7924_RESET_REG, 0b10101010); + if (ret) + return ret; + } + + return 0; +}; + +static void ads7924_reg_disable(void *data) +{ + regulator_disable(data); +} + +static void ads7924_set_idle_mode(void *data) +{ + ads7924_set_conv_mode(data, ADS7924_MODECNTRL_IDLE); +} + +static int ads7924_probe(struct i2c_client *client) +{ + struct iio_dev *indio_dev; + struct ads7924_data *data; + struct device *dev = &client->dev; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + + data->dev = dev; + + /* Initialize the reset GPIO as output with an initial value of 0. */ + data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(data->reset_gpio)) + return dev_err_probe(dev, PTR_ERR(data->reset_gpio), + "failed to get request reset GPIO\n"); + + mutex_init(&data->lock); + + indio_dev->name = "ads7924"; + indio_dev->modes = INDIO_DIRECT_MODE; + + indio_dev->channels = ads7924_channels; + indio_dev->num_channels = ARRAY_SIZE(ads7924_channels); + indio_dev->info = &ads7924_info; + + ret = ads7924_get_channels_config(client, indio_dev); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to get channels configuration\n"); + + data->regmap = devm_regmap_init_i2c(client, &ads7924_regmap_config); + if (IS_ERR(data->regmap)) + return dev_err_probe(dev, PTR_ERR(data->regmap), + "failed to init regmap\n"); + + data->vref_reg = devm_regulator_get(dev, "vref"); + if (IS_ERR(data->vref_reg)) + return dev_err_probe(dev, PTR_ERR(data->vref_reg), + "failed to get vref regulator\n"); + + ret = regulator_enable(data->vref_reg); + if (ret) + return dev_err_probe(dev, ret, + "failed to enable regulator\n"); + + ret = devm_add_action_or_reset(dev, ads7924_reg_disable, data->vref_reg); + if (ret) + return dev_err_probe(dev, ret, + "failed to add regulator disable action\n"); + + ret = ads7924_reset(indio_dev); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to reset device\n"); + + ret = devm_add_action_or_reset(dev, ads7924_set_idle_mode, data); + if (ret) + return dev_err_probe(dev, ret, + "failed to add idle mode action\n"); + + ret = ads7924_set_conv_mode(data, ADS7924_MODECNTRL_AUTO_SCAN); + if (ret) + return dev_err_probe(dev, ret, + "failed to set conversion mode\n"); + + /* Use minimum signal acquire time. */ + ret = regmap_update_bits(data->regmap, ADS7924_ACQCONFIG_REG, + ADS7924_ACQTIME_MASK, + FIELD_PREP(ADS7924_ACQTIME_MASK, 0)); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to configure signal acquire time\n"); + + /* Disable power-up time. */ + ret = regmap_update_bits(data->regmap, ADS7924_PWRCONFIG_REG, + ADS7924_PWRUPTIME_MASK, + FIELD_PREP(ADS7924_PWRUPTIME_MASK, 0)); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to configure power-up time\n"); + + data->conv_invalid = true; + + ret = devm_iio_device_register(dev, indio_dev); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to register IIO device\n"); + + return 0; +} + +static const struct i2c_device_id ads7924_id[] = { + { "ads7924", 0 }, + {} +}; +MODULE_DEVICE_TABLE(i2c, ads7924_id); + +static const struct of_device_id ads7924_of_match[] = { + { .compatible = "ti,ads7924", }, + {} +}; +MODULE_DEVICE_TABLE(of, ads7924_of_match); + +static struct i2c_driver ads7924_driver = { + .driver = { + .name = "ads7924", + .of_match_table = ads7924_of_match, + }, + .probe_new = ads7924_probe, + .id_table = ads7924_id, +}; + +module_i2c_driver(ads7924_driver); + +MODULE_AUTHOR("Hugo Villeneuve <hvilleneuve@dimonoff.com>"); +MODULE_DESCRIPTION("Texas Instruments ADS7924 ADC I2C driver"); +MODULE_LICENSE("GPL");