2021-07-29 12:56:46

by Luo Jie

[permalink] [raw]
Subject: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more
ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx.

Signed-off-by: Luo Jie <[email protected]>
---
...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 ++++++++++++++++---
1 file changed, 28 insertions(+), 4 deletions(-)
rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} (58%)

diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
similarity index 58%
rename from Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
rename to Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
index 0c973310ada0..5bdeb461523b 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
@@ -1,10 +1,10 @@
# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
%YAML 1.2
---
-$id: http://devicetree.org/schemas/net/qcom,ipq4019-mdio.yaml#
+$id: http://devicetree.org/schemas/net/qcom,ipq-mdio.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings
+title: Qualcomm IPQ MDIO Controller Device Tree Bindings

maintainers:
- Robert Marko <[email protected]>
@@ -14,7 +14,9 @@ allOf:

properties:
compatible:
- const: qcom,ipq4019-mdio
+ oneOf:
+ - const: qcom,ipq4019-mdio
+ - const: qcom,ipq-mdio

"#address-cells":
const: 1
@@ -23,7 +25,29 @@ properties:
const: 0

reg:
- maxItems: 1
+ maxItems: 2
+
+ clocks:
+ items:
+ - description: MDIO clock
+
+ clock-names:
+ items:
+ - const: gcc_mdio_ahb_clk
+
+ resets:
+ items:
+ - description: MDIO reset & GEPHY hardware reset
+
+ reset-names:
+ items:
+ - const: gephy_mdc_rst
+
+ phy-reset-gpios:
+ maxItems: 3
+ description:
+ The phandle and specifier for the GPIO that controls the RESET
+ lines of PHY devices on that MDIO bus.

required:
- compatible
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



2021-07-29 13:20:59

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

> @@ -23,7 +25,29 @@ properties:
> const: 0
>
> reg:
> - maxItems: 1
> + maxItems: 2
> +
> + clocks:
> + items:
> + - description: MDIO clock
> +
> + clock-names:
> + items:
> + - const: gcc_mdio_ahb_clk
> +
> + resets:
> + items:
> + - description: MDIO reset & GEPHY hardware reset
> +
> + reset-names:
> + items:
> + - const: gephy_mdc_rst
> +
> + phy-reset-gpios:
> + maxItems: 3
> + description:
> + The phandle and specifier for the GPIO that controls the RESET
> + lines of PHY devices on that MDIO bus.

This is clearly not a rename. It is great you are adding missing
properties, but please do it as a separate patch.

Andrew

2021-07-29 13:59:44

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

On Thu, 29 Jul 2021 20:53:58 +0800, Luo Jie wrote:
> rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more
> ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx.
>
> Signed-off-by: Luo Jie <[email protected]>
> ---
> ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 ++++++++++++++++---
> 1 file changed, 28 insertions(+), 4 deletions(-)
> rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} (58%)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/qcom,ipq-mdio.example.dt.yaml: mdio@90000: reg: [[589824, 100]] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1511253

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


2021-07-29 17:32:17

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

On Thu, Jul 29, 2021 at 6:54 AM Luo Jie <[email protected]> wrote:
>
> rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more
> ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx.
>
> Signed-off-by: Luo Jie <[email protected]>
> ---
> ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 ++++++++++++++++---
> 1 file changed, 28 insertions(+), 4 deletions(-)
> rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} (58%)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
> similarity index 58%
> rename from Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> rename to Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
> index 0c973310ada0..5bdeb461523b 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
> @@ -1,10 +1,10 @@
> # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> %YAML 1.2
> ---
> -$id: http://devicetree.org/schemas/net/qcom,ipq4019-mdio.yaml#
> +$id: http://devicetree.org/schemas/net/qcom,ipq-mdio.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings
> +title: Qualcomm IPQ MDIO Controller Device Tree Bindings
>
> maintainers:
> - Robert Marko <[email protected]>
> @@ -14,7 +14,9 @@ allOf:
>
> properties:
> compatible:
> - const: qcom,ipq4019-mdio
> + oneOf:
> + - const: qcom,ipq4019-mdio
> + - const: qcom,ipq-mdio

This is more than the commit log suggests. A generic compatible by
itself is not sufficient. If other chips have the same block, just use
'qcom,ipq4019-mdio'. They should also have a compatible for the new
SoC in case it's not 'the same'.

Also, use 'enum' rather than oneOf plus const.

>
> "#address-cells":
> const: 1
> @@ -23,7 +25,29 @@ properties:
> const: 0
>
> reg:
> - maxItems: 1
> + maxItems: 2

This breaks compatibility because now 1 entry is not valid.

> +
> + clocks:
> + items:
> + - description: MDIO clock
> +
> + clock-names:
> + items:
> + - const: gcc_mdio_ahb_clk
> +
> + resets:
> + items:
> + - description: MDIO reset & GEPHY hardware reset
> +
> + reset-names:
> + items:
> + - const: gephy_mdc_rst

These all now apply to 'qcom,ipq4019-mdio'. The h/w had no clocks or
resets and now does?

You don't need *-names when there is only 1.

> + phy-reset-gpios:
> + maxItems: 3
> + description:
> + The phandle and specifier for the GPIO that controls the RESET
> + lines of PHY devices on that MDIO bus.

This belongs in the phy node since the reset is connected to the phy.

>
> required:
> - compatible
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

2021-08-02 06:03:31

by Luo Jie

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

