On 25 September 2022 13:20:09 GMT+03:00, Stephan Gerhold <stephan@xxxxxxxxxxx> 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