Add Intel ComboPhy driver

PATCH RESEND Superseded Dilip Kota 3 participants 9 messages May 15, 2020 – May 19, 2020

Subsystems (2)

Device tree bindings Generic PHY framework

lore

Share & tools

lore

Dilip Kota

author

2020-05-15 08:15 UTC #0 lore raw

v8 cover Add Intel ComboPhy driver

This patch series adds Intel ComboPhy driver, respective yaml schemas

Changes on v8:
As per PHY Maintainer's request add description in comments for doing register access through register map framework.

Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap() definition and use device_node_get_regmap()

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 (3):
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/phy/intel/Kconfig                          |  14 +
 drivers/phy/intel/Makefile                         |   1 +
 drivers/phy/intel/phy-intel-combo.c                | 632 +++++++++++++++++++++
 include/dt-bindings/phy/phy.h                      |   1 +
 5 files changed, 749 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

Dilip Kota

author

2020-05-15 08:15 UTC #1 lore raw

PATCH RESEND v8 1/3 dt-bindings: phy: Add PHY_TYPE_XPCS definition

Add definition for Ethernet PCS phy type.

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

Acked-by Rob Herring <[email protected]>

---
Changes on v8:
No Change

Changes on v7:
No Change

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

Dilip Kota

author

2020-05-15 08:15 UTC #2 lore raw

PATCH RESEND v8 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]>

Reviewed-by Rob Herring <[email protected]>

---
Changes on v8:
No Change.

Changes on v7:
No Change.

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

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

Dilip Kota

author

2020-05-15 08:15 UTC #3 lore raw

PATCH RESEND v8 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 v8:
As per PHY Maintainer's request add description for doing register access through regmap in comments.

Changes on v7:
Use device_node_to_regmap instead of fwnode_to_regmap

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 | 632 ++++++++++++++++++++++++++++++++++++
 3 files changed, 647 insertions(+)
 create mode 100644 drivers/phy/intel/phy-intel-combo.c

drivers/phy/intel/Kconfig +14 -0

index 4ea6a8897cd7..3b40eb7b4fb4 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 OF && 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 +632 -0

632 lines not on this page · raw message

Signature · 1 line

2.11.0

Vinod Koul

maintainer Acked-by on the v8 series

2020-05-18 12:41 UTC #4 lore raw

In reply to Dilip Kota

Quoted 6 lines · Dilip Kota

This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v8:
As per PHY Maintainer's request add description in comments for doing register access through register map framework.

Quoted 7 lines

Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap() definition and use device_node_get_regmap()
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

Acked-by Vinod Koul <[email protected]>

Signature · 1 line

~Vinod

Kishon Vijay Abraham I

2020-05-18 13:49 UTC #5 lore raw

In reply to Dilip Kota

Dilip,

Quoted 9 lines · Dilip Kota

This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v8:
As per PHY Maintainer's request add description in comments for doing register access through register map framework.
Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap() definition and use device_node_get_regmap()

Can you fix this warning and resend the patch? drivers/phy/intel/phy-intel-combo.c:229:6: warning: ‘cb_mode’ may be used uninitialized in this function [-Wmaybe-uninitialized]
ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode); ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/phy/intel/phy-intel-combo.c:204:24: note: ‘cb_mode’ was declared here

  enum intel_combo_mode cb_mode;
                        ^~~~~~~

Thanks
Kishon

Quoted 18 lines

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 (3):
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/phy/intel/Kconfig                          |  14 +
 drivers/phy/intel/Makefile                         |   1 +
 drivers/phy/intel/phy-intel-combo.c                | 632 +++++++++++++++++++++
 include/dt-bindings/phy/phy.h                      |   1 +
 5 files changed, 749 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
 create mode 100644 drivers/phy/intel/phy-intel-combo.c

Dilip Kota

author

2020-05-19 03:56 UTC #6 lore raw

Quoted 12 lines · Kishon Vijay Abraham I

Dilip,
Quoted 9 lines · Dilip Kota
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v8:
As per PHY Maintainer's request add description in comments for doing register access through register map framework.
Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap() definition and use device_node_get_regmap()

Quoted 8 lines

Can you fix this warning and resend the patch? drivers/phy/intel/phy-intel-combo.c:229:6: warning: ‘cb_mode’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode);
   ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/phy/intel/phy-intel-combo.c:204:24: note: ‘cb_mode’ was declared here
   enum intel_combo_mode cb_mode;
                         ^~~~~~~

