Skip to contentSkip to conversation

Add Intel ComboPhy driver

PATCH Superseded Dilip Kota 3 participants 10 messages Apr 3, 2020 – Apr 29, 2020

Subsystems (4)

v6 of 9 Superseded Latest is v9 →

lore
Share & tools
lore

Dilip Kota

author
#0 lore raw

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

Changes on v6:
Rebase patches on the latest maintainer's branch
https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7

Dilip Kota (4):
mfd: syscon: Add fwnode_to_regmap
dt-bindings: phy: Add PHY_TYPE_XPCS definition
dt-bindings: phy: Add YAML schemas for Intel ComboPhy
phy: intel: Add driver support for ComboPhy

 .../devicetree/bindings/phy/intel,combo-phy.yaml   | 101 ++++
 drivers/mfd/syscon.c                               |   8 +
 drivers/phy/intel/Kconfig                          |  14 +
 drivers/phy/intel/Makefile                         |   1 +
 drivers/phy/intel/phy-intel-combo.c                | 626 +++++++++++++++++++++
 include/dt-bindings/phy/phy.h                      |   1 +
 include/linux/mfd/syscon.h                         |   6 +
 7 files changed, 757 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
 create mode 100644 drivers/phy/intel/phy-intel-combo.c
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH v6 1/4 mfd: syscon: Add fwnode_to_regmap

Traverse regmap handle entry from firmware node handle.

---
Changes on v5:
No changes

Changes on v5:
No changes

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 3a97816d0cba..e085c50816b9 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 7f20e9b502a5..dacab0b4a091 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(
@@ -34,6 +35,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
author
#7 lore raw

PATCH v6 2/4 dt-bindings: phy: Add PHY_TYPE_XPCS definition

In reply to Dilip Kota

Add definition for Ethernet PCS phy type.

---
Changes on v6:
Add Acked-by: Rob Herring <[email protected]>

 include/dt-bindings/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/phy/phy.h +1 -0
index 1f3f866fae7b..3727ef72138b 100644
@@ -17,5 +17,6 @@
 #define PHY_TYPE_USB3		4
 #define PHY_TYPE_UFS		5
 #define PHY_TYPE_DP		6
+#define PHY_TYPE_XPCS		7
 
 #endif /* _DT_BINDINGS_PHY */
Signature · 1 line
2.11.0
author
#8 lore raw

PATCH v6 3/4 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.

---
Changes on v6:
Add Reviewed-by: Rob Herring <[email protected]>

Changes on v5:
Add changes as per Rob Herring inputs:
Use include/dt-bindings/phy/phy.h values to set intel,phy-mode. Move children node properties to parent node and remove children
node completely.

Changes on v4:
No Change.

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   | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +101 -0
new file mode 100644
index 000000000000..347d0cdfb80d
@@ -0,0 +1,101 @@
+# 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: 4
+
+  reset-names:
+    items:
+      - const: phy
+      - const: core
+      - const: iphy0
+      - const: iphy1
+
+  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.h for values.
+
+  "#phy-cells":
+    const: 1
+
+required:
+  - compatible
+  - clocks
+  - reg
+  - reg-names
+  - intel,syscfg
+  - intel,hsio
+  - intel,phy-mode
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/phy/phy.h>
+    combophy@d0a00000 {
+        compatible = "intel,combophy-lgm", "intel,combo-phy";
+        clocks = <&cgu0 1>;
+        #phy-cells = <1>;
+        reg = <0xd0a00000 0x40000>,
+              <0xd0a40000 0x1000>;
+        reg-names = "core", "app";
+        resets = <&rcu0 0x50 6>,
+                 <&rcu0 0x50 17>,
+                 <&rcu0 0x50 23>,
+                 <&rcu0 0x50 24>;
+        reset-names = "phy", "core", "iphy0", "iphy1";
+        intel,syscfg = <&sysconf 0>;
+        intel,hsio = <&hsiol 0>;
+        intel,phy-mode = <PHY_TYPE_PCIE>;
+        intel,aggregation;
+    };
Signature · 1 line
2.11.0
author
#9 lore raw

PATCH v6 4/4 phy: intel: Add driver support for ComboPhy

In reply to Dilip Kota

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

---
Changes on v6:
No changes

Changes on v5:
Add changes as per inputs from Andy and Rob:
DT node uses phy-mode values as defined in "include/dt-bindings/phy/phy.h",
add changes to handle it.
ComboPhy no longer has children nodes, and children node properties(reset)
moved to parent node, so do the code changes accordingly.
Add _xlate() function to pass the appropriate phy handle.
Fix couple of nitpicks.

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 | 626 ++++++++++++++++++++++++++++++++++++
 3 files changed, 641 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 +626 -0

626 lines not on this page · raw message

Signature · 1 line
2.11.0
#2 lore raw

Re: [RESEND PATCH v6 1/4] mfd: syscon: Add fwnode_to_regmap

In reply to Dilip Kota
Quoted 22 lines · Dilip Kota

Traverse regmap handle entry from firmware node handle.

---
Changes on v5:
No changes

Changes on v5:
No changes

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 +0 -0
index 3a97816d0cba..e085c50816b9 100644

drivers/mfd/syscon.c struct regmap *device_node_to_regmap(struct device_node *np) · line 183

Quoted 6 lines
drivers/mfd/syscon.c +3 -0
@@ -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);

You are assuming that the fwnode was Device Tree pointer.

