2022-09-01 07:44:40

by Iskren Chernev

[permalink] [raw]
Subject: [PATCH 08/14] dt-bindings: ufs: qcom: Add sm6115 binding

Add SM6115 UFS to DT schema.

Signed-off-by: Iskren Chernev <[email protected]>
---
.../devicetree/bindings/ufs/qcom,ufs.yaml | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index f2d6298d926c..7c5f6e2e6d4c 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -28,6 +28,7 @@ properties:
- qcom,msm8998-ufshc
- qcom,sc8280xp-ufshc
- qcom,sdm845-ufshc
+ - qcom,sm6115-ufshc
- qcom,sm6350-ufshc
- qcom,sm8150-ufshc
- qcom,sm8250-ufshc
@@ -178,6 +179,31 @@ allOf:
minItems: 1
maxItems: 1

+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sm6115-ufshc
+ then:
+ properties:
+ clocks:
+ minItems: 8
+ maxItems: 8
+ clock-names:
+ items:
+ - const: core_clk
+ - const: bus_aggr_clk
+ - const: iface_clk
+ - const: core_clk_unipro
+ - const: core_clk_ice
+ - const: ref_clk
+ - const: tx_lane0_sync_clk
+ - const: rx_lane0_sync_clk
+ reg:
+ minItems: 1
+ maxItems: 1
+
# TODO: define clock bindings for qcom,msm8994-ufshc

unevaluatedProperties: false
--
2.37.2


2022-09-01 17:30:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 08/14] dt-bindings: ufs: qcom: Add sm6115 binding

On 01/09/2022 10:24, Iskren Chernev wrote:
> Add SM6115 UFS to DT schema.
>
> Signed-off-by: Iskren Chernev <[email protected]>
> ---
> .../devicetree/bindings/ufs/qcom,ufs.yaml | 26 +++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> index f2d6298d926c..7c5f6e2e6d4c 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> @@ -28,6 +28,7 @@ properties:
> - qcom,msm8998-ufshc
> - qcom,sc8280xp-ufshc
> - qcom,sdm845-ufshc
> + - qcom,sm6115-ufshc
> - qcom,sm6350-ufshc
> - qcom,sm8150-ufshc
> - qcom,sm8250-ufshc
> @@ -178,6 +179,31 @@ allOf:
> minItems: 1
> maxItems: 1
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,sm6115-ufshc
> + then:
> + properties:
> + clocks:
> + minItems: 8
> + maxItems: 8
> + clock-names:
> + items:
> + - const: core_clk
> + - const: bus_aggr_clk
> + - const: iface_clk
> + - const: core_clk_unipro
> + - const: core_clk_ice

Use existing name and put it in the same place as existing variant - sdm845:
ice_core_clk


Best regards,
Krzysztof

2022-09-03 17:03:34

by Iskren Chernev

[permalink] [raw]
Subject: Re: [PATCH 08/14] dt-bindings: ufs: qcom: Add sm6115 binding



On 9/1/22 19:11, Krzysztof Kozlowski wrote:
> On 01/09/2022 10:24, Iskren Chernev wrote:
>> Add SM6115 UFS to DT schema.
>>
>> Signed-off-by: Iskren Chernev <[email protected]>
>> ---
>> .../devicetree/bindings/ufs/qcom,ufs.yaml | 26 +++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> index f2d6298d926c..7c5f6e2e6d4c 100644
>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> @@ -28,6 +28,7 @@ properties:
>> - qcom,msm8998-ufshc
>> - qcom,sc8280xp-ufshc
>> - qcom,sdm845-ufshc
>> + - qcom,sm6115-ufshc
>> - qcom,sm6350-ufshc
>> - qcom,sm8150-ufshc
>> - qcom,sm8250-ufshc
>> @@ -178,6 +179,31 @@ allOf:
>> minItems: 1
>> maxItems: 1
>>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,sm6115-ufshc
>> + then:
>> + properties:
>> + clocks:
>> + minItems: 8
>> + maxItems: 8
>> + clock-names:
>> + items:
>> + - const: core_clk
>> + - const: bus_aggr_clk
>> + - const: iface_clk
>> + - const: core_clk_unipro
>> + - const: core_clk_ice
>
> Use existing name and put it in the same place as existing variant - sdm845:
> ice_core_clk

The only problem with sdm845 bindings is the presence of rx_lane1_sync_clk
clock. I'm guessing I could pass zeros there, because it shouldn't be used. Or
it could be moved to last property and then min/maxItems to guard, but that is
a change to something more-or-less immutable.

> Best regards,
> Krzysztof

Regards,
Iskren

2022-09-04 20:10:29

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 08/14] dt-bindings: ufs: qcom: Add sm6115 binding

On 03/09/2022 19:54, Iskren Chernev wrote:
>
>
> On 9/1/22 19:11, Krzysztof Kozlowski wrote:
>> On 01/09/2022 10:24, Iskren Chernev wrote:
>>> Add SM6115 UFS to DT schema.
>>>
>>> Signed-off-by: Iskren Chernev <[email protected]>
>>> ---
>>> .../devicetree/bindings/ufs/qcom,ufs.yaml | 26 +++++++++++++++++++
>>> 1 file changed, 26 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>> index f2d6298d926c..7c5f6e2e6d4c 100644
>>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>> @@ -28,6 +28,7 @@ properties:
>>> - qcom,msm8998-ufshc
>>> - qcom,sc8280xp-ufshc
>>> - qcom,sdm845-ufshc
>>> + - qcom,sm6115-ufshc
>>> - qcom,sm6350-ufshc
>>> - qcom,sm8150-ufshc
>>> - qcom,sm8250-ufshc
>>> @@ -178,6 +179,31 @@ allOf:
>>> minItems: 1
>>> maxItems: 1
>>>
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - qcom,sm6115-ufshc
>>> + then:
>>> + properties:
>>> + clocks:
>>> + minItems: 8
>>> + maxItems: 8
>>> + clock-names:
>>> + items:
>>> + - const: core_clk
>>> + - const: bus_aggr_clk
>>> + - const: iface_clk
>>> + - const: core_clk_unipro
>>> + - const: core_clk_ice
>>
>> Use existing name and put it in the same place as existing variant - sdm845:
>> ice_core_clk
>
> The only problem with sdm845 bindings is the presence of rx_lane1_sync_clk
> clock. I'm guessing I could pass zeros there, because it shouldn't be used. Or
> it could be moved to last property and then min/maxItems to guard, but that is
> a change to something more-or-less immutable.

I don't understand - what is the problem here. How presence of some
clock affects name of other clock and its place/location in list of clocks?


Best regards,
Krzysztof

2022-09-05 07:33:21

by Iskren Chernev

[permalink] [raw]
Subject: Re: [PATCH 08/14] dt-bindings: ufs: qcom: Add sm6115 binding



On 9/4/22 22:10, Krzysztof Kozlowski wrote:
> On 03/09/2022 19:54, Iskren Chernev wrote:
>>
>>
>> On 9/1/22 19:11, Krzysztof Kozlowski wrote:
>>> On 01/09/2022 10:24, Iskren Chernev wrote:
>>>> Add SM6115 UFS to DT schema.
>>>>
>>>> Signed-off-by: Iskren Chernev <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/ufs/qcom,ufs.yaml | 26 +++++++++++++++++++
>>>> 1 file changed, 26 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>>> index f2d6298d926c..7c5f6e2e6d4c 100644
>>>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>>> @@ -28,6 +28,7 @@ properties:
>>>> - qcom,msm8998-ufshc
>>>> - qcom,sc8280xp-ufshc
>>>> - qcom,sdm845-ufshc
>>>> + - qcom,sm6115-ufshc
>>>> - qcom,sm6350-ufshc
>>>> - qcom,sm8150-ufshc
>>>> - qcom,sm8250-ufshc
>>>> @@ -178,6 +179,31 @@ allOf:
>>>> minItems: 1
>>>> maxItems: 1
>>>>
>>>> + - if:
>>>> + properties:
>>>> + compatible:
>>>> + contains:
>>>> + enum:
>>>> + - qcom,sm6115-ufshc
>>>> + then:
>>>> + properties:
>>>> + clocks:
>>>> + minItems: 8
>>>> + maxItems: 8
>>>> + clock-names:
>>>> + items:
>>>> + - const: core_clk
>>>> + - const: bus_aggr_clk
>>>> + - const: iface_clk
>>>> + - const: core_clk_unipro
>>>> + - const: core_clk_ice
>>>
>>> Use existing name and put it in the same place as existing variant - sdm845:
>>> ice_core_clk
>>
>> The only problem with sdm845 bindings is the presence of rx_lane1_sync_clk
>> clock. I'm guessing I could pass zeros there, because it shouldn't be used. Or
>> it could be moved to last property and then min/maxItems to guard, but that is
>> a change to something more-or-less immutable.
>
> I don't understand - what is the problem here. How presence of some
> clock affects name of other clock and its place/location in list of clocks?

