[PATCH] regmap-irq: Update interrupt clear register for proper reset

[PATCH] regmap-irq: Update interrupt clear register for proper reset

[Prasad Kumpatla]

With the existing logic where clear_ack is true, interrupt clear register
reset is not handled properly. Due to this only the first interrupts get
processed properly and further interrupts are blocked due to interrupt
clear register is not reset. So writing 0x00 and 0xff(when ack_invert is
true) should have no effect, other than clearing the ACKs just set.

Fixes: 3a6f0fb7b8eb ("regmap: irq: Add support to clear ack registers")
Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com>
---
 drivers/base/regmap/regmap-irq.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index d2656581a608..22b4c98bc026 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -189,11 +189,9 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
 				ret = regmap_write(map, reg, d->mask_buf[i]);
 			if (d->chip->clear_ack) {
 				if (d->chip->ack_invert && !ret)
-					ret = regmap_write(map, reg,
-							   d->mask_buf[i]);
+					ret = regmap_write(map, reg, 0xff);
 				else if (!ret)
-					ret = regmap_write(map, reg,
-							   ~d->mask_buf[i]);
+					ret = regmap_write(map, reg, 0x00);
 			}
 			if (ret != 0)
 				dev_err(d->map->dev, "Failed to ack 0x%x: %d\n",
@@ -556,11 +554,9 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 						data->status_buf[i]);
 			if (chip->clear_ack) {
 				if (chip->ack_invert && !ret)
-					ret = regmap_write(map, reg,
-							data->status_buf[i]);
+					ret = regmap_write(map, reg, 0xff);
 				else if (!ret)
-					ret = regmap_write(map, reg,
-							~data->status_buf[i]);
+					ret = regmap_write(map, reg, 0x00);
 			}
 			if (ret != 0)
 				dev_err(map->dev, "Failed to ack 0x%x: %d\n",
@@ -817,13 +813,9 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 					d->status_buf[i] & d->mask_buf[i]);
 			if (chip->clear_ack) {
 				if (chip->ack_invert && !ret)
-					ret = regmap_write(map, reg,
-						(d->status_buf[i] &
-						 d->mask_buf[i]));
+					ret = regmap_write(map, reg, 0xff);
 				else if (!ret)
-					ret = regmap_write(map, reg,
-						~(d->status_buf[i] &
-						  d->mask_buf[i]));
+					ret = regmap_write(map, reg, 0x00);
 			}
 			if (ret != 0) {
 				dev_err(map->dev, "Failed to ack 0x%x: %d\n",
-- 
2.17.1

Re: [PATCH] regmap-irq: Update interrupt clear register for proper reset

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Wed, Feb 16, 2022 at 03:30:26PM +0530, Prasad Kumpatla wrote:
> With the existing logic where clear_ack is true, interrupt clear register
> reset is not handled properly. Due to this only the first interrupts get
> processed properly and further interrupts are blocked due to interrupt
> clear register is not reset. So writing 0x00 and 0xff(when ack_invert is
> true) should have no effect, other than clearing the ACKs just set.

This commit log still has the problem that it's not clearly describing
the problem and I've forgotten again - in what way is the interrupt
clear reset not handled properly?  What should happen and what does
happen?

>  				if (d->chip->ack_invert && !ret)
> -					ret = regmap_write(map, reg,
> -							   d->mask_buf[i]);
> +					ret = regmap_write(map, reg, 0xff);

Why only 0xff and not UINT_MAX?

>  				else if (!ret)
> -					ret = regmap_write(map, reg,
> -							   ~d->mask_buf[i]);
> +					ret = regmap_write(map, reg, 0x00);

Please write 0 normally, no need for 00.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap-irq: Update interrupt clear register for proper reset

[Prasad Kumpatla]

On 2/16/2022 10:28 PM, Mark Brown wrote:
Thanks for your time Mark Brown.
> On Wed, Feb 16, 2022 at 03:30:26PM +0530, Prasad Kumpatla wrote:
>> With the existing logic where clear_ack is true, interrupt clear register
>> reset is not handled properly. Due to this only the first interrupts get
>> processed properly and further interrupts are blocked due to interrupt
>> clear register is not reset. So writing 0x00 and 0xff(when ack_invert is
>> true) should have no effect, other than clearing the ACKs just set.
> This commit log still has the problem that it's not clearly describing
> the problem and I've forgotten again - in what way is the interrupt
> clear reset not handled properly?  What should happen and what does
> happen?
Okay, will elaborate the commit message and resend.
>>   				if (d->chip->ack_invert && !ret)
>> -					ret = regmap_write(map, reg,
>> -							   d->mask_buf[i]);
>> +					ret = regmap_write(map, reg, 0xff);
> Why only 0xff and not UINT_MAX?
Okay.
>
>>   				else if (!ret)
>> -					ret = regmap_write(map, reg,
>> -							   ~d->mask_buf[i]);
>> +					ret = regmap_write(map, reg, 0x00);
> Please write 0 normally, no need for 00.
Okay.