Re: [PATCH v2 1/2] dt-bindings: phy: intel-sdxc-phy: Add YAML schema for LGM SDXC PHY

From: Ramuthevar, Vadivel MuruganX
Date: Wed Aug 28 2019 - 21:45:44 EST


Hi Langer,

Thank you for the review comments.

On 28/8/2019 11:37 PM, Langer, Thomas wrote:
Hi Vadivel,

+...
diff --git a/Documentation/devicetree/bindings/phy/intel,syscon.yaml
b/Documentation/devicetree/bindings/phy/intel,syscon.yaml
new file mode 100644
index 000000000000..d0b78805e49f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/intel,syscon.yaml
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/intel,syscon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Syscon for eMMC/SDXC PHY Device Tree Bindings
This says the binding is for eMMC/SDXC
Agreed, fix it.
+
+maintainers:
+ - Ramuthevar Vadivel Murugan
<vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx>
+
+properties:
+ compatible:
+ const: intel,syscon
but this is a generic syscon, behind which are many registers, not only for
eMMC/SDXC. Also, the registers will be different for each SoC and needed for
many different drivers, that is why in your example it is called "chiptop"
-> toplevel registers not belonging to a specific HW module.

Rob: Do you also think this "intel,syscon" is too generic?
And the binding should be outside the "phy" folder?

What is the way to support different SoCs with this?
Must the driver referencing this syscon be aware of these differences?

[Vadivel] : most of the IP drivers are using syscon, please suggest me to keep in the right place since

it is common to all(w.r.t Intel's Lightning Mountain).

Best Regards
Vadivel
+
+ reg:
+ maxItems: 1
+
+ "#reset-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - "#reset-cells"
+
+examples:
+ - |
+ sysconf: chiptop@e0020000 {
+ compatible = "intel,syscon", "syscon";
+ reg = <0xe0020000 0x100>;
+ #reset-cells = <1>;
+ };
--
2.11.0
Best regards,
Thomas