Re: omap5 fixing palmas IRQ_TYPE_NONE warning leads to gpadc timeouts

[Jon Hunter]

On 26/11/2018 19:32, Tony Lindgren wrote:
> * Thierry Reding <treding@xxxxxxxxxx> [181126 10:25]:
>> On Mon, Nov 26, 2018 at 11:49:54AM +0200, Peter Ujfalusi wrote:
>>> The register map documentation I have states the following:
>>> bit7 INT_POLARITY Select the polarity of the INT output line
>>> 0: Interrupt line (INT) is low when interrupt is pending (default) RW
>>> 1: Interrupt line (INT) is high when interrupt is pending
>>>
>>> By default the Palmas irq is active low.
>>
>> That would confirm that the driver code is correct. My understanding is
>> that the PMC on Tegra expects a low-active IRQ from the PMIC, so we need
>> to invert the interrupt again in the PMC.
> 
> But then why Tegra need to set PALMAS_POLARITY_CTRL_INT_POLARITY
> if dts has IRQ_TYPE_LEVEL_HIGH? Shouldn't the Palmas default low
> setting be correct for Tegra if PMC expects active-low interrupt
> and then inverts it for GIC?

So I think what is going on here is ...

1. For Tegra, the interrupt parent the palmas interrupt in DT is the GIC
   not the PMC. The PMC does not register an interrupt controller
   (although to be correct probably should have. I think it had been
    discussed in the past and Stephen W may know the history here). So
   the interrupt polarity has to be HIGH otherwise setting the trigger
   type in the GIC will fail (as it only supports rising edge or level
   high IIRC).
2. However, as Thierry mentioned the Tegra PMC wants an active low
   interrupt and so the PMC inverts it on entering the PMC. However,
   given that the GIC interrupts must be active high, the PMC must
   invert again between the PMC and GIC.

So looking back my description in the change 7e9d474954f4 was not quite
accurate because the interrupt from palmas is active high but the PMC
inverts it. I think that this is quite confusing because we don't have a
good way to describe this in the DT. If we made the PMC an interrupt
controller then it would probably be a lot clearer. However, for
historical reasons this was not done.

Cheers
Jon

-- 
nvpublic