Add Intel ComboPhy driver

PATCH Superseded Dilip Kota 3 participants 12 messages Mar 2, 2020 – Mar 13, 2020

Subsystems (4)

Device tree bindings Multifunction devices (MFD) Generic PHY framework System configuration (syscon)

lore

Share & tools

lore

Dilip Kota

author

2020-03-02 08:43 UTC #0 lore raw

v4 cover Add Intel ComboPhy driver

This patch series adds Intel Combophy driver, respective yaml schemas and a kernel API fwnode_to_regmap().
ComboPhy driver calls it to get regmap handle through firmware node.

Dilip Kota (3):
mfd: syscon: Add fwnode_to_regmap
dt-bindings: phy: Add YAML schemas for Intel Combophy
phy: intel: Add driver support for Combophy

 .../devicetree/bindings/phy/intel,combo-phy.yaml   | 123 ++++
 drivers/mfd/syscon.c                               |   8 +
 drivers/phy/intel/Kconfig                          |  14 +
 drivers/phy/intel/Makefile                         |   1 +
 drivers/phy/intel/phy-intel-combo.c                | 661 +++++++++++++++++++++
 include/dt-bindings/phy/phy-intel-combophy.h       |  10 +
 include/linux/mfd/syscon.h                         |   6 +
 7 files changed, 823 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
 create mode 100644 drivers/phy/intel/phy-intel-combo.c
 create mode 100644 include/dt-bindings/phy/phy-intel-combophy.h

Signature · 1 line

2.11.0

Dilip Kota

author

2020-03-02 08:43 UTC #1 lore raw

PATCH v4 1/3 mfd: syscon: Add fwnode_to_regmap

Traverse regmap handle entry from firmware node handle.

Signed-off-by Dilip Kota <[email protected]>

---
Changes on v4:
No changes

 drivers/mfd/syscon.c       | 8 ++++++++
 include/linux/mfd/syscon.h | 6 ++++++
 2 files changed, 14 insertions(+)

drivers/mfd/syscon.c +8 -0

