netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode
@ 2024-04-22  8:46 Steffen Trumtrar
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Steffen Trumtrar @ 2024-04-22  8:46 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

This series adds support for setting the TX_CLK direction in the eQOS
ethernet core on the i.MX93 when RMII mode is used.

According to AN14149, when the i.MX93 ethernet controller is used in
RMII mode, the TX_CLK *must* be set to output mode. 

Add a devicetree property with the register to set this bit and parse it
in the dwmac-imx driver.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
Steffen Trumtrar (3):
      dt-bindings: net: mx93: add enet_clk_sel binding
      arm64: dts: imx93: add enet_clk_sel
      net: stmicro: imx: set TX_CLK direction in RMII mode

 .../devicetree/bindings/net/nxp,dwmac-imx.yaml     | 10 ++++++++
 arch/arm64/boot/dts/freescale/imx93.dtsi           |  1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c    | 27 ++++++++++++++++++++++
 3 files changed, 38 insertions(+)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240422-v6-9-topic-imx93-eqos-rmii-3a2cb421c81d

Best regards,
-- 
Steffen Trumtrar <s.trumtrar@pengutronix.de>


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

* [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding
  2024-04-22  8:46 [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode Steffen Trumtrar
@ 2024-04-22  8:46 ` Steffen Trumtrar
  2024-04-22  8:58   ` Sascha Hauer
                     ` (3 more replies)
  2024-04-22  8:46 ` [PATCH 2/3] arm64: dts: imx93: add enet_clk_sel Steffen Trumtrar
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 14+ messages in thread
From: Steffen Trumtrar @ 2024-04-22  8:46 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

When the eQOS on the i.MX93 is used in RMII mode, the TX_CLK must be set
to output mode. To do this, the ENET_CLK_SEL register must be accessed.
This register is located in a GPR register space.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
index 4c01cae7c93a7..1d1c8b90da871 100644
--- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
+++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
@@ -56,6 +56,16 @@ properties:
         - tx
         - mem
 
+  enet_clk_sel:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to the GPR syscon
+          - description: the offset of the GPR register
+    description:
+      Should be phandle/offset pair. The phandle to the syscon node which
+      encompases the GPR register, and the offset of the GPR register.
+
   intf_mode:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     items:

-- 
2.43.2


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

* [PATCH 2/3] arm64: dts: imx93: add enet_clk_sel
  2024-04-22  8:46 [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode Steffen Trumtrar
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
@ 2024-04-22  8:46 ` Steffen Trumtrar
  2024-04-22  8:46 ` [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode Steffen Trumtrar
  2024-04-22  9:28 ` [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK " Sébastien Szymanski
  3 siblings, 0 replies; 14+ messages in thread
From: Steffen Trumtrar @ 2024-04-22  8:46 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

The ENET_CLK_SEL register is at offset 0x2c in the wakeupmix_gpr
register and needed to set the TX_CLK direction in case of RMII mode.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx93.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 601c94e1fac8e..116ff9c15709b 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -1051,6 +1051,7 @@ eqos: ethernet@428a0000 {
 							 <&clk IMX93_CLK_SYS_PLL_PFD0_DIV2>;
 				assigned-clock-rates = <100000000>, <250000000>;
 				intf_mode = <&wakeupmix_gpr 0x28>;
+				enet_clk_sel = <&wakeupmix_gpr 0x2c>;
 				snps,clk-csr = <0>;
 				status = "disabled";
 			};

-- 
2.43.2


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

* [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode
  2024-04-22  8:46 [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode Steffen Trumtrar
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
  2024-04-22  8:46 ` [PATCH 2/3] arm64: dts: imx93: add enet_clk_sel Steffen Trumtrar
@ 2024-04-22  8:46 ` Steffen Trumtrar
  2024-04-22  8:51   ` Ahmad Fatoum
                     ` (2 more replies)
  2024-04-22  9:28 ` [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK " Sébastien Szymanski
  3 siblings, 3 replies; 14+ messages in thread
From: Steffen Trumtrar @ 2024-04-22  8:46 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

In case of RMII connection, the TX_CLK must be set to output direction.
Parse the register and offset from the devicetree and set the direction
of the TX_CLK when the property was provided.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c | 27 +++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index 6b65420e11b5c..0fc81a626a664 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -37,6 +37,9 @@
 #define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 1)
 #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
 
+#define MX93_GPR_ENET_QOS_TX_CLK_SEL_MASK	GENMASK(1, 1)
+#define MX93_GPR_ENET_QOS_TX_CLK_SEL		(0x1 << 1)
+
 #define DMA_BUS_MODE			0x00001000
 #define DMA_BUS_MODE_SFT_RESET		(0x1 << 0)
 #define RMII_RESET_SPEED		(0x3 << 14)
@@ -57,7 +60,9 @@ struct imx_priv_data {
 	struct clk *clk_tx;
 	struct clk *clk_mem;
 	struct regmap *intf_regmap;
+	struct regmap *enet_clk_regmap;
 	u32 intf_reg_off;
+	u32 enet_clk_reg_off;
 	bool rmii_refclk_ext;
 	void __iomem *base_addr;
 
@@ -116,6 +121,18 @@ static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		val = MX93_GPR_ENET_QOS_INTF_SEL_RMII;
+
+		/* According to NXP AN14149, the direction of the
+		 * TX_CLK must be set to output in RMII mode.
+		 */
+		if (dwmac->enet_clk_regmap)
+			regmap_update_bits(dwmac->enet_clk_regmap,
+					   dwmac->enet_clk_reg_off,
+					   MX93_GPR_ENET_QOS_TX_CLK_SEL_MASK,
+					   MX93_GPR_ENET_QOS_TX_CLK_SEL);
+		else
+			dev_warn(dwmac->dev, "TX_CLK can't be set to output mode.\n");
+
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
@@ -310,6 +327,16 @@ imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
 			dev_err(dev, "Can't get intf mode reg offset (%d)\n", err);
 			return err;
 		}
+
+		dwmac->enet_clk_regmap = syscon_regmap_lookup_by_phandle(np, "enet_clk_sel");
+		if (IS_ERR(dwmac->enet_clk_regmap))
+			return PTR_ERR(dwmac->enet_clk_regmap);
+
+		err = of_property_read_u32_index(np, "enet_clk_sel", 1, &dwmac->enet_clk_reg_off);
+		if (err) {
+			dev_err(dev, "Can't get enet clk sel reg offset (%d)\n", err);
+			return err;
+		}
 	}
 
 	return err;

-- 
2.43.2


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

* Re: [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode
  2024-04-22  8:46 ` [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode Steffen Trumtrar
@ 2024-04-22  8:51   ` Ahmad Fatoum
  2024-04-22  9:04   ` Marc Kleine-Budde
  2024-04-22  9:12   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2024-04-22  8:51 UTC (permalink / raw)
  To: Steffen Trumtrar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Clark Wang, Linux Team, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin
  Cc: imx, devicetree, netdev, linux-kernel, linux-stm32,
	linux-arm-kernel

Hello Steffen,

On 22.04.24 10:46, Steffen Trumtrar wrote:
> +		dwmac->enet_clk_regmap = syscon_regmap_lookup_by_phandle(np, "enet_clk_sel");
> +		if (IS_ERR(dwmac->enet_clk_regmap))
> +			return PTR_ERR(dwmac->enet_clk_regmap);
> +
> +		err = of_property_read_u32_index(np, "enet_clk_sel", 1, &dwmac->enet_clk_reg_off);
> +		if (err) {
> +			dev_err(dev, "Can't get enet clk sel reg offset (%d)\n", err);
> +			return err;
> +		}

This looks like the property is not optional. The property needs to stay optional
as not to break backwards compatibility with older device trees.

Cheers,
Ahmad

>  	}
>  
>  	return err;
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


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

* Re: [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
@ 2024-04-22  8:58   ` Sascha Hauer
  2024-04-23  6:46     ` Steffen Trumtrar
  2024-04-22  9:11   ` Krzysztof Kozlowski
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2024-04-22  8:58 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Pengutronix Kernel Team, Fabio Estevam, Clark Wang, Linux Team,
	Alexandre Torgue, Jose Abreu, Maxime Coquelin, netdev, devicetree,
	imx, linux-arm-kernel, linux-kernel, linux-stm32

On Mon, Apr 22, 2024 at 10:46:17AM +0200, Steffen Trumtrar wrote:
> When the eQOS on the i.MX93 is used in RMII mode, the TX_CLK must be set
> to output mode. To do this, the ENET_CLK_SEL register must be accessed.
> This register is located in a GPR register space.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> index 4c01cae7c93a7..1d1c8b90da871 100644
> --- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> +++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> @@ -56,6 +56,16 @@ properties:
>          - tx
>          - mem
>  
> +  enet_clk_sel:

Krzysztof will likely write the same in a moment, but anyway:

Please no underscores in property names, see
https://docs.kernel.org/devicetree/bindings/dts-coding-style.html

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode
  2024-04-22  8:46 ` [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode Steffen Trumtrar
  2024-04-22  8:51   ` Ahmad Fatoum
@ 2024-04-22  9:04   ` Marc Kleine-Budde
  2024-04-22  9:12   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 14+ messages in thread
From: Marc Kleine-Budde @ 2024-04-22  9:04 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin, imx,
	devicetree, netdev, linux-kernel, linux-stm32, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 3109 bytes --]

On 22.04.2024 10:46:19, Steffen Trumtrar wrote:
> In case of RMII connection, the TX_CLK must be set to output direction.
> Parse the register and offset from the devicetree and set the direction
> of the TX_CLK when the property was provided.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c | 27 +++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
> index 6b65420e11b5c..0fc81a626a664 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
> @@ -37,6 +37,9 @@
>  #define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 1)
>  #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
>  
> +#define MX93_GPR_ENET_QOS_TX_CLK_SEL_MASK	GENMASK(1, 1)
> +#define MX93_GPR_ENET_QOS_TX_CLK_SEL		(0x1 << 1)
> +
>  #define DMA_BUS_MODE			0x00001000
>  #define DMA_BUS_MODE_SFT_RESET		(0x1 << 0)
>  #define RMII_RESET_SPEED		(0x3 << 14)
> @@ -57,7 +60,9 @@ struct imx_priv_data {
>  	struct clk *clk_tx;
>  	struct clk *clk_mem;
>  	struct regmap *intf_regmap;
> +	struct regmap *enet_clk_regmap;
>  	u32 intf_reg_off;
> +	u32 enet_clk_reg_off;
>  	bool rmii_refclk_ext;
>  	void __iomem *base_addr;
>  
> @@ -116,6 +121,18 @@ static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
>  		break;
>  	case PHY_INTERFACE_MODE_RMII:
>  		val = MX93_GPR_ENET_QOS_INTF_SEL_RMII;
> +
> +		/* According to NXP AN14149, the direction of the
> +		 * TX_CLK must be set to output in RMII mode.
> +		 */
> +		if (dwmac->enet_clk_regmap)
> +			regmap_update_bits(dwmac->enet_clk_regmap,
> +					   dwmac->enet_clk_reg_off,
> +					   MX93_GPR_ENET_QOS_TX_CLK_SEL_MASK,
> +					   MX93_GPR_ENET_QOS_TX_CLK_SEL);

Please add error handling.

> +		else
> +			dev_warn(dwmac->dev, "TX_CLK can't be set to output mode.\n");

As far as I understand the AN14149, setting the TX_CLK_SEL is mandatory
for PHY_INTERFACE_MODE_RMII. IMHO this should be error, shouldn't it?

> +
>  		break;
>  	case PHY_INTERFACE_MODE_RGMII:
>  	case PHY_INTERFACE_MODE_RGMII_ID:
> @@ -310,6 +327,16 @@ imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
>  			dev_err(dev, "Can't get intf mode reg offset (%d)\n", err);
>  			return err;
>  		}
> +
> +		dwmac->enet_clk_regmap = syscon_regmap_lookup_by_phandle(np, "enet_clk_sel");
> +		if (IS_ERR(dwmac->enet_clk_regmap))
> +			return PTR_ERR(dwmac->enet_clk_regmap);
> +
> +		err = of_property_read_u32_index(np, "enet_clk_sel", 1, &dwmac->enet_clk_reg_off);
> +		if (err) {
> +			dev_err(dev, "Can't get enet clk sel reg offset (%d)\n", err);
> +			return err;
> +		}

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
  2024-04-22  8:58   ` Sascha Hauer
@ 2024-04-22  9:11   ` Krzysztof Kozlowski
  2024-04-22  9:11   ` Krzysztof Kozlowski
  2024-04-22 16:02   ` Andrew Lunn
  3 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-04-22  9:11 UTC (permalink / raw)
  To: Steffen Trumtrar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Clark Wang, Linux Team, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

On 22/04/2024 10:46, Steffen Trumtrar wrote:
> When the eQOS on the i.MX93 is used in RMII mode, the TX_CLK must be set
> to output mode. To do this, the ENET_CLK_SEL register must be accessed.
> This register is located in a GPR register space.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> index 4c01cae7c93a7..1d1c8b90da871 100644
> --- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> +++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml
> @@ -56,6 +56,16 @@ properties:
>          - tx
>          - mem
>  
> +  enet_clk_sel:

Except what Sasha wrote, also missing vendor prefix. That's not a
generic property.

> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to the GPR syscon
> +          - description: the offset of the GPR register
> +    description:
> +      Should be phandle/offset pair. The phandle to the syscon node which
> +      encompases the GPR register, and the offset of the GPR register.

That's redundant. Provide full description in the items. You can say
here what is the purpose of this phandle.

Best regards,
Krzysztof


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

* Re: [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
  2024-04-22  8:58   ` Sascha Hauer
  2024-04-22  9:11   ` Krzysztof Kozlowski
@ 2024-04-22  9:11   ` Krzysztof Kozlowski
  2024-04-22 16:02   ` Andrew Lunn
  3 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-04-22  9:11 UTC (permalink / raw)
  To: Steffen Trumtrar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Clark Wang, Linux Team, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

On 22/04/2024 10:46, Steffen Trumtrar wrote:
> When the eQOS on the i.MX93 is used in RMII mode, the TX_CLK must be set
> to output mode. To do this, the ENET_CLK_SEL register must be accessed.
> This register is located in a GPR register space.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

Best regards,
Krzysztof


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

* Re: [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode
  2024-04-22  8:46 ` [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode Steffen Trumtrar
  2024-04-22  8:51   ` Ahmad Fatoum
  2024-04-22  9:04   ` Marc Kleine-Budde
@ 2024-04-22  9:12   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2024-04-22  9:12 UTC (permalink / raw)
  To: Steffen Trumtrar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Clark Wang, Linux Team, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

On 22/04/2024 10:46, Steffen Trumtrar wrote:
> In case of RMII connection, the TX_CLK must be set to output direction.
> Parse the register and offset from the devicetree and set the direction
> of the TX_CLK when the property was provided.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c | 27 +++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
> index 6b65420e11b5c..0fc81a626a664 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
> @@ -37,6 +37,9 @@
>  #define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 1)
>  #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
>  
> +#define MX93_GPR_ENET_QOS_TX_CLK_SEL_MASK	GENMASK(1, 1)
> +#define MX93_GPR_ENET_QOS_TX_CLK_SEL		(0x1 << 1)
> +
>  #define DMA_BUS_MODE			0x00001000
>  #define DMA_BUS_MODE_SFT_RESET		(0x1 << 0)
>  #define RMII_RESET_SPEED		(0x3 << 14)
> @@ -57,7 +60,9 @@ struct imx_priv_data {
>  	struct clk *clk_tx;
>  	struct clk *clk_mem;
>  	struct regmap *intf_regmap;
> +	struct regmap *enet_clk_regmap;
>  	u32 intf_reg_off;
> +	u32 enet_clk_reg_off;
>  	bool rmii_refclk_ext;
>  	void __iomem *base_addr;
>  
> @@ -116,6 +121,18 @@ static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
>  		break;
>  	case PHY_INTERFACE_MODE_RMII:
>  		val = MX93_GPR_ENET_QOS_INTF_SEL_RMII;
> +
> +		/* According to NXP AN14149, the direction of the
> +		 * TX_CLK must be set to output in RMII mode.
> +		 */
> +		if (dwmac->enet_clk_regmap)
> +			regmap_update_bits(dwmac->enet_clk_regmap,
> +					   dwmac->enet_clk_reg_off,
> +					   MX93_GPR_ENET_QOS_TX_CLK_SEL_MASK,
> +					   MX93_GPR_ENET_QOS_TX_CLK_SEL);
> +		else
> +			dev_warn(dwmac->dev, "TX_CLK can't be set to output mode.\n");
> +
>  		break;
>  	case PHY_INTERFACE_MODE_RGMII:
>  	case PHY_INTERFACE_MODE_RGMII_ID:
> @@ -310,6 +327,16 @@ imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
>  			dev_err(dev, "Can't get intf mode reg offset (%d)\n", err);
>  			return err;
>  		}
> +
> +		dwmac->enet_clk_regmap = syscon_regmap_lookup_by_phandle(np, "enet_clk_sel");
> +		if (IS_ERR(dwmac->enet_clk_regmap))
> +			return PTR_ERR(dwmac->enet_clk_regmap);

This looks like breaking ABI. Please test your changes without the DTS.
Does the DTS pass dtbs_check? Does the driver probe correctly with such DTS?

Best regards,
Krzysztof


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

* Re: [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode
  2024-04-22  8:46 [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode Steffen Trumtrar
                   ` (2 preceding siblings ...)
  2024-04-22  8:46 ` [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode Steffen Trumtrar
@ 2024-04-22  9:28 ` Sébastien Szymanski
  2024-04-23  6:43   ` Steffen Trumtrar
  3 siblings, 1 reply; 14+ messages in thread
From: Sébastien Szymanski @ 2024-04-22  9:28 UTC (permalink / raw)
  To: Steffen Trumtrar, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Clark Wang, Linux Team, Alexandre Torgue, Jose Abreu,
	Maxime Coquelin
  Cc: netdev, devicetree, imx, linux-arm-kernel, linux-kernel,
	linux-stm32

Hello,

On 4/22/24 10:46, Steffen Trumtrar wrote:
> This series adds support for setting the TX_CLK direction in the eQOS
> ethernet core on the i.MX93 when RMII mode is used.
> 
> According to AN14149, when the i.MX93 ethernet controller is used in
> RMII mode, the TX_CLK *must* be set to output mode.

Must ? I don't think that is true. Downstream NXP kernel has an option 
to set TX_CLK as an input:

https://github.com/nxp-imx/linux-imx/blob/lf-6.6.y/Documentation/devicetree/bindings/net/nxp%2Cdwmac-imx.yaml#L69

https://github.com/nxp-imx/linux-imx/commit/fbc17f6f7919d03c275fc48b0400c212475b60ec

Regards,

> 
> Add a devicetree property with the register to set this bit and parse it
> in the dwmac-imx driver.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
> Steffen Trumtrar (3):
>        dt-bindings: net: mx93: add enet_clk_sel binding
>        arm64: dts: imx93: add enet_clk_sel
>        net: stmicro: imx: set TX_CLK direction in RMII mode
> 
>   .../devicetree/bindings/net/nxp,dwmac-imx.yaml     | 10 ++++++++
>   arch/arm64/boot/dts/freescale/imx93.dtsi           |  1 +
>   drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c    | 27 ++++++++++++++++++++++
>   3 files changed, 38 insertions(+)
> ---
> base-commit: 4cece764965020c22cff7665b18a012006359095
> change-id: 20240422-v6-9-topic-imx93-eqos-rmii-3a2cb421c81d
> 
> Best regards,

-- 
Sébastien Szymanski, Armadeus Systems
Software engineer


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

* Re: [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding
  2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
                     ` (2 preceding siblings ...)
  2024-04-22  9:11   ` Krzysztof Kozlowski
@ 2024-04-22 16:02   ` Andrew Lunn
  3 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2024-04-22 16:02 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin, netdev,
	devicetree, imx, linux-arm-kernel, linux-kernel, linux-stm32

> +  enet_clk_sel:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to the GPR syscon
> +          - description: the offset of the GPR register
> +    description:
> +      Should be phandle/offset pair. The phandle to the syscon node which
> +      encompases the GPR register, and the offset of the GPR register.
> +

net/nxp,dwmac-imx.yaml

  intf_mode:
    $ref: /schemas/types.yaml#/definitions/phandle-array
    items:
      - items:
          - description: phandle to the GPR syscon
          - description: the offset of the GPR register
    description:
      Should be phandle/offset pair. The phandle to the syscon node which
      encompases the GPR register, and the offset of the GPR register.

dma/fsl,imx-sdma.yaml

  gpr:
    $ref: /schemas/types.yaml#/definitions/phandle
    description: The phandle to the General Purpose Register (GPR) node

memory-controllers/fsl/fsl,imx-weim.yaml

 fsl,weim-cs-gpr:
    $ref: /schemas/types.yaml#/definitions/phandle
    description: |
      Phandle to the system General Purpose Register controller that contains
      WEIM CS GPR register, e.g. IOMUXC_GPR1 on i.MX6Q. IOMUXC_GPR1[11:0]
      should be set up as one of the following 4 possible values depending on
      the CS space configuration.

      IOMUXC_GPR1[11:0]    CS0    CS1    CS2    CS3
      ---------------------------------------------
              05          128M     0M     0M     0M
              033          64M    64M     0M     0M
              0113         64M    32M    32M     0M
              01111        32M    32M    32M    32M

      In case that the property is absent, the reset value or what bootloader
      sets up in IOMUXC_GPR1[11:0] will be used.

How about defining that the General Purpose Registers property is
once, rather than per device which needs access to it?

      Andrew

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

* Re: [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode
  2024-04-22  9:28 ` [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK " Sébastien Szymanski
@ 2024-04-23  6:43   ` Steffen Trumtrar
  0 siblings, 0 replies; 14+ messages in thread
From: Steffen Trumtrar @ 2024-04-23  6:43 UTC (permalink / raw)
  To: Sébastien Szymanski
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Clark Wang,
	Linux Team, Alexandre Torgue, Jose Abreu, Maxime Coquelin, netdev,
	devicetree, imx, linux-arm-kernel, linux-kernel, linux-stm32


Hi,

On 2024-04-22 at 11:28 +02, Sébastien Szymanski <sebastien.szymanski@armadeus.com> wrote: 
 
> Hello,  On 4/22/24 10:46, Steffen Trumtrar wrote: 
> > This series adds support for setting the TX_CLK direction in the eQOS ethernet core on the i.MX93 when RMII mode is used.  According to AN14149, when the i.MX93 ethernet controller is used in RMII mode, the TX_CLK *must* be set to output mode. 
>  Must ? I don't think that is true. Downstream NXP kernel has an option to set TX_CLK as an input: 
>

re-reading that passage again, it only says "other registers that must be configured" and that the ENET_QOS_CLK_TX_CLK_SEL bit "is 0b1" for RMII. So, maybe you are right. 


Thanks,
Steffen

> https://github.com/nxp-imx/linux-imx/blob/lf-6.6.y/Documentation/devicetree/bindings/net/nxp%2Cdwmac-imx.yaml#L69
> 
> https://github.com/nxp-imx/linux-imx/commit/fbc17f6f7919d03c275fc48b0400c212475b60ec
> 

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

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

* Re: [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding
  2024-04-22  8:58   ` Sascha Hauer
@ 2024-04-23  6:46     ` Steffen Trumtrar
  0 siblings, 0 replies; 14+ messages in thread
From: Steffen Trumtrar @ 2024-04-23  6:46 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Pengutronix Kernel Team, Fabio Estevam, Clark Wang, Linux Team,
	Alexandre Torgue, Jose Abreu, Maxime Coquelin, netdev, devicetree,
	imx, linux-arm-kernel, linux-kernel, linux-stm32

On 2024-04-22 at 10:58 +02, Sascha Hauer <s.hauer@pengutronix.de> wrote: 
 
> On Mon, Apr 22, 2024 at 10:46:17AM +0200, Steffen Trumtrar wrote: 
> > When the eQOS on the i.MX93 is used in RMII mode, the TX_CLK must be set to output mode. To do this, the ENET_CLK_SEL register must be accessed.  This register is located in a GPR register space.   Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> --- 
> >  Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) 
> >  diff --git a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml index 4c01cae7c93a7..1d1c8b90da871 100644 --- a/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml +++ b/Documentation/devicetree/bindings/net/nxp,dwmac-imx.yaml @@ -56,6 +56,16 @@ properties: 
> >          - tx - mem 
> >   
> > +  enet_clk_sel: 
>  Krzysztof will likely write the same in a moment, but anyway:  Please no underscores in property names, see https://docs.kernel.org/devicetree/bindings/dts-coding-style.html 

That's what you get, when you just mindlessly copy existing property names :(
You are right, of course.


Thanks
Steffen

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

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

end of thread, other threads:[~2024-04-23  6:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-22  8:46 [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK in RMII mode Steffen Trumtrar
2024-04-22  8:46 ` [PATCH 1/3] dt-bindings: net: mx93: add enet_clk_sel binding Steffen Trumtrar
2024-04-22  8:58   ` Sascha Hauer
2024-04-23  6:46     ` Steffen Trumtrar
2024-04-22  9:11   ` Krzysztof Kozlowski
2024-04-22  9:11   ` Krzysztof Kozlowski
2024-04-22 16:02   ` Andrew Lunn
2024-04-22  8:46 ` [PATCH 2/3] arm64: dts: imx93: add enet_clk_sel Steffen Trumtrar
2024-04-22  8:46 ` [PATCH 3/3] net: stmicro: imx: set TX_CLK direction in RMII mode Steffen Trumtrar
2024-04-22  8:51   ` Ahmad Fatoum
2024-04-22  9:04   ` Marc Kleine-Budde
2024-04-22  9:12   ` Krzysztof Kozlowski
2024-04-22  9:28 ` [PATCH 0/3] arm64: mx93: etherrnet: set TX_CLK " Sébastien Szymanski
2024-04-23  6:43   ` Steffen Trumtrar

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