On Thu, Aug 31, 2023 at 11:20:39AM -0500, Vlad Karpovich wrote:
From: Ricardo Rivera-Matos rriveram@opensource.cirrus.com
Checks the index computed by the virq offset before printing the error condition in cs35l45_spk_safe_err() handler.
Signed-off-by: Ricardo Rivera-Matos rriveram@opensource.cirrus.com Signed-off-by: Vlad Karpovich vkarpovi@opensource.cirrus.com
sound/soc/codecs/cs35l45.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c index 2ac4612402eb..02b1172d2647 100644 --- a/sound/soc/codecs/cs35l45.c +++ b/sound/soc/codecs/cs35l45.c @@ -1023,7 +1023,10 @@ static irqreturn_t cs35l45_spk_safe_err(int irq, void *data)
i = irq - regmap_irq_get_virq(cs35l45->irq_data, 0);
- dev_err(cs35l45->dev, "%s condition detected!\n", cs35l45_irqs[i].name);
- if (i < 0 || i >= ARRAY_SIZE(cs35l45_irqs))
I am happy enough for this to be merged, since it clearly does no harm. So:
Acked-by: Charles Keepax ckeepax@opensource.cirrus.com
But I do still have a slight reservation of what is the point of this error check? This handler is static and can only be called from within cs35l45.c and the only code that registers IRQs goes through the cs35l45_irqs array and registers IRQs from there, so how does this ever end up with i being out of bounds?
And whilst I would not add this to this patch. I do also think perhaps Ricardo had a point in his email, the IRQ handler should probably be renamed, since it handles more than just the spk_safe_err's, perhaps something like cs35l45_report_err. On why the watchdog and global error call this as well, that was a review comment on an earlier patch since the handlers for those errors only printed a message, they might as well be combined with the spk_safe error that also only printed a message. If at some point separate handling is added for them they can be split out.
Thanks, Charles