Re: [PATCH 0/4] Add LVTS support for mt8192

[Nícolas F. R. A. Prado]

On Thu, Apr 27, 2023 at 04:08:13PM +0200, Balsam CHIHI wrote:
> On Thu, Apr 27, 2023 at 1:20 AM Nícolas F. R. A. Prado
> <nfraprado@xxxxxxxxxxxxx> wrote:
> >
> > On Tue, Apr 25, 2023 at 01:28:39PM +0200, Balsam CHIHI wrote:
> > > On Tue, Apr 25, 2023 at 12:00 PM Chen-Yu Tsai <wenst@xxxxxxxxxxxx> wrote:
> > > >
> > > > On Tue, Apr 25, 2023 at 6:21 AM Nícolas F. R. A. Prado
> > > > <nfraprado@xxxxxxxxxxxxx> wrote:
> > > > >
> > > > > On Tue, Mar 28, 2023 at 02:20:24AM +0200, Balsam CHIHI wrote:
> > > > > > On Sat, Mar 25, 2023 at 5:33 AM Chen-Yu Tsai <wenst@xxxxxxxxxxxx> wrote:
> > > > > > >
> > > > > > > On Wed, Mar 22, 2023 at 8:48 PM Balsam CHIHI <bchihi@xxxxxxxxxxxx> wrote:
> > [..]
> > > > > >
> > > > > > Hi Chen-Yu Tsai,
> > > > > >
> > > > > > Thank you very much for helping me testing this suggestion.
> > > > > >
> > > > > > Indeed, calibration data is stored differently in the mt8192 compared to mt8195.
> > > > > > So, the mt8192's support will be delayed for now, to allow further debugging.
> > > > > >
> > > > > > In the mean time, we will only continue to upstream the remaining
> > > > > > mt8195's source code, so it will get full LVTS support.
> > > > > > A new series will be submitted soon.
> > > > >
> > > > > Hi Balsam,
> > > > >
> > > > > like Chen-Yu mentioned, the calibration data is stored with 4 byte alignment for
> > > > > MT8192, but the data that is split between non-contiguous bytes is for the
> > > > > thermal controllers (called Resistor-Capacitor Calibration downstream) not the
> > > > > sensors. The controller calibration isn't currently handled in this driver (and
> > > > > downstream it also isn't used, since a current value is read from the controller
> > > > > instead), so we can just ignore those.
> > > > >
> > > > > The patch below adjusts the addresseses for the sensors and gives me reasonable
> > > > > reads, so the machine no longer reboots. Can you integrate it into your series?
> > > >
> > > > Not sure what I got wrong, but on my machine the VPU0 and VPU1 zone interrupts
> > > > are still tripping excessively. The readings seem normal though. Specifically,
> > > > it's bits 16 and 17 that are tripping.
> > > >
> > >
> > > Hi Chen-Yu,
> > >
> > > Thank you for testing!
> > >
> > > As the readings are normal that proves that calibration data offsets
> > > are correct.
> > > would you like that I send the v2 of series to add mt8192 support?
> > > Then we could deal with the interrupts later in a separate fix,
> > > because the interrupt code in common for both SoC (mt8192 and mt8195)?
> > >
> > > Does Nícolas also have tripping interrupts?
> > > On my side, I've got no interrupts tripping on mt8195.
> > >
> > > Any other suggestions (a question for everyone)?
> >
> > Hi,
> >
> > sorry for the delay.
> >
> > Indeed the interrupts are constantly tripping on mt8192 here as well.
> >
> > I do not see the same bits as Chen-Yu mentioned however, I see
> >
> > LVTS_MONINTSTS = 0x08070000
> >
> > which corresponds to
> >
> >         Hot threshold on sense point 3
> >         high to normal offset on sense point 2
> >         high offset on sense point 2
> >         low offset on sense point 2
> >
> > and it's the same on all controllers and domains here, which is weird. I noticed
> > we have offset interrupts enabled even though we don't configure the values for
> > those, but even after disabling them and clearing the status register, the
> > interrupts keep triggering and the status is the same, so for some reason
> > LVTS_MONINT doesn't seem to be honored.
> >
> > I also tried using the filtered mode instead of immediate for the sensors, and
> > that together with disabling the extra interrupts, got me a zeroed
> > LVTS_MONINTSTS. However no interrupts seem to be triggered at all (nor
> > LVTS_MONINTSTS updated) when the temperature goes over the configured one in
> > LVTS_HTHRE.
> >
> > I tried the driver on mt8195 (Tomato chromebook) as well, and it has the same
> > LVTS_MONINTSTS = 0x08070000
> > even though the interrupts aren't being triggered, but in fact I don't see them
> > triggering over the threshold either, so I suspect the irq number might be
> > incorrectly described in the DT there.
> >
> > Do either of you have it working correctly on mt8195?
> >
> > Anyway, I'll keep digging and reply here when I find a solution.
> 
> Hi Nícolas,
> 
> Thank your for your time testing and investigating the interrupt issues!
> 
> I only have an mt8195 based board (i1200-demo), and I could not
> trigger any interrupt on it.
> I whish that MediaTek could reply to this thread to give us more
> information (I avoid disclosing MediaTek's internal information).
> And now, it's clear that mt8192 interrupts does work at least (but not
> properly, may be we could fix it at driver level).
> 
> It's been a couple of days since I sent a v2 of the series that adds
> LVTS support for mt8192 SoC (+ Suspend and Resume, + Doc update):
> "https://lore.kernel.org/all/20230425133052.199767-1-bchihi@xxxxxxxxxxxx/";.
> I wish that it will be applied very soon, then we could patch the core driver.
> 
> My colleagues "Alexandre Mergnat (amergnat@xxxxxxxxxxxx)" and
> "Alexandre Bailon (abailon@xxxxxxxxxxxx)" are now part of this
> project.
> Please let them know of future information.
> 
> Thanks again for suggesting solutions!

Hi,

finally managed to fix the issues. I had mis-read the interrupt status bits,
which made things a whole lot more confusing...

I CC'ed you on the series, but for the archive this is it:
https://lore.kernel.org/all/20230428195347.3832687-1-nfraprado@xxxxxxxxxxxxx/

Please review/test it if you have the time.

I have one extra comment regarding the mt8192 support, but I'll write it on the
v2 of this series.

Thanks,
Nícolas