Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Sandeep Maheswaram <sanm@codeaurora.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Stephen Boyd <swboyd@chromium.org>,
	Doug Anderson <dianders@chromium.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, Manu Gautam <mgautam@codeaurora.org>
Subject: Re: [PATCH v3 1/4] dt-bindings: phy: qcom,qmp: Convert QMP phy bindings to yaml
Date: Wed, 12 Feb 2020 12:16:33 -0800
Message-ID: <20200212201633.GB50449@google.com> (raw)
In-Reply-To: <1581506488-26881-2-git-send-email-sanm@codeaurora.org>

Hi Sandeep,

On Wed, Feb 12, 2020 at 04:51:25PM +0530, Sandeep Maheswaram wrote:
> Convert QMP phy  bindings to DT schema format using json-schema.

nit: s/phy  bindings/PHY bindings/

> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  .../devicetree/bindings/phy/qcom,qmp-phy.yaml      | 283 +++++++++++++++++++++
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 227 -----------------
>  2 files changed, 283 insertions(+), 227 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
>  delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> new file mode 100644
> index 0000000..b39a594
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> @@ -0,0 +1,283 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/phy/qcom,qmp-phy.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm QMP PHY controller
> +
> +maintainers:
> +  - Manu Gautam <mgautam@codeaurora.org>
> +
> +description:
> +  QMP phy controller supports physical layer functionality for a number of
> +  controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,ipq8074-qmp-pcie-phy
> +      - qcom,msm8996-qmp-pcie-phy
> +      - qcom,msm8996-qmp-usb3-phy
> +      - qcom,msm8998-qmp-usb3-phy
> +      - qcom,msm8998-qmp-ufs-phy
> +      - qcom,msm8998-qmp-pcie-phy
> +      - qcom,sdm845-qmp-usb3-phy
> +      - qcom,sdm845-qmp-usb3-uni-phy
> +      - qcom,sdm845-qmp-ufs-phy
> +      - qcom,sm8150-qmp-ufs-phy

nit: sort in alphabetical order (i.e. -pcie, -ufs, -usb3)?

> +
> +  reg:
> +    minItems: 1
> +    items:
> +      - description: Address and length of PHY's common serdes block.
> +      - description: Address and length of the DP_COM control block.
> +
> +  reg-names:
> +    items:
> +      - const: reg-base
> +      - const: dp_com
> +
> +  "#clock-cells":
> +     enum: [ 1, 2 ]
> +
> +  "#address-cells":
> +    enum: [ 1, 2 ]
> +
> +  "#size-cells":
> +    enum: [ 1, 2 ]
> +
> +  clocks:
> +    maxItems: 4
> +    minItems: 1

nit: minItems before maxItems, which is the order humans expect, also
it is the prevalent order in other .yaml bindings.

Do we actually need min/maxItems? I would expect the below rules for each
compatible string to take care of it, however I'm not a schema expert.

> +
> +  clock-names:
> +    maxItems: 4
> +    minItems: 1
> +  resets:
> +    maxItems: 3
> +    minItems: 1
> +
> +  reset-names:
> +    maxItems: 3
> +    minItems: 1
> +
> +  vdda-phy-supply:
> +    description:
> +        Phandle to a regulator supply to PHY core block.
> +
> +  vdda-pll-supply:
> +    description:
> +        Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +  vddp-ref-clk-supply:
> +    description:
> +        Phandle to a regulator supply to any specific refclk
> +        pll block.
> +
> +#Required nodes:
> +patternProperties:
> +  "^lanes@[0-9a-f]+$":
> +    type: object
> +    description:
> +      Each device node of QMP phy is required to have as many child nodes as
> +      the number of lanes the PHY has.
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - "#address-cells"
> +  - "#size-cells"
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - vdda-phy-supply
> +  - vdda-pll-supply
> +
> +additionalProperties: false
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +             enum:

fix indentation (2 blanks, not 3)

> +               - qcom,sdm845-qmp-usb3-phy
> +               - qcom,sdm845-qmp-usb3-uni-phy
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Phy aux clock.
> +            - description: Phy config clock.
> +            - description: 19.2 MHz ref clk.
> +            - description: Phy common block aux clock.
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: cfg_ahb
> +            - const: ref
> +            - const: com_aux
> +        resets:
> +          items:
> +            - description: reset of phy block.
> +            - description: phy common block reset.
> +        reset-names:
> +          items:
> +             - const: phy
> +             - const: common

