diff mbox series

[v3,1/3] dt-bindings: mmc: Add support for rk3576 dw-mshc

Message ID 20240814223555.3695-2-detlev.casanova@collabora.com
State New
Headers show
Series Add dw_mmc support for rk3576 | expand

Commit Message

Detlev Casanova Aug. 14, 2024, 10:34 p.m. UTC
Add the compatible string for rockchip,rk3576-dw-mshc and add support
for the rockchip,v2-tuning flag, a new feature of this core.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Heiko Stübner Aug. 15, 2024, 1:49 p.m. UTC | #1
Am Donnerstag, 15. August 2024, 00:34:00 CEST schrieb Detlev Casanova:
> Add the compatible string for rockchip,rk3576-dw-mshc and add support
> for the rockchip,v2-tuning flag, a new feature of this core.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> index 211cd0b0bc5f3..0543cdb51c657 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> @@ -39,6 +39,7 @@ properties:
>                - rockchip,rk3368-dw-mshc
>                - rockchip,rk3399-dw-mshc
>                - rockchip,rk3568-dw-mshc
> +              - rockchip,rk3576-dw-mshc
>                - rockchip,rk3588-dw-mshc
>                - rockchip,rv1108-dw-mshc
>                - rockchip,rv1126-dw-mshc

this would mark the rk3576-dw-mshc as being the "same" as the
core rk3288 variant. rk3288 was the first controller introducing the
clock tuning for higher speeds. with the clocks being part of the CRU.

As we can see in later patches, this rk3576 though changes that
setup with moving the tunable clock configurations into the controller
itself.

So please don't claim to be compatible to the 3288, but instead start
a new block for this new set of controllers:


  compatible:
    oneOf:
      # for Rockchip RK2928 and before RK3288
      - const: rockchip,rk2928-dw-mshc
      # for Rockchip RK3288
      - const: rockchip,rk3288-dw-mshc
      - items:
          - enum:
              - rockchip,px30-dw-mshc
              - rockchip,rk1808-dw-mshc
              - rockchip,rk3036-dw-mshc
              - rockchip,rk3128-dw-mshc
              - rockchip,rk3228-dw-mshc
              - rockchip,rk3308-dw-mshc
              - rockchip,rk3328-dw-mshc
              - rockchip,rk3368-dw-mshc
              - rockchip,rk3399-dw-mshc
              - rockchip,rk3568-dw-mshc
              - rockchip,rk3588-dw-mshc
              - rockchip,rv1108-dw-mshc
              - rockchip,rv1126-dw-mshc
          - const: rockchip,rk3288-dw-mshc
+      # for Rockchip RK3576 with phase tuning inside the controller
+      - const: rockchip,rk3576-dw-mshc

That way you can simplify the dt-parsing code too.


Heiko
Krzysztof Kozlowski Aug. 16, 2024, 6:52 a.m. UTC | #2
On 15/08/2024 15:49, Heiko Stübner wrote:
> Am Donnerstag, 15. August 2024, 00:34:00 CEST schrieb Detlev Casanova:
>> Add the compatible string for rockchip,rk3576-dw-mshc and add support
>> for the rockchip,v2-tuning flag, a new feature of this core.
>>
>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>> ---
>>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
>> index 211cd0b0bc5f3..0543cdb51c657 100644
>> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
>> @@ -39,6 +39,7 @@ properties:
>>                - rockchip,rk3368-dw-mshc
>>                - rockchip,rk3399-dw-mshc
>>                - rockchip,rk3568-dw-mshc
>> +              - rockchip,rk3576-dw-mshc
>>                - rockchip,rk3588-dw-mshc
>>                - rockchip,rv1108-dw-mshc
>>                - rockchip,rv1126-dw-mshc
> 
> this would mark the rk3576-dw-mshc as being the "same" as the

Not the same, but compatible.

> core rk3288 variant. rk3288 was the first controller introducing the
> clock tuning for higher speeds. with the clocks being part of the CRU.
> 
> As we can see in later patches, this rk3576 though changes that
> setup with moving the tunable clock configurations into the controller
> itself.
> 
> So please don't claim to be compatible to the 3288, but instead start
> a new block for this new set of controllers:

The question is can new device work with old compatible (without new
features)?


Best regards,
Krzysztof
Heiko Stübner Aug. 16, 2024, 7:45 a.m. UTC | #3
Am Freitag, 16. August 2024, 08:52:04 CEST schrieb Krzysztof Kozlowski:
> On 15/08/2024 15:49, Heiko Stübner wrote:
> > Am Donnerstag, 15. August 2024, 00:34:00 CEST schrieb Detlev Casanova:
> >> Add the compatible string for rockchip,rk3576-dw-mshc and add support
> >> for the rockchip,v2-tuning flag, a new feature of this core.
> >>
> >> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> >> ---
> >>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> >> index 211cd0b0bc5f3..0543cdb51c657 100644
> >> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> >> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> >> @@ -39,6 +39,7 @@ properties:
> >>                - rockchip,rk3368-dw-mshc
> >>                - rockchip,rk3399-dw-mshc
> >>                - rockchip,rk3568-dw-mshc
> >> +              - rockchip,rk3576-dw-mshc
> >>                - rockchip,rk3588-dw-mshc
> >>                - rockchip,rv1108-dw-mshc
> >>                - rockchip,rv1126-dw-mshc
> > 
> > this would mark the rk3576-dw-mshc as being the "same" as the
> 
> Not the same, but compatible.
> 
> > core rk3288 variant. rk3288 was the first controller introducing the
> > clock tuning for higher speeds. with the clocks being part of the CRU.
> > 
> > As we can see in later patches, this rk3576 though changes that
> > setup with moving the tunable clock configurations into the controller
> > itself.
> > 
> > So please don't claim to be compatible to the 3288, but instead start
> > a new block for this new set of controllers:
> 
> The question is can new device work with old compatible (without new
> features)?

the rk3288 and following have their clock phase tuning for hs-modes in
the main soc's clock controller. Hence you have the "ciu-drive" and
"ciu-sample" clocks [0].

On the rk3576 (and probably following) those clock phase settings moved
inside the mmc controller itself. So there are no external phase clocks
anymore.

So right now we have two types in the binding, the rk2928 type [1],
used on the old rk3066 and rk3188 socs, that did not support mmc hs-modes
and the rk3288-type which introduced phase tuning via clocks from the
main clock controller.

The rk3576 now switches to having phase tuning in the mmc controller
itself. So throwing that on unmodified code for the rk3288 will get you
degraded functionality, because the tuning won't work because there are
no "ciu-drive" and "ciu-sample" anymore .


And with that separate compatible we could also "tighten" the binding
a bit to make those additional clocks more explicit for the rk3288 type.


Heiko


[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399.dtsi#n410
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml#n27
Krzysztof Kozlowski Aug. 16, 2024, 8:23 a.m. UTC | #4
On 16/08/2024 09:45, Heiko Stübner wrote:
> Am Freitag, 16. August 2024, 08:52:04 CEST schrieb Krzysztof Kozlowski:
>> On 15/08/2024 15:49, Heiko Stübner wrote:
>>> Am Donnerstag, 15. August 2024, 00:34:00 CEST schrieb Detlev Casanova:
>>>> Add the compatible string for rockchip,rk3576-dw-mshc and add support
>>>> for the rockchip,v2-tuning flag, a new feature of this core.
>>>>
>>>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
>>>> index 211cd0b0bc5f3..0543cdb51c657 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
>>>> @@ -39,6 +39,7 @@ properties:
>>>>                - rockchip,rk3368-dw-mshc
>>>>                - rockchip,rk3399-dw-mshc
>>>>                - rockchip,rk3568-dw-mshc
>>>> +              - rockchip,rk3576-dw-mshc
>>>>                - rockchip,rk3588-dw-mshc
>>>>                - rockchip,rv1108-dw-mshc
>>>>                - rockchip,rv1126-dw-mshc
>>>
>>> this would mark the rk3576-dw-mshc as being the "same" as the
>>
>> Not the same, but compatible.
>>
>>> core rk3288 variant. rk3288 was the first controller introducing the
>>> clock tuning for higher speeds. with the clocks being part of the CRU.
>>>
>>> As we can see in later patches, this rk3576 though changes that
>>> setup with moving the tunable clock configurations into the controller
>>> itself.
>>>
>>> So please don't claim to be compatible to the 3288, but instead start
>>> a new block for this new set of controllers:
>>
>> The question is can new device work with old compatible (without new
>> features)?
> 
> the rk3288 and following have their clock phase tuning for hs-modes in
> the main soc's clock controller. Hence you have the "ciu-drive" and
> "ciu-sample" clocks [0].
> 
> On the rk3576 (and probably following) those clock phase settings moved
> inside the mmc controller itself. So there are no external phase clocks
> anymore.
> 
> So right now we have two types in the binding, the rk2928 type [1],
> used on the old rk3066 and rk3188 socs, that did not support mmc hs-modes
> and the rk3288-type which introduced phase tuning via clocks from the
> main clock controller.
> 
> The rk3576 now switches to having phase tuning in the mmc controller
> itself. So throwing that on unmodified code for the rk3288 will get you
> degraded functionality, because the tuning won't work because there are
> no "ciu-drive" and "ciu-sample" anymore .

One could still argue that rest of programming model is the same, thus
"degraded" mode counts as compatibility, but I do not insist on that.

> 
> 
> And with that separate compatible we could also "tighten" the binding
> a bit to make those additional clocks more explicit for the rk3288 type.
This you can, and actually should, do with existing binding. Maybe just
a bit more tricky/complicated code.

Anyway, fine with both compatibility-approaches.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
index 211cd0b0bc5f3..0543cdb51c657 100644
--- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
+++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
@@ -39,6 +39,7 @@  properties:
               - rockchip,rk3368-dw-mshc
               - rockchip,rk3399-dw-mshc
               - rockchip,rk3568-dw-mshc
+              - rockchip,rk3576-dw-mshc
               - rockchip,rk3588-dw-mshc
               - rockchip,rv1108-dw-mshc
               - rockchip,rv1126-dw-mshc