index e22197c832e8..08064ffc5394 100644
@@ -178,6 +178,14 @@ struct regmap *device_node_to_regmap(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(device_node_to_regmap);
 
+struct regmap *fwnode_to_regmap(struct fwnode_handle *fwnode)
+{
+	struct device_node *np = to_of_node(fwnode);
+
+	return device_node_get_regmap(np, false);
+}
+EXPORT_SYMBOL_GPL(fwnode_to_regmap);
+
 struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
 	if (!of_device_is_compatible(np, "syscon"))

include/linux/mfd/syscon.h +6 -0

index 112dc66262cc..a024dd88bdd4 100644
@@ -18,6 +18,7 @@ struct device_node;
 
 #ifdef CONFIG_MFD_SYSCON
 extern struct regmap *device_node_to_regmap(struct device_node *np);
+extern struct regmap *fwnode_to_regmap(struct fwnode_handle *fwnode);
 extern struct regmap *syscon_node_to_regmap(struct device_node *np);
 extern struct regmap *syscon_regmap_lookup_by_compatible(const char *s);
 extern struct regmap *syscon_regmap_lookup_by_phandle(

@@ -29,6 +30,11 @@ static inline struct regmap *device_node_to_regmap(struct device_node *np)
 	return ERR_PTR(-ENOTSUPP);
 }
 
+static inline struct regmap *fwnode_to_regmap(struct fwnode_handle *fwnode)
+{
+	return ERR_PTR(-ENOTSUPP);
+}
+
 static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
 	return ERR_PTR(-ENOTSUPP);

Signature · 1 line

2.11.0

Dilip Kota

author

2020-03-02 08:43 UTC #2 lore raw

PATCH v4 2/3 dt-bindings: phy: Add YAML schemas for Intel Combophy

In reply to Dilip Kota

Combophy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.

Signed-off-by Dilip Kota <[email protected]>

---
Changes on v4:
No changes.

Changes on v3:

Add include/dt-bindings/phy/phy-intel-combphy.h for phy modes. Add SoC specific compatible "intel,combophy-lgm".
Correct the nodename pattern.
clocks description removed and add maxItems entry.
Remove "simple-bus" as it expects minimum one address
cell and size cell in the children node. Call devm_of_platform_populate() in the driver to perform "simple-bus" functionality.

Changes on v2:

Add custom 'select'
Pass hardware instance entries with phandles and
remove cell-index and bid entries
Clock, register address space, are same for the children.
So move them to parent node.
Two PHY instances cannot run in different modes,
so move the phy-mode entry to parent node.
Add second child entry in the DT example.

 .../devicetree/bindings/phy/intel,combo-phy.yaml   | 123 +++++++++++++++++++++
 include/dt-bindings/phy/phy-intel-combophy.h       |  10 ++
 2 files changed, 133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
 create mode 100644 include/dt-bindings/phy/phy-intel-combophy.h

Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +123 -0

new file mode 100644
index 000000000000..f9bae37fab17
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/intel,combo-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel ComboPhy Subsystem
+
+maintainers:
+  - Dilip Kota <[email protected]>
+
+description: |
+  Intel Combophy subsystem supports PHYs for PCIe, EMAC and SATA
+  controllers. A single Combophy provides two PHY instances.
+
+properties:
+  $nodename:
+    pattern: "combophy(@.*|-[0-9a-f])*$"
+
+  compatible:
+    items:
+      - const: intel,combophy-lgm
+      - const: intel,combo-phy
+
+  clocks:
+    maxItems: 1
+
+  reg:
+    items:
+      - description: ComboPhy core registers
+      - description: PCIe app core control registers
+
+  reg-names:
+    items:
+      - const: core
+      - const: app
+
+  resets:
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: phy
+      - const: core
+
+  intel,syscfg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: Chip configuration registers handle and ComboPhy instance id
+
+  intel,hsio:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: HSIO registers handle and ComboPhy instance id on NOC
+
+  intel,aggregation:
+    type: boolean
+    description: |
+      Specify the flag to configure ComboPHY in dual lane mode.
+
+  intel,phy-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Mode of the two phys in ComboPhy.
+      See dt-bindings/phy/phy-intel-combophy.h for values.
+
+patternProperties:
+  "^phy@[0-9]+$":
+    type: object
+
+    properties:
+      compatible:
+        const: intel,phydev
+
+      "#phy-cells":
+        const: 0
+
+      resets:
+        description: |
+          reset handle according to the PHY mode.
+          See ../reset/reset.txt for details.
+
+    required:
+      - compatible
+      - "#phy-cells"
+
+required:
+  - compatible
+  - clocks
+  - reg
+  - reg-names
+  - intel,syscfg
+  - intel,hsio
+  - intel,phy-mode
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/phy/phy-intel-combophy.h>
+    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy";
+        clocks = <&cgu0 1>;
+        reg = <0xd0a00000 0x40000>,
+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>;
+        reset-names = "phy", "core";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <COMBO_PHY_PCIE>;
+
+        phy@0 {
+            compatible = "intel,phydev";
+            #phy-cells = <0>;
+            resets = <&rcu0 0x50 23>;
+        };
+
+        phy@1 {
+            compatible = "intel,phydev";
+            #phy-cells = <0>;
+            resets = <&rcu0 0x50 24>;
+        };
+    };

include/dt-bindings/phy/phy-intel-combophy.h new file +10 -0

new file mode 100644
index 000000000000..bd7f6377f4ef
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DT_BINDINGS_INTEL_COMBOPHY
+#define _DT_BINDINGS_INTEL_COMBOPHY
+
+#define COMBO_PHY_PCIE	0
+#define COMBO_PHY_XPCS	1
+#define COMBO_PHY_SATA	2
+
+#endif /* _DT_BINDINGS_INTEL_COMBOPHY*/

Signature · 1 line

2.11.0

