Re: [PATCH v4] dt-bindings: phy: rockchip-emmc-phy: Convert to dtschema

[Shresth Prasad <shresthprasad7@gmail.com>]

18 June 2024 11:50:41 am Krzysztof Kozlowski <krzk@kernel.org>:

> On 17/06/2024 20:14, Shresth Prasad wrote:
> 
>>>> +examples:
>>>> +  - |
>>>> +    grf: syscon@ff770000 {
>>> 
>>> Drop label... actually entire node looks not needed.
>> 
>> From what I understand, this `phy` node should be a sub-node of a `grf`
>> node which is why it is part of the example.
>> 
>>> 
>>>> +      compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
>>> 
>>> Drop
>>> 
>>>> +      reg = <0xff770000 0x10000>;
>>> 
>>> Drop
>> 
>> Removing `reg` causes the following warning:
>> Warning (unit_address_vs_reg): /example-0/syscon@ff770000: node has a
>> unit name, but no reg or ranges property
>> 
>> Please let me know what the prefered solution would be here.
> 
> Obviously you need to drop entire node... You cannot just drop reg and
> leave unit address.
> 
>> 
>>> 
>>> 
>>>> +      #address-cells = <1>;
>>>> +      #size-cells = <1>;
>>>> +
>>>> +      emmcphy: phy@f780 {
>>> 
>>> Drop label
>>> 
>>>> +        compatible = "rockchip,rk3399-emmc-phy";
>>>> +        reg = <0xf780 0x20>;
>>>> +        clocks = <&sdhci>;
>>>> +        clock-names = "emmcclk";
>>>> +        drive-impedance-ohm = <50>;
>>>> +        #phy-cells = <0>;
>>>> +      };
>>>> +    };
>>>> diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
>>>> deleted file mode 100644
>>>> index 57d28c0d5696..000000000000
>>>> --- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
>>>> +++ /dev/null
>>>> @@ -1,43 +0,0 @@
>>>> -Rockchip EMMC PHY
>>>> ------------------------
>>>> -
>>>> -Required properties:
>>>> - - compatible: rockchip,rk3399-emmc-phy
>>>> - - #phy-cells: must be 0
>>>> - - reg: PHY register address offset and length in "general
>>>> -   register files"
>>>> -
>>>> -Optional properties:
>>>> - - clock-names: Should contain "emmcclk".  Although this is listed as optional
>>>> -             (because most boards can get basic functionality without having
>>>> -             access to it), it is strongly suggested.
>>>> -             See ../clock/clock-bindings.txt for details.
>>>> - - clocks: Should have a phandle to the card clock exported by the SDHCI driver.
>>>> - - drive-impedance-ohm: Specifies the drive impedance in Ohm.
>>>> -                        Possible values are 33, 40, 50, 66 and 100.
>>>> -                        If not set, the default value of 50 will be applied.
>>>> - - rockchip,enable-strobe-pulldown: Enable internal pull-down for the strobe
>>>> -                                    line.  If not set, pull-down is not used.
>>>> - - rockchip,output-tapdelay-select: Specifies the phyctrl_otapdlysec register.
>>>> -                                    If not set, the register defaults to 0x4.
>>>> -                                    Maximum value 0xf.
>>>> -
>>>> -Example:
>>>> -
>>>> -
>>>> -grf: syscon@ff770000 {
>>>> -     compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
>>>> -     #address-cells = <1>;
>>>> -     #size-cells = <1>;
>>>> -
>>>> -...
>>>> -
>>>> -     emmcphy: phy@f780 {
>>>> -             compatible = "rockchip,rk3399-emmc-phy";
>>>> -             reg = <0xf780 0x20>;
>>>> -             clocks = <&sdhci>;
>>>> -             clock-names = "emmcclk";
>>>> -             drive-impedance-ohm = <50>;
>>>> -             #phy-cells = <0>;
>>>> -     };
>>>> -};
>>>> diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
>>>> index 79798c747476..6e1b1cdea680 100644
>>>> --- a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
>>>> @@ -176,9 +176,12 @@ allOf:
>>>>              Documentation/devicetree/bindings/phy/rockchip-pcie-phy.txt
>>>> 
>>>>        patternProperties:
>>>> -        "phy@[0-9a-f]+$":
>>>> -          description:
>>>> -            Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
>>>> +        "^phy@[0-9a-f]+$":
>>>> +          type: object
>>>> +
>>> 
>>> Drop blank line
>>> 
>>>> +          $ref: /schemas/phy/rockchip,rk3399-emmc-phy.yaml#
>>>> +
>>> 
>>> Drop blank line
>> 
>> The rest of the document also has these blank lines, which is why I've
>> also kept them here. Are you sure I should remove them?
> 
> Yes
> 
>> 
>>> 
>>>> +          unevaluatedProperties: false
>>>> 
>>>>    - if:
>>>>        properties:
>>> 
>>> Nothing in example? Isn't the example for 3399?
>>> 
>>> We want only one complete example of such multi-children devices, so the
>>> example can be moved and included in existing one here.
>> 
>> The example in this file is actually for `rockchip,rk3399-usb2phy` and
>> not `rockchip,rk3399-emmc-phy` which is why I haven't touched it.
> 
> What? That's gref, not usb2phy.
> 
> This patch and your explanations are very confusing.
> 
> 
> 
> Best regards,
> Krzysztof

Sorry about this, I'm still quite new to doing this.
I ask that you be a bit more patient with me, I'm still trying to learn
how things work here...

Regards,
Shresth