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 [off-list ref] wrote:

quoted

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 <redacted>
---
 ...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.

quoted

   "#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.

quoted

+
+  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.

quoted

+  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.

quoted

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