Re: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver

[Fenglin Wu <fenglin.wu@oss.qualcomm.com>]

On 7/18/2026 12:51 AM, Dmitry Torokhov wrote:
...

Thank you for reviewing the change!

>> +
>> +static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
>> +{
>> +	if (h->irq_enabled == enable)
>> +		return;
> 
> Should t you know if given code runs with interrupts disabled or
> enabled? I believe this tracking and the wrapper should be removed.
> 
>> +
>> +	if (enable)
>> +		enable_irq(h->fifo_empty_irq);
>> +	else
>> +		disable_irq(h->fifo_empty_irq);
>> +
>> +	h->irq_enabled = enable;
>> +}
>> +
In normal handling, the code should be able to track the IRQ status. The
IRQ is not auto enabled after the registration. It's only enabled when
the upload data doesn't fit in the initial FIFO fill and requires to
refill based on the interrupt. And after play is done, the IRQ can be
disabled.

There are error paths or concurrency cases which might cause the IRQ
being disabled multiple times, for example:
1) IRQ is enabled when playing an effect with a long FIFO data
2) During the FIFO refill, if any SPMI write errors, and if the SPMI bus
issue persists, there would be an IRQ storm as the IRQ is still kept as
enabled and the HW FIFO is still 'empty'. So I need to disable the IRQ
to avoid the IRQ storming before the play is stopped in step 3) below.
3) Stop the play and disable the IRQ, either when the play is done, or
when userspace issues a stop command.

There are potential multiple times of IRQ disabling in such cases, I
created this helper function to track the IRQ status to prevent
disabling the IRQ permanently.

BTW, Sashiko AI flagged a deadlock issue on the 'disable_irq()' usage,
when a stop() command came 1st and acquired the 'fifo_lock' and then the
IRQ thread is scheduled, in 'disable_irq()" the IRQ handler would
compete the 'fifo_lock' and cause a deadlock. I will need to change it
to use the _nosync() version consider how to prevent the races of FIFO
resources being used by the IRQ handler after it is freed in stop().


>> +
>> +/*
>> + * haptics_fifo_empty_irq: Threaded IRQ handler for the FIFO-empty interrupt.
>> + *
>> + * While a FIFO play is in progress the hardware fires this interrupt when
>> + * the number of samples in the FIFO drops below the programmed threshold.
>> + * The handler refills the FIFO from the effect's data buffer.  When all
>> + * samples have been written the threshold is set to zero. The HW would
>> + * stop the play automatically after all of the samples in FIFO memory are
>> + * played out.
>> + */
>> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
>> +{
>> +	struct qcom_haptics *h = dev_id;
>> +	u32 sts, to_write;
>> +	int ret;
>> +
>> +	ret = regmap_read(h->regmap,
>> +			  h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
>> +	if (ret || !(sts & FIFO_EMPTY_BIT))
>> +		return IRQ_HANDLED;
>> +
>> +	guard(mutex)(&h->fifo_lock);
>> +
>> +	if (!h->fifo_data)
>> +		return IRQ_HANDLED;
>> +
>> +	/* Refill: write the next chunk */
>> +	to_write = min_t(u32, h->data_len - h->data_written,
>> +			 h->fifo_len - FIFO_EMPTY_THRESH);
>> +	ret = haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
>> +	if (ret) {
>> +		dev_err(h->dev, "refill FIFO samples failed, ret=%d\n", ret);
>> +		/*
>> +		 * If data refilling is failed,stop the HW play and disable the
>> +		 * IRQ to prevent the FIFO empty IRQ being fired continuously.
>> +		 */
> 
> Is recovery possible after this?

It depends on how serious the problem is. Ideally, the SPMI write access
error above should only indicate a bus transaction fault but the haptics
HW should still work on its FIFO data which has already been programmed.

If the SPMI write fault just happens temporarily, it might only cause
the haptics driving waveform distortion and impact on the vibration
effect. If the SPMI write fault is caused by SPMI bus hung, then the
haptics module won't be functional anymore as all of the commands are
SPMI based. For safety, I stop the play (doesn't work in the latter
case) and disable the IRQ to prevent the IRQ being fired continuously.

Thanks
Fenglin