devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY
@ 2025-06-02  9:53 George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 1/5] clk: qcom: gcc-ipq5018: fix GE PHY reset George Moussalem via B4 Relay
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-02  9:53 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Philipp Zabel, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk,
	George Moussalem, Konrad Dybcio

The IPQ5018 SoC contains an internal Gigabit Ethernet PHY with its
output pins that provide an MDI interface to either an external switch
in a PHY to PHY link architecture or directly to an attached RJ45
connector.

The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
802.3az EEE.

The LDO controller found in the IPQ5018 SoC needs to be enabled to drive
power to the CMN Ethernet Block (CMN BLK) which the GE PHY depends on.
The LDO must be enabled in TCSR by writing to a specific register.

In a phy to phy architecture, DAC values need to be set to accommodate
for the short cable length.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
Changes in v3:
- Replace bitmask of GEPHY_MISC_ARES with GENMASK as suggested by Konrad
- Removed references to RX and TX clocks as the driver need not
  explicitly enable them. The GCC gatecontrols and routes the PHY's
  output clocks, registered in the DT as fixed clocks, back to the PHY.
  The bindings file has been updated accordingly.
- Removed acquisition and enablement of RX and TX clocks from the driver
- Link to v2: https://lore.kernel.org/r/20250528-ipq5018-ge-phy-v2-0-dd063674c71c@outlook.com

Changes in v2:
- Moved values for MDAC and EDAC into the driver and converted DT
  property qca,dac to a new boolean: qcom,dac-preset-short-cable as per
  discussion.
- Added compatible string along with a condition with a description of
  properties including clocks, resets, and qcom,dac-preset-short-cable
  in the bindings to address bindings issues reported by Rob and to
  bypass restrictions on nr of clocks and resets in ethernet-phy.yaml
- Added example to bindings file
- Renamed all instances of IPQ5018_PHY_MMD3* macros to IPQ5018_PHY_PCS*
- Removed qca,eth-ldo-ready property and moved the TCSR register to the
  mdio bus the phy is on as there's already support for setting this reg
  property in the mdio-ipq4019 driver as per commit:
  23a890d493e3ec1e957bc925fabb120962ae90a7
- Explicitly probe on PHY ID as otherwise the PHY wouldn't come up and
  initialize as found during further testing when the kernel is flashed
  to NAND
- Link to v1: https://lore.kernel.org/r/20250525-ipq5018-ge-phy-v1-0-ddab8854e253@outlook.com

---
George Moussalem (5):
      clk: qcom: gcc-ipq5018: fix GE PHY reset
      dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support
      net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
      arm64: dts: qcom: ipq5018: Add MDIO buses
      arm64: dts: qcom: ipq5018: Add GE PHY to internal mdio bus

 .../devicetree/bindings/net/qca,ar803x.yaml        |  39 +++++
 arch/arm64/boot/dts/qcom/ipq5018.dtsi              |  48 +++++-
 drivers/clk/qcom/gcc-ipq5018.c                     |   2 +-
 drivers/net/phy/qcom/Kconfig                       |   2 +-
 drivers/net/phy/qcom/at803x.c                      | 185 ++++++++++++++++++++-
 5 files changed, 264 insertions(+), 12 deletions(-)
---
base-commit: ebfff09f63e3efb6b75b0328b3536d3ce0e26565
change-id: 20250430-ipq5018-ge-phy-db654afa4ced

Best regards,
-- 
George Moussalem <george.moussalem@outlook.com>



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/5] clk: qcom: gcc-ipq5018: fix GE PHY reset
  2025-06-02  9:53 [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY George Moussalem via B4 Relay
@ 2025-06-02  9:53 ` George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support George Moussalem via B4 Relay
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-02  9:53 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Philipp Zabel, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk,
	George Moussalem, Konrad Dybcio

From: George Moussalem <george.moussalem@outlook.com>

The MISC reset is supposed to trigger a resets across the MDC, DSP, and
RX & TX clocks of the IPQ5018 internal GE PHY. So let's set the bitmask
of the reset definition accordingly in the GCC as per the downstream
driver.

Link: https://git.codelinaro.org/clo/qsdk/oss/kernel/linux-ipq-5.4/-/commit/00743c3e82fa87cba4460e7a2ba32f473a9ce932

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 drivers/clk/qcom/gcc-ipq5018.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-ipq5018.c b/drivers/clk/qcom/gcc-ipq5018.c
index 70f5dcb96700f55da1fb19fc893d22350a7e63bf..6eb86c034fda18c38dcd9726f0903841252381da 100644
--- a/drivers/clk/qcom/gcc-ipq5018.c
+++ b/drivers/clk/qcom/gcc-ipq5018.c
@@ -3660,7 +3660,7 @@ static const struct qcom_reset_map gcc_ipq5018_resets[] = {
 	[GCC_WCSS_AXI_S_ARES] = { 0x59008, 6 },
 	[GCC_WCSS_Q6_BCR] = { 0x18004, 0 },
 	[GCC_WCSSAON_RESET] = { 0x59010, 0},
-	[GCC_GEPHY_MISC_ARES] = { 0x56004, 0 },
+	[GCC_GEPHY_MISC_ARES] = { 0x56004, .bitmask = GENMASK(3, 0) },
 };
 
 static const struct of_device_id gcc_ipq5018_match_table[] = {

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support
  2025-06-02  9:53 [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 1/5] clk: qcom: gcc-ipq5018: fix GE PHY reset George Moussalem via B4 Relay
@ 2025-06-02  9:53 ` George Moussalem via B4 Relay
  2025-06-05 18:14   ` Rob Herring
  2025-06-02  9:53 ` [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal " George Moussalem via B4 Relay
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-02  9:53 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Philipp Zabel, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk,
	George Moussalem

From: George Moussalem <george.moussalem@outlook.com>

Document the IPQ5018 Internal Gigabit Ethernet PHY found in the IPQ5018
SoC. Its output pins provide an MDI interface to either an external
switch in a PHY to PHY link scenario or is directly attached to an RJ45
connector.

The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
802.3az EEE.

For operation, the LDO controller found in the IPQ5018 SoC for which
there is provision in the mdio-4019 driver.

Two common archictures across IPQ5018 boards are:
1. IPQ5018 PHY --> MDI --> RJ45 connector
2. IPQ5018 PHY --> MDI --> External PHY
In a phy to phy architecture, the DAC needs to be configured to
accommodate for the short cable length. As such, add an optional boolean
property so the driver sets preset DAC register values accordingly.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 .../devicetree/bindings/net/qca,ar803x.yaml        | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
index 3acd09f0da863137f8a05e435a1fd28a536c2acd..fce167412896edbf49371129e3e7e87312eee051 100644
--- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
+++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
@@ -16,8 +16,32 @@ description: |
 
 allOf:
   - $ref: ethernet-phy.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - ethernet-phy-id004d.d0c0
+
+    then:
+      properties:
+        reg:
+          const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC
+        resets:
+          items:
+            - description:
+                GE PHY MISC reset which triggers a reset across MDC, DSP, RX, and TX lines.
+        qcom,dac-preset-short-cable:
+          description:
+            Set if this phy is connected to another phy to adjust the values for
+            MDAC and EDAC to adjust amplitude, bias current settings, and error
+            detection and correction algorithm to accommodate for short cable length.
+            If not set, it is assumed the MDI output pins of this PHY are directly
+            connected to an RJ45 connector and default DAC values will be used.
+          type: boolean
 
 properties:
+
   qca,clk-out-frequency:
     description: Clock output frequency in Hertz.
     $ref: /schemas/types.yaml#/definitions/uint32
@@ -132,3 +156,18 @@ examples:
             };
         };
     };
