diff mbox series

[3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings

Message ID 20210729125358.5227-3-luoj@codeaurora.org
State New
Headers show
Series [1/3] net: mdio-ipq4019: Add mdio reset function | expand

Commit Message

Jie Luo July 29, 2021, 12:53 p.m. UTC
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 <luoj@codeaurora.org>
---
 ...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%)

Comments

Andrew Lunn July 29, 2021, 1:17 p.m. UTC | #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.

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

	    Andrew
Jie Luo Aug. 2, 2021, 7:19 a.m. UTC | #2
On 2021-07-30 01:29, Rob Herring wrote:
> On Thu, Jul 29, 2021 at 6:54 AM Luo Jie <luoj@codeaurora.org> 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 <luoj@codeaurora.org>

>> ---

>>  ...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 <robert.marko@sartura.hr>

>> @@ -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

>>
Andrew Lunn Aug. 2, 2021, 1:39 p.m. UTC | #3
> > 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
Jie Luo Aug. 4, 2021, 2:37 a.m. UTC | #4
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.
diff mbox series

Patch

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 <robert.marko@sartura.hr>
@@ -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