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

[Stephan Gerhold <stephan@gerhold.net>]

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.

Also, aren't you also going against the custom here by moving the fuse
hardware details to the DT? :)

> [...]
> > 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).

Thanks,
Stephan