+  - |
+    #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* add alias to set qcom,dac-preset-short-cable on boards that need it */
+        ge_phy: ethernet-phy@7 {
+            compatible = "ethernet-phy-id004d.d0c0";
+            reg = <7>;
+
+            resets = <&gcc GCC_GEPHY_MISC_ARES>;
+        };
+    };

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-02  9:53 [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 1/5] clk: qcom: gcc-ipq5018: fix GE PHY reset George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support George Moussalem via B4 Relay
@ 2025-06-02  9:53 ` George Moussalem via B4 Relay
  2025-06-02 12:41   ` Andrew Lunn
  2025-06-16 11:01   ` Philipp Zabel
  2025-06-02  9:53 ` [PATCH v3 4/5] arm64: dts: qcom: ipq5018: Add MDIO buses George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 5/5] arm64: dts: qcom: ipq5018: Add GE PHY to internal mdio bus George Moussalem via B4 Relay
  4 siblings, 2 replies; 16+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-02  9:53 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Philipp Zabel, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk,
	George Moussalem

From: George Moussalem <george.moussalem@outlook.com>

The IPQ5018 SoC contains a single internal Gigabit Ethernet PHY which
provides an MDI interface directly to an RJ45 connector or an external
switch over a PHY to PHY link.

The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
802.3az EEE.

Let's add support for this PHY in the at803x driver as it falls within
the Qualcomm Atheros OUI.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 drivers/net/phy/qcom/Kconfig  |   2 +-
 drivers/net/phy/qcom/at803x.c | 185 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 178 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/qcom/Kconfig b/drivers/net/phy/qcom/Kconfig
index 570626cc8e14d3e6615f74a6377f0f7c9f723e89..84239e08a8dfa466b0a7b2a5ec724a168b692cd2 100644
--- a/drivers/net/phy/qcom/Kconfig
+++ b/drivers/net/phy/qcom/Kconfig
@@ -7,7 +7,7 @@ config AT803X_PHY
 	select QCOM_NET_PHYLIB
 	depends on REGULATOR
 	help
-	  Currently supports the AR8030, AR8031, AR8033, AR8035 model
+	  Currently supports the AR8030, AR8031, AR8033, AR8035, IPQ5018 model
 
 config QCA83XX_PHY
 	tristate "Qualcomm Atheros QCA833x PHYs"
diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
index 26350b962890b0321153d74758b13d817407d094..c148e245b5391c5da374ace8609dcdfd8284732d 100644
--- a/drivers/net/phy/qcom/at803x.c
+++ b/drivers/net/phy/qcom/at803x.c
@@ -7,19 +7,24 @@
  * Author: Matus Ujhelyi <ujhelyi.m@gmail.com>
  */
 
-#include <linux/phy.h>
-#include <linux/module.h>
-#include <linux/string.h>
-#include <linux/netdevice.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool_netlink.h>
-#include <linux/bitfield.h>
-#include <linux/regulator/of_regulator.h>
-#include <linux/regulator/driver.h>
-#include <linux/regulator/consumer.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
 #include <linux/of.h>
+#include <linux/phy.h>
 #include <linux/phylink.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/reset.h>
 #include <linux/sfp.h>
+#include <linux/string.h>
 #include <dt-bindings/net/qca-ar803x.h>
 
 #include "qcom.h"
@@ -96,6 +101,8 @@
 #define ATH8035_PHY_ID				0x004dd072
 #define AT8030_PHY_ID_MASK			0xffffffef
 
+#define IPQ5018_PHY_ID				0x004dd0c0
+
 #define QCA9561_PHY_ID				0x004dd042
 
 #define AT803X_PAGE_FIBER			0
