Received-SPF: pass (google.com: domain of linux-kernel+bounces-212980-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <cc66cca1-33db-4f30-afcf-d256a959896b@yandex.com>
Date: Thu, 13 Jun 2024 11:44:30 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3] dt-bindings: phy: rockchip-emmc-phy: Convert to
 dtschema
To: Shresth Prasad <shresthprasad7@gmail.com>, vkoul@kernel.org,
 kishon@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
 heiko@sntech.de, sebastian.reichel@collabora.com, s.hauer@pengutronix.de,
 cristian.ciocaltea@collabora.com, andy.yan@rock-chips.com
Cc: linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
 linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org,
 linux-kernel@vger.kernel.org, skhan@linuxfoundation.org,
 javier.carrasco.cruz@gmail.com
References: <20240613085812.4020-2-shresthprasad7@gmail.com>
Content-Language: en-US
From: Johan Jonker <jbx6244@yandex.com>
In-Reply-To: <20240613085812.4020-2-shresthprasad7@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 6/13/24 10:58, Shresth Prasad wrote:
> Convert txt bindings of Rockchip EMMC PHY to dtschema to allow
> for validation.
> 
> Signed-off-by: Shresth Prasad <shresthprasad7@gmail.com>
> ---

Add ack request from phy maintainer here.

> Changes in v3:
>     - fix `reg` in example being too long
> 
> Tested against `rockchip/rk3399-firefly.dtb`, `rockchip/rk3399-orangepi.dtb`
> and `rockchip/rk3399-pinebook-pro.dtb`.
> 
>  .../bindings/phy/rockchip,emmc-phy.yaml       | 79 +++++++++++++++++++
>  .../bindings/phy/rockchip-emmc-phy.txt        | 43 ----------
>  .../devicetree/bindings/soc/rockchip/grf.yaml |  2 +-
>  3 files changed, 80 insertions(+), 44 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/rockchip,emmc-phy.yaml
>  delete mode 100644 Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/rockchip,emmc-phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,emmc-phy.yaml
> new file mode 100644
> index 000000000000..85d74b343991
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/rockchip,emmc-phy.yaml
> @@ -0,0 +1,79 @@

> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)

You are converting an existing document, so GPL 2 only.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/rockchip,emmc-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip EMMC PHY
> +
> +maintainers:

> +  - Shresth Prasad <shresthprasad7@gmail.com>

Prefer to add someone that can immediately respond if Rob H. wants to delete something.
Use the Rockchip DT maintainer here:
  - Heiko Stuebner <heiko@sntech.de>

> +
> +properties:

> +  "#phy-cells":
> +    const: 0

Move this to the bottom of this list.

> +
> +  compatible:
> +    const: rockchip,rk3399-emmc-phy
> +
> +  reg:

> +    description:
> +      PHY register address offset and length in "general register files"

remove

> +    maxItems: 1
> +
> +  clock-names:

> +    description: |
> +      Although this is not a required property (because most boards can get
> +      basic functionality without having access to it), it is strongly
> +      suggested.

remove
No need for descriptions if there's just 1 clock.

> +    const: emmcclk
> +
> +  clocks:

> +    description:
> +      Should have a phandle to the card clock exported by the SDHCI driver.

remove 

> +    maxItems: 1
> +
> +  drive-impedance-ohm:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Specifies the drive impedance in Ohm.
> +    enum: [33, 40, 50, 66, 100]
> +    default: 50
> +
> +  rockchip,enable-strobe-pulldown:
> +    type: boolean
> +    description: |
> +      Enable internal pull-down for the strobe
> +      line.  If not set, pull-down is not used.
> +
> +  rockchip,output-tapdelay-select:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Specifies the phyctrl_otapdlysec register.
> +    default: 0x4
> +    maximum: 0xf
> +
> +required:

> +  - "#phy-cells"

Move at the bottom of this list.

> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    grf: syscon@ff770000 {
> +      compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
> +      reg = <0xff770000 0x10000>;
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +
> +      emmcphy: phy@f780 {

> +        #phy-cells = <0>;

Move at the bottom of this list.
Order of Properties in Device Node:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n112

> +        compatible = "rockchip,rk3399-emmc-phy";
> +        reg = <0xf780 0x20>;
> +        clocks = <&sdhci>;
> +        clock-names = "emmcclk";
> +        drive-impedance-ohm = <50>;
> +      };
> +    };
> 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..1f88416657cc 100644

> --- a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml

grf.yaml can be busy at times. Let Heiko take care of the merge order.
Ask for an ack from the phy maintainers in your commit message below a "---"

> @@ -178,7 +178,7 @@ allOf:
>        patternProperties:
>          "phy@[0-9a-f]+$":

>            description:
> -            Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
> +            Documentation/devicetree/bindings/phy/rockchip,emmc-phy.yaml

Integrate rockchip,emmc-phy.yaml with grf.yaml with $ref.
Remove above, use/test below:

          $ref: /schemas/phy/rockchip,emmc-phy.yaml#

          unevaluatedProperties: false


>  
>    - if:
>        properties: