Hi Doug,
On 12/10/2020 6:13 AM, Doug Anderson wrote:
Hi,
On Tue, Dec 8, 2020 at 9:54 PM Maulik Shah <mkshah@xxxxxxxxxxxxxx> wrote:
but as long as its IRQ is in disabled/masked state it
doesn't matter.
...but there's no requirement that someone would need to disable/mask
an interrupt while switching the muxing, is there? So it does matter.
its only when the GPIO is again set to IRQ mode with set_mux callback,
the phantom IRQ needs clear to start as clean.
So we should check only for if (i == pctrl->soc->gpio_func) then clear
phantom IRQ.
The same is case with .direction_output callback, when GPIO is used as
output say as clock, need not clear any phantom IRQ,
The reason is with every pulse of clock it can latch as pending IRQ in
GIC_ISPEND as long as it stay as output mode/clock.
its only when switching back GPIO from output direction to input & IRQ
function, need to clear the phantom IRQ.
so we do not require clear phantom irq in .direction_output callback.
I think all the above explanation is with the model that the interrupt
detection logic is still happening even when muxed away. I don't
believe that's true.
Its not the interrupt detection logic that is still happening when muxed
away, but the GPIO line is routed to GIC from PDC.
The GPIO line get forwarded when the system is active/out of system
level low power mode to GIC irrespective of whether GPIO is used as
interrupt or not.
Due to this it can still latch the IRQ at GIC after switching to lets
say Rx mode, whenever the line has any data recive, the line state
toggles can be latched as error interrupt at GIC.
From my tests, though, I strongly believe that the pin is only visible
to the PDC if it's muxed as GPIO. Specifically, in my tests I did
this (with all my patches applied so there were no phantom interrupts
when remuxing):
a) Muxed the pin away from GPIO to special function, but _didn't_ mask
the interrupt.
b) Toggled the line a whole bunch. These caused no interrupts at all.
c) Muxed back to GPIO.
To me this is quite strong evidence that the muxing is "earlier" in
the path than the connection to the PDC. In other words, if you
change the mux away from GPIO then the PDC stops seeing it and thus
the GIC also stops seeing it. The GIC can't latch what it can't see.
This means while you're in "Rx mode" it can't be latched.
OK, so just in case this somehow only happens in S3, I also tried this
(with my patch from https://crrev.com/c/2556012):
a) Muxed away from GPIO ("bogus" pinmux)
b) Enter S3.
c) Toggle the GPIO a whole bunch ("wp enable / wp disable" on Cr50).
d) Wake from S3.
e) Check to see if the interrupt fired a bunch. It didn't fire at all
In my test code the interrupt is not masked, only muxed away. That
means that if, somehow, the PDC was still observing it then we'd see
the interrupt fire. We don't.
Unless I messed up in my tests (always possible, though by this point
I've run them a number of times) then it still feels like your mental
model is wrong, or it's always possible I'm still misunderstanding
your model. Regardless, rather than trying to re-explain your model
can you please confirm that you've written test code to confirm your
mental model? If so, can you please provide this test code? I've
provided several test patches proving out my mental model.
Its not a mental model, its how the line is connected to GIC.
GPIO follows the path, TLMM to PDC to GIC.
PDC donot know if the line is muxed away from GPIO to some other
function, so it can stop forwarding to GIC.
I have slightly modified your test case (see at
https://crrev.com/c/2584729) which is as per what i used in my testing.
Here is what i am doing, setting GPIO to a fixed function (function 2 here)
Note that function 0 is the GPIO (interrupt mode).
1) Pull up the GPIO in function 2
2) Pull down the GPIO in function 2
Repeat above steps, and you will see fake interrupt every time pull down/up.
This proves that if you mux away from GPIO then still PDC sees the line
and can latch the interrupt at GIC.
Thanks,
Maulik
As the interrupt is in disabled state it won't be sent to CPU.
Its only when the driver chooses to switch back to interrupt mode we
want to clear the error interrupt latched to start as clean. same is the
case when used as output direction.
Hope above is clear.
Unfortunately, it's still not. :( Can I convince you to provide a
test patch and a set of steps that will demonstrate the problem you're
worried about? Specifically:
a) Maybe you're talking about the initial switch from a plain GPIO
input to making it an interrupt for the first time? Are you worried
about a phantom interrupt in this case? After patch #1 I think we're
safe because pdc_gic_set_type() will always clear the interrupt,
right?
b) You say "switch back to interrupt mode". Are you imagining that a
driver does something like this:
request_irq();
...
free_irq();
...
request_irq();
If you're worried about that then we can implement irq_shutdown() for
PDC and then make sure we clear on the first enable after a shutdown,
I guess?
c) Maybe when you say "switch back to interrupt mode" you mean
something else? If you are talking about muxing away and then muxing
back then I think we already have this covered. If you are talking
about masking/unmasking then the whole point is that we _do_ want
interrupts latched while masked, right?
OK, I'm going to send out a v3 just to get the already-identified
problems fixed and also to allow landing of patch #1 in the series,
which I think is all agreed upon. My request to you is that if you
think my code misses a specific case to provide some test patches to
demonstrate that case.
-Doug
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