@@ -108,6 +115,50 @@
 /* disable hibernation mode */
 #define AT803X_DISABLE_HIBERNATION_MODE		BIT(2)
 
+#define IPQ5018_PHY_FIFO_CONTROL		0x19
+#define IPQ5018_PHY_FIFO_RESET			GENMASK(1, 0)
+
+#define IPQ5018_PHY_DEBUG_EDAC			0x4380
+#define IPQ5018_PHY_MMD1_MDAC			0x8100
+#define IPQ5018_PHY_DAC_MASK			GENMASK(15, 8)
+
+/* MDAC and EDAC values for short cable length */
+#define IPQ5018_PHY_DEBUG_EDAC_VAL		0x10
+#define IPQ5018_PHY_MMD1_MDAC_VAL		0x10
+
+#define IPQ5018_PHY_MMD1_MSE_THRESH1		0x1000
+#define IPQ5018_PHY_MMD1_MSE_THRESH2		0x1001
+#define IPQ5018_PHY_PCS_AZ_CTRL1		0x8008
+#define IPQ5018_PHY_PCS_AZ_CTRL2		0x8009
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL3	0x8074
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL4	0x8075
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL5	0x8076
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL6	0x8077
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL7	0x8078
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL9	0x807a
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL13	0x807e
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL14	0x807f
+
+#define IPQ5018_PHY_MMD1_MSE_THRESH1_VAL	0xf1
+#define IPQ5018_PHY_MMD1_MSE_THRESH2_VAL	0x1f6
+#define IPQ5018_PHY_PCS_AZ_CTRL1_VAL		0x7880
+#define IPQ5018_PHY_PCS_AZ_CTRL2_VAL		0xc8
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL3_VAL	0xc040
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL4_VAL	0xa060
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL5_VAL	0xc040
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL6_VAL	0xa060
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL7_VAL	0xc24c
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL9_VAL	0xc060
+#define IPQ5018_PHY_PCS_CDT_THRESH_CTRL13_VAL	0xb060
+#define IPQ5018_PHY_PCS_NEAR_ECHO_THRESH_VAL	0x90b0
+
+#define IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE		0x1
+#define IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_MASK	GENMASK(7, 4)
+#define IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_DEFAULT	0x50
+#define IPQ5018_PHY_DEBUG_ANA_DAC_FILTER	0xa080
+
+#define IPQ5018_TCSR_ETH_LDO_READY		BIT(0)
+
 MODULE_DESCRIPTION("Qualcomm Atheros AR803x PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
@@ -133,6 +184,11 @@ struct at803x_context {
 	u16 led_control;
 };
 
+struct ipq5018_priv {
+	struct reset_control *rst;
+	bool set_short_cable_dac;
+};
+
 static int at803x_write_page(struct phy_device *phydev, int page)
 {
 	int mask;
@@ -987,6 +1043,105 @@ static int at8035_probe(struct phy_device *phydev)
 	return at8035_parse_dt(phydev);
 }
 
+static int ipq5018_cable_test_start(struct phy_device *phydev)
+{
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL3,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL3_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL4,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL4_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL5,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL5_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL6,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL6_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL7,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL7_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL9,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL9_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL13,
+		      IPQ5018_PHY_PCS_CDT_THRESH_CTRL13_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_CDT_THRESH_CTRL3,
+		      IPQ5018_PHY_PCS_NEAR_ECHO_THRESH_VAL);
+
+	/* we do all the (time consuming) work later */
+	return 0;
+}
+
+static int ipq5018_config_init(struct phy_device *phydev)
+{
+	struct ipq5018_priv *priv = phydev->priv;
+	u16 val = 0;
+
+	/*
+	 * set LDO efuse: first temporarily store ANA_DAC_FILTER value from
+	 * debug register as it will be reset once the ANA_LDO_EFUSE register
+	 * is written to
+	 */
+	val = at803x_debug_reg_read(phydev, IPQ5018_PHY_DEBUG_ANA_DAC_FILTER);
+	at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE,
+			      IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_MASK,
+			      IPQ5018_PHY_DEBUG_ANA_LDO_EFUSE_DEFAULT);
+	at803x_debug_reg_write(phydev, IPQ5018_PHY_DEBUG_ANA_DAC_FILTER, val);
+
+	/* set 8023AZ CTRL values */
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_AZ_CTRL1,
+		      IPQ5018_PHY_PCS_AZ_CTRL1_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PCS, IPQ5018_PHY_PCS_AZ_CTRL2,
+		      IPQ5018_PHY_PCS_AZ_CTRL2_VAL);
+
+	/* set MSE threshold values */
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MSE_THRESH1,
+		      IPQ5018_PHY_MMD1_MSE_THRESH1_VAL);
+	phy_write_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MSE_THRESH2,
+		      IPQ5018_PHY_MMD1_MSE_THRESH2_VAL);
+
+	/* PHY DAC values are optional and only set in a PHY to PHY link architecture */
+	if (priv->set_short_cable_dac) {
+		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
+		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
+			       IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_MMD1_MDAC_VAL);
+
+		/* setting EDAC (Error-detection and Correction) in debug register */
+		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
+				      IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_DEBUG_EDAC_VAL);
+	}
+
+	return 0;
+}
+
+static void ipq5018_link_change_notify(struct phy_device *phydev)
+{
+	mdiobus_modify_changed(phydev->mdio.bus, phydev->mdio.addr,
+			       IPQ5018_PHY_FIFO_CONTROL, IPQ5018_PHY_FIFO_RESET,
+			       phydev->link ? IPQ5018_PHY_FIFO_RESET : 0);
+}
+
+static int ipq5018_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct ipq5018_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->set_short_cable_dac = of_property_read_bool(dev->of_node,
+							  "qcom,dac-preset-short-cable");
+
+	priv->rst = devm_reset_control_array_get_exclusive(dev);
+	if (IS_ERR_OR_NULL(priv->rst))
+		return dev_err_probe(dev, PTR_ERR(priv->rst),
+				     "failed to acquire reset\n");
+
+	ret = reset_control_reset(priv->rst);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to reset\n");
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
 static struct phy_driver at803x_driver[] = {
 {
 	/* Qualcomm Atheros AR8035 */
@@ -1078,6 +1233,19 @@ static struct phy_driver at803x_driver[] = {
 	.read_status		= at803x_read_status,
 	.soft_reset		= genphy_soft_reset,
 	.config_aneg		= at803x_config_aneg,
+}, {
+	PHY_ID_MATCH_EXACT(IPQ5018_PHY_ID),
+	.name			= "Qualcomm Atheros IPQ5018 internal PHY",
+	.flags			= PHY_IS_INTERNAL | PHY_POLL_CABLE_TEST,
+	.probe			= ipq5018_probe,
+	.config_init		= ipq5018_config_init,
+	.link_change_notify	= ipq5018_link_change_notify,
+	.read_status		= at803x_read_status,
+	.config_intr		= at803x_config_intr,
+	.handle_interrupt	= at803x_handle_interrupt,
+	.cable_test_start	= ipq5018_cable_test_start,
+	.cable_test_get_status	= qca808x_cable_test_get_status,
+	.soft_reset		= genphy_soft_reset,
 }, {
 	/* Qualcomm Atheros QCA9561 */
 	PHY_ID_MATCH_EXACT(QCA9561_PHY_ID),
@@ -1104,6 +1272,7 @@ static const struct mdio_device_id __maybe_unused atheros_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(ATH8032_PHY_ID) },
 	{ PHY_ID_MATCH_EXACT(ATH8035_PHY_ID) },
 	{ PHY_ID_MATCH_EXACT(ATH9331_PHY_ID) },
+	{ PHY_ID_MATCH_EXACT(IPQ5018_PHY_ID) },
 	{ PHY_ID_MATCH_EXACT(QCA9561_PHY_ID) },
 	{ }
 };

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/5] arm64: dts: qcom: ipq5018: Add MDIO buses
  2025-06-02  9:53 [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY George Moussalem via B4 Relay
                   ` (2 preceding siblings ...)
  2025-06-02  9:53 ` [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal " George Moussalem via B4 Relay
@ 2025-06-02  9:53 ` George Moussalem via B4 Relay
  2025-06-02  9:53 ` [PATCH v3 5/5] arm64: dts: qcom: ipq5018: Add GE PHY to internal mdio bus George Moussalem via B4 Relay
  4 siblings, 0 replies; 16+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-02  9:53 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Philipp Zabel, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk,
	George Moussalem, Konrad Dybcio

From: George Moussalem <george.moussalem@outlook.com>

IPQ5018 contains two mdio buses of which one bus is used to control the
SoC's internal GE PHY, while the other bus is connected to external PHYs
or switches.

There's already support for IPQ5018 in the mdio-ipq4019 driver, so let's
simply add the mdio nodes for them.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 arch/arm64/boot/dts/qcom/ipq5018.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 130360014c5e14c778e348d37e601f60325b0b14..03ebc3e305b267c98a034c41ce47a39269afce75 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -182,6 +182,30 @@ pcie0_phy: phy@86000 {
 			status = "disabled";
 		};
 
+		mdio0: mdio@88000 {
+			compatible = "qcom,ipq5018-mdio";
+			reg = <0x00088000 0x64>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			clocks = <&gcc GCC_MDIO0_AHB_CLK>;
+			clock-names = "gcc_mdio_ahb_clk";
+
+			status = "disabled";
+		};
+
+		mdio1: mdio@90000 {
+			compatible = "qcom,ipq5018-mdio";
+			reg = <0x00090000 0x64>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			clocks = <&gcc GCC_MDIO1_AHB_CLK>;
+			clock-names = "gcc_mdio_ahb_clk";
+
+			status = "disabled";
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq5018-tlmm";
 			reg = <0x01000000 0x300000>;

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/5] arm64: dts: qcom: ipq5018: Add GE PHY to internal mdio bus
  2025-06-02  9:53 [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY George Moussalem via B4 Relay
                   ` (3 preceding siblings ...)
  2025-06-02  9:53 ` [PATCH v3 4/5] arm64: dts: qcom: ipq5018: Add MDIO buses George Moussalem via B4 Relay
@ 2025-06-02  9:53 ` George Moussalem via B4 Relay
  4 siblings, 0 replies; 16+ messages in thread
From: George Moussalem via B4 Relay @ 2025-06-02  9:53 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Philipp Zabel, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk,
	George Moussalem, Konrad Dybcio

From: George Moussalem <george.moussalem@outlook.com>

The IPQ5018 SoC contains an internal GE PHY, always at phy address 7.
As such, let's add the GE PHY node to the SoC dtsi.

The LDO controller found in the SoC must be enabled to provide constant
low voltages to the PHY. The mdio-ipq4019 driver already has support
for this, so adding the appropriate TCSR register offset.

In addition, the GE PHY outputs both the RX and TX clocks to the GCC
which gate controls them and routes them back to the PHY itself.
So let's create two DT fixed clocks and register them in the GCC node.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 arch/arm64/boot/dts/qcom/ipq5018.dtsi | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 03ebc3e305b267c98a034c41ce47a39269afce75..d47ad62b01991fafa51e7082bd1fcf6670d9b0bc 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -16,6 +16,18 @@ / {
 	#size-cells = <2>;
 
 	clocks {
+		gephy_rx_clk: gephy-rx-clk {
+			compatible = "fixed-clock";
+			clock-frequency = <125000000>;
+			#clock-cells = <0>;
+		};
+
+		gephy_tx_clk: gephy-tx-clk {
+			compatible = "fixed-clock";
+			clock-frequency = <125000000>;
+			#clock-cells = <0>;
+		};
+
 		sleep_clk: sleep-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
@@ -184,7 +196,8 @@ pcie0_phy: phy@86000 {
 
 		mdio0: mdio@88000 {
 			compatible = "qcom,ipq5018-mdio";
-			reg = <0x00088000 0x64>;
+			reg = <0x00088000 0x64>,
+			      <0x019475c4 0x4>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 
@@ -192,6 +205,13 @@ mdio0: mdio@88000 {
 			clock-names = "gcc_mdio_ahb_clk";
 
 			status = "disabled";
+
+			ge_phy: ethernet-phy@7 {
+				compatible = "ethernet-phy-id004d.d0c0";
+				reg = <7>;
+
+				resets = <&gcc GCC_GEPHY_MISC_ARES>;
+			};
 		};
 
 		mdio1: mdio@90000 {
@@ -232,8 +252,8 @@ gcc: clock-controller@1800000 {
 				 <&pcie0_phy>,
 				 <&pcie1_phy>,
 				 <0>,
-				 <0>,
-				 <0>,
+				 <&gephy_rx_clk>,
+				 <&gephy_tx_clk>,
 				 <0>,
 				 <0>;
 			#clock-cells = <1>;

-- 
2.49.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-02  9:53 ` [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal " George Moussalem via B4 Relay
@ 2025-06-02 12:41   ` Andrew Lunn
  2025-06-02 13:02     ` George Moussalem
  2025-06-05 18:18     ` Rob Herring
  2025-06-16 11:01   ` Philipp Zabel
  1 sibling, 2 replies; 16+ messages in thread
From: Andrew Lunn @ 2025-06-02 12:41 UTC (permalink / raw)
  To: george.moussalem
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

> +	/* PHY DAC values are optional and only set in a PHY to PHY link architecture */
> +	if (priv->set_short_cable_dac) {
> +		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
> +		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
> +			       IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_MMD1_MDAC_VAL);
> +
> +		/* setting EDAC (Error-detection and Correction) in debug register */
> +		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
> +				      IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_DEBUG_EDAC_VAL);
> +	}

In the binding you say:


+            If not set, it is assumed the MDI output pins of this PHY are directly
+            connected to an RJ45 connector and default DAC values will be used.

So shouldn't there be an else clause here setting these two values to
their default, undoing what the bootloader might of done etc.

Or you can change the binding, and say something like:

+            If not set, DAC values are not modified.

We often need a tristate in DT, set something, unset something, leave
it as it is. But that does not exist in DT in an easy form :-(

	Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-02 12:41   ` Andrew Lunn
@ 2025-06-02 13:02     ` George Moussalem
  2025-06-02 13:49       ` Andrew Lunn
  2025-06-05 18:18     ` Rob Herring
  1 sibling, 1 reply; 16+ messages in thread
From: George Moussalem @ 2025-06-02 13:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Hi Andrew,

On 6/2/25 16:41, Andrew Lunn wrote:
>> +	/* PHY DAC values are optional and only set in a PHY to PHY link architecture */
>> +	if (priv->set_short_cable_dac) {
>> +		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
>> +		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
>> +			       IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_MMD1_MDAC_VAL);
>> +
>> +		/* setting EDAC (Error-detection and Correction) in debug register */
>> +		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
>> +				      IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_DEBUG_EDAC_VAL);
>> +	}
> 
> In the binding you say:
> 
> 
> +            If not set, it is assumed the MDI output pins of this PHY are directly
> +            connected to an RJ45 connector and default DAC values will be used.
> 
> So shouldn't there be an else clause here setting these two values to
> their default, undoing what the bootloader might of done etc.

DAC values are only set if the property is set in the DTS. If the 
property is not set, the default values set by the PHY itself are used, 
and as you mentioned below, DAC values aren't modified by the driver.

> 
> Or you can change the binding, and say something like:
> 
> +            If not set, DAC values are not modified.

sure, anything else you need changed for v4?

> 
> We often need a tristate in DT, set something, unset something, leave
> it as it is. But that does not exist in DT in an easy form :-(

> 
> 	Andrew

Best regards,
George

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-02 13:02     ` George Moussalem
@ 2025-06-02 13:49       ` Andrew Lunn
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2025-06-02 13:49 UTC (permalink / raw)
  To: George Moussalem
  Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

> > So shouldn't there be an else clause here setting these two values to
> > their default, undoing what the bootloader might of done etc.
> 
> DAC values are only set if the property is set in the DTS. If the property
> is not set, the default values set by the PHY itself are used, and as you
> mentioned below, DAC values aren't modified by the driver.
> 
> > 
> > Or you can change the binding, and say something like:
> > 
> > +            If not set, DAC values are not modified.
> 
> sure, anything else you need changed for v4?

We have seen cases where the bootloader does the wrong thing, e.g. in
this case, hard coded for short cable. A DT developer than looks at
the binding, sees that the defaults should be used, and are confused.
By accurately wording the binding, that the values are left untouched,
it gives the DT developer a hint where to look, at the bootloader.

So in general, i tend to be picky about what does it mean if the
property is not present, because its a detail which is often
overlooked.

I did not notice anything else, but net-next is closed at the moment
for the merge window, so you need to wait a while before submitting it
for merging.

	Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support
  2025-06-02  9:53 ` [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support George Moussalem via B4 Relay
@ 2025-06-05 18:14   ` Rob Herring
  2025-06-06 10:54     ` George Moussalem
  0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2025-06-05 18:14 UTC (permalink / raw)
  To: George Moussalem
  Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

On Mon, Jun 02, 2025 at 01:53:14PM +0400, George Moussalem wrote:
> Document the IPQ5018 Internal Gigabit Ethernet PHY found in the IPQ5018
> SoC. Its output pins provide an MDI interface to either an external
> switch in a PHY to PHY link scenario or is directly attached to an RJ45
> connector.
> 
> The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
> 802.3az EEE.
> 
> For operation, the LDO controller found in the IPQ5018 SoC for which
> there is provision in the mdio-4019 driver.
> 
> Two common archictures across IPQ5018 boards are:
> 1. IPQ5018 PHY --> MDI --> RJ45 connector
> 2. IPQ5018 PHY --> MDI --> External PHY
> In a phy to phy architecture, the DAC needs to be configured to
> accommodate for the short cable length. As such, add an optional boolean
> property so the driver sets preset DAC register values accordingly.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>  .../devicetree/bindings/net/qca,ar803x.yaml        | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> index 3acd09f0da863137f8a05e435a1fd28a536c2acd..fce167412896edbf49371129e3e7e87312eee051 100644
> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> @@ -16,8 +16,32 @@ description: |
>  
>  allOf:
>    - $ref: ethernet-phy.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ethernet-phy-id004d.d0c0
> +
> +    then:
> +      properties:
> +        reg:
> +          const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC

blank line

> +        resets:
> +          items:
> +            - description:
> +                GE PHY MISC reset which triggers a reset across MDC, DSP, RX, and TX lines.

blank line

> +        qcom,dac-preset-short-cable:
> +          description:
> +            Set if this phy is connected to another phy to adjust the values for
> +            MDAC and EDAC to adjust amplitude, bias current settings, and error
> +            detection and correction algorithm to accommodate for short cable length.
> +            If not set, it is assumed the MDI output pins of this PHY are directly
> +            connected to an RJ45 connector and default DAC values will be used.
> +          type: boolean
>  
>  properties:
> +

Drop

But this schema is broken. There's no way for it to be applied to a node 
because there is no compatible defined in this schema nor a 'select'. 
You can introduce an error and see (e.g. 'qcom,dac-preset-short-cable = 
"foo";'). Really, any phy using these properties should have a specific 
compatible defined here.

>    qca,clk-out-frequency:
>      description: Clock output frequency in Hertz.
>      $ref: /schemas/types.yaml#/definitions/uint32
> @@ -132,3 +156,18 @@ examples:
>              };
>          };
>      };
> +  - |
> +    #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
> +
> +    mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        /* add alias to set qcom,dac-preset-short-cable on boards that need it */
> +        ge_phy: ethernet-phy@7 {
> +            compatible = "ethernet-phy-id004d.d0c0";
> +            reg = <7>;
> +
> +            resets = <&gcc GCC_GEPHY_MISC_ARES>;
> +        };
> +    };
> 
> -- 
> 2.49.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-02 12:41   ` Andrew Lunn
  2025-06-02 13:02     ` George Moussalem
@ 2025-06-05 18:18     ` Rob Herring
  1 sibling, 0 replies; 16+ messages in thread
From: Rob Herring @ 2025-06-05 18:18 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: george.moussalem, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

On Mon, Jun 02, 2025 at 02:41:56PM +0200, Andrew Lunn wrote:
> > +	/* PHY DAC values are optional and only set in a PHY to PHY link architecture */
> > +	if (priv->set_short_cable_dac) {
> > +		/* setting MDAC (Multi-level Digital-to-Analog Converter) in MMD1 */
> > +		phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, IPQ5018_PHY_MMD1_MDAC,
> > +			       IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_MMD1_MDAC_VAL);
> > +
> > +		/* setting EDAC (Error-detection and Correction) in debug register */
> > +		at803x_debug_reg_mask(phydev, IPQ5018_PHY_DEBUG_EDAC,
> > +				      IPQ5018_PHY_DAC_MASK, IPQ5018_PHY_DEBUG_EDAC_VAL);
> > +	}
> 
> In the binding you say:
> 
> 
> +            If not set, it is assumed the MDI output pins of this PHY are directly
> +            connected to an RJ45 connector and default DAC values will be used.
> 
> So shouldn't there be an else clause here setting these two values to
> their default, undoing what the bootloader might of done etc.
> 
> Or you can change the binding, and say something like:
> 
> +            If not set, DAC values are not modified.
> 
> We often need a tristate in DT, set something, unset something, leave
> it as it is. But that does not exist in DT in an easy form :-(

I'm happy to define that and have thought about it. That does up the 
minimum version of dtschema required, but we pretty much expect people 
to use the latest anyways.

Rob

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support
  2025-06-05 18:14   ` Rob Herring
@ 2025-06-06 10:54     ` George Moussalem
  2025-06-06 13:01       ` Andrew Lunn
  0 siblings, 1 reply; 16+ messages in thread
From: George Moussalem @ 2025-06-06 10:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

Hi Rob,

On 6/5/25 22:14, Rob Herring wrote:
> On Mon, Jun 02, 2025 at 01:53:14PM +0400, George Moussalem wrote:
>> Document the IPQ5018 Internal Gigabit Ethernet PHY found in the IPQ5018
>> SoC. Its output pins provide an MDI interface to either an external
>> switch in a PHY to PHY link scenario or is directly attached to an RJ45
>> connector.
>>
>> The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
>> 802.3az EEE.
>>
>> For operation, the LDO controller found in the IPQ5018 SoC for which
>> there is provision in the mdio-4019 driver.
>>
>> Two common archictures across IPQ5018 boards are:
>> 1. IPQ5018 PHY --> MDI --> RJ45 connector
>> 2. IPQ5018 PHY --> MDI --> External PHY
>> In a phy to phy architecture, the DAC needs to be configured to
>> accommodate for the short cable length. As such, add an optional boolean
>> property so the driver sets preset DAC register values accordingly.
>>
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>> ---
>>   .../devicetree/bindings/net/qca,ar803x.yaml        | 39 ++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
>> index 3acd09f0da863137f8a05e435a1fd28a536c2acd..fce167412896edbf49371129e3e7e87312eee051 100644
>> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
>> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
>> @@ -16,8 +16,32 @@ description: |
>>   
>>   allOf:
>>     - $ref: ethernet-phy.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - ethernet-phy-id004d.d0c0
>> +
>> +    then:
>> +      properties:
>> +        reg:
>> +          const: 7  # This PHY is always at MDIO address 7 in the IPQ5018 SoC
> 
> blank line

added newline

> 
>> +        resets:
>> +          items:
>> +            - description:
>> +                GE PHY MISC reset which triggers a reset across MDC, DSP, RX, and TX lines.
> 
> blank line

added newline
> 
>> +        qcom,dac-preset-short-cable:
>> +          description:
>> +            Set if this phy is connected to another phy to adjust the values for
>> +            MDAC and EDAC to adjust amplitude, bias current settings, and error
>> +            detection and correction algorithm to accommodate for short cable length.
>> +            If not set, it is assumed the MDI output pins of this PHY are directly
>> +            connected to an RJ45 connector and default DAC values will be used.
>> +          type: boolean
>>   
>>   properties:
>> +
> 
> Drop

removed

> 
> But this schema is broken. There's no way for it to be applied to a node
> because there is no compatible defined in this schema nor a 'select'.
> You can introduce an error and see (e.g. 'qcom,dac-preset-short-cable =
> "foo";'). Really, any phy using these properties should have a specific
> compatible defined here.

added PHY ID as compatible in v4 which I'll send out once the merge 
window reopens.

Under 'properties' node:
   compatible:
     enum:
       - ethernet-phy-id004d.d0c0

Q: do I need to add the PHY IDs of all PHYs that the qca803x driver 
covers or will this one suffice?

> 
>>     qca,clk-out-frequency:
>>       description: Clock output frequency in Hertz.
>>       $ref: /schemas/types.yaml#/definitions/uint32
>> @@ -132,3 +156,18 @@ examples:
>>               };
>>           };
>>       };
>> +  - |
>> +    #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
>> +
>> +    mdio {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        /* add alias to set qcom,dac-preset-short-cable on boards that need it */
>> +        ge_phy: ethernet-phy@7 {
>> +            compatible = "ethernet-phy-id004d.d0c0";
>> +            reg = <7>;
>> +
>> +            resets = <&gcc GCC_GEPHY_MISC_ARES>;
>> +        };
>> +    };
>>
>> -- 
>> 2.49.0
>>
> 

Best regards,
George


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support
  2025-06-06 10:54     ` George Moussalem
@ 2025-06-06 13:01       ` Andrew Lunn
  2025-06-07  7:32         ` Russell King (Oracle)
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2025-06-06 13:01 UTC (permalink / raw)
  To: George Moussalem
  Cc: Rob Herring, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

> Under 'properties' node:
>   compatible:
>     enum:
>       - ethernet-phy-id004d.d0c0
> 
> Q: do I need to add the PHY IDs of all PHYs that the qca803x driver covers
> or will this one suffice?

The history is complicated, because PHYs can be enumerated, which
makes compatible mean something different to the usual for devices
which cannot be enumerated.

I personally would search the in tree DT files and find any which list
a qca803x compatible, and add those.

	Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support
  2025-06-06 13:01       ` Andrew Lunn
@ 2025-06-07  7:32         ` Russell King (Oracle)
  0 siblings, 0 replies; 16+ messages in thread
From: Russell King (Oracle) @ 2025-06-07  7:32 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: George Moussalem, Rob Herring, Heiner Kallweit, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Krzysztof Kozlowski,
	Conor Dooley, Florian Fainelli, Philipp Zabel, Bjorn Andersson,
	Konrad Dybcio, Michael Turquette, Stephen Boyd, netdev,
	devicetree, linux-kernel, linux-arm-msm, linux-clk

On Fri, Jun 06, 2025 at 03:01:26PM +0200, Andrew Lunn wrote:
> > Under 'properties' node:
> >   compatible:
> >     enum:
> >       - ethernet-phy-id004d.d0c0
> > 
> > Q: do I need to add the PHY IDs of all PHYs that the qca803x driver covers
> > or will this one suffice?
> 
> The history is complicated, because PHYs can be enumerated

... provided one doesn't wire up the reset pin to a GPIO and then
declare that in DT as a reset pin for the PHY, thus holding the PHY
in reset while we try to probe what's on the bus, making the ID
unreadable.

The down-side to providing the ID in the compatible is we lose the
revision, so if a new revision of the PHY ends up being fitted
part way through production, the kernel has no way to know.

Sadly, we can't just read the PHY ID when we've released reset
because the ID may be provided in DT because the one in the device
is not reliable / wrong.

What's done on SolidRun platforms is that the PHY reset is connected
to a GPIO, but that is controlled by the boot loader and not by the
kernel. All PHY resets are deasserted before the kernel is entered,
and the reset GPIOs are in DT as "hogged" GPIOs. This allows phylib
to operate normally without any of this faff.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-02  9:53 ` [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal " George Moussalem via B4 Relay
  2025-06-02 12:41   ` Andrew Lunn
@ 2025-06-16 11:01   ` Philipp Zabel
  2025-06-16 12:38     ` George Moussalem
  1 sibling, 1 reply; 16+ messages in thread
From: Philipp Zabel @ 2025-06-16 11:01 UTC (permalink / raw)
  To: george.moussalem, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk

On Mo, 2025-06-02 at 13:53 +0400, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> The IPQ5018 SoC contains a single internal Gigabit Ethernet PHY which
> provides an MDI interface directly to an RJ45 connector or an external
> switch over a PHY to PHY link.
> 
> The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
> 802.3az EEE.
> 
> Let's add support for this PHY in the at803x driver as it falls within
> the Qualcomm Atheros OUI.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
>  drivers/net/phy/qcom/Kconfig  |   2 +-
>  drivers/net/phy/qcom/at803x.c | 185 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 178 insertions(+), 9 deletions(-)
> 
[...]
> diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
> index 26350b962890b0321153d74758b13d817407d094..c148e245b5391c5da374ace8609dcdfd8284732d 100644
> --- a/drivers/net/phy/qcom/at803x.c
> +++ b/drivers/net/phy/qcom/at803x.c
> @@ -7,19 +7,24 @@
[...]
> +static int ipq5018_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct ipq5018_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->set_short_cable_dac = of_property_read_bool(dev->of_node,
> +							  "qcom,dac-preset-short-cable");
> +
> +	priv->rst = devm_reset_control_array_get_exclusive(dev);

Both dt-bindings and dts patch only show a single reset. Is there a
reason this is a reset_control_array?

regards
Philipp

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal PHY support
  2025-06-16 11:01   ` Philipp Zabel
@ 2025-06-16 12:38     ` George Moussalem
  0 siblings, 0 replies; 16+ messages in thread
From: George Moussalem @ 2025-06-16 12:38 UTC (permalink / raw)
  To: Philipp Zabel, Andrew Lunn, Heiner Kallweit, Russell King,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
	Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd
  Cc: netdev, devicetree, linux-kernel, linux-arm-msm, linux-clk

Hi Philipp,

On 6/16/25 15:01, Philipp Zabel wrote:
> On Mo, 2025-06-02 at 13:53 +0400, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@outlook.com>
>>
>> The IPQ5018 SoC contains a single internal Gigabit Ethernet PHY which
>> provides an MDI interface directly to an RJ45 connector or an external
>> switch over a PHY to PHY link.
>>
>> The PHY supports 10/100/1000 mbps link modes, CDT, auto-negotiation and
>> 802.3az EEE.
>>
>> Let's add support for this PHY in the at803x driver as it falls within
>> the Qualcomm Atheros OUI.
>>
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>> ---
>>  drivers/net/phy/qcom/Kconfig  |   2 +-
>>  drivers/net/phy/qcom/at803x.c | 185 ++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 178 insertions(+), 9 deletions(-)
>>
> [...]
>> diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
>> index 26350b962890b0321153d74758b13d817407d094..c148e245b5391c5da374ace8609dcdfd8284732d 100644
>> --- a/drivers/net/phy/qcom/at803x.c
>> +++ b/drivers/net/phy/qcom/at803x.c
>> @@ -7,19 +7,24 @@
> [...]
>> +static int ipq5018_probe(struct phy_device *phydev)
>> +{
>> +	struct device *dev = &phydev->mdio.dev;
>> +	struct ipq5018_priv *priv;
>> +	int ret;
>> +
>> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->set_short_cable_dac = of_property_read_bool(dev->of_node,
>> +							  "qcom,dac-preset-short-cable");
>> +
>> +	priv->rst = devm_reset_control_array_get_exclusive(dev);
> 
> Both dt-bindings and dts patch only show a single reset. Is there a
> reason this is a reset_control_array?

The series started with multiple resets, but due to patch 1 (using a
bitmask to trigger multiple resets) and the restriction of max 1 reset
in ethernet-phy.yaml, there's no need for calling the array function
anymore. Need me to change to devm_reset_control_get_exclusive?

If so, don't mind me asking, do I send 2 patch sets? (complete set of 5
and a separate set of 2 to net-next as requested by Yakub)? Or just the
entire patch set to net-next?

> 
> regards
> Philipp

Best regards,
George


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-06-16 12:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-02  9:53 [PATCH v3 0/5] Add support for the IPQ5018 Internal GE PHY George Moussalem via B4 Relay
2025-06-02  9:53 ` [PATCH v3 1/5] clk: qcom: gcc-ipq5018: fix GE PHY reset George Moussalem via B4 Relay
2025-06-02  9:53 ` [PATCH v3 2/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support George Moussalem via B4 Relay
2025-06-05 18:14   ` Rob Herring
2025-06-06 10:54     ` George Moussalem
2025-06-06 13:01       ` Andrew Lunn
2025-06-07  7:32         ` Russell King (Oracle)
2025-06-02  9:53 ` [PATCH v3 3/5] net: phy: qcom: at803x: Add Qualcomm IPQ5018 Internal " George Moussalem via B4 Relay
2025-06-02 12:41   ` Andrew Lunn
2025-06-02 13:02     ` George Moussalem
2025-06-02 13:49       ` Andrew Lunn
2025-06-05 18:18     ` Rob Herring
2025-06-16 11:01   ` Philipp Zabel
2025-06-16 12:38     ` George Moussalem
2025-06-02  9:53 ` [PATCH v3 4/5] arm64: dts: qcom: ipq5018: Add MDIO buses George Moussalem via B4 Relay
2025-06-02  9:53 ` [PATCH v3 5/5] arm64: dts: qcom: ipq5018: Add GE PHY to internal mdio bus George Moussalem via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).