2022-04-19 16:05:04

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH v3 1/4] dt-bindings: phy: qcom,qmp: Mark '#clock-cells' as a 'optional' property

'#clock-cells' is not a required property for qmp-phy(s) in the
'/' node, but it should be is used in 'phy@' subnode (where it is
actually a 'required' property). Fix the same.

This also fixes the following 'make dtbs_check' warning(s):

sm8350-microsoft-surface-duo2.dt.yaml: phy@1d87000:
'#clock-cells' is a required property

Cc: Bjorn Andersson <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Rob Herring <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
index 8b850c5ab116..c39ead81ecd7 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
@@ -66,9 +66,6 @@ properties:
- description: Address and length of PHY's common serdes block.
- description: Address and length of PHY's DP_COM control block.

- "#clock-cells":
- enum: [ 1, 2 ]
-
"#address-cells":
enum: [ 1, 2 ]

@@ -112,11 +109,13 @@ patternProperties:
description:
Each device node of QMP phy is required to have as many child nodes as
the number of lanes the PHY has.
+ properties:
+ "#clock-cells":
+ enum: [ 0, 1, 2 ]

required:
- compatible
- reg
- - "#clock-cells"
- "#address-cells"
- "#size-cells"
- ranges
@@ -468,7 +467,6 @@ examples:
usb_2_qmpphy: phy-wrapper@88eb000 {
compatible = "qcom,sdm845-qmp-usb3-uni-phy";
reg = <0x088eb000 0x18c>;
- #clock-cells = <1>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x088eb000 0x2000>;
--
2.35.1


2022-04-19 17:12:15

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] dt-bindings: phy: qcom,qmp: Mark '#clock-cells' as a 'optional' property

On Tue, 19 Apr 2022 02:25:06 +0530, Bhupesh Sharma wrote:
> '#clock-cells' is not a required property for qmp-phy(s) in the
> '/' node, but it should be is used in 'phy@' subnode (where it is
> actually a 'required' property). Fix the same.
>
> This also fixes the following 'make dtbs_check' warning(s):
>
> sm8350-microsoft-surface-duo2.dt.yaml: phy@1d87000:
> '#clock-cells' is a required property
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


phy@1c07000: 'lanes@1c06000' does not match any of the regexes: '^phy@[0-9a-f]+$', 'pinctrl-[0-9]+'
arch/arm/boot/dts/qcom-sdx55-mtp.dtb
arch/arm/boot/dts/qcom-sdx55-t55.dtb
arch/arm/boot/dts/qcom-sdx55-telit-fn980-tlb.dtb

phy@1c0e000: 'lanes@1c0e200' does not match any of the regexes: '^phy@[0-9a-f]+$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sc7280-crd.dtb
arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dtb
arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dtb
arch/arm64/boot/dts/qcom/sc7280-idp2.dtb
arch/arm64/boot/dts/qcom/sc7280-idp.dtb

phy@1d87000: 'lanes@1d87400', 'vdda-max-microamp', 'vdda-pll-max-microamp' do not match any of the regexes: '^phy@[0-9a-f]+$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sm8450-hdk.dtb
arch/arm64/boot/dts/qcom/sm8450-qrd.dtb

phy@1d87000: 'vdda-max-microamp', 'vdda-pll-max-microamp' do not match any of the regexes: '^phy@[0-9a-f]+$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/sm8350-microsoft-surface-duo2.dtb

phy@627000: 'vdda-phy-max-microamp', 'vdda-pll-max-microamp', 'vddp-ref-clk-always-on', 'vddp-ref-clk-max-microamp' do not match any of the regexes: '^phy@[0-9a-f]+$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dtb
arch/arm64/boot/dts/qcom/msm8996-xiaomi-scorpio.dtb

phy-wrapper@88e9000: 'vdda-phy-supply' is a required property
arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx214.dtb
arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dtb

phy-wrapper@88e9000: 'vdda-pll-supply' is a required property
arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx214.dtb
arch/arm64/boot/dts/qcom/sm8350-sony-xperia-sagami-pdx215.dtb

ssphy@78000: '#clock-cells', 'lane@78200' do not match any of the regexes: '^phy@[0-9a-f]+$', 'pinctrl-[0-9]+'
arch/arm64/boot/dts/qcom/ipq6018-cp01-c1.dtb

2022-04-24 03:25:22

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] dt-bindings: phy: qcom,qmp: Mark '#clock-cells' as a 'optional' property

On Mon 18 Apr 13:55 PDT 2022, Bhupesh Sharma wrote:

> '#clock-cells' is not a required property for qmp-phy(s) in the
> '/' node, but it should be is used in 'phy@' subnode (where it is
> actually a 'required' property). Fix the same.
>

