Re: [PATCH v7 04/10] mfd: sec: add support for S2MU005 PMIC

["Kaustabh Chakraborty" <kauschluss@disroot.org>]

On 2026-05-15 22:25 +00:00, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing `.use_ack = true` in `s2mu005_irq_chip` causes an infinite interrupt storm.
> --
>
> commit 6e3b1141ae78b94854e84436a682b1574f6933ac
> Author: Kaustabh Chakraborty <kauschluss@disroot.org>
>
> mfd: sec: add support for S2MU005 PMIC
>
> This commit adds support for Samsung's S2MU005 PMIC. It introduces a new
> device type and handles its specific revision register fetching, as well
> as providing its interrupt chips and child cell definitions.
>
>> diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
>> index 133188391f7c2..42862807be1a0 100644
>> --- a/drivers/mfd/sec-irq.c
>> +++ b/drivers/mfd/sec-irq.c
>
> [ ... ]
>
>> @@ -337,6 +397,16 @@ static const struct regmap_irq_chip s2mpu05_irq_chip = {
>>  	.ack_base = S2MPU05_REG_INT1,
>>  };
>>  
>> +static const struct regmap_irq_chip s2mu005_irq_chip = {
>> +	.name = "s2mu005",
>> +	.irqs = s2mu005_irqs,
>> +	.num_irqs = ARRAY_SIZE(s2mu005_irqs),
>> +	.num_regs = 4,
>> +	.status_base = S2MU005_REG_CHGR_INT1,
>> +	.mask_base = S2MU005_REG_CHGR_INT1M,
>> +	.get_irq_reg = s2mu005_irq_get_reg,
>> +};
>
> Does this structure need .use_ack = true?
>
> In s2mu005_irq_chip, .ack_base is not explicitly set, so it implicitly
> initializes to 0. The .status_base is set to S2MU005_REG_CHGR_INT1, which is
> the first enumerator in s2mu005_reg and thus equals 0.
>
> The regmap-irq core framework relies on the condition if (chip->ack_base ||
> chip->use_ack) to determine if it should write back to the hardware registers
> to acknowledge and clear the interrupts. Because both evaluate to 0 (false),
> regmap-irq assumes the chip is clear-on-read and completely skips the ACK
> execution.
>
> Since Samsung PMICs require a Write-1-to-Clear sequence to clear interrupt
> status registers, without the ACK write, the hardware PMIC continuously
> asserts the physical IRQ line.
>
> As the interrupt uses IRQF_ONESHOT, the moment the threaded handler completes
> and unmasks the interrupt at the controller, the still-asserted hardware line
> will immediately re-trigger the interrupt.
>
> Could this loop cause an infinite interrupt storm, starving the CPU and
> leading to a hard lockup?

This actually does not happen, this PMIC device doesn't require ack.
There's no interrupt storm when testing, and neither does the downstream
driver implement ack-ing code.