qcom,sdm845-ufshc has 9 clocks, one of which is rx_lane1_sync_clk.
qcom,sm6115-ufshc has 8 clocks (all of the ones in sdm845 without
rx_lane1_sync_clk). So if I'm understanding correctly, you want to put the
sm6115 with sdm845, which means re-use the clocks and reg specification from
sdm845, which means sm6115 will "inherit" this rx_lane1_sync_clk, and then
I have to put it in DT (otherwise the schema would complain), and I'm asking if
I can put an empty (i.e <0 0>) value, so schema is satisfied but clock is still
not really passed.

> Best regards,
> Krzysztof

2022-09-05 10:07:18

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 08/14] dt-bindings: ufs: qcom: Add sm6115 binding

On 05/09/2022 09:29, Iskren Chernev wrote:
>
>
> On 9/4/22 22:10, Krzysztof Kozlowski wrote:
>> On 03/09/2022 19:54, Iskren Chernev wrote:
>>>
>>>
>>> On 9/1/22 19:11, Krzysztof Kozlowski wrote:
>>>> On 01/09/2022 10:24, Iskren Chernev wrote:
>>>>> Add SM6115 UFS to DT schema.
>>>>>
>>>>> Signed-off-by: Iskren Chernev <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/ufs/qcom,ufs.yaml | 26 +++++++++++++++++++
>>>>> 1 file changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>>>> index f2d6298d926c..7c5f6e2e6d4c 100644
>>>>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>>>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>>>> @@ -28,6 +28,7 @@ properties:
>>>>> - qcom,msm8998-ufshc
>>>>> - qcom,sc8280xp-ufshc
>>>>> - qcom,sdm845-ufshc
>>>>> + - qcom,sm6115-ufshc
>>>>> - qcom,sm6350-ufshc
>>>>> - qcom,sm8150-ufshc
>>>>> - qcom,sm8250-ufshc
>>>>> @@ -178,6 +179,31 @@ allOf:
>>>>> minItems: 1
>>>>> maxItems: 1
>>>>>
>>>>> + - if:
>>>>> + properties:
>>>>> + compatible:
>>>>> + contains:
>>>>> + enum:
>>>>> + - qcom,sm6115-ufshc
>>>>> + then:
>>>>> + properties:
>>>>> + clocks:
>>>>> + minItems: 8
>>>>> + maxItems: 8
>>>>> + clock-names:
>>>>> + items:
>>>>> + - const: core_clk
>>>>> + - const: bus_aggr_clk
>>>>> + - const: iface_clk
>>>>> + - const: core_clk_unipro
>>>>> + - const: core_clk_ice
>>>>
>>>> Use existing name and put it in the same place as existing variant - sdm845:
>>>> ice_core_clk
>>>
>>> The only problem with sdm845 bindings is the presence of rx_lane1_sync_clk
>>> clock. I'm guessing I could pass zeros there, because it shouldn't be used. Or
>>> it could be moved to last property and then min/maxItems to guard, but that is
>>> a change to something more-or-less immutable.
>>
>> I don't understand - what is the problem here. How presence of some
>> clock affects name of other clock and its place/location in list of clocks?
>
> qcom,sdm845-ufshc has 9 clocks, one of which is rx_lane1_sync_clk.
> qcom,sm6115-ufshc has 8 clocks (all of the ones in sdm845 without
> rx_lane1_sync_clk). So if I'm understanding correctly, you want to put the
> sm6115 with sdm845, which means re-use the clocks and reg specification from
> sdm845, which means sm6115 will "inherit" this rx_lane1_sync_clk, and then
> I have to put it in DT (otherwise the schema would complain), and I'm asking if
> I can put an empty (i.e <0 0>) value, so schema is satisfied but clock is still
> not really passed.

No. I want to use the same order and naming as sdm845 variant, but in
your own/dedicated if:else: entry. Just do not create inconsistencies
when not needed. If inconsistency is needed here (which I think is not),
please explain more.

Best regards,
Krzysztof