Re: Aw: [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: add lvts commands and their sizes to driver data

[Mason Chang <mason-cw.chang@mediatek.com>]

Hi Frank,

First of all, thank you for providing the link to the actual issue
case. This issue does not affect all MT7988 ICs. Based on the
information collected from users and production lines, we have found
that about 2% of the ICs show severe temperature anomalies without this
patch.

> Hi Mason,
> 
> thank you for working on this.
> 
> I have not yet tested the series, but did not have the issue (which
> should be solved by it) reported [1].
> 
> So just my thoughts when looking through changes
> 
> > Gesendet: Montag, 26. Mai 2025 um 12:26
> > Von: "Mason Chang" <mason-cw.chang@mediatek.com>
> > Betreff: [PATCH 2/3] thermal/drivers/mediatek/lvts_thermal: add
> > lvts commands and their sizes to driver data
> > 
> > Add LVTS commands and their sizes to driver data in preparation for
> > adding different commands.
> > 
> > Signed-off-by: Mason Chang <mason-cw.chang@mediatek.com>
> > ---
> >  drivers/thermal/mediatek/lvts_thermal.c | 65 ++++++++++++++++++++-
> > ----
> >  1 file changed, 52 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/thermal/mediatek/lvts_thermal.c
> > b/drivers/thermal/mediatek/lvts_thermal.c
> > index 7e4f56831..5b7bf29a7 100644
> > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > @@ -96,17 +96,6 @@
> > 
> >  #define LVTS_MINIMUM_THRESHOLD               20000
> > 
> > -static const u32 default_conn_cmds[] = { 0xC103FFFF, 0xC502FF55 };
> > -/*
> > - * Write device mask: 0xC1030000
> > - */
> > -static const u32 default_init_cmds[] = {
> > -     0xC1030E01, 0xC1030CFC, 0xC1030A8C, 0xC103098D, 0xC10308F1,
> > -     0xC10307A6, 0xC10306B8, 0xC1030500, 0xC1030420, 0xC1030300,
> > -     0xC1030030, 0xC10300F6, 0xC1030050, 0xC1030060, 0xC10300AC,
> > -     0xC10300FC, 0xC103009D, 0xC10300F1, 0xC10300E1
> > -};
> 
> could you please move this block in part 1 to the position used here
> in v2 to avoid deletion/adding again here?

To clearly separate changes and maintain the principle of minimal
modifications, you can see that in patch [1/3], the functions
lvts_ctrl_connect and lvts_ctrl_initialize call the commands.
Therefore, the commands cannot be moved to the part in patch [2/3].

> Maybe magic numbers can be described a bit?

This is just an initialization sequence, it cannot be adjusted.

> >  static int golden_temp = LVTS_GOLDEN_TEMP_DEFAULT;
> >  static int golden_temp_offset;
> > 
> ...
> > @@ -1446,6 +1442,17 @@ static int lvts_resume(struct device *dev)
> >       return 0;
> >  }
> > 
> > +static const u32 default_conn_cmds[] = { 0xC103FFFF, 0xC502FF55 };
> > +/*
> > + * Write device mask: 0xC1030000
> > + */
> > +static const u32 default_init_cmds[] = {
> > +     0xC1030E01, 0xC1030CFC, 0xC1030A8C, 0xC103098D, 0xC10308F1,
> > +     0xC10307A6, 0xC10306B8, 0xC1030500, 0xC1030420, 0xC1030300,
> > +     0xC1030030, 0xC10300F6, 0xC1030050, 0xC1030060, 0xC10300AC,
> > +     0xC10300FC, 0xC103009D, 0xC10300F1, 0xC10300E1
> > +};
> > +
> >  /*
> >   * The MT8186 calibration data is stored as packed 3-byte little-
> > endian
> >   * values using a weird layout that makes sense only when viewed
> > as a 32-bit
> > @@ -1740,7 +1747,11 @@ static const struct lvts_ctrl_data
> > mt8195_lvts_ap_data_ctrl[] = {
>                                                                      
> regards Frank
> 
> [1]
> https://urldefense.com/v3/__https://github.com/openwrt/openwrt/pull/18750*issuecomment-2877554514__;Iw!!CTRNKA9wMg0ARbw!lVkeuQXjfQ1pWGSvoDCEmf0FEFQqqQDEb0ovZ5cSsNiXi7hud5epYUyl9xFjO6U7vukgfBY1Ue_-xJ78F5Qe7rw4UzwJyA$

Thank you for focusing on this issue!

Sincerely
Mason