On 2021-07-29 21:17, Andrew Lunn wrote:
>> @@ -23,7 +25,29 @@ properties:
>> const: 0
>>
>> reg:
>> - maxItems: 1
>> + maxItems: 2
>> +
>> + clocks:
>> + items:
>> + - description: MDIO clock
>> +
>> + clock-names:
>> + items:
>> + - const: gcc_mdio_ahb_clk
>> +
>> + resets:
>> + items:
>> + - description: MDIO reset & GEPHY hardware reset
>> +
>> + reset-names:
>> + items:
>> + - const: gephy_mdc_rst
>> +
>> + phy-reset-gpios:
>> + maxItems: 3
>> + description:
>> + The phandle and specifier for the GPIO that controls the RESET
>> + lines of PHY devices on that MDIO bus.
>
> This is clearly not a rename. It is great you are adding missing
> properties, but please do it as a separate patch.
>
> Andrew

> Hi Andrew, thanks for the comments, will separate it out in the next
> patch set.

2021-08-02 07:23:23

by Luo Jie

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

On 2021-07-30 01:29, Rob Herring wrote:
> On Thu, Jul 29, 2021 at 6:54 AM Luo Jie <[email protected]> wrote:
>>
>> rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more
>> ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx.
>>
>> Signed-off-by: Luo Jie <[email protected]>
>> ---
>> ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32
>> ++++++++++++++++---
>> 1 file changed, 28 insertions(+), 4 deletions(-)
>> rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml
>> => qcom,ipq-mdio.yaml} (58%)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>> b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
>> similarity index 58%
>> rename from
>> Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>> rename to Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
>> index 0c973310ada0..5bdeb461523b 100644
>> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>> +++ b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml
>> @@ -1,10 +1,10 @@
>> # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> %YAML 1.2
>> ---
>> -$id: http://devicetree.org/schemas/net/qcom,ipq4019-mdio.yaml#
>> +$id: http://devicetree.org/schemas/net/qcom,ipq-mdio.yaml#
>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>
>> -title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings
>> +title: Qualcomm IPQ MDIO Controller Device Tree Bindings
>>
>> maintainers:
>> - Robert Marko <[email protected]>
>> @@ -14,7 +14,9 @@ allOf:
>>
>> properties:
>> compatible:
>> - const: qcom,ipq4019-mdio
>> + oneOf:
>> + - const: qcom,ipq4019-mdio
>> + - const: qcom,ipq-mdio
>
> This is more than the commit log suggests. A generic compatible by
> itself is not sufficient. If other chips have the same block, just use
> 'qcom,ipq4019-mdio'. They should also have a compatible for the new
> SoC in case it's not 'the same'.
>
> Also, use 'enum' rather than oneOf plus const.
>
> Hi Rob
> Thanks for the comments, will keep the compatible "qcom,ipq4019-mdio"
> unchanged,
> and add the new compatible name by using 'enum' in the next patch set.
> the commit log will be updated in the next patch set.
>>
>> "#address-cells":
>> const: 1
>> @@ -23,7 +25,29 @@ properties:
>> const: 0
>>
>> reg:
>> - maxItems: 1
>> + maxItems: 2
>
> This breaks compatibility because now 1 entry is not valid.
>
> will update it by using "minItems: 1, maxItems: 2" in the next patch
> set.
>
>> +
>> + clocks:
>> + items:
>> + - description: MDIO clock
>> +
>> + clock-names:
>> + items:
>> + - const: gcc_mdio_ahb_clk
>> +
>> + resets:
>> + items:
>> + - description: MDIO reset & GEPHY hardware reset
>> +
>> + reset-names:
>> + items:
>> + - const: gephy_mdc_rst
>
> These all now apply to 'qcom,ipq4019-mdio'. The h/w had no clocks or
> resets and now does?
>
> You don't need *-names when there is only 1.
>
> Hi Rob
> thanks for the comment, clocks is for configuring the source clock of
> MDIO bus,
> which is apply to ipq4019 and the new chip set such as ipq807x, ipq50xx
> and ipq60xx,
> which is not configured because of uboot configuring it, kernel should
> not depends on
> the configuration of uboot, so i add it.
> will remove the *-name in the next patch set.
>
>> + phy-reset-gpios:
>> + maxItems: 3
>> + description:
>> + The phandle and specifier for the GPIO that controls the RESET
>> + lines of PHY devices on that MDIO bus.
>
> This belongs in the phy node since the reset is connected to the phy.
>
> since the phylib code can't satisfy resetting PHY in IPQ chipset,
> phylib resets phy by
> configuring GPIO output value to 1, then to 0. however the PHY reset in
> IPQ chipset need
> to configuring GPIO output value to 0, then to 1 for the PHY reset, so
> i put the phy-reset-gpios here.
>
>>
>> required:
>> - compatible
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>

2021-08-02 13:40:34

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

> > since the phylib code can't satisfy resetting PHY in IPQ chipset, phylib
> > resets phy by
> > configuring GPIO output value to 1, then to 0. however the PHY reset in
> > IPQ chipset need
> > to configuring GPIO output value to 0, then to 1 for the PHY reset, so i
> > put the phy-reset-gpios here.

Look at the active low DT property of a GPIO.

Andrew

2021-08-04 03:35:49

by Luo Jie

[permalink] [raw]
Subject: Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings


On 8/2/2021 9:39 PM, Andrew Lunn wrote:
>>> since the phylib code can't satisfy resetting PHY in IPQ chipset, phylib
>>> resets phy by
>>> configuring GPIO output value to 1, then to 0. however the PHY reset in
>>> IPQ chipset need
>>> to configuring GPIO output value to 0, then to 1 for the PHY reset, so i
>>> put the phy-reset-gpios here.
> Look at the active low DT property of a GPIO.
>
> Andrew

> thanks Andrew, will update it in the next patch set.