diff mbox series

[2/3] net: stmmac: qcom-ethqos: Enable RX programmable swap on qcs615

Message ID 20241225-support_10m100m-v1-2-4b52ef48b488@quicinc.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series Support tuning the RX sampling swap of the MAC. | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Yijie Yang Dec. 25, 2024, 10:04 a.m. UTC 
The sampling timing of the MAC varies per board due to differences in
hardware layout or peer PHY, even within the same version. For instance,
the EMAC version on the qcs615-ride board is below 3, but it requires
swapping the RX timing to enable 10M/100M speeds. Therefore, this setting
should be adjustable rather than solely determined by the version.
The RGMII_CONFIG2_RX_PROG_SWAP bit is used to switch the RX sampling
timing between the rising edge and the falling edge of the clock.
Consequently, the condition for setting this bit should be revised to
ensure correct data sampling.
The compatible string matching for 'qcom,sa8540p-ride' in this change is
intended to ensure ABI compatibility for DTS files that do not include the
new 'qcom,rx-prog-swap' property, allowing legacy boards to function as
before. This board is the only one among legacy boards using RGMII and
needs to enable RX swap.

Signed-off-by: Yijie Yang <quic_yijiyang@quicinc.com>
---
 .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c    | 36 ++++++++++++----------
 1 file changed, 20 insertions(+), 16 deletions(-)

Comments

Krzysztof Kozlowski Dec. 25, 2024, 11:37 a.m. UTC | #1 
On 25/12/2024 11:04, Yijie Yang wrote:

>  static int qcom_ethqos_probe(struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.of_node, *root;
>  	const struct ethqos_emac_driver_data *data;
>  	struct plat_stmmacenet_data *plat_dat;
>  	struct stmmac_resources stmmac_res;
> @@ -810,6 +805,15 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
>  	ret = of_get_phy_mode(np, &ethqos->phy_mode);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to get phy mode\n");
> +
> +	root = of_find_node_by_path("/");
> +	if (root && of_device_is_compatible(root, "qcom,sa8540p-ride"))


Nope, your drivers are not supposed to poke root compatibles. Drop and
fix your driver to behave correctly for all existing devices.

> +		ethqos->needs_rx_prog_swap = true;
> +	else
> +		ethqos->needs_rx_prog_swap =
Best regards,
Krzysztof
Yijie Yang Dec. 26, 2024, 2:29 a.m. UTC | #2 
On 2024-12-25 19:37, Krzysztof Kozlowski wrote:
> On 25/12/2024 11:04, Yijie Yang wrote:
> 
>>   static int qcom_ethqos_probe(struct platform_device *pdev)
>>   {
>> -	struct device_node *np = pdev->dev.of_node;
>> +	struct device_node *np = pdev->dev.of_node, *root;
>>   	const struct ethqos_emac_driver_data *data;
>>   	struct plat_stmmacenet_data *plat_dat;
>>   	struct stmmac_resources stmmac_res;
>> @@ -810,6 +805,15 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
>>   	ret = of_get_phy_mode(np, &ethqos->phy_mode);
>>   	if (ret)
>>   		return dev_err_probe(dev, ret, "Failed to get phy mode\n");
>> +
>> +	root = of_find_node_by_path("/");
>> +	if (root && of_device_is_compatible(root, "qcom,sa8540p-ride"))
> 
> 
> Nope, your drivers are not supposed to poke root compatibles. Drop and
> fix your driver to behave correctly for all existing devices.
> 

Since this change introduces a new flag in the DTS, we must maintain ABI 
compatibility with the kernel. The new flag is specific to the board, so 
I need to ensure root nodes are matched to allow older boards to 
continue functioning as before. I'm happy to adopt that approach if 
there are any more elegant solutions.

>> +		ethqos->needs_rx_prog_swap = true;
>> +	else
>> +		ethqos->needs_rx_prog_swap =
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 2a5b38723635b5ef9233ca4709e99dd5ddf06b77..cf9633489975bc89480d065ae723a92723a12b04 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -120,6 +120,7 @@  struct qcom_ethqos {
 	bool rgmii_config_loopback_en;
 	bool has_emac_ge_3;
 	bool needs_sgmii_loopback;
+	bool needs_rx_prog_swap;
 };
 
 static int rgmii_readl(struct qcom_ethqos *ethqos, unsigned int offset)
@@ -401,6 +402,7 @@  static int ethqos_dll_configure(struct qcom_ethqos *ethqos)
 static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
 {
 	struct device *dev = &ethqos->pdev->dev;
+	int rx_prog_swap = 0;
 	int phase_shift;
 	int loopback;
 
@@ -421,6 +423,9 @@  static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
 	else
 		loopback = 0;
 
+	if (ethqos->needs_rx_prog_swap)
+		rx_prog_swap = RGMII_CONFIG2_RX_PROG_SWAP;
+
 	/* Select RGMII, write 0 to interface select */
 	rgmii_updatel(ethqos, RGMII_CONFIG_INTF_SEL,
 		      0, RGMII_IO_MACRO_CONFIG);
@@ -484,14 +489,8 @@  static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
 			      BIT(6), RGMII_IO_MACRO_CONFIG);
 		rgmii_updatel(ethqos, RGMII_CONFIG2_RSVD_CONFIG15,
 			      0, RGMII_IO_MACRO_CONFIG2);
-
-		if (ethqos->has_emac_ge_3)
-			rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
-				      RGMII_CONFIG2_RX_PROG_SWAP,
-				      RGMII_IO_MACRO_CONFIG2);
-		else
-			rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
-				      0, RGMII_IO_MACRO_CONFIG2);
+		rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP, rx_prog_swap,
+			      RGMII_IO_MACRO_CONFIG2);
 
 		/* Write 0x5 to PRG_RCLK_DLY_CODE */
 		rgmii_updatel(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
@@ -525,13 +524,9 @@  static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
 			      RGMII_IO_MACRO_CONFIG);
 		rgmii_updatel(ethqos, RGMII_CONFIG2_RSVD_CONFIG15,
 			      0, RGMII_IO_MACRO_CONFIG2);
-		if (ethqos->has_emac_ge_3)
-			rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
-				      RGMII_CONFIG2_RX_PROG_SWAP,
-				      RGMII_IO_MACRO_CONFIG2);
-		else
-			rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
-				      0, RGMII_IO_MACRO_CONFIG2);
+		rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP, rx_prog_swap,
+			      RGMII_IO_MACRO_CONFIG2);
+
 		/* Write 0x5 to PRG_RCLK_DLY_CODE */
 		rgmii_updatel(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
 			      (BIT(29) | BIT(27)), SDCC_HC_REG_DDR_CONFIG);
@@ -782,7 +777,7 @@  static void ethqos_ptp_clk_freq_config(struct stmmac_priv *priv)
 
 static int qcom_ethqos_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node, *root;
 	const struct ethqos_emac_driver_data *data;
 	struct plat_stmmacenet_data *plat_dat;
 	struct stmmac_resources stmmac_res;
@@ -810,6 +805,15 @@  static int qcom_ethqos_probe(struct platform_device *pdev)
 	ret = of_get_phy_mode(np, &ethqos->phy_mode);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to get phy mode\n");
+
+	root = of_find_node_by_path("/");
+	if (root && of_device_is_compatible(root, "qcom,sa8540p-ride"))
+		ethqos->needs_rx_prog_swap = true;
+	else
+		ethqos->needs_rx_prog_swap =
+			of_property_read_bool(np, "qcom,rx-prog-swap");
+	of_node_put(root);
+
 	switch (ethqos->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID: