Re: [PATCH v2 1/2] thermal: mediatek: add support for MT7986 and MT7981

[AngeloGioacchino Del Regno]

Il 05/12/22 18:42, Daniel Golle ha scritto:
> Hi Angelo,
>
> On Thu, Dec 01, 2022 at 02:24:17PM +0100, AngeloGioacchino Del Regno wrote:
> > Il 30/11/22 14:19, Daniel Golle ha scritto:
> > > Add support for V3 generation thermal found in MT7986 and MT7981 SoCs.
> > > Brings code to assign values from efuse as well as new function to
> > > convert raw temperature to millidegree celsius, as found in MediaTek's
> > > SDK sources (but cleaned up and de-duplicated)
> > >
> > > [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/baf36c7eef477aae1f8f2653b6c29e2caf48475b
> > > Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> > > Reviewed-by: Henry Yen <henry.yen@xxxxxxxxxxxx>
> > > ---
> > > Changes since v1: Drop use of adc_oe field in efuse, Henry Yen confirmed
> > > its use has been dropped intentionally in MTK SDK as well.
> > >
> > >    drivers/thermal/mtk_thermal.c | 122 +++++++++++++++++++++++++++++++++-
> > >    1 file changed, 119 insertions(+), 3 deletions(-)

...

> > What about optimizing this with assigning a function pointer?
> > Like that, we wouldn't check any version in there... as in that case we'd
> > simply do something like
> >
> > temp = conf->raw_to_mcelsius(mt, conf->bank...blahblah...);
> >
> > ...and this would also mean that the snippet saying "the first read of a sensor
> > often contains very high bogus temperature value [...]" would get merged in the v2
> > of raw_to_mcelsius (as that function is used only in mtk_thermal_bank_temperature).
>
> I found that Amjad Ouled-Ameur is taking care of converting that series
> of if-else options into a function pointer allowing to easily call the
> right raw_to_mcelsius function.
>
> [PATCH v7 0/4] thermal: mediatek: Add support for MT8365 SoC
> https://lore.kernel.org/linux-arm-kernel/4121bb6b-30db-7a23-f4c8-40afdda7a0b5@xxxxxxxxxx/T/
>
> Should I wait until this series is merged and then submit support
> for MT7986 thermal on top of that?

Right - that may be sensible. Please coordinate with him, so that one of the two
series actually depends on the other (and they don't conflict with each other).

I've added him to the loop.

Regards,
Angelo