Re: [RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Stephan Gerhold <stephan@gerhold.net>]

On Sun, Sep 25, 2022 at 02:21:11PM +0300, Dmitry Baryshkov wrote:
> On 25 September 2022 13:20:09 GMT+03:00, Stephan Gerhold <stephan@gerhold.net> wrote:
> >On Sat, Sep 24, 2022 at 09:58:56PM +0300, Dmitry Baryshkov wrote:
> >> On 22/09/2022 20:23, Stephan Gerhold wrote:
> >> > On Sat, Sep 10, 2022 at 03:46:51PM +0300, Dmitry Baryshkov wrote:
> >> > > Historically the tsens driver fetches the calibration data as a blob and
> >> > > then parses the blob on its own. This results in semi-duplicated code
> >> > > spreading over the platform-specific functions.
> >> > > 
> >> > > This patch series changes tsens calibration code to use pre-parsed nvmem
> >> > > cells rather than parsing the blob in the driver. For backwards
> >> > > compatibility the old code is left in place for msm8916 and qcs404, two
> >> > > platforms which have in-tree DT files. For msm8974 the original function
> >> > > is left intact, since it differs significantly (and I can not test the
> >> > > code on msm8974). For all other affected platforms the old parsing code
> >> > > has been dropped as a part of this RFC.
> >> > > 
> >> > > The code was tested on msm8916 and qcs404 only, thus it is being sent as
> >> > > an RFC.
> >> > > 
> >> > 
> >> > Thanks a lot for working on this!
> >> > 
> >> > After thinking about this for a while I wonder if we can go even a step
> >> > further: Can we drop SoC-specific code entirely for 8939 and 9607 and
> >> > match the generic compatible (qcom,tsens-v0_1)? This would allow most
> >> > v0.1 plaforms to use generic code like for qcom,tsens-v2.
> >> 
> >> While this idea looks appealing, I think it's a bit against our custom to
> >> put hardware details into the driver rather than putting them into the DT.
> >> So, I think, the 8939 will have to stay as is, while for the 9607 and maybe
> >> several other devices it should be possible to create a fallback entry.
> >> 
> >
> >IMHO the existing tsens-v2 support is a good example that it's sometimes
> >better to have some minor hardware details in the DT so the driver does
> >not have to be changed for every single platform. Extending from
> >specifying the number of sensors in the DT to the exact set of sensors
> >is not a very big step.
> 
> Fine, I will take a look.
> 

Thanks!

> >
> >Also, aren't you also going against the custom here by moving the fuse
> >hardware details to the DT? :)
> 
> Not quite. Fuses are completely software thing. 
> 

Good point, but this depends on the point of view: Since those fuses are
likely "burned in" at the factory I would consider them to be part of
"hardware" like everything else. Even if I wanted to I probably couldn't
change anything about the layout, at least as a user (and probably even
as a customer). :-)

> >
> >> [...]
> >> > And actually there are two revisions of 8939, the older one has one
> >> > sensor less (msm-3.10: msm8939-common.dtsi vs msm8939-v3.0.dtsi).
> >> > This could also be easily handled from the DT without any code changes:
> >> > 
> >> > 	qcom,sensors = <0 1 2 3 5 6 7 8 9>;
> >> 
> >> Usually we only care about the latest revision of the chip, earlier
> >> revisions typically correspond to engineering samples, never hitting the
> >> actual consumer devices.
> >> 
> >
> >I'm afraid we might have to care about both revisions here - I recently
> >checked a couple of MSM8939 devices in postmarketOS and there are
> >definitely two different revisions used in production - they are easily
> >identifiable since they have different CPU revisions in the "lscpu"
> >output (Cortex-A53 r0p1 vs r0p4).
> 
> Ugh. 
> 

Indeed.

Thanks,
Stephan