* [PATCH v3] regmap-irq: Update interrupt clear register for proper reset
@ 2022-02-17 8:50 Prasad Kumpatla
2022-02-17 13:17 ` Charles Keepax
2022-02-17 16:22 ` Mark Brown
0 siblings, 2 replies; 4+ messages in thread
From: Prasad Kumpatla @ 2022-02-17 8:50 UTC (permalink / raw)
To: Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
Charles Keepax, Srinivas Kandagatla
Cc: Prasad Kumpatla
With the existing logic where clear_ack is true (HW doesn’t support
auto clear for ICR), 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 not resetting interrupt
clear register.
Example for issue case where Invert_ack is false and clear_ack is true:
Say Default ISR=0x00 & ICR=0x00 and ISR is triggered with 2
interrupts making ISR = 0x11.
Step 1: Say ISR is set 0x11 (store status_buff = ISR). ISR needs to
be cleared with the help of ICR once the Interrupt is processed.
Step 2: Write ICR = 0x11 (status_buff), this will clear the ISR to 0x00.
Step 3: Issue - In the existing code, ICR is written with ICR =
~(status_buff) i.e ICR = 0xEE -> This will block all the interrupts
from raising except for interrupts 0 and 4. So expectation here is to
reset ICR, which will unblock all the interrupts.
if (chip->clear_ack) {
if (chip->ack_invert && !ret)
........
else if (!ret)
ret = regmap_write(map, reg,
~data->status_buf[i]);
So writing 0 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..4a446259a184 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, UINT_MAX);
else if (!ret)
- ret = regmap_write(map, reg,
- ~d->mask_buf[i]);
+ ret = regmap_write(map, reg, 0);
}
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, UINT_MAX);
else if (!ret)
- ret = regmap_write(map, reg,
- ~data->status_buf[i]);
+ ret = regmap_write(map, reg, 0);
}
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, UINT_MAX);
else if (!ret)
- ret = regmap_write(map, reg,
- ~(d->status_buf[i] &
- d->mask_buf[i]));
+ ret = regmap_write(map, reg, 0);
}
if (ret != 0) {
dev_err(map->dev, "Failed to ack 0x%x: %d\n",
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] regmap-irq: Update interrupt clear register for proper reset
2022-02-17 8:50 [PATCH v3] regmap-irq: Update interrupt clear register for proper reset Prasad Kumpatla
@ 2022-02-17 13:17 ` Charles Keepax
2022-02-17 14:09 ` Marek Szyprowski
2022-02-17 16:22 ` Mark Brown
1 sibling, 1 reply; 4+ messages in thread
From: Charles Keepax @ 2022-02-17 13:17 UTC (permalink / raw)
To: Prasad Kumpatla
Cc: Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
Srinivas Kandagatla, Marek Szyprowski
On Thu, Feb 17, 2022 at 02:20:07PM +0530, Prasad Kumpatla wrote:
+ Marek Szyprowski, as the old version of the patch was causing
problems on his system.
> With the existing logic where clear_ack is true (HW doesn’t support
> auto clear for ICR), 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 not resetting interrupt
> clear register.
>
> Example for issue case where Invert_ack is false and clear_ack is true:
>
> Say Default ISR=0x00 & ICR=0x00 and ISR is triggered with 2
> interrupts making ISR = 0x11.
>
> Step 1: Say ISR is set 0x11 (store status_buff = ISR). ISR needs to
> be cleared with the help of ICR once the Interrupt is processed.
>
> Step 2: Write ICR = 0x11 (status_buff), this will clear the ISR to 0x00.
>
> Step 3: Issue - In the existing code, ICR is written with ICR =
> ~(status_buff) i.e ICR = 0xEE -> This will block all the interrupts
> from raising except for interrupts 0 and 4. So expectation here is to
> reset ICR, which will unblock all the interrupts.
>
> if (chip->clear_ack) {
> if (chip->ack_invert && !ret)
> ........
> else if (!ret)
> ret = regmap_write(map, reg,
> ~data->status_buf[i]);
>
> So writing 0 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>
> ---
I think this version looks like it should work to me.
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks,
Charles
> 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..4a446259a184 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, UINT_MAX);
> else if (!ret)
> - ret = regmap_write(map, reg,
> - ~d->mask_buf[i]);
> + ret = regmap_write(map, reg, 0);
> }
> 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, UINT_MAX);
> else if (!ret)
> - ret = regmap_write(map, reg,
> - ~data->status_buf[i]);
> + ret = regmap_write(map, reg, 0);
> }
> 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, UINT_MAX);
> else if (!ret)
> - ret = regmap_write(map, reg,
> - ~(d->status_buf[i] &
> - d->mask_buf[i]));
> + ret = regmap_write(map, reg, 0);
> }
> if (ret != 0) {
> dev_err(map->dev, "Failed to ack 0x%x: %d\n",
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] regmap-irq: Update interrupt clear register for proper reset
2022-02-17 13:17 ` Charles Keepax
@ 2022-02-17 14:09 ` Marek Szyprowski
0 siblings, 0 replies; 4+ messages in thread
From: Marek Szyprowski @ 2022-02-17 14:09 UTC (permalink / raw)
To: Charles Keepax, Prasad Kumpatla
Cc: Mark Brown, Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
Srinivas Kandagatla
On 17.02.2022 14:17, Charles Keepax wrote:
> On Thu, Feb 17, 2022 at 02:20:07PM +0530, Prasad Kumpatla wrote:
>
> + Marek Szyprowski, as the old version of the patch was causing
> problems on his system.
Both devices affected by the previous version of the patch work fine
with this version.
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> With the existing logic where clear_ack is true (HW doesn’t support
>> auto clear for ICR), 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 not resetting interrupt
>> clear register.
>>
>> Example for issue case where Invert_ack is false and clear_ack is true:
>>
>> Say Default ISR=0x00 & ICR=0x00 and ISR is triggered with 2
>> interrupts making ISR = 0x11.
>>
>> Step 1: Say ISR is set 0x11 (store status_buff = ISR). ISR needs to
>> be cleared with the help of ICR once the Interrupt is processed.
>>
>> Step 2: Write ICR = 0x11 (status_buff), this will clear the ISR to 0x00.
>>
>> Step 3: Issue - In the existing code, ICR is written with ICR =
>> ~(status_buff) i.e ICR = 0xEE -> This will block all the interrupts
>> from raising except for interrupts 0 and 4. So expectation here is to
>> reset ICR, which will unblock all the interrupts.
>>
>> if (chip->clear_ack) {
>> if (chip->ack_invert && !ret)
>> ........
>> else if (!ret)
>> ret = regmap_write(map, reg,
>> ~data->status_buf[i]);
>>
>> So writing 0 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>
>> ---
> I think this version looks like it should work to me.
>
> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
>
> Thanks,
> Charles
>
>> 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..4a446259a184 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, UINT_MAX);
>> else if (!ret)
>> - ret = regmap_write(map, reg,
>> - ~d->mask_buf[i]);
>> + ret = regmap_write(map, reg, 0);
>> }
>> 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, UINT_MAX);
>> else if (!ret)
>> - ret = regmap_write(map, reg,
>> - ~data->status_buf[i]);
>> + ret = regmap_write(map, reg, 0);
>> }
>> 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, UINT_MAX);
>> else if (!ret)
>> - ret = regmap_write(map, reg,
>> - ~(d->status_buf[i] &
>> - d->mask_buf[i]));
>> + ret = regmap_write(map, reg, 0);
>> }
>> if (ret != 0) {
>> dev_err(map->dev, "Failed to ack 0x%x: %d\n",
>> --
>> 2.17.1
>>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] regmap-irq: Update interrupt clear register for proper reset
2022-02-17 8:50 [PATCH v3] regmap-irq: Update interrupt clear register for proper reset Prasad Kumpatla
2022-02-17 13:17 ` Charles Keepax
@ 2022-02-17 16:22 ` Mark Brown
1 sibling, 0 replies; 4+ messages in thread
From: Mark Brown @ 2022-02-17 16:22 UTC (permalink / raw)
To: Srinivas Kandagatla, Charles Keepax, Greg Kroah-Hartman,
linux-kernel, Prasad Kumpatla, Rafael J. Wysocki
On Thu, 17 Feb 2022 14:20:07 +0530, Prasad Kumpatla wrote:
> With the existing logic where clear_ack is true (HW doesn’t support
> auto clear for ICR), 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 not resetting interrupt
> clear register.
>
> Example for issue case where Invert_ack is false and clear_ack is true:
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-linus
Thanks!
[1/1] regmap-irq: Update interrupt clear register for proper reset
commit: d04ad245d67a3991dfea5e108e4c452c2ab39bac
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-17 16:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 8:50 [PATCH v3] regmap-irq: Update interrupt clear register for proper reset Prasad Kumpatla
2022-02-17 13:17 ` Charles Keepax
2022-02-17 14:09 ` Marek Szyprowski
2022-02-17 16:22 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.