Re: [PATCH v8 1/5] dt-bindings: soc: qcom: cpr3: Add bindings for CPR3 driver

From: Dmitry Baryshkov
Date: Wed Jan 11 2023 - 10:58:56 EST


On 11/01/2023 15:30, Konrad Dybcio wrote:


On 11.01.2023 03:18, Dmitry Baryshkov wrote:
On 10/01/2023 20:54, Konrad Dybcio wrote:


On 10.01.2023 18:56, Konrad Dybcio wrote:
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>

Add the bindings for the CPR3 driver to the documentation.

Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
[Konrad: Add type reference to acc-syscon; update AGdR's email]
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
Need to add

qcom,opp-oloop-vadj
qcom,opp-cloop-vadj

And note that at least for CPR3 they are different between fusing revisions. I see that for CPRh (esp. for 8998v2) they are the same, but this is not the case for 8996 (CPR3).
If we both mean the "speed bin"-dependent values, the driver
reads the fuse value but currently does nothing. My guess would
be that Angelo omitted it, as - just like you pointed out - MSM8998
(and SDM660 for that matter) don't really use it. I suppose I could
take care of that in bindings by making this an array and handle it
separately in a different patchset, as the per-revision values
aren't *that much* different, and again aren't really of concern for
the first round of supported SoCs.

No, there are two dimensions there: speed-bin and then cpr fusing_rev, see:

ret = nvmem_cell_read_variable_le_u32(dev, "cpr_fuse_revision", &drv->fusing_rev);

While the speed bin determines overall SoC performance characteristics, cpr_fuse_revision determines how we interpret the fuse values. E.g. on msm8996 some of loop adjustment values depend on fusing_rev. I interpreted this as 'the manufacturer could not decide on how to measure things'. May be we can support only the latest fusing rev (like we do for the SoC versions). But I can not guarantee that it would be enough.

--
With best wishes
Dmitry