I noticed this warning while preparing the patch.
It sounds like false warning because:
1.) "cb_mode" is initialized in the switch case based on the "mode = cbphy->phy_mode;"
2.) cbphy->phy_mode is initialized during the probe in "intel_cbphy_fwnode_parse()" with one of the 3 values. PHY_PCIE_MODE, PHY_SATA_MODE, PHY_XPCS_MODE.
3.) There is no chance of "cbphy->phy_mode" having different value.
4.) And "cb_mode" will be initialized according to the "mode = cbphy->phy_mode;"
5.) Hence, there is no chance of "cb_mode" getting accessed uninitialized.

Regards,
Dilip

Quoted 20 lines

Thanks
Kishon
Quoted 18 lines
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 (3):
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/phy/intel/Kconfig                          |  14 +
  drivers/phy/intel/Makefile                         |   1 +
  drivers/phy/intel/phy-intel-combo.c                | 632 +++++++++++++++++++++
  include/dt-bindings/phy/phy.h                      |   1 +
  5 files changed, 749 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml create mode 100644 drivers/phy/intel/phy-intel-combo.c

Kishon Vijay Abraham I

2020-05-19 05:18 UTC #7 lore raw

Dilip,

Quoted 22 lines · Dilip Kota

Quoted 12 lines · Kishon Vijay Abraham I
Dilip,
Quoted 9 lines · Dilip Kota
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v8:
   As per PHY Maintainer's request add description in comments for doing register access through register map framework.
Changes on v7:
   As per System control driver maintainer's inputs remove fwnode_to_regmap() definition and use device_node_get_regmap()
Quoted 8 lines
Can you fix this warning and resend the patch? drivers/phy/intel/phy-intel-combo.c:229:6: warning: ‘cb_mode’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode); ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/phy/intel/phy-intel-combo.c:204:24: note: ‘cb_mode’ was declared here enum intel_combo_mode cb_mode;
                         ^~~~~~~

Quoted 10 lines

I noticed this warning while preparing the patch.
It sounds like false warning because:
1.) "cb_mode" is initialized in the switch case based on the "mode = cbphy->phy_mode;"
2.) cbphy->phy_mode is initialized during the probe in "intel_cbphy_fwnode_parse()" with one of the 3 values. PHY_PCIE_MODE, PHY_SATA_MODE, PHY_XPCS_MODE.
3.) There is no chance of "cbphy->phy_mode" having different value. 4.) And "cb_mode" will be initialized according to the "mode = cbphy->phy_mode;" 5.) Hence, there is no chance of "cb_mode" getting accessed uninitialized.

Let's try to keep the compiler happy. Please fix this warning.

Thanks
Kishon

Dilip Kota

author

2020-05-19 05:25 UTC #8 lore raw

Quoted 34 lines · Kishon Vijay Abraham I

Dilip,
Quoted 21 lines · Dilip Kota
Quoted 12 lines · Kishon Vijay Abraham I
Dilip,
Quoted 9 lines · Dilip Kota
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v8:
   As per PHY Maintainer's request add description in comments for doing register access through register map framework.
Changes on v7:
   As per System control driver maintainer's inputs remove fwnode_to_regmap() definition and use device_node_get_regmap()
Quoted 8 lines
Can you fix this warning and resend the patch? drivers/phy/intel/phy-intel-combo.c:229:6: warning: ‘cb_mode’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode); ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/phy/intel/phy-intel-combo.c:204:24: note: ‘cb_mode’ was declared here
   enum intel_combo_mode cb_mode;
                         ^~~~~~~
Quoted 10 lines
I noticed this warning while preparing the patch.
It sounds like false warning because:
1.) "cb_mode" is initialized in the switch case based on the "mode = cbphy->phy_mode;"
2.) cbphy->phy_mode is initialized during the probe in "intel_cbphy_fwnode_parse()" with one of the 3 values. PHY_PCIE_MODE, PHY_SATA_MODE, PHY_XPCS_MODE.
3.) There is no chance of "cbphy->phy_mode" having different value. 4.) And "cb_mode" will be initialized according to the "mode = cbphy->phy_mode;" 5.) Hence, there is no chance of "cb_mode" getting accessed uninitialized.

Quoted 1 line

Let's try to keep the compiler happy. Please fix this warning.

Sure, will fix it and send the patch series.

Quoted 3 lines

Thanks
Kishon