Re: [PATCH v7 06/10] iio: adc: Support ROHM BD79124 ADC

[Matti Vaittinen]

On 13/03/2025 15:19, Andy Shevchenko wrote:
> On Thu, Mar 13, 2025 at 09:19:03AM +0200, Matti Vaittinen wrote:

> > +	/*
> > +	 * 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(data->map, BD79124_REG_EVENT_FLAG_HI, &i_hi);
> > +	if (ret)
> > +		return IRQ_NONE;
> > +
> > +	ret = regmap_read(data->map, BD79124_REG_EVENT_FLAG_LO, &i_lo);
> > +	if (ret)
> > +		return IRQ_NONE;
>
> Only I don't get why you can't use bulk read here.
> The registers seem to be sequential.

After taking another look - there seems to be undocumented register 
(0x1b) between the BD79124_REG_EVENT_FLAG_HI (0x1a) and the 
BD79124_REG_EVENT_FLAG_LO (0x1c).

I won't touch it unless there is a real verified performance problem.

> > +	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(iio_dev, ecode, data->timestamp);
> > +			/*
> > +			 * The BD79124 keeps the IRQ asserted for as long as
> > +			 * the voltage exceeds the threshold. It causes the IRQ
> > +			 * to keep firing.
> > +			 *
> > +			 * Disable the event for the channel and schedule the
> > +			 * re-enabling the event later to prevent storm of
> > +			 * events.
> > +			 */
> > +			ret = bd79124_event_ratelimit_hi(data, i);
> > +			if (ret)
> > +				return IRQ_NONE;
> > +		}
> > +		if (BIT(i) & i_lo) {
> > +			ecode = IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, i,
> > +						     IIO_EV_TYPE_THRESH,
> > +						     IIO_EV_DIR_FALLING);
> > +
> > +			iio_push_event(iio_dev, ecode, data->timestamp);
> > +			ret = bd79124_event_ratelimit_lo(data, i);
> > +			if (ret)
> > +				return IRQ_NONE;
> > +		}
> > +	}
> > +
> > +	ret = regmap_write(data->map, BD79124_REG_EVENT_FLAG_HI, i_hi);
> > +	if (ret)
> > +		return IRQ_NONE;
> > +
> > +	ret = regmap_write(data->map, BD79124_REG_EVENT_FLAG_LO, i_lo);
> > +	if (ret)
> > +		return IRQ_NONE;
>
> In the similar way bulk write.

definitely not due to the 0x1b.

> > +	return IRQ_HANDLED;
> > +}