Rob Herring

maintainer

2020-03-03 01:50 UTC #3 lore raw

Re: [PATCH v4 2/3] dt-bindings: phy: Add YAML schemas for Intel Combophy

Quoted 144 lines · Dilip Kota

Combophy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.

Signed-off-by Dilip Kota <[email protected]>

---
Changes on v4:
No changes.

Changes on v3:

Changes on v2:

 .../devicetree/bindings/phy/intel,combo-phy.yaml   | 123 +++++++++++++++++++++
 include/dt-bindings/phy/phy-intel-combophy.h       |  10 ++
 2 files changed, 133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
 create mode 100644 include/dt-bindings/phy/phy-intel-combophy.h

Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +102 -0

new file mode 100644
index 000000000000..f9bae37fab17
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/intel,combo-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel ComboPhy Subsystem
+
+maintainers:
+  - Dilip Kota <[email protected]>
+
+description: |
+  Intel Combophy subsystem supports PHYs for PCIe, EMAC and SATA
+  controllers. A single Combophy provides two PHY instances.
+
+properties:
+  $nodename:
+    pattern: "combophy(@.*|-[0-9a-f])*$"
+
+  compatible:
+    items:
+      - const: intel,combophy-lgm
+      - const: intel,combo-phy
+
+  clocks:
+    maxItems: 1
+
+  reg:
+    items:
+      - description: ComboPhy core registers
+      - description: PCIe app core control registers
+
+  reg-names:
+    items:
+      - const: core
+      - const: app
+
+  resets:
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: phy
+      - const: core
+
+  intel,syscfg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: Chip configuration registers handle and ComboPhy instance id
+
+  intel,hsio:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: HSIO registers handle and ComboPhy instance id on NOC
+
+  intel,aggregation:
+    type: boolean
+    description: |
+      Specify the flag to configure ComboPHY in dual lane mode.
+
+  intel,phy-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Mode of the two phys in ComboPhy.
+      See dt-bindings/phy/phy-intel-combophy.h for values.
+
+patternProperties:
+  "^phy@[0-9]+$":
+    type: object
+
+    properties:
+      compatible:
+        const: intel,phydev
+
+      "#phy-cells":
+        const: 0
+
+      resets:
+        description: |
+          reset handle according to the PHY mode.
+          See ../reset/reset.txt for details.
+
+    required:
+      - compatible
+      - "#phy-cells"
+
+required:
+  - compatible
+  - clocks
+  - reg
+  - reg-names
+  - intel,syscfg
+  - intel,hsio
+  - intel,phy-mode
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/phy/phy-intel-combophy.h>
+    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy";
+        clocks = <&cgu0 1>;
+        reg = <0xd0a00000 0x40000>,

Documentation/devicetree/bindings/phy/intel,combo-phy.yaml line 112

Quoted 10 lines

Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +10 -0

+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>;
+        reset-names = "phy", "core";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <COMBO_PHY_PCIE>;
+
+        phy@0 {

You need a 'reg' property to go with a unit-address.

Really, I'd just simplify this to make parent 'resets' be 4 entries and put '#phy-cells = <1>;' in the parent. Then you don't need these child nodes.

Quoted 17 lines

Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +11 -0

+            compatible = "intel,phydev";
+            #phy-cells = <0>;
+            resets = <&rcu0 0x50 23>;
+        };
+
+        phy@1 {
+            compatible = "intel,phydev";
+            #phy-cells = <0>;
+            resets = <&rcu0 0x50 24>;
+        };
+    };

include/dt-bindings/phy/phy-intel-combophy.h new file +0 -0

new file mode 100644
index 000000000000..bd7f6377f4ef

include/dt-bindings/phy/phy-intel-combophy.h line 8

Quoted 8 lines

include/dt-bindings/phy/phy-intel-combophy.h new file +8 -0

@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DT_BINDINGS_INTEL_COMBOPHY
+#define _DT_BINDINGS_INTEL_COMBOPHY
+
+#define COMBO_PHY_PCIE	0
+#define COMBO_PHY_XPCS	1
+#define COMBO_PHY_SATA	2

Use the PHY_TYPE_* defines we already have and extend as you need to.

Quoted 5 lines

include/dt-bindings/phy/phy-intel-combophy.h new file +2 -0
+
+#endif /* _DT_BINDINGS_INTEL_COMBOPHY*/
Signature · 1 line
2.11.0

Dilip Kota

author

2020-03-03 09:24 UTC #4 lore raw

Re: [PATCH v4 2/3] dt-bindings: phy: Add YAML schemas for Intel Combophy

Quoted 9 lines · Rob Herring

Quoted 8 lines · Dilip Kota
Combophy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.
Signed-off-by Dilip Kota <[email protected]>
---
Changes on v4:
No changes.

...

Quoted 21 lines

Quoted 9 lines
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +9 -0
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/phy/phy-intel-combophy.h>
+    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy";
+        clocks = <&cgu0 1>;
+        reg = <0xd0a00000 0x40000>,
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml line 112
Quoted 10 lines
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +10 -0
+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>;
+        reset-names = "phy", "core";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <COMBO_PHY_PCIE>;
+
+        phy@0 {
You need a 'reg' property to go with a unit-address.

Quoted 3 lines

Really, I'd just simplify this to make parent 'resets' be 4 entries and put '#phy-cells = <1>;' in the parent. Then you don't need these child nodes.

If child nodes are not present, use case like PCIe controller-0 using phy@0 and PCIe controller-1 using phy@1 wont be possible.

Quoted 19 lines

Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +11 -0

+            compatible = "intel,phydev";
+            #phy-cells = <0>;
+            resets = <&rcu0 0x50 23>;
+        };
+
+        phy@1 {
+            compatible = "intel,phydev";
+            #phy-cells = <0>;
+            resets = <&rcu0 0x50 24>;
+        };
+    };

include/dt-bindings/phy/phy-intel-combophy.h new file +1 -0

new file mode 100644
index 000000000000..bd7f6377f4ef
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */

Quoted 8 lines

include/dt-bindings/phy/phy-intel-combophy.h line 8
Quoted 7 lines
include/dt-bindings/phy/phy-intel-combophy.h new file +7 -0
+
+#ifndef _DT_BINDINGS_INTEL_COMBOPHY
+#define _DT_BINDINGS_INTEL_COMBOPHY
+
+#define COMBO_PHY_PCIE	0
+#define COMBO_PHY_XPCS	1
+#define COMBO_PHY_SATA	2
Use the PHY_TYPE_* defines we already have and extend as you need to.

Sure, will do the same.

Regards,
Dilip

Quoted 6 lines

Quoted 6 lines
include/dt-bindings/phy/phy-intel-combophy.h new file +2 -0
+
+#endif /* _DT_BINDINGS_INTEL_COMBOPHY*/
Signature · 1 line
2.11.0

Rob Herring

maintainer

2020-03-03 16:26 UTC #5 lore raw

Re: [PATCH v4 2/3] dt-bindings: phy: Add YAML schemas for Intel Combophy

Quoted 34 lines · Dilip Kota