fix indentation (3 -> 2)

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +             enum:

fix indentation (3 -> 2)

> +               - qcom,msm8996-qmp-pcie-phy
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Phy aux clock.
> +            - description: Phy config clock.
> +            - description: 19.2 MHz ref clk.
> +

nit: remove empty line or add it everywhere.

> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: cfg_ahb
> +            - const: ref
> +        resets:
> +          items:
> +            - description: reset of phy block.
> +            - description: phy common block reset.
> +            - description: phy's ahb cfg block reset.
> +        reset-names:
> +          items:
> +             - const: phy
> +             - const: common
> +             - const: cfg

fix indentation (3 -> 2)

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +             enum:

fix indentation (3 -> 2)

> +               - qcom,msm8996-qmp-pcie-phy
> +               - qcom,msm8996-qmp-usb3-phy
> +               - qcom,msm8998-qmp-pcie-phy
> +               - qcom,msm8998-qmp-usb3-phy
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Phy aux clock.
> +            - description: Phy config clock.
> +            - description: 19.2 MHz ref clk.
> +        clock-names:
> +          items:
> +            - const: aux
> +            - const: cfg_ahb
> +            - const: ref
> +        resets:
> +          items:
> +            - description: reset of phy block.
> +            - description: phy common block reset.
> +        reset-names:
> +          items:
> +             - const: phy
> +             - const: common

According to the .txt binding this is not correct for
'qcom,msm8996-qmp-pcie-phy':

  For "qcom,msm8996-qmp-pcie-phy" must contain:
			"phy", "common", "cfg".

This also matches the actual use in arch/arm64/boot/dts/qcom/msm8996.dtsi

Also the indentation needs to be fixed (3 -> 2)

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +             enum:

fix indentation (3 -> 2)

> +               - qcom,msm8998-qmp-ufs-phy
> +               - qcom,sdm845-qmp-ufs-phy
> +               - qcom,sm8150-qmp-ufs-phy
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: 19.2 MHz ref clk.
> +            - description: Phy reference aux clock.
> +        clock-names:
> +          items:
> +            - const: ref
> +            - const: ref_aux
> +        resets:
> +          items:
> +            - description: PHY reset in the UFS controller.
> +        reset-names:
> +          items:
> +            - const: ufsphy
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +             enum:

fix indentation (3 -> 2)

Thanks

Matthias

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12 11:21 [PATCH v3 0/4] Add QMP V3 USB3 PHY support for SC7180 Sandeep Maheswaram
2020-02-12 11:21 ` [PATCH v3 1/4] dt-bindings: phy: qcom,qmp: Convert QMP phy bindings to yaml Sandeep Maheswaram
2020-02-12 20:16   ` Matthias Kaehlcke [this message]
2020-02-13 20:42   ` Rob Herring
2020-02-12 11:21 ` [PATCH v3 2/4] dt-bindings: phy: qcom,qmp: Add support for SC7180 Sandeep Maheswaram
2020-02-12 20:30   ` Matthias Kaehlcke
2020-02-18 23:05   ` Rob Herring
2020-02-12 11:21 ` [PATCH v3 3/4] phy: qcom-qmp: Add QMP V3 USB3 PHY " Sandeep Maheswaram
2020-02-12 20:52   ` Matthias Kaehlcke
2020-02-12 11:21 ` [PATCH v3 4/4] arm64: dts: qcom: sc7180: Correct qmp phy reset entries Sandeep Maheswaram
2020-02-12 20:56   ` Matthias Kaehlcke
2020-03-05 18:57 ` [PATCH v3 0/4] Add QMP V3 USB3 PHY support for SC7180 Matthias Kaehlcke
2020-03-06 11:21   ` Sandeep Maheswaram (Temp)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200212201633.GB50449@google.com \
    --to=mka@chromium.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=kishon@ti.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mgautam@codeaurora.org \
    --cc=robh+dt@kernel.org \
    --cc=sanm@codeaurora.org \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git