18 June 2024 11:50:41 am Krzysztof Kozlowski <krzk@xxxxxxxxxx>:
> 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
