Re: [PATCH 01/18] dt-bindings: opp: opp-v2-kryo-cpu: support Qualcomm Krait SoCs

Dmitry Baryshkov
2023-06-14T20:11:31Z

On 14/06/2023 19:01, Krzysztof Kozlowski wrote:
> On 12/06/2023 07:39, Dmitry Baryshkov wrote:
>> Exted the opp-v2-kryo-cpu.yaml to support defining OPP tables for the
>> previous generation of Qualcomm CPUs, 32-bit Krait-based platforms.
>>
>> It makes no sense to use 'operating-points-v2-kryo-cpu' compatibility
>> node for the Krait cores. Add support for the Krait-specific
>> 'operating-points-v2-krait-cpu' compatibility string and the relevant
>> opp-microvolt subclasses properties.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> .../devicetree/bindings/opp/opp-v2-kryo-cpu.yaml | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
>> index bbbad31ae4ca..93ec778bf333 100644
>> --- a/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
>> +++ b/Documentation/devicetree/bindings/opp/opp-v2-kryo-cpu.yaml
>> @@ -26,7 +26,9 @@ description: |
>>
>> properties:
>> compatible:
>> - const: operating-points-v2-kryo-cpu
>> + enum:
>> + - operating-points-v2-krait-cpu
>> + - operating-points-v2-kryo-cpu
>>
>> nvmem-cells:
>> description: |
>> @@ -63,14 +65,15 @@ patternProperties:
>> 5: MSM8996SG, speedbin 1
>> 6: MSM8996SG, speedbin 2
>> 7-31: unused
>> - enum: [0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
>> - 0x9, 0xd, 0xe, 0xf,
>> - 0x10, 0x20, 0x30, 0x70]
>
> Why?
>
>> + $ref: /schemas/types.yaml#/definitions/uint32
>
> You are changing the type. No. It should be fixed instead (enum applies
> to items).

Currenlty this bindings are only usable for msm8996/msm8996pro. As such
we listed opp-supported-hw values that are applicable to this platform.
This series adds support for apq8064 platform, which will add new items
to this enum. I think it is not very sensible to list all of them here.

However granted there is already a good enough base type definition, I
think it would be better to drop the $ref, drop the enum, add ': true'
(is it necessary if we have a description already?) and expand
documentation.

>
>>
>> clock-latency-ns: true
>>
>> required-opps: true
>>
>> + patternProperties:
>> + '^opp-microvolt-speed[0-9]+-pvs[0-9]+$': true
>
> I don't think it is a common property, so it needs description and
> specific type. Specifically "pvs[0-9]" something entirely new.

Ack.

>
>
>
> Best regards,
> Krzysztof
>

--
With best wishes
Dmitry