| Message ID | e44851669ce7e91d1295ab7352535c93b89d35bf.1738328714.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support ROHM BD79124 ADC/GPO | expand |
On Fri, 31 Jan 2025 15:37:48 +0200 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports > an automatic measurement mode, with an alarm interrupt for out-of-window > measurements. The window is configurable for each channel. > > The I2C protocol for manual start of the measurement and data reading is > somewhat peculiar. It requires the master to do clock stretching after > sending the I2C slave-address until the slave has captured the data. > Needless to say this is not well suopported by the I2C controllers. From what I recall that is in the I2C spec, so in theory should be supported. Ah well. > > Thus the driver does not support the BD79124's manual measurement mode > but implements the measurements using automatic measurement mode relying > on the BD79124's ability of storing latest measurements into register. > > The driver does also support configuring the threshold events for > detecting the out-of-window events. > > The BD79124 keeps asserting IRQ for as long as the measured voltage is > out of the configured window. Thus the driver disables the event when > first event is handled. This prevents the user-space from choking on the > events - but it also requires the user space to reconfigure and > re-enable the monitored event when it wants to keep monitoring for new > occurrences. > > It is worth noting that the ADC input pins can be also configured as > general purpose outputs. The pin mode should be configured using pincmux > driver. We shouldn't be presenting channels that are configure for GPIOs as ADC channels. It is very rare that there is a usecase for any dynamic switching. Normally it's a case of what is wired and so static. Hence build the iio_chan_spec array for just the channels you want, not the the lot. Channel sub nodes in the DT are how we most commonly specify what is wired. Few other comments inline, Jonathan > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > --- > > Regarding disabling the event upon reception - is this totally strange? > Is regular userspace compeletely unprepared for this, and better > prepared for handling large amounts of continuous events? > > The BD79124 should not cause a total CPU-blocking IRQ storm because the > driver uses the autonomous sequencer mode - which has minimum of 0.75 > millisecond delay between measurements. So, new IRQs can be raised with > this interval. (The 0.75 mS includes handling and acking / status reading > delays - so there is still not much time for things done outside of the > IRQ handling...) > diff --git a/drivers/iio/adc/rohm-bd79124-adc.c b/drivers/iio/adc/rohm-bd79124-adc.c > new file mode 100644 > index 000000000000..7c95a1de1e71 > --- /dev/null > +++ b/drivers/iio/adc/rohm-bd79124-adc.c > @@ -0,0 +1,890 @@ > +enum { > + BD79124_CH_0, If they just map to integers of the same name, may not be worth bothering with the enum! > + BD79124_CH_1, > + BD79124_CH_2, > + BD79124_CH_3, > + BD79124_CH_4, > + BD79124_CH_5, > + BD79124_CH_6, > + BD79124_CH_7, > + BD79124_MAX_NUM_CHANNELS > +}; > +struct bd79124_raw { > + u8 bit0_3; /* Is set in high bits of the byte */ > + u8 bit4_11; > +}; > +#define BD79124_RAW_TO_INT(r) ((r.bit4_11 << 4) | (r.bit0_3 >> 4)) You could do this as an endian conversion and a single shift I think. Might be slightly simpler. > + > +/* > + * The high and low limits as well as the recent result values are stored in > + * the same way in 2 consequent registers. The first register contains 4 bits > + * of the value. These bits are stored in the high bits [7:4] of register, but > + * they represent the low bits [3:0] of the value. > + * The value bits [11:4] are stored in the next register. > + * > + * Read data from register and convert to integer. > + */ > +static int bd79124_read_reg_to_int(struct bd79124_data *d, int reg, > + unsigned int *val) > +{ > + int ret; > + struct bd79124_raw raw; > + > + ret = regmap_bulk_read(d->map, reg, &raw, sizeof(raw)); > + if (ret) > + dev_dbg(d->dev, "bulk_read failed %d\n", ret); If it failed, then shouldn't set *val. > + *val = BD79124_RAW_TO_INT(raw); > + > + return ret; > +} > + > +/* > + * The high and low limits as well as the recent result values are stored in > + * the same way in 2 consequent registers. The first register contains 4 bits > + * of the value. These bits are stored in the high bits [7:4] of register, but > + * they represent the low bits [3:0] of the value. > + * The value bits [11:4] are stored in the next regoster. > + * > + * Conver the integer to register format and write it using rmw cycle. > + */ > +static int bd79124_write_int_to_reg(struct bd79124_data *d, int reg, > + unsigned int val) > +{ > + struct bd79124_raw raw; > + int ret, tmp; > + > + raw.bit4_11 = (u8)(val >> 4); > + raw.bit0_3 = (u8)(val << 4); > + > + ret = regmap_read(d->map, reg, &tmp); > + if (ret) > + return ret; > + > + raw.bit0_3 |= (0xf & tmp); > + > + return regmap_bulk_write(d->map, reg, &raw, sizeof(raw)); > +} > + > +static const struct iio_event_spec bd79124_events[] = { > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_RISING, > + .mask_separate = BIT(IIO_EV_INFO_VALUE) | > + BIT(IIO_EV_INFO_ENABLE), > + }, > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_FALLING, > + .mask_separate = BIT(IIO_EV_INFO_VALUE) | > + BIT(IIO_EV_INFO_ENABLE), > + }, > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_EITHER, > + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS), > + }, > +}; > + > +#define BD79124_CHAN(idx) { \ > + .type = IIO_VOLTAGE, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .indexed = 1, \ > + .channel = idx, \ > +} > + > +#define BD79124_CHAN_EV(idx) { \ > + .type = IIO_VOLTAGE, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ > + .indexed = 1, \ > + .channel = idx, \ > + .event_spec = bd79124_events, \ > + .num_event_specs = ARRAY_SIZE(bd79124_events), \ > +} > + > +static const struct iio_chan_spec bd79124_channels[] = { > + BD79124_CHAN_EV(0), > + BD79124_CHAN_EV(1), > + BD79124_CHAN_EV(2), > + BD79124_CHAN_EV(3), > + BD79124_CHAN_EV(4), > + BD79124_CHAN_EV(5), > + BD79124_CHAN_EV(6), > + BD79124_CHAN_EV(7), > +}; > + > +static const struct iio_chan_spec bd79124_channels_noirq[] = { > + BD79124_CHAN(0), > + BD79124_CHAN(1), > + BD79124_CHAN(2), > + BD79124_CHAN(3), > + BD79124_CHAN(4), > + BD79124_CHAN(5), > + BD79124_CHAN(6), > + BD79124_CHAN(7), > +}; > + > +/* > + * The BD79124 supports muxing the pins as ADC inputs or as a general purpose > + * output. This muxing is handled by a pinmux driver. Here we just check the > + * settings from the register, and disallow using the pin if pinmux is set to > + * GPO. > + * > + * NOTE: This driver does not perform any locking to ensure the pinmux stays > + * toggled to ADC for the duration of the whatever operation is being done. > + * It is responsibility of the user to configure the pinmux. > + */ > +static bool bd79124_chan_is_adc(struct bd79124_data *d, unsigned int offset) > +{ > + int ret, val; > + > + ret = regmap_read(d->map, BD79124_REG_PINCFG, &val); > + /* If read fails, don't allow using as AIN (to be on a safe side) */ > + if (ret) > + return 0; > + > + return !(val & BIT(offset)); > +} > + > +static int bd79124_read_event_value(struct iio_dev *idev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int *val, > + int *val2) > +{ > + struct bd79124_data *d = iio_priv(idev); > + int ret, reg; > + > + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) > + return -EINVAL; > + > + /* ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan->channel)) > + return -EBUSY; > + > + switch (info) { > + case IIO_EV_INFO_VALUE: > + if (dir == IIO_EV_DIR_RISING) > + reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel); > + else if (dir == IIO_EV_DIR_FALLING) > + reg = BD79124_GET_LOW_LIMIT_REG(chan->channel); > + else > + return -EINVAL; > + > + ret = bd79124_read_reg_to_int(d, reg, val); > + if (ret) > + return ret; > + > + return IIO_VAL_INT; > + > + case IIO_EV_INFO_HYSTERESIS: > + reg = BD79124_GET_HYSTERESIS_REG(chan->channel); > + ret = regmap_read(d->map, reg, val); > + if (ret) > + return ret; > + /* Mask the non hysteresis bits */ > + *val &= BD79124_MASK_HYSTERESIS; > + /* > + * The data-sheet says the hysteresis register value needs to be > + * sifted left by 3 (or multiplied by 8, depending on the > + * page :] ) > + */ > + *val <<= 3; > + > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > +} > + > +static int bd79124_start_measurement(struct bd79124_data *d, int chan) > +{ > + int val, ret, regval; > + > + /* ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan)) > + return -EBUSY; > + > + /* See if already started */ > + ret = regmap_read(d->map, BD79124_REG_AUTO_CHANNELS, &val); > + if (val & BIT(chan)) > + return 0; > + > + /* Stop the sequencer */ > + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > + if (ret) > + return ret; > + > + /* Add the channel to measured channels */ > + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, val | BIT(chan)); > + if (ret) > + return ret; > + > + /* Restart the sequencer */ > + ret = regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > + if (ret) > + return ret; > + > + /* > + * Start the measurement at the background. Don't bother checking if > + * it was started, regmap has cache > + */ > + regval = FIELD_PREP(BD79124_MASK_CONV_MODE, BD79124_CONV_MODE_AUTO); > + > + return regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, > + BD79124_MASK_CONV_MODE, regval); > +} > + > +static int bd79124_stop_measurement(struct bd79124_data *d, int chan) > +{ > + int val, ret; > + > + /* Ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan)) > + return -EBUSY; > + > + /* If alarm is requested for the channel we won't stop measurement */ > + if (d->alarm_monitored[chan]) > + return 0; > + > + /* See if already stopped */ > + ret = regmap_read(d->map, BD79124_REG_AUTO_CHANNELS, &val); > + if (!(val & BIT(chan))) > + return 0; > + > + /* Stop the sequencer */ > + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > + > + /* Clear the channel from the measured channels */ > + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, > + (~BIT(chan)) & val); > + if (ret) > + return ret; > + > + /* Stop background conversion if it was the last channel */ > + if (!((~BIT(chan)) & val)) { > + int regval = FIELD_PREP(BD79124_MASK_CONV_MODE, > + BD79124_CONV_MODE_MANSEQ); > + > + ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, > + BD79124_MASK_CONV_MODE, regval); > + if (ret) > + return ret; > + } > + > + /* Restart the sequencer */ > + return regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > +} > + > +static int bd79124_read_event_config(struct iio_dev *idev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir) > +{ > + struct bd79124_data *d = iio_priv(idev); > + int val, ret, reg, disabled; > + > + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) > + return -EINVAL; > + > + /* ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan->channel)) > + return -EBUSY; > + > + ret = regmap_read(d->map, BD79124_REG_ALERT_CH_SEL, &val); > + if (ret) > + return ret; > + > + /* The event is disabled if alerts for the channel are disabled */ > + if (!(val & BIT(chan->channel))) > + return 0; > + > + /* > + * If alerts are on, then the event may be disabled if limit is set to > + * the one extreme. (HW does not support disabling rising/falling > + * thresholds independently. Hence we resort to setting high limit to > + * MAX, or low limit to 0 to try effectively disable thresholds). > + */ > + if (dir == IIO_EV_DIR_RISING) { > + reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel); > + disabled = BD79124_HIGH_LIMIT_MAX; > + } else if (dir == IIO_EV_DIR_FALLING) { > + reg = BD79124_GET_LOW_LIMIT_REG(chan->channel); > + disabled = BD79124_LOW_LIMIT_MIN; > + } else { > + return -EINVAL; > + } > + ret = bd79124_read_reg_to_int(d, reg, &val); > + if (ret) > + return ret; > + > + return val != disabled; > +} > + > +static int bd79124_disable_event(struct bd79124_data *d, > + enum iio_event_direction dir, int channel) > +{ > + int dir_bit = BIT(dir), reg; > + unsigned int limit; > + > + d->alarm_monitored[channel] &= (~dir_bit); > + /* > + * Set thresholds either to 0 or to 2^12 - 1 as appropriate to prevent > + * alerts and thus disable event generation. > + */ > + if (dir == IIO_EV_DIR_RISING) { > + reg = BD79124_GET_HIGH_LIMIT_REG(channel); > + limit = BD79124_HIGH_LIMIT_MAX; > + } else if (dir == IIO_EV_DIR_FALLING) { > + reg = BD79124_GET_LOW_LIMIT_REG(channel); > + limit = BD79124_LOW_LIMIT_MIN; > + } else { > + return -EINVAL; > + } > + > + /* > + * Stop measurement if there is no more events to monitor. > + * We don't bother checking the retval because the limit > + * setting should in any case effectively disable the alarm. > + */ > + if (!d->alarm_monitored[channel]) { > + bd79124_stop_measurement(d, channel); > + regmap_clear_bits(d->map, BD79124_REG_ALERT_CH_SEL, > + BIT(channel)); > + } > + > + return bd79124_write_int_to_reg(d, reg, limit); > +} > + > +/* Do we need to disable the measurement for the duration of the limit conf ? */ > +static int bd79124_write_event_config(struct iio_dev *idev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, bool state) > +{ > + struct bd79124_data *d = iio_priv(idev); > + int dir_bit = BIT(dir); > + > + guard(mutex)(&d->mutex); > + > + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) > + return -EINVAL; > + > + /* ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan->channel)) > + return -EBUSY; > + > + if (state) { > + int ret; > + > + /* Set channel to be measured */ > + ret = bd79124_start_measurement(d, chan->channel); > + if (ret) > + return ret; > + > + d->alarm_monitored[chan->channel] |= dir_bit; > + > + /* Add the channel to the list of monitored channels */ > + ret = regmap_set_bits(d->map, BD79124_REG_ALERT_CH_SEL, > + BIT(chan->channel)); > + if (ret) > + return ret; > + > + /* > + * Enable comparator. Trust the regmap cache, no need to check > + * if it was already enabled. > + * > + * We could do this in the hw-init, but there may be users who > + * never enable alarms and for them it makes sense to not > + * enable the comparator at probe. > + */ > + return regmap_set_bits(d->map, BD79124_REG_GEN_CFG, > + BD79124_MASK_DWC_EN); > + } > + > + return bd79124_disable_event(d, dir, chan->channel); > +} > + > +static int bd79124_write_event_value(struct iio_dev *idev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + enum iio_event_info info, int val, > + int val2) > +{ > + struct bd79124_data *d = iio_priv(idev); > + int reg; > + > + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) > + return -EINVAL; > + > + /* ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan->channel)) > + return -EBUSY; > + > + switch (info) { > + case IIO_EV_INFO_VALUE: > + if (dir == IIO_EV_DIR_RISING) > + reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel); > + else if (dir == IIO_EV_DIR_FALLING) > + reg = BD79124_GET_LOW_LIMIT_REG(chan->channel); > + else > + return -EINVAL; > + > + return bd79124_write_int_to_reg(d, reg, val); > + > + case IIO_EV_INFO_HYSTERESIS: > + reg = BD79124_GET_HYSTERESIS_REG(chan->channel); > + val >>= 3; > + > + return regmap_update_bits(d->map, reg, BD79124_MASK_HYSTERESIS, > + val); > + default: > + return -EINVAL; > + } > +} > + > +static int bd79124_read_last_result(struct bd79124_data *d, int chan, > + int *result) > +{ > + struct bd79124_raw raw; > + int ret; > + > + ret = regmap_bulk_read(d->map, BD79124_GET_RECENT_RES_REG(chan), &raw, > + sizeof(raw)); > + if (ret) > + return ret; > + > + *result = BD79124_RAW_TO_INT(raw); > + > + return 0; > +} > + > +static int bd79124_single_chan_seq_end(struct bd79124_data *d, int old) > +{ > + int ret; > + > + /* Stop the sequencer */ Fairly obvious from code. Maybe drop these sequencer related comments. > + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > + if (ret) > + return ret; > + > + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, old); > + if (ret) > + return ret; > + > + /* Restart the sequencer */ > + return regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > +} > + > +static int bd79124_read_raw(struct iio_dev *idev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long m) > +{ > + struct bd79124_data *d = iio_priv(idev); > + int ret; > + > + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) > + return -EINVAL; > + > + switch (m) { > + case IIO_CHAN_INFO_RAW: > + { > + int old_chan_cfg, tmp; > + int regval = FIELD_PREP(BD79124_MASK_CONV_MODE, > + BD79124_CONV_MODE_AUTO); > + > + guard(mutex)(&d->mutex); > + > + /* ensure pinmux is set to ADC */ > + if (!bd79124_chan_is_adc(d, chan->channel)) > + return -EBUSY; If you are exposing the channel in sysfs it better not be set to be a GPIO. You need to generate the chan_spec from what is wired for ADC usage vs what is wired for GPIO. > + > + /* > + * Start the automatic conversion. This is needed here if no > + * events have been enabled. > + */ > + ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, > + BD79124_MASK_CONV_MODE, regval); ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, BD79124_MASK_CONV_MODE, FIELD_PREP(BD79124_MASK_CONV_MODE, > + BD79124_CONV_MODE_AUTO)); Is a tiny bit long but meh, it keeps the information all in one place so I think still worth doing it in one go. If not, just set regval the line above this call. > + if (ret) > + return ret; > + > + ret = bd79124_single_chan_seq(d, chan->channel, &old_chan_cfg); > + if (ret) > + return ret; > + > + /* > + * The maximum conversion time is 6 uS. Ensure the sample is > + * ready The second bit not needed. /* Maximum conversion time is 6 usecs */ is fine. > + */ > + udelay(6); > + > + ret = bd79124_read_last_result(d, chan->channel, val); > + /* Try unconditionally returning the chan config */ We can see the return to old config from the code. Comment should say why instead. > + tmp = bd79124_single_chan_seq_end(d, old_chan_cfg); > + if (tmp) > + dev_err(d->dev, > + "Failed to return config. Alarms may be disabled\n"); > + > + if (ret) > + return ret; > + > + return IIO_VAL_INT; > + } > + case IIO_CHAN_INFO_SCALE: > + *val = d->vmax / 1000; > + *val2 = BD79124_ADC_BITS; > + return IIO_VAL_FRACTIONAL_LOG2; > + default: > + return -EINVAL; > + } > +} > +static irqreturn_t bd79124_event_handler(int irq, void *priv) > +{ > + int ret, i_hi, i_lo, i; > + struct iio_dev *idev = priv; > + struct bd79124_data *d = iio_priv(idev); > + > + /* > + * Return IRQ_NONE if bailing-out without acking. This allows the IRQ > + * subsystem to disable the offending IRQ line if we get a hardware > + * problem. This behaviour has saved my poor bottom a few times in the > + * past as, instead of getting unusably unresponsive, the system has > + * spilled out the magic words "...nobody cared". *laughs*. Maybe the comment isn't strictly necessary but it cheered up my Friday. > + */ > + ret = regmap_read(d->map, BD79124_REG_EVENT_FLAG_HI, &i_hi); > + if (ret) > + return IRQ_NONE; > + > + ret = regmap_read(d->map, BD79124_REG_EVENT_FLAG_LO, &i_lo); > + if (ret) > + return IRQ_NONE; > + > + if (!i_lo && !i_hi) > + return IRQ_NONE; > + > + for (i = 0; i < BD79124_MAX_NUM_CHANNELS; i++) { > + u64 ecode; > + > + if (BIT(i) & i_hi) { Maybe cleaner as a pair of for_each_set_bit() loops. > + ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i, > + IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING); > + > + iio_push_event(idev, ecode, d->timestamp); > + /* > + * The BD79124 keeps the IRQ asserted for as long as > + * the voltage exceeds the threshold. It may not serve > + * the purpose to keep the IRQ firing and events > + * generated in a loop because it may yield the > + * userspace to have some problems when event handling > + * there is slow. > + * > + * Thus, we disable the event for the channel. Userspace > + * needs to re-enable the event. That's not pretty. So I'd prefer a timeout and autoreenable if we can. > + * > + * We don't check the result as there is not much to do. > + * Also, this should not lead to total IRQ storm > + * because the BD79124 is running in autonomous mode, > + * which means there is by minimum 0.75 mS idle time > + * between changing the channels. That should be > + * sufficient to show some life on system, even if the > + * event handling couldn't keep up. > + */ > + bd79124_disable_event(d, IIO_EV_DIR_RISING, i); > + } > + if (BIT(i) & i_lo) { > + ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i, > + IIO_EV_TYPE_THRESH, IIO_EV_DIR_FALLING); > + > + iio_push_event(idev, ecode, d->timestamp); The same interrupt thing doesn't apply to falling? That's odd. > + } > + } > + > + ret = regmap_write(d->map, BD79124_REG_EVENT_FLAG_HI, i_hi); > + if (ret) > + return IRQ_NONE; > + > + ret = regmap_write(d->map, BD79124_REG_EVENT_FLAG_LO, i_lo); > + if (ret) > + return IRQ_NONE; > + > + return IRQ_HANDLED; > +} > +static int bd79124_hw_init(struct bd79124_data *d) > +{ > + int ret, regval; > + > + /* Stop auto sequencer */ > + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_START); > + if (ret) > + return ret; > + > + /* Enable writing the measured values to the regsters */ > + ret = regmap_set_bits(d->map, BD79124_REG_GEN_CFG, > + BD79124_MASK_STATS_EN); Check alignment of that second line. Generally align to just after ( > + if (ret) > + return ret; > + > + /* Set no channels to be auto-measured */ > + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, 0x0); > + if (ret) > + return ret; > + > + /* Set no channels to be manually measured */ > + ret = regmap_write(d->map, BD79124_REG_MANUAL_CHANNELS, 0x0); > + if (ret) > + return ret; > + > + /* Set the measurement interval to 0.75 mS */ > + > + regval = FIELD_PREP(BD79124_MASK_AUTO_INTERVAL, BD79124_INTERVAL_075); > + ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, > + BD79124_MASK_AUTO_INTERVAL, regval); > + if (ret) > + return ret; > + > + /* Sequencer mode to auto */ > + ret = regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, > + BD79124_MASK_SEQ_SEQ); > + if (ret) > + return ret; > + > + regval = FIELD_PREP(BD79124_MASK_CONV_MODE, BD79124_CONV_MODE_MANSEQ); > + /* Don't start the measurement */ > + return regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, > + BD79124_MASK_CONV_MODE, BD79124_CONV_MODE_MANSEQ); > + > +} > + > +#define BD79124_VDD_MAX 5250000 > +#define BD79124_VDD_MIN 2700000 They are real numbers, so if you need them but them values inline. > + > +static int bd79124_probe(struct platform_device *pdev) > +{ > + struct bd79124_data *d; > + struct iio_dev *idev; Maybe copy more common naming iio_dev, indio_dev etc just for familiarity. For device data, d is a little brief! > + int ret, irq, *parent_data; > + > + idev = devm_iio_device_alloc(&pdev->dev, sizeof(*d)); > + if (!idev) > + return -ENOMEM; > + > + d = iio_priv(idev); > + > + parent_data = dev_get_drvdata(pdev->dev.parent); > + if (!parent_data) > + return dev_err_probe(&pdev->dev, -EINVAL, > + "reference voltage missing\n"); > + > + /* > + * Recommended VDD voltage from the data-sheet: > + * Analog/Digital Supply Voltage VDD 2.70 - 5.25 V > + */ > + d->vmax = *parent_data; > + if (d->vmax < BD79124_VDD_MIN || d->vmax > BD79124_VDD_MAX) { Normally we consider this 'not our problem'. If someone has wired their system wrong we don't bother to tell them. I don't mind having the check though if you find it particularly useful. > + return dev_err_probe(d->dev, -EINVAL, > + "VDD (%d) out of range [%d - %d]\n", > + d->vmax, BD79124_VDD_MIN, BD79124_VDD_MAX); > + > + return -EINVAL; Already returned. > + } > + > + irq = platform_get_irq_byname_optional(pdev, "thresh-alert"); Is there more than one? If not why do we need to do it by name? > + if (irq < 0) { > + if (irq == -EPROBE_DEFER) > + return irq; > + > + idev->channels = &bd79124_channels_noirq[0]; > + idev->num_channels = ARRAY_SIZE(bd79124_channels_noirq); > + dev_dbg(d->dev, "No IRQ found, events disabled\n"); > + } else { > + idev->channels = &bd79124_channels[0]; > + idev->num_channels = ARRAY_SIZE(bd79124_channels); > + } > + > + idev->info = &bd79124_info; > + idev->name = "bd79124"; > + idev->modes = INDIO_DIRECT_MODE; > + > + d->dev = &pdev->dev; > + d->map = dev_get_regmap(d->dev->parent, NULL); > + if (!d->map) > + return dev_err_probe(d->dev, -ENODEV, "No regmap\n"); > + > + mutex_init(&d->mutex); > + > + ret = bd79124_hw_init(d); > + if (ret) > + return ret; > + > + if (irq > 0) { > + ret = devm_request_threaded_irq(d->dev, irq, bd79124_irq_handler, > + &bd79124_event_handler, IRQF_ONESHOT, > + "adc-thresh-alert", idev); > + if (ret) > + return dev_err_probe(d->dev, ret, > + "Failed to register IRQ\n"); > + } > + > + return devm_iio_device_register(d->dev, idev); > +}
On 31/01/2025 19:41, Jonathan Cameron wrote: > On Fri, 31 Jan 2025 15:37:48 +0200 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports >> an automatic measurement mode, with an alarm interrupt for out-of-window >> measurements. The window is configurable for each channel. >> >> The I2C protocol for manual start of the measurement and data reading is >> somewhat peculiar. It requires the master to do clock stretching after >> sending the I2C slave-address until the slave has captured the data. >> Needless to say this is not well suopported by the I2C controllers. > > From what I recall that is in the I2C spec, so in theory should be supported. > Ah well. Could be I am mistaken then. Or, maybe I just misused the term "master to do clock stretching". I know that it is not rare the slave device is keeping the clock down for extended period (in this case so that the measurement would be completed) - but at least I am not aware of any APIs which could be used to cause the _master_ side to keep the SCL low for an extended period after receiving the ACK (after sending the slave address). In this case it would require this driver to be able to set a time for how long the master would keep SCL low after sensing the slave address, before sending the "command" bytes. |S|ADDRESS+R|a|STRETCH|8-bit-i2c-frame|A|8-bit-i2c-frame|A|STRETCH|8-bit-i2c... Above denotes this "master stretching". CAPITALs are initiated by master, lowercase by slave. S, is start, a is ack and R is read-bit. If there is a standard way to implement this in Linux side, then I might consider using it as it'd allowed much higher capture rates. >> It is worth noting that the ADC input pins can be also configured as >> general purpose outputs. The pin mode should be configured using pincmux >> driver. > > We shouldn't be presenting channels that are configure for GPIOs as > ADC channels. It is very rare that there is a usecase for any > dynamic switching. Thanks :) If the dynamic switching is rare, then you're definitely right. I need to see if using the pinmux still makes sense, and if we can implement this while using (separate) pinmux driver. > Normally it's a case of what is wired and > so static. I should implement a device which can be controlled via it's analog output line :) If nothing else then a device shutting down when it's output is pulled low ;) ...Well, I have no real use-case for dynamic config either. > Hence build the iio_chan_spec array for just the > channels you want, not the the lot. Channel sub nodes in the > DT are how we most commonly specify what is wired. Hmm. That'd mean the ADC channels _must_ be defined in DT in order to be usable(?) Well, if this is the usual way, then it should be well known by users. Thanks. ... >> + if (BIT(i) & i_lo) { >> + ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i, >> + IIO_EV_TYPE_THRESH, IIO_EV_DIR_FALLING); >> + >> + iio_push_event(idev, ecode, d->timestamp); > The same interrupt thing doesn't apply to falling? That's odd. > It does. So, not odd, just a regular bug :) Thanks! >> + } >> + } >> + ... >> + >> + irq = platform_get_irq_byname_optional(pdev, "thresh-alert"); > > Is there more than one? If not why do we need to do it by name? I kind of like doing it by name when the IRQs come from a MFD device - which can name them. It is no real extra cost (well, maybe bytes of the added string in kernel, but I guess it's not relevant with only few interrupts) - but it makes it very hard to get it wrong, and it documents the purpose of the IRQ. >> + if (irq < 0) { >> + if (irq == -EPROBE_DEFER) >> + return irq; ... I do thank you for, and agree with, all the rest of the comments! Yours, -- Matti
On Sat, 1 Feb 2025 17:38:20 +0200 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > On 31/01/2025 19:41, Jonathan Cameron wrote: > > On Fri, 31 Jan 2025 15:37:48 +0200 > > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > > >> The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports > >> an automatic measurement mode, with an alarm interrupt for out-of-window > >> measurements. The window is configurable for each channel. > >> > >> The I2C protocol for manual start of the measurement and data reading is > >> somewhat peculiar. It requires the master to do clock stretching after > >> sending the I2C slave-address until the slave has captured the data. > >> Needless to say this is not well suopported by the I2C controllers. > > > > From what I recall that is in the I2C spec, so in theory should be supported. > > Ah well. > > Could be I am mistaken then. Or, maybe I just misused the term "master > to do clock stretching". > > I know that it is not rare the slave device is keeping the clock down > for extended period (in this case so that the measurement would be > completed) - but at least I am not aware of any APIs which could be used > to cause the _master_ side to keep the SCL low for an extended period > after receiving the ACK (after sending the slave address). In this case > it would require this driver to be able to set a time for how long the > master would keep SCL low after sensing the slave address, before > sending the "command" bytes. > > |S|ADDRESS+R|a|STRETCH|8-bit-i2c-frame|A|8-bit-i2c-frame|A|STRETCH|8-bit-i2c... > > Above denotes this "master stretching". CAPITALs are initiated by > master, lowercase by slave. S, is start, a is ack and R is read-bit. Ah. That is indeed more unusual. You were correct that i was thinking of the client side doing the stretching! > > If there is a standard way to implement this in Linux side, then I might > consider using it as it'd allowed much higher capture rates. Not that I'm aware of. Wolfram, have you seen anything like this? > > >> It is worth noting that the ADC input pins can be also configured as > >> general purpose outputs. The pin mode should be configured using pincmux > >> driver. > > > > We shouldn't be presenting channels that are configure for GPIOs as > > ADC channels. It is very rare that there is a usecase for any > > dynamic switching. > > Thanks :) If the dynamic switching is rare, then you're definitely > right. I need to see if using the pinmux still makes sense, and if we > can implement this while using (separate) pinmux driver. > > > Normally it's a case of what is wired and > > so static. > > I should implement a device which can be controlled via it's analog > output line :) If nothing else then a device shutting down when it's > output is pulled low ;) > > ...Well, I have no real use-case for dynamic config either. > > > Hence build the iio_chan_spec array for just the > > channels you want, not the the lot. Channel sub nodes in the > > DT are how we most commonly specify what is wired. > > Hmm. That'd mean the ADC channels _must_ be defined in DT in order to be > usable(?) Well, if this is the usual way, then it should be well known > by users. Thanks. Yes. We basically have two types of binding wrt to channels. 1) Always there - no explicit binding, but also no way to describe anything specific about the channels. 2) Subnode per channel with stuff from adc.yaml and anything device specific. Only channels that that have a node are enabled. There are a few drivers that for historical reasons support both options with 'no channels' meaning 'all channels'. J
On 31/01/2025 19:41, Jonathan Cameron wrote: > On Fri, 31 Jan 2025 15:37:48 +0200 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports >> an automatic measurement mode, with an alarm interrupt for out-of-window >> measurements. The window is configurable for each channel. >> Hi Jonathan, I just sent the v2, where I (think I) addressed all comments except ones below. Just wanted to point out what was not changed and why :) ... > >> +struct bd79124_raw { >> + u8 bit0_3; /* Is set in high bits of the byte */ >> + u8 bit4_11; >> +}; >> +#define BD79124_RAW_TO_INT(r) ((r.bit4_11 << 4) | (r.bit0_3 >> 4)) > You could do this as an endian conversion and a single shift I think. > Might be slightly simpler. I kept this struct with bytes matching the register spec. Doing the endian conversion and then shifting would probably have worked, but my head hurts when I try thinking how the bits settle there. Especially if this is done on a big-endian machine. I can rework this for v3 if you feel very strongly about this. ... > >> +static irqreturn_t bd79124_event_handler(int irq, void *priv) >> +{ >> + int ret, i_hi, i_lo, i; >> + struct iio_dev *idev = priv; >> + struct bd79124_data *d = iio_priv(idev); >> + >> + /* >> + * Return IRQ_NONE if bailing-out without acking. This allows the IRQ >> + * subsystem to disable the offending IRQ line if we get a hardware >> + * problem. This behaviour has saved my poor bottom a few times in the >> + * past as, instead of getting unusably unresponsive, the system has >> + * spilled out the magic words "...nobody cared". > *laughs*. Maybe the comment isn't strictly necessary but it cheered > up my Friday. >> + */ >> + ret = regmap_read(d->map, BD79124_REG_EVENT_FLAG_HI, &i_hi); >> + if (ret) >> + return IRQ_NONE; >> + >> + ret = regmap_read(d->map, BD79124_REG_EVENT_FLAG_LO, &i_lo); >> + if (ret) >> + return IRQ_NONE; >> + >> + if (!i_lo && !i_hi) >> + return IRQ_NONE; >> + >> + for (i = 0; i < BD79124_MAX_NUM_CHANNELS; i++) { >> + u64 ecode; >> + >> + if (BIT(i) & i_hi) { > Maybe cleaner as a pair of > > for_each_set_bit() loops. > I kept the original for 2 reasons. 1. the main reason is that the for_each_set_bit() would want the value read from a register to be in long. Regmap wants to use int. Solving this produced (in my 'humblish' opinion) less readable code. 2. The current implementation has only one loop, which should perhaps be a tiny bit more efficient. >> + ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i, >> + IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING); >> + >> + iio_push_event(idev, ecode, d->timestamp); >> + /* >> + * The BD79124 keeps the IRQ asserted for as long as >> + * the voltage exceeds the threshold. It may not serve >> + * the purpose to keep the IRQ firing and events >> + * generated in a loop because it may yield the >> + * userspace to have some problems when event handling >> + * there is slow. >> + * >> + * Thus, we disable the event for the channel. Userspace >> + * needs to re-enable the event. > > That's not pretty. So I'd prefer a timeout and autoreenable if we can. And I did this, but with constant 1 sec 'grace time' instead of modifiable time-out. I believe this suffices and keeps it simpler. Yours, -- Matti
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 849c90203071..195a61ba5cf4 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -1188,6 +1188,16 @@ config RN5T618_ADC This driver can also be built as a module. If so, the module will be called rn5t618-adc. +config ROHM_BD79124 + tristate "Rohm BD79124 ADC driver" + depends on MFD_ROHM_BD79124 + help + Say yes here to build support for the ROHM BD79124 ADC. The + ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports + also an automatic measurement mode, with an alarm interrupt for + out-of-window measurements. The window is configurable for each + channel. + config ROCKCHIP_SARADC tristate "Rockchip SARADC driver" depends on ARCH_ROCKCHIP || COMPILE_TEST diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile index ee19afba62b7..7049d984682d 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -107,6 +107,7 @@ obj-$(CONFIG_QCOM_VADC_COMMON) += qcom-vadc-common.o obj-$(CONFIG_RCAR_GYRO_ADC) += rcar-gyroadc.o obj-$(CONFIG_RICHTEK_RTQ6056) += rtq6056.o obj-$(CONFIG_RN5T618_ADC) += rn5t618-adc.o +obj-$(CONFIG_ROHM_BD79124) += rohm-bd79124-adc.o obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o obj-$(CONFIG_SC27XX_ADC) += sc27xx_adc.o diff --git a/drivers/iio/adc/rohm-bd79124-adc.c b/drivers/iio/adc/rohm-bd79124-adc.c new file mode 100644 index 000000000000..7c95a1de1e71 --- /dev/null +++ b/drivers/iio/adc/rohm-bd79124-adc.c @@ -0,0 +1,890 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * ROHM ADC driver for BD79124 ADC/GPO device + * https://fscdn.rohm.com/en/products/databook/datasheet/ic/data_converter/dac/bd79124muf-c-e.pdf + * + * Copyright (c) 2025, ROHM Semiconductor. + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/byteorder/generic.h> +#include <linux/device.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/irqreturn.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/types.h> + +#include <linux/iio/events.h> +#include <linux/iio/iio.h> +#include <linux/mfd/rohm-bd79124.h> + +#define BD79124_ADC_BITS 12 +#define BD79124_MASK_CONV_MODE GENMASK(6, 5) +#define BD79124_MASK_AUTO_INTERVAL GENMASK(1, 0) +#define BD79124_CONV_MODE_MANSEQ 0 +#define BD79124_CONV_MODE_AUTO 1 +#define BD79124_INTERVAL_075 0 +#define BD79124_INTERVAL_150 1 +#define BD79124_INTERVAL_300 2 +#define BD79124_INTERVAL_600 3 + +#define BD79124_MASK_DWC_EN BIT(4) +#define BD79124_MASK_STATS_EN BIT(5) +#define BD79124_MASK_SEQ_START BIT(4) +#define BD79124_MASK_SEQ_MODE GENMASK(1, 0) +#define BD79124_MASK_SEQ_MANUAL 0 +#define BD79124_MASK_SEQ_SEQ 1 + +#define BD79124_MASK_HYSTERESIS GENMASK(3, 0) +#define BD79124_LOW_LIMIT_MIN 0 +#define BD79124_HIGH_LIMIT_MAX GENMASK(11, 0) + +/* + * The high limit, low limit and last measurement result are each stored in + * 2 consequtive registers. 4 bits are in the high bits of the 1.st register + * and 8 bits in the next register. + * + * These macros return the address of the 1.st reg for the given channel + */ +#define BD79124_GET_HIGH_LIMIT_REG(ch) (BD79124_REG_HYSTERESIS_CH0 + (ch) * 4) +#define BD79124_GET_LOW_LIMIT_REG(ch) (BD79124_REG_EVENTCOUNT_CH0 + (ch) * 4) +#define BD79124_GET_RECENT_RES_REG(ch) (BD79124_REG_RECENT_CH0_LSB + (ch) * 2) + +/* + * The hysteresis for a channel is stored in the same register where the + * 4 bits of high limit reside. + */ +#define BD79124_GET_HYSTERESIS_REG(ch) BD79124_GET_HIGH_LIMIT_REG(ch) + +enum { + BD79124_CH_0, + BD79124_CH_1, + BD79124_CH_2, + BD79124_CH_3, + BD79124_CH_4, + BD79124_CH_5, + BD79124_CH_6, + BD79124_CH_7, + BD79124_MAX_NUM_CHANNELS +}; + +struct bd79124_data { + s64 timestamp; + struct regmap *map; + struct device *dev; + int vmax; + int alarm_monitored[BD79124_MAX_NUM_CHANNELS]; + /* + * The BD79124 is configured to run the measurements in the background. + * This is done for the event monitoring as well as for the read_raw(). + * Protect the measurement starting/stopping using a mutex. + */ + struct mutex mutex; +}; + +struct bd79124_raw { + u8 bit0_3; /* Is set in high bits of the byte */ + u8 bit4_11; +}; +#define BD79124_RAW_TO_INT(r) ((r.bit4_11 << 4) | (r.bit0_3 >> 4)) + +/* + * The high and low limits as well as the recent result values are stored in + * the same way in 2 consequent registers. The first register contains 4 bits + * of the value. These bits are stored in the high bits [7:4] of register, but + * they represent the low bits [3:0] of the value. + * The value bits [11:4] are stored in the next register. + * + * Read data from register and convert to integer. + */ +static int bd79124_read_reg_to_int(struct bd79124_data *d, int reg, + unsigned int *val) +{ + int ret; + struct bd79124_raw raw; + + ret = regmap_bulk_read(d->map, reg, &raw, sizeof(raw)); + if (ret) + dev_dbg(d->dev, "bulk_read failed %d\n", ret); + *val = BD79124_RAW_TO_INT(raw); + + return ret; +} + +/* + * The high and low limits as well as the recent result values are stored in + * the same way in 2 consequent registers. The first register contains 4 bits + * of the value. These bits are stored in the high bits [7:4] of register, but + * they represent the low bits [3:0] of the value. + * The value bits [11:4] are stored in the next regoster. + * + * Conver the integer to register format and write it using rmw cycle. + */ +static int bd79124_write_int_to_reg(struct bd79124_data *d, int reg, + unsigned int val) +{ + struct bd79124_raw raw; + int ret, tmp; + + raw.bit4_11 = (u8)(val >> 4); + raw.bit0_3 = (u8)(val << 4); + + ret = regmap_read(d->map, reg, &tmp); + if (ret) + return ret; + + raw.bit0_3 |= (0xf & tmp); + + return regmap_bulk_write(d->map, reg, &raw, sizeof(raw)); +} + +static const struct iio_event_spec bd79124_events[] = { + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_RISING, + .mask_separate = BIT(IIO_EV_INFO_VALUE) | + BIT(IIO_EV_INFO_ENABLE), + }, + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_FALLING, + .mask_separate = BIT(IIO_EV_INFO_VALUE) | + BIT(IIO_EV_INFO_ENABLE), + }, + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_EITHER, + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS), + }, +}; + +#define BD79124_CHAN(idx) { \ + .type = IIO_VOLTAGE, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .indexed = 1, \ + .channel = idx, \ +} + +#define BD79124_CHAN_EV(idx) { \ + .type = IIO_VOLTAGE, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ + .indexed = 1, \ + .channel = idx, \ + .event_spec = bd79124_events, \ + .num_event_specs = ARRAY_SIZE(bd79124_events), \ +} + +static const struct iio_chan_spec bd79124_channels[] = { + BD79124_CHAN_EV(0), + BD79124_CHAN_EV(1), + BD79124_CHAN_EV(2), + BD79124_CHAN_EV(3), + BD79124_CHAN_EV(4), + BD79124_CHAN_EV(5), + BD79124_CHAN_EV(6), + BD79124_CHAN_EV(7), +}; + +static const struct iio_chan_spec bd79124_channels_noirq[] = { + BD79124_CHAN(0), + BD79124_CHAN(1), + BD79124_CHAN(2), + BD79124_CHAN(3), + BD79124_CHAN(4), + BD79124_CHAN(5), + BD79124_CHAN(6), + BD79124_CHAN(7), +}; + +/* + * The BD79124 supports muxing the pins as ADC inputs or as a general purpose + * output. This muxing is handled by a pinmux driver. Here we just check the + * settings from the register, and disallow using the pin if pinmux is set to + * GPO. + * + * NOTE: This driver does not perform any locking to ensure the pinmux stays + * toggled to ADC for the duration of the whatever operation is being done. + * It is responsibility of the user to configure the pinmux. + */ +static bool bd79124_chan_is_adc(struct bd79124_data *d, unsigned int offset) +{ + int ret, val; + + ret = regmap_read(d->map, BD79124_REG_PINCFG, &val); + /* If read fails, don't allow using as AIN (to be on a safe side) */ + if (ret) + return 0; + + return !(val & BIT(offset)); +} + +static int bd79124_read_event_value(struct iio_dev *idev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, int *val, + int *val2) +{ + struct bd79124_data *d = iio_priv(idev); + int ret, reg; + + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) + return -EINVAL; + + /* ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan->channel)) + return -EBUSY; + + switch (info) { + case IIO_EV_INFO_VALUE: + if (dir == IIO_EV_DIR_RISING) + reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel); + else if (dir == IIO_EV_DIR_FALLING) + reg = BD79124_GET_LOW_LIMIT_REG(chan->channel); + else + return -EINVAL; + + ret = bd79124_read_reg_to_int(d, reg, val); + if (ret) + return ret; + + return IIO_VAL_INT; + + case IIO_EV_INFO_HYSTERESIS: + reg = BD79124_GET_HYSTERESIS_REG(chan->channel); + ret = regmap_read(d->map, reg, val); + if (ret) + return ret; + /* Mask the non hysteresis bits */ + *val &= BD79124_MASK_HYSTERESIS; + /* + * The data-sheet says the hysteresis register value needs to be + * sifted left by 3 (or multiplied by 8, depending on the + * page :] ) + */ + *val <<= 3; + + return IIO_VAL_INT; + default: + return -EINVAL; + } +} + +static int bd79124_start_measurement(struct bd79124_data *d, int chan) +{ + int val, ret, regval; + + /* ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan)) + return -EBUSY; + + /* See if already started */ + ret = regmap_read(d->map, BD79124_REG_AUTO_CHANNELS, &val); + if (val & BIT(chan)) + return 0; + + /* Stop the sequencer */ + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); + if (ret) + return ret; + + /* Add the channel to measured channels */ + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, val | BIT(chan)); + if (ret) + return ret; + + /* Restart the sequencer */ + ret = regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); + if (ret) + return ret; + + /* + * Start the measurement at the background. Don't bother checking if + * it was started, regmap has cache + */ + regval = FIELD_PREP(BD79124_MASK_CONV_MODE, BD79124_CONV_MODE_AUTO); + + return regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, + BD79124_MASK_CONV_MODE, regval); +} + +static int bd79124_stop_measurement(struct bd79124_data *d, int chan) +{ + int val, ret; + + /* Ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan)) + return -EBUSY; + + /* If alarm is requested for the channel we won't stop measurement */ + if (d->alarm_monitored[chan]) + return 0; + + /* See if already stopped */ + ret = regmap_read(d->map, BD79124_REG_AUTO_CHANNELS, &val); + if (!(val & BIT(chan))) + return 0; + + /* Stop the sequencer */ + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); + + /* Clear the channel from the measured channels */ + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, + (~BIT(chan)) & val); + if (ret) + return ret; + + /* Stop background conversion if it was the last channel */ + if (!((~BIT(chan)) & val)) { + int regval = FIELD_PREP(BD79124_MASK_CONV_MODE, + BD79124_CONV_MODE_MANSEQ); + + ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, + BD79124_MASK_CONV_MODE, regval); + if (ret) + return ret; + } + + /* Restart the sequencer */ + return regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); +} + +static int bd79124_read_event_config(struct iio_dev *idev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir) +{ + struct bd79124_data *d = iio_priv(idev); + int val, ret, reg, disabled; + + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) + return -EINVAL; + + /* ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan->channel)) + return -EBUSY; + + ret = regmap_read(d->map, BD79124_REG_ALERT_CH_SEL, &val); + if (ret) + return ret; + + /* The event is disabled if alerts for the channel are disabled */ + if (!(val & BIT(chan->channel))) + return 0; + + /* + * If alerts are on, then the event may be disabled if limit is set to + * the one extreme. (HW does not support disabling rising/falling + * thresholds independently. Hence we resort to setting high limit to + * MAX, or low limit to 0 to try effectively disable thresholds). + */ + if (dir == IIO_EV_DIR_RISING) { + reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel); + disabled = BD79124_HIGH_LIMIT_MAX; + } else if (dir == IIO_EV_DIR_FALLING) { + reg = BD79124_GET_LOW_LIMIT_REG(chan->channel); + disabled = BD79124_LOW_LIMIT_MIN; + } else { + return -EINVAL; + } + ret = bd79124_read_reg_to_int(d, reg, &val); + if (ret) + return ret; + + return val != disabled; +} + +static int bd79124_disable_event(struct bd79124_data *d, + enum iio_event_direction dir, int channel) +{ + int dir_bit = BIT(dir), reg; + unsigned int limit; + + d->alarm_monitored[channel] &= (~dir_bit); + /* + * Set thresholds either to 0 or to 2^12 - 1 as appropriate to prevent + * alerts and thus disable event generation. + */ + if (dir == IIO_EV_DIR_RISING) { + reg = BD79124_GET_HIGH_LIMIT_REG(channel); + limit = BD79124_HIGH_LIMIT_MAX; + } else if (dir == IIO_EV_DIR_FALLING) { + reg = BD79124_GET_LOW_LIMIT_REG(channel); + limit = BD79124_LOW_LIMIT_MIN; + } else { + return -EINVAL; + } + + /* + * Stop measurement if there is no more events to monitor. + * We don't bother checking the retval because the limit + * setting should in any case effectively disable the alarm. + */ + if (!d->alarm_monitored[channel]) { + bd79124_stop_measurement(d, channel); + regmap_clear_bits(d->map, BD79124_REG_ALERT_CH_SEL, + BIT(channel)); + } + + return bd79124_write_int_to_reg(d, reg, limit); +} + +/* Do we need to disable the measurement for the duration of the limit conf ? */ +static int bd79124_write_event_config(struct iio_dev *idev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, bool state) +{ + struct bd79124_data *d = iio_priv(idev); + int dir_bit = BIT(dir); + + guard(mutex)(&d->mutex); + + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) + return -EINVAL; + + /* ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan->channel)) + return -EBUSY; + + if (state) { + int ret; + + /* Set channel to be measured */ + ret = bd79124_start_measurement(d, chan->channel); + if (ret) + return ret; + + d->alarm_monitored[chan->channel] |= dir_bit; + + /* Add the channel to the list of monitored channels */ + ret = regmap_set_bits(d->map, BD79124_REG_ALERT_CH_SEL, + BIT(chan->channel)); + if (ret) + return ret; + + /* + * Enable comparator. Trust the regmap cache, no need to check + * if it was already enabled. + * + * We could do this in the hw-init, but there may be users who + * never enable alarms and for them it makes sense to not + * enable the comparator at probe. + */ + return regmap_set_bits(d->map, BD79124_REG_GEN_CFG, + BD79124_MASK_DWC_EN); + } + + return bd79124_disable_event(d, dir, chan->channel); +} + +static int bd79124_write_event_value(struct iio_dev *idev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + enum iio_event_info info, int val, + int val2) +{ + struct bd79124_data *d = iio_priv(idev); + int reg; + + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) + return -EINVAL; + + /* ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan->channel)) + return -EBUSY; + + switch (info) { + case IIO_EV_INFO_VALUE: + if (dir == IIO_EV_DIR_RISING) + reg = BD79124_GET_HIGH_LIMIT_REG(chan->channel); + else if (dir == IIO_EV_DIR_FALLING) + reg = BD79124_GET_LOW_LIMIT_REG(chan->channel); + else + return -EINVAL; + + return bd79124_write_int_to_reg(d, reg, val); + + case IIO_EV_INFO_HYSTERESIS: + reg = BD79124_GET_HYSTERESIS_REG(chan->channel); + val >>= 3; + + return regmap_update_bits(d->map, reg, BD79124_MASK_HYSTERESIS, + val); + default: + return -EINVAL; + } +} + +static int bd79124_read_last_result(struct bd79124_data *d, int chan, + int *result) +{ + struct bd79124_raw raw; + int ret; + + ret = regmap_bulk_read(d->map, BD79124_GET_RECENT_RES_REG(chan), &raw, + sizeof(raw)); + if (ret) + return ret; + + *result = BD79124_RAW_TO_INT(raw); + + return 0; +} + +static int bd79124_single_chan_seq(struct bd79124_data *d, int chan, int *old) +{ + int ret; + + /* Stop the sequencer */ + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); + if (ret) + return ret; + + /* + * It may be we have some channels monitored for alarms so we want to + * cache the old config and return it when the single channel + * measurement has been completed. Read the old config. + */ + ret = regmap_read(d->map, BD79124_REG_AUTO_CHANNELS, old); + if (ret) + return ret; + + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, BIT(chan)); + if (ret) + return ret; + + /* Restart the sequencer */ + return regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); +} + +static int bd79124_single_chan_seq_end(struct bd79124_data *d, int old) +{ + int ret; + + /* Stop the sequencer */ + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); + if (ret) + return ret; + + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, old); + if (ret) + return ret; + + /* Restart the sequencer */ + return regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); +} + +static int bd79124_read_raw(struct iio_dev *idev, + struct iio_chan_spec const *chan, + int *val, int *val2, long m) +{ + struct bd79124_data *d = iio_priv(idev); + int ret; + + if (chan->channel >= BD79124_MAX_NUM_CHANNELS) + return -EINVAL; + + switch (m) { + case IIO_CHAN_INFO_RAW: + { + int old_chan_cfg, tmp; + int regval = FIELD_PREP(BD79124_MASK_CONV_MODE, + BD79124_CONV_MODE_AUTO); + + guard(mutex)(&d->mutex); + + /* ensure pinmux is set to ADC */ + if (!bd79124_chan_is_adc(d, chan->channel)) + return -EBUSY; + + /* + * Start the automatic conversion. This is needed here if no + * events have been enabled. + */ + ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, + BD79124_MASK_CONV_MODE, regval); + if (ret) + return ret; + + ret = bd79124_single_chan_seq(d, chan->channel, &old_chan_cfg); + if (ret) + return ret; + + /* + * The maximum conversion time is 6 uS. Ensure the sample is + * ready + */ + udelay(6); + + ret = bd79124_read_last_result(d, chan->channel, val); + /* Try unconditionally returning the chan config */ + tmp = bd79124_single_chan_seq_end(d, old_chan_cfg); + if (tmp) + dev_err(d->dev, + "Failed to return config. Alarms may be disabled\n"); + + if (ret) + return ret; + + return IIO_VAL_INT; + } + case IIO_CHAN_INFO_SCALE: + *val = d->vmax / 1000; + *val2 = BD79124_ADC_BITS; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } +} + +static const struct iio_info bd79124_info = { + .read_raw = bd79124_read_raw, + .read_event_config = &bd79124_read_event_config, + .write_event_config = &bd79124_write_event_config, + .read_event_value = &bd79124_read_event_value, + .write_event_value = &bd79124_write_event_value, +}; + +static irqreturn_t bd79124_event_handler(int irq, void *priv) +{ + int ret, i_hi, i_lo, i; + struct iio_dev *idev = priv; + struct bd79124_data *d = iio_priv(idev); + + /* + * Return IRQ_NONE if bailing-out without acking. This allows the IRQ + * subsystem to disable the offending IRQ line if we get a hardware + * problem. This behaviour has saved my poor bottom a few times in the + * past as, instead of getting unusably unresponsive, the system has + * spilled out the magic words "...nobody cared". + */ + ret = regmap_read(d->map, BD79124_REG_EVENT_FLAG_HI, &i_hi); + if (ret) + return IRQ_NONE; + + ret = regmap_read(d->map, BD79124_REG_EVENT_FLAG_LO, &i_lo); + if (ret) + return IRQ_NONE; + + if (!i_lo && !i_hi) + return IRQ_NONE; + + for (i = 0; i < BD79124_MAX_NUM_CHANNELS; i++) { + u64 ecode; + + if (BIT(i) & i_hi) { + ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i, + IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING); + + iio_push_event(idev, ecode, d->timestamp); + /* + * The BD79124 keeps the IRQ asserted for as long as + * the voltage exceeds the threshold. It may not serve + * the purpose to keep the IRQ firing and events + * generated in a loop because it may yield the + * userspace to have some problems when event handling + * there is slow. + * + * Thus, we disable the event for the channel. Userspace + * needs to re-enable the event. + * + * We don't check the result as there is not much to do. + * Also, this should not lead to total IRQ storm + * because the BD79124 is running in autonomous mode, + * which means there is by minimum 0.75 mS idle time + * between changing the channels. That should be + * sufficient to show some life on system, even if the + * event handling couldn't keep up. + */ + bd79124_disable_event(d, IIO_EV_DIR_RISING, i); + } + if (BIT(i) & i_lo) { + ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i, + IIO_EV_TYPE_THRESH, IIO_EV_DIR_FALLING); + + iio_push_event(idev, ecode, d->timestamp); + } + } + + ret = regmap_write(d->map, BD79124_REG_EVENT_FLAG_HI, i_hi); + if (ret) + return IRQ_NONE; + + ret = regmap_write(d->map, BD79124_REG_EVENT_FLAG_LO, i_lo); + if (ret) + return IRQ_NONE; + + return IRQ_HANDLED; +} + +static irqreturn_t bd79124_irq_handler(int irq, void *priv) +{ + struct iio_dev *idev = priv; + struct bd79124_data *d = iio_priv(idev); + + d->timestamp = iio_get_time_ns(idev); + + return IRQ_WAKE_THREAD; +} + +static int bd79124_hw_init(struct bd79124_data *d) +{ + int ret, regval; + + /* Stop auto sequencer */ + ret = regmap_clear_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_START); + if (ret) + return ret; + + /* Enable writing the measured values to the regsters */ + ret = regmap_set_bits(d->map, BD79124_REG_GEN_CFG, + BD79124_MASK_STATS_EN); + if (ret) + return ret; + + /* Set no channels to be auto-measured */ + ret = regmap_write(d->map, BD79124_REG_AUTO_CHANNELS, 0x0); + if (ret) + return ret; + + /* Set no channels to be manually measured */ + ret = regmap_write(d->map, BD79124_REG_MANUAL_CHANNELS, 0x0); + if (ret) + return ret; + + /* Set the measurement interval to 0.75 mS */ + + regval = FIELD_PREP(BD79124_MASK_AUTO_INTERVAL, BD79124_INTERVAL_075); + ret = regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, + BD79124_MASK_AUTO_INTERVAL, regval); + if (ret) + return ret; + + /* Sequencer mode to auto */ + ret = regmap_set_bits(d->map, BD79124_REG_SEQUENCE_CFG, + BD79124_MASK_SEQ_SEQ); + if (ret) + return ret; + + regval = FIELD_PREP(BD79124_MASK_CONV_MODE, BD79124_CONV_MODE_MANSEQ); + /* Don't start the measurement */ + return regmap_update_bits(d->map, BD79124_REG_OPMODE_CFG, + BD79124_MASK_CONV_MODE, BD79124_CONV_MODE_MANSEQ); + +} + +#define BD79124_VDD_MAX 5250000 +#define BD79124_VDD_MIN 2700000 + +static int bd79124_probe(struct platform_device *pdev) +{ + struct bd79124_data *d; + struct iio_dev *idev; + int ret, irq, *parent_data; + + idev = devm_iio_device_alloc(&pdev->dev, sizeof(*d)); + if (!idev) + return -ENOMEM; + + d = iio_priv(idev); + + parent_data = dev_get_drvdata(pdev->dev.parent); + if (!parent_data) + return dev_err_probe(&pdev->dev, -EINVAL, + "reference voltage missing\n"); + + /* + * Recommended VDD voltage from the data-sheet: + * Analog/Digital Supply Voltage VDD 2.70 - 5.25 V + */ + d->vmax = *parent_data; + if (d->vmax < BD79124_VDD_MIN || d->vmax > BD79124_VDD_MAX) { + return dev_err_probe(d->dev, -EINVAL, + "VDD (%d) out of range [%d - %d]\n", + d->vmax, BD79124_VDD_MIN, BD79124_VDD_MAX); + + return -EINVAL; + } + + irq = platform_get_irq_byname_optional(pdev, "thresh-alert"); + if (irq < 0) { + if (irq == -EPROBE_DEFER) + return irq; + + idev->channels = &bd79124_channels_noirq[0]; + idev->num_channels = ARRAY_SIZE(bd79124_channels_noirq); + dev_dbg(d->dev, "No IRQ found, events disabled\n"); + } else { + idev->channels = &bd79124_channels[0]; + idev->num_channels = ARRAY_SIZE(bd79124_channels); + } + + idev->info = &bd79124_info; + idev->name = "bd79124"; + idev->modes = INDIO_DIRECT_MODE; + + d->dev = &pdev->dev; + d->map = dev_get_regmap(d->dev->parent, NULL); + if (!d->map) + return dev_err_probe(d->dev, -ENODEV, "No regmap\n"); + + mutex_init(&d->mutex); + + ret = bd79124_hw_init(d); + if (ret) + return ret; + + if (irq > 0) { + ret = devm_request_threaded_irq(d->dev, irq, bd79124_irq_handler, + &bd79124_event_handler, IRQF_ONESHOT, + "adc-thresh-alert", idev); + if (ret) + return dev_err_probe(d->dev, ret, + "Failed to register IRQ\n"); + } + + return devm_iio_device_register(d->dev, idev); +} + +static const struct platform_device_id bd79124_adc_id[] = { + { "bd79124-adc", }, + { } +}; +MODULE_DEVICE_TABLE(platform, bd79124_adc_id); + +static struct platform_driver bd79124_driver = { + .driver = { + .name = "bd79124-adc", + /* + * Probing explicitly requires a few millisecond of sleep. + * Enabling the VDD regulator may include ramp up rates. + */ + .probe_type = PROBE_PREFER_ASYNCHRONOUS, + }, + .probe = bd79124_probe, + .id_table = bd79124_adc_id, +}; +module_platform_driver(bd79124_driver); + +MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>"); +MODULE_DESCRIPTION("Driver for ROHM BD79124 ADC"); +MODULE_LICENSE("GPL");
The ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports an automatic measurement mode, with an alarm interrupt for out-of-window measurements. The window is configurable for each channel. The I2C protocol for manual start of the measurement and data reading is somewhat peculiar. It requires the master to do clock stretching after sending the I2C slave-address until the slave has captured the data. Needless to say this is not well suopported by the I2C controllers. Thus the driver does not support the BD79124's manual measurement mode but implements the measurements using automatic measurement mode relying on the BD79124's ability of storing latest measurements into register. The driver does also support configuring the threshold events for detecting the out-of-window events. The BD79124 keeps asserting IRQ for as long as the measured voltage is out of the configured window. Thus the driver disables the event when first event is handled. This prevents the user-space from choking on the events - but it also requires the user space to reconfigure and re-enable the monitored event when it wants to keep monitoring for new occurrences. It is worth noting that the ADC input pins can be also configured as general purpose outputs. The pin mode should be configured using pincmux driver. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Regarding disabling the event upon reception - is this totally strange? Is regular userspace compeletely unprepared for this, and better prepared for handling large amounts of continuous events? The BD79124 should not cause a total CPU-blocking IRQ storm because the driver uses the autonomous sequencer mode - which has minimum of 0.75 millisecond delay between measurements. So, new IRQs can be raised with this interval. (The 0.75 mS includes handling and acking / status reading delays - so there is still not much time for things done outside of the IRQ handling...) --- drivers/iio/adc/Kconfig | 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/rohm-bd79124-adc.c | 890 +++++++++++++++++++++++++++++ 3 files changed, 901 insertions(+) create mode 100644 drivers/iio/adc/rohm-bd79124-adc.c