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

Dmitry Baryshkov
2022-09-25T11:21:24Z

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.

>
>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.

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

>
>Thanks,
>Stephan

--
With best wishes
Dmitry