Quoted 9 lines · Rob Herring
Quoted 8 lines · Dilip Kota
Combophy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.
Signed-off-by Dilip Kota <[email protected]>
---
Changes on v4:
No changes.
...
Quoted 21 lines
Quoted 9 lines
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +9 -0
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/phy/phy-intel-combophy.h>
+    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy";
+        clocks = <&cgu0 1>;
+        reg = <0xd0a00000 0x40000>,
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml line 112
Quoted 10 lines
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +10 -0
+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>;
+        reset-names = "phy", "core";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <COMBO_PHY_PCIE>;
+
+        phy@0 {
You need a 'reg' property to go with a unit-address.

Quoted 5 lines

Quoted 3 lines
Really, I'd just simplify this to make parent 'resets' be 4 entries and put '#phy-cells = <1>;' in the parent. Then you don't need these child nodes.
If child nodes are not present, use case like PCIe controller-0 using phy@0 and PCIe controller-1 using phy@1 wont be possible.

Yes, it will be.

For controller-0:
phys = <&phy 0>;

For controller-1:
phys = <&phy 1>;

Rob

Dilip Kota

author

2020-03-04 09:16 UTC #6 lore raw

Re: [PATCH v4 2/3] dt-bindings: phy: Add YAML schemas for Intel Combophy

Quoted 39 lines · Rob Herring

Quoted 33 lines · Dilip Kota
Quoted 9 lines · Rob Herring
Quoted 8 lines · Dilip Kota
Combophy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.
Signed-off-by Dilip Kota <[email protected]>
---
Changes on v4:
No changes.
...
Quoted 21 lines
Quoted 9 lines
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +9 -0
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/phy/phy-intel-combophy.h>
+    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy";
+        clocks = <&cgu0 1>;
+        reg = <0xd0a00000 0x40000>,
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml line 112
Quoted 10 lines
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +10 -0
+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>;
+        reset-names = "phy", "core";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <COMBO_PHY_PCIE>;
+
+        phy@0 {
You need a 'reg' property to go with a unit-address.
Quoted 5 lines
Quoted 3 lines
Really, I'd just simplify this to make parent 'resets' be 4 entries and put '#phy-cells = <1>;' in the parent. Then you don't need these child nodes.
If child nodes are not present, use case like PCIe controller-0 using phy@0 and PCIe controller-1 using phy@1 wont be possible.

Quoted 7 lines

Yes, it will be.
For controller-0:
phys = <&phy 0>;
For controller-1:
phys = <&phy 1>;

OH got it, arg cell can be utilized for PHY id.
I started working on your suggestion in simplifying it, but below point is haunting while doing the changes. So felt to check with you whether the better one is going with existing DT node or the one without child nodes!. Existing DT node skeleton, replicates hardware design ComboPhy with 2 PHYs. (ComboPhy as parent node and 2PHYs as child nodes)

Regards,
Dilip

Quoted 2 lines

Rob

Dilip Kota

author

2020-03-13 08:36 UTC #7 lore raw

Re: [PATCH v4 2/3] dt-bindings: phy: Add YAML schemas for Intel Combophy

Hi Rob,

Quoted 9 lines · Dilip Kota

Quoted 7 lines · Rob Herring
Quoted 6 lines · Dilip Kota
Quoted 4 lines · Rob Herring
Quoted 3 lines · Dilip Kota
Combophy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.

...

Quoted 28 lines

Quoted 28 lines
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
+ - |
+    #include <dt-bindings/phy/phy-intel-combophy.h> +    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy"; +        clocks = <&cgu0 1>;
+        reg = <0xd0a00000 0x40000>,
+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>;
+        reset-names = "phy", "core";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <COMBO_PHY_PCIE>;
+
+        phy@0 {
Quoted 7 lines
Quoted 5 lines
You need a 'reg' property to go with a unit-address.
Really, I'd just simplify this to make parent 'resets' be 4 entries and put '#phy-cells = <1>;' in the parent. Then you don't need these child nodes.
If child nodes are not present, use case like PCIe controller-0 using phy@0 and PCIe controller-1 using phy@1 wont be possible.
Yes, it will be.
For controller-0:
phys = <&phy 0>;

Quoted 6 lines

Quoted 2 lines
For controller-1:
phys = <&phy 1>;
OH got it, arg cell can be utilized for PHY id.
I started working on your suggestion in simplifying it, but below point is haunting while doing the changes. So felt to check with you whether the better one is going with existing DT node or the one without child nodes!. Existing DT node skeleton, replicates hardware design ComboPhy with 2 PHYs. (ComboPhy as parent node and 2PHYs as child nodes)

In the patchwork, i see the patch state is 'Change Requested', so felt to keep a remainder mail for your inputs on above query. I have waiting to push the appropriate code changes based on your comment.

Thanks,
Dilip

Quoted 6 lines

Regards,
Dilip
Quoted 2 lines
Rob

Dilip Kota

author

2020-03-02 08:43 UTC #8 lore raw

PATCH v4 3/3 phy: intel: Add driver support for ComboPhy

In reply to Dilip Kota

ComboPhy subsystem provides PHYs for various
controllers like PCIe, SATA and EMAC.

Signed-off-by Dilip Kota <[email protected]>

---
Changes on v4:
Address review comments
Remove dependency on OF config
Update copyright to 2019-2020
Define register macro PAD_DIS_CFG instead of const variable inside function. Improve the error prints, and error returns.
Call put_device(dev), for get_dev_from_fwnode()
Move platform_set_drvdata() at the end of the probe().
Correct alignment in phy_ops intel_cbphy_ops.
Correct commented lines with proper vocabulary and punctuation. Add/remove commas for the required constant arrays and enums. Remove in driver:
linux/kernel.h, not required
macros: PCIE_PHY_MPLLA_CTRL, PCIE_PHY_MPLLB_CTRL
temp variable u32 prop;
Change function names:

     intel_cbphy_iphy_dt_parse() -> intel_cbphy_iphy_fwnode_parse()
     intel_cbphy_dt_sanity_check() -> intel_cbphy_sanity_check()
     intel_cbphy_dt_parse() -> intel_cbphy_fwnode_parse()

Changes on v3:
Remove intel_iphy_names
Remove struct phy in struct intel_cbphy_iphy
Imporve if conditions logic
Use fwnode_to_regmap()
Call devm_of_platform_populate() to populate child nodes Fix reset sequence during phy_init
Add SoC specific compatible "intel,combophy-lgm"
Add description for enums
Remove default case in switch {} intel_cbphy_set_mode() as it
never happens.
Use mutex_lock to synchronise combophy initialization across
two phys.
Change init_cnt to u32 datatype as it is within mutex lock. Correct error handling of
fwnode_property_read_u32_array(fwnode, "intel,phy-mode", ...)

 drivers/phy/intel/Kconfig           |  14 +
 drivers/phy/intel/Makefile          |   1 +
 drivers/phy/intel/phy-intel-combo.c | 661 ++++++++++++++++++++++++++++++++++++
 3 files changed, 676 insertions(+)
 create mode 100644 drivers/phy/intel/phy-intel-combo.c

drivers/phy/intel/Kconfig +14 -0

index 4ea6a8897cd7..14705b80ec8b 100644
@@ -2,6 +2,20 @@
 #
 # Phy drivers for Intel Lightning Mountain(LGM) platform
 #
+config PHY_INTEL_COMBO
+	bool "Intel ComboPHY driver"
+	depends on X86 || COMPILE_TEST
+	depends on HAS_IOMEM
+	select MFD_SYSCON
+	select GENERIC_PHY
+	select REGMAP
+	help
+	  Enable this to support Intel ComboPhy.
+
+	  This driver configures ComboPhy subsystem on Intel gateway
+	  chipsets which provides PHYs for various controllers, EMAC,
+	  SATA and PCIe.
+
 config PHY_INTEL_EMMC
 	tristate "Intel EMMC PHY driver"
 	select GENERIC_PHY

drivers/phy/intel/Makefile +1 -0

index 6b876a75599d..233d530dadde 100644
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_INTEL_COMBO)		+= phy-intel-combo.o
 obj-$(CONFIG_PHY_INTEL_EMMC)            += phy-intel-emmc.o

drivers/phy/intel/phy-intel-combo.c new file +661 -0

661 lines not on this page · raw message

Signature · 1 line

2.11.0

Andy Shevchenko

2020-03-02 11:19 UTC #9 lore raw

Re: [PATCH v4 3/3] phy: intel: Add driver support for ComboPhy

Quoted 2 lines · Dilip Kota

ComboPhy subsystem provides PHYs for various
controllers like PCIe, SATA and EMAC.

Thanks for an update, my (few minor) comments below.

...

drivers/phy/intel/phy-intel-combo.c line 58

Quoted 5 lines

drivers/phy/intel/phy-intel-combo.c new file +4 -0
+enum intel_phy_mode {
+	PHY_PCIE_MODE = 0,
+	PHY_XPCS_MODE,
 
+	PHY_SATA_MODE

From here it's not visible that above is the only possible values. Maybe in the future you will have another mode.
So, I suggest to leave comma here...

drivers/phy/intel/phy-intel-combo.c line 67

Quoted 9 lines

drivers/phy/intel/phy-intel-combo.c new file +7 -0

+};
 
+enum intel_combo_mode {
+	PCIE0_PCIE1_MODE = 0,
+	PCIE_DL_MODE,
+	RXAUI_MODE,
+	XPCS0_XPCS1_MODE,
 
+	SATA0_SATA1_MODE

...and here...

drivers/phy/intel/phy-intel-combo.c line 72

Quoted 6 lines

drivers/phy/intel/phy-intel-combo.c new file +5 -0
+};
+
+enum aggregated_mode {
+	PHY_SL_MODE,
 
+	PHY_DL_MODE

...and here.

Quoted 1 line

+};

...

Quoted 6 lines

drivers/phy/intel/phy-intel-combo.c new file +6 -0

+static int intel_cbphy_iphy_cfg(struct intel_cbphy_iphy *iphy,
+				int (*phy_cfg)(struct intel_cbphy_iphy *))
+{
+	struct intel_combo_phy *cbphy = iphy->parent;
+	struct intel_cbphy_iphy *sphy;
+	int ret;

drivers/phy/intel/phy-intel-combo.c line 154

Quoted 10 lines

drivers/phy/intel/phy-intel-combo.c new file +9 -0

+
+	ret = phy_cfg(iphy);
+	if (ret)
+		return ret;
+
+	if (cbphy->aggr_mode != PHY_DL_MODE)
+		return 0;
+
 
+	sphy = &cbphy->iphy[PHY_1];

Do you really need temporary variable here?

Quoted 3 lines

drivers/phy/intel/phy-intel-combo.c new file +3 -0
+
+	return phy_cfg(sphy);
+}

...

Review

Quoted 1 line

(quoted diff) +1 -0
+	if (!cbphy->init_cnt) {

	if (init_cnt)
		return 0;

Quoted 5 lines

drivers/phy/intel/phy-intel-combo.c new file +5 -0

+		clk_disable_unprepare(cbphy->core_clk);
+		intel_cbphy_rst_assert(cbphy);
+	}
+
+	return 0;

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Dilip Kota

author

2020-03-03 08:41 UTC #10 lore raw

Re: [PATCH v4 3/3] phy: intel: Add driver support for ComboPhy

Quoted 7 lines · Andy Shevchenko

Quoted 2 lines · Dilip Kota
ComboPhy subsystem provides PHYs for various
controllers like PCIe, SATA and EMAC.
Thanks for an update, my (few minor) comments below.
...

Quoted 7 lines

drivers/phy/intel/phy-intel-combo.c line 58
Quoted 4 lines
drivers/phy/intel/phy-intel-combo.c new file +4 -0
+enum intel_phy_mode {
+	PHY_PCIE_MODE = 0,
+	PHY_XPCS_MODE,
+	PHY_SATA_MODE
From here it's not visible that above is the only possible values.
Maybe in the future you will have another mode.
So, I suggest to leave comma here...

There won't be no further modes,...
i can still add the comma at all the places pointed.

Quoted 20 lines

drivers/phy/intel/phy-intel-combo.c line 67
Quoted 8 lines
drivers/phy/intel/phy-intel-combo.c new file +7 -0
+};
+enum intel_combo_mode {
+	PCIE0_PCIE1_MODE = 0,
+	PCIE_DL_MODE,
+	RXAUI_MODE,
+	XPCS0_XPCS1_MODE,
+	SATA0_SATA1_MODE
...and here...
drivers/phy/intel/phy-intel-combo.c line 72
Quoted 5 lines
drivers/phy/intel/phy-intel-combo.c new file +5 -0
+};
+
+enum aggregated_mode {
+	PHY_SL_MODE,
+	PHY_DL_MODE
...and here.
Quoted 1 line
+};
...

Quoted 16 lines

Quoted 5 lines
drivers/phy/intel/phy-intel-combo.c new file +5 -0
+static int intel_cbphy_iphy_cfg(struct intel_cbphy_iphy *iphy,
+				int (*phy_cfg)(struct intel_cbphy_iphy *))
+{
+	struct intel_combo_phy *cbphy = iphy->parent;
+	struct intel_cbphy_iphy *sphy;
drivers/phy/intel/phy-intel-combo.c line 154
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10 -0
+	int ret;
+
+	ret = phy_cfg(iphy);
+	if (ret)
+		return ret;
+
+	if (cbphy->aggr_mode != PHY_DL_MODE)
+		return 0;
+
+	sphy = &cbphy->iphy[PHY_1];
Do you really need temporary variable here?

Can be removed, i will update in the next patch.

Quoted 11 lines

Quoted 4 lines
drivers/phy/intel/phy-intel-combo.c new file +3 -0
+
+	return phy_cfg(sphy);
+}
...
Review
Quoted 1 line
(quoted diff) +1 -0
+	if (!cbphy->init_cnt) {
	if (init_cnt)
		return 0;
?

Sure, will do the same.

Thanks,
Dilip

Quoted 5 lines

Quoted 5 lines
drivers/phy/intel/phy-intel-combo.c new file +5 -0
+		clk_disable_unprepare(cbphy->core_clk);
+		intel_cbphy_rst_assert(cbphy);
+	}
+
+	return 0;

Andy Shevchenko

2020-03-03 10:07 UTC #11 lore raw

Re: [PATCH v4 3/3] phy: intel: Add driver support for ComboPhy

Quoted 5 lines · Dilip Kota

Quoted 4 lines · Andy Shevchenko
Quoted 2 lines · Dilip Kota
ComboPhy subsystem provides PHYs for various
controllers like PCIe, SATA and EMAC.
Thanks for an update, my (few minor) comments below.

...

Quoted 8 lines

Quoted 7 lines
drivers/phy/intel/phy-intel-combo.c line 58
Quoted 4 lines
drivers/phy/intel/phy-intel-combo.c new file +4 -0
+enum intel_phy_mode {
+	PHY_PCIE_MODE = 0,
+	PHY_XPCS_MODE,
+	PHY_SATA_MODE
From here it's not visible that above is the only possible values.
Maybe in the future you will have another mode.
So, I suggest to leave comma here...
There won't be no further modes,...

Again, this is not obvious from the naming scheme in use. And how do you know the future?

Quoted 1 line

i can still add the comma at all the places pointed.

Just use a common sense.

Quoted 17 lines

Quoted 17 lines
drivers/phy/intel/phy-intel-combo.c line 67
Quoted 7 lines
drivers/phy/intel/phy-intel-combo.c new file +7 -0
+};
+enum intel_combo_mode {
+	PCIE0_PCIE1_MODE = 0,
+	PCIE_DL_MODE,
+	RXAUI_MODE,
+	XPCS0_XPCS1_MODE,
+	SATA0_SATA1_MODE
...and here...
drivers/phy/intel/phy-intel-combo.c line 72
Quoted 5 lines
drivers/phy/intel/phy-intel-combo.c new file +5 -0
+};
+
+enum aggregated_mode {
+	PHY_SL_MODE,
+	PHY_DL_MODE
...and here.
Quoted 1 line
+};

Signature · 2 lines

With Best Regards,
Andy Shevchenko