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

[Matti Vaittinen <mazziesaccount@gmail.com>]

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;
>> +}