It's not that #clock-cells is "not a required property", it's that the
clock comes out of the phy (the child node), so there is no clocks
provided by the parent device.


Please rewrite the commit message.

> This also fixes the following 'make dtbs_check' warning(s):
>
> sm8350-microsoft-surface-duo2.dt.yaml: phy@1d87000:
> '#clock-cells' is a required property
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: Rob Herring <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> index 8b850c5ab116..c39ead81ecd7 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> @@ -66,9 +66,6 @@ properties:
> - description: Address and length of PHY's common serdes block.
> - description: Address and length of PHY's DP_COM control block.
>
> - "#clock-cells":
> - enum: [ 1, 2 ]
> -
> "#address-cells":
> enum: [ 1, 2 ]
>
> @@ -112,11 +109,13 @@ patternProperties:
> description:
> Each device node of QMP phy is required to have as many child nodes as
> the number of lanes the PHY has.
> + properties:
> + "#clock-cells":
> + enum: [ 0, 1, 2 ]

The commit message doesn't mention the fact that 0 is also a valid
value. Perhaps just keep it [1, 2] in this patch?

Regards,
Bjorn

>
> required:
> - compatible
> - reg
> - - "#clock-cells"
> - "#address-cells"
> - "#size-cells"
> - ranges
> @@ -468,7 +467,6 @@ examples:
> usb_2_qmpphy: phy-wrapper@88eb000 {
> compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> reg = <0x088eb000 0x18c>;
> - #clock-cells = <1>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x088eb000 0x2000>;
> --
> 2.35.1
>

2022-05-15 10:37:39

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] dt-bindings: phy: qcom,qmp: Mark '#clock-cells' as a 'optional' property

Hi Bjorn,

On Sat, 23 Apr 2022 at 21:14, Bjorn Andersson
<[email protected]> wrote:
>
> On Mon 18 Apr 13:55 PDT 2022, Bhupesh Sharma wrote:
>
> > '#clock-cells' is not a required property for qmp-phy(s) in the
> > '/' node, but it should be is used in 'phy@' subnode (where it is
> > actually a 'required' property). Fix the same.
> >
>
> It's not that #clock-cells is "not a required property", it's that the
> clock comes out of the phy (the child node), so there is no clocks
> provided by the parent device.
>
>
> Please rewrite the commit message.

Ok.

> > This also fixes the following 'make dtbs_check' warning(s):
> >
> > sm8350-microsoft-surface-duo2.dt.yaml: phy@1d87000:
> > '#clock-cells' is a required property
> >
> > Cc: Bjorn Andersson <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> > index 8b850c5ab116..c39ead81ecd7 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/qcom,qmp-phy.yaml
> > @@ -66,9 +66,6 @@ properties:
> > - description: Address and length of PHY's common serdes block.
> > - description: Address and length of PHY's DP_COM control block.
> >
> > - "#clock-cells":
> > - enum: [ 1, 2 ]
> > -
> > "#address-cells":
> > enum: [ 1, 2 ]
> >
> > @@ -112,11 +109,13 @@ patternProperties:
> > description:
> > Each device node of QMP phy is required to have as many child nodes as
> > the number of lanes the PHY has.
> > + properties:
> > + "#clock-cells":
> > + enum: [ 0, 1, 2 ]
>
> The commit message doesn't mention the fact that 0 is also a valid
> value. Perhaps just keep it [1, 2] in this patch?

0 is a valid value as mentioned in the example inside the dt-binding
example itself.
For e.g. see the 'sdm845-qmp-pcie-phy' node:

pcie0_phy: phy@1c06000 {
compatible = "qcom,sdm845-qmp-pcie-phy";
<..snip..>

pcie0_lane: phy@1c06200 {
<..snip..>

#clock-cells = <0>;

So, without [ 0, 1, 2 ] in the yaml bindings we get the following
error while running '$ make dt_binding_check' :
qcom,qmp-phy.example.dtb: phy-wrapper@88eb000:
phy@200:#clock-cells:0:0: 0 is not one of [1, 2]

Thanks,
Bhupesh

> >
> > required:
> > - compatible
> > - reg
> > - - "#clock-cells"
> > - "#address-cells"
> > - "#size-cells"
> > - ranges
> > @@ -468,7 +467,6 @@ examples:
> > usb_2_qmpphy: phy-wrapper@88eb000 {
> > compatible = "qcom,sdm845-qmp-usb3-uni-phy";
> > reg = <0x088eb000 0x18c>;
> > - #clock-cells = <1>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> > ranges = <0x0 0x088eb000 0x2000>;
> > --
> > 2.35.1
> >