The point of a fwnode is that it could be one of multiple types.

What if it was a pointer to an ACPI property?

Quoted 31 lines
drivers/mfd/syscon.c +4 -0
+	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 7f20e9b502a5..dacab0b4a091 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(
@@ -34,6 +35,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 · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#3 lore raw

Re: [RESEND PATCH v6 1/4] mfd: syscon: Add fwnode_to_regmap

Quoted 27 lines · Lee Jones
Quoted 25 lines · Dilip Kota

Traverse regmap handle entry from firmware node handle.

---
Changes on v5:
No changes

Changes on v5:
No changes

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 +0 -0
index 3a97816d0cba..e085c50816b9 100644
@@ -178,6 +178,14 @@ struct regmap *device_node_to_regmap(struct device_node *np)
  }
  EXPORT_SYMBOL_GPL(device_node_to_regmap);
 
Quoted 8 lines

drivers/mfd/syscon.c struct regmap *device_node_to_regmap(struct device_node *np) · line 183

Quoted 3 lines
drivers/mfd/syscon.c +3 -0
+struct regmap *fwnode_to_regmap(struct fwnode_handle *fwnode)
+{
+	struct device_node *np = to_of_node(fwnode);

You are assuming that the fwnode was Device Tree pointer.

The point of a fwnode is that it could be one of multiple types.

What if it was a pointer to an ACPI property?

Yes, i missed to check in other perspective. Thanks for pointing it.
While going through the System control driver to address the query, i noticed that System Control driver is talking with 'of' framework only. (No ACPI)

So, i think to add a defensive check and return error pointer if 'to_of_node' returns NULL As System control Driver cannot talk with ACPI, so fwnode_to_regmap() cannot talk and return error.

Or, the other option is removing the 'fwnode_to_regmap()' definition itself, to avoid confusion as fwnode can point to 'OF', 'ACPI'or 'swnode'.

But, i feel return error for ACPI or oother, looks better because 'device_node' has fwnode pointer. And provide description in the header file, mentioning function is success for 'OF' and returns error for the rest.

Regards,
Dilip

Quoted 2 lines
#4 lore raw

Re: [RESEND PATCH v6 1/4] mfd: syscon: Add fwnode_to_regmap

Quoted 45 lines · Dilip Kota
Quoted 30 lines · Lee Jones
Quoted 21 lines · Dilip Kota

Traverse regmap handle entry from firmware node handle.

---
Changes on v5:
No changes
Changes on v5:
No changes

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 +0 -0
index 3a97816d0cba..e085c50816b9 100644

drivers/mfd/syscon.c struct regmap *device_node_to_regmap(struct device_node *np) · line 182

Quoted 5 lines
drivers/mfd/syscon.c +3 -0
@@ -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);

You are assuming that the fwnode was Device Tree pointer.

Quoted 3 lines

The point of a fwnode is that it could be one of multiple types.

What if it was a pointer to an ACPI property?

Yes, i missed to check in other perspective. Thanks for pointing it. While going through the System control driver to address the query, i noticed that System Control
driver is talking with 'of' framework only. (No ACPI)

So, i think to add a defensive check and return error pointer if 'to_of_node' returns NULL
As System control Driver cannot talk with ACPI, so fwnode_to_regmap() cannot talk and return error.

Quoted 8 lines

Or, the other option is removing the 'fwnode_to_regmap()' definition itself, to avoid confusion as fwnode can
point to 'OF', 'ACPI'or 'swnode'.

But, i feel return error for ACPI or oother, looks better because 'device_node' has fwnode pointer. And provide description
in the header file, mentioning function is success for 'OF' and returns error for the rest.

I don't think this patch adds much to be honest.

Better to just do:

    device_node_get_regmap(to_of_node(fwnode), false);

... from the call site I think.

Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#5 lore raw

Re: [RESEND PATCH v6 1/4] mfd: syscon: Add fwnode_to_regmap

Quoted 7 lines · Lee Jones
Quoted 5 lines · Dilip Kota

But, i feel return error for ACPI or oother, looks better because 'device_node' has fwnode pointer. And provide description
in the header file, mentioning function is success for 'OF' and returns error for the rest.

Quoted 7 lines

I don't think this patch adds much to be honest.

Better to just do:

    device_node_get_regmap(to_of_node(fwnode), false);

... from the call site I think.

Agreed, or just use the of_node pointer consistently in the driver that uses it and avoid the use of the fwnode interface entirely when dealing with a modern driver that does not need to support board files any more.

Arnd

author
#6 lore raw

Re: [RESEND PATCH v6 1/4] mfd: syscon: Add fwnode_to_regmap

Quoted 10 lines · Arnd Bergmann
Quoted 9 lines · Lee Jones
Quoted 4 lines · Dilip Kota

But, i feel return error for ACPI or oother, looks better because 'device_node' has fwnode pointer. And provide description
in the header file, mentioning function is success for 'OF' and returns error for the rest.

I don't think this patch adds much to be honest.

Better to just do:

Quoted 8 lines
Quoted 3 lines
     device_node_get_regmap(to_of_node(fwnode), false);

... from the call site I think.

Agreed, or just use the of_node pointer consistently in the driver that uses it and avoid the use of the fwnode interface entirely when dealing with a modern driver that does not need to support board files any more.

Arnd

Ok, I will do it in the driver itself and remove the fwnode_to_regmap() definition.

Regards,
Dilip