[PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Lukasz Majewski]

The NXP's vf610 soc is equipped with L2 switch IP block from More
Than IP (MTIP) vendor.

It requires special clock (VF610_CLK_ESW) to be operational.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/clk/imx/clk-vf610.c             | 1 +
 include/dt-bindings/clock/vf610-clock.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
index 9e11f1c7c397..405bf48a1d28 100644
--- a/drivers/clk/imx/clk-vf610.c
+++ b/drivers/clk/imx/clk-vf610.c
@@ -309,6 +309,7 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
 	clk[VF610_CLK_ENET_TS] = imx_clk_gate("enet_ts", "enet_ts_sel", CCM_CSCDR1, 23);
 	clk[VF610_CLK_ENET0] = imx_clk_gate2("enet0", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(0));
 	clk[VF610_CLK_ENET1] = imx_clk_gate2("enet1", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(1));
+	clk[VF610_CLK_ESW] = imx_clk_gate2("esw", "ipg_bus", CCM_CCGR10, CCM_CCGRx_CGn(8));
 
 	clk[VF610_CLK_PIT] = imx_clk_gate2("pit", "ipg_bus", CCM_CCGR1, CCM_CCGRx_CGn(7));
 
diff --git a/include/dt-bindings/clock/vf610-clock.h b/include/dt-bindings/clock/vf610-clock.h
index 373644e46747..95446f1bee16 100644
--- a/include/dt-bindings/clock/vf610-clock.h
+++ b/include/dt-bindings/clock/vf610-clock.h
@@ -197,6 +197,7 @@
 #define VF610_CLK_TCON1			188
 #define VF610_CLK_CAAM			189
 #define VF610_CLK_CRC			190
-#define VF610_CLK_END			191
+#define VF610_CLK_ESW			191
+#define VF610_CLK_END			192
 
 #endif /* __DT_BINDINGS_CLOCK_VF610_H */
-- 
2.40.1

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Krzysztof Kozlowski]

On 19/02/2025 12:49, Lukasz Majewski wrote:
> The NXP's vf610 soc is equipped with L2 switch IP block from More
> Than IP (MTIP) vendor.
> 
> It requires special clock (VF610_CLK_ESW) to be operational.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
>  drivers/clk/imx/clk-vf610.c             | 1 +
>  include/dt-bindings/clock/vf610-clock.h | 3 ++-


Bindings are a separate patch.

>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/imx/clk-vf610.c b/drivers/clk/imx/clk-vf610.c
> index 9e11f1c7c397..405bf48a1d28 100644
> --- a/drivers/clk/imx/clk-vf610.c
> +++ b/drivers/clk/imx/clk-vf610.c
> @@ -309,6 +309,7 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
>  	clk[VF610_CLK_ENET_TS] = imx_clk_gate("enet_ts", "enet_ts_sel", CCM_CSCDR1, 23);
>  	clk[VF610_CLK_ENET0] = imx_clk_gate2("enet0", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(0));
>  	clk[VF610_CLK_ENET1] = imx_clk_gate2("enet1", "ipg_bus", CCM_CCGR9, CCM_CCGRx_CGn(1));
> +	clk[VF610_CLK_ESW] = imx_clk_gate2("esw", "ipg_bus", CCM_CCGR10, CCM_CCGRx_CGn(8));
>  
>  	clk[VF610_CLK_PIT] = imx_clk_gate2("pit", "ipg_bus", CCM_CCGR1, CCM_CCGRx_CGn(7));
>  
> diff --git a/include/dt-bindings/clock/vf610-clock.h b/include/dt-bindings/clock/vf610-clock.h
> index 373644e46747..95446f1bee16 100644
> --- a/include/dt-bindings/clock/vf610-clock.h
> +++ b/include/dt-bindings/clock/vf610-clock.h
> @@ -197,6 +197,7 @@
>  #define VF610_CLK_TCON1			188
>  #define VF610_CLK_CAAM			189
>  #define VF610_CLK_CRC			190
> -#define VF610_CLK_END			191
> +#define VF610_CLK_ESW			191
> +#define VF610_CLK_END			192


This defiine should not change. If you need to change it, means it is
not an ABI. Just like for other cases, I suggest to drop the define in
preparatory patch.


Best regards,
Krzysztof

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Fabio Estevam]

Hi Lukasz,

On Wed, Feb 19, 2025 at 9:06 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:

> This defiine should not change. If you need to change it, means it is
> not an ABI. Just like for other cases, I suggest to drop the define in
> preparatory patch.

You can follow the same approach that was done for i.MX93.

Please check the two commits below:

commit c0813ce2e5b0d1174782aff30d366509377abc7b
Author: Pengfei Li <pengfei.li_1@nxp.com>
Date:   Wed Oct 23 11:46:48 2024 -0700

    dt-bindings: clock: imx93: Drop IMX93_CLK_END macro definition

    IMX93_CLK_END should be dropped as it is not part of the ABI.

    Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
    Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
    Acked-by: Peng Fan <peng.fan@nxp.com>
    Acked-by: Conor Dooley <conor.dooley@microchip.com>
    Link: https://lore.kernel.org/r/20241023184651.381265-3-pengfei.li_1@nxp.com
    Signed-off-by: Abel Vesa <abel.vesa@linaro.org>


commit 0af18ba60752e8a4ba34404c1d9a4a799da690f5
Author: Pengfei Li <pengfei.li_1@nxp.com>
Date:   Wed Oct 23 11:46:47 2024 -0700

    clk: imx93: Move IMX93_CLK_END macro to clk driver

    IMX93_CLK_END was previously defined in imx93-clock.h to indicate
    the number of clocks. However, it is not part of the ABI. For starters
    it does no really appear in DTS. But what's more important - new clocks
    are described later, which contradicts this define in binding header.
    So move this macro to clock driver.

    Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
    Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
    Link: https://lore.kernel.org/r/20241023184651.381265-2-pengfei.li_1@nxp.com
    Signed-off-by: Abel Vesa <abel.vesa@linaro.org>

Also, the Subject should not contain 'arm'. You could use:

clk: vf610: Add support for the Ethernet switch clock

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Andrew Lunn]

On Wed, Feb 19, 2025 at 12:49:36PM +0100, Lukasz Majewski wrote:
> The NXP's vf610 soc is equipped with L2 switch IP block from More
> Than IP (MTIP) vendor.
> 
> It requires special clock (VF610_CLK_ESW) to be operational.

So you have a driver for this switch? It has been talked about in the
past, but nobody made any progress with it. Ah, it was you in 2020. It
will be interesting to see what you came up with in the end, pure
switchdev or a DSA driver.

	Andrew

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Lukasz Majewski]

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

Hi Andrew,

> On Wed, Feb 19, 2025 at 12:49:36PM +0100, Lukasz Majewski wrote:
> > The NXP's vf610 soc is equipped with L2 switch IP block from More
> > Than IP (MTIP) vendor.
> > 
> > It requires special clock (VF610_CLK_ESW) to be operational.  
> 
> So you have a driver for this switch? It has been talked about in the
> past, but nobody made any progress with it. Ah, it was you in 2020.

Yes, I'm going to try another time to upstream it.... :-)

> It
> will be interesting to see what you came up with in the end, pure
> switchdev or a DSA driver.

I think it would be:

1. Standalone driver, which would configure the L2 switch from the very
beginning to work (this is different from FEC on imx28/vf610 where
switch is bypassed)

2. It will use the in-switch registers to have two network interfaces
separated. As a result - it may be slower than the fec_main.c in this
use case.

3. When somebody call "bridge ..." on it - then the in-switch
separation would be disabled. This is the "normal" state of operation
for L2 switch, which would be a HW accelerator for bridging.

4. The switchdev would be used to manage it

5. This would be just a very simple driver - just bridging and startup
of the L2 switch.

After we would have a consensus (i.e. it would be pulled to mainline) -
I would proceed further.

I will try to not touch fec_main.c driver - just write standalone, new
for MoreThanIP L2 switch driver.

If somebody would like to use FEC, then he will insert the proper
module. If switch, another one can be inserted, depending o the target
use case.

> 
> 	Andrew


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

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

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Andrew Lunn]

On Wed, Feb 19, 2025 at 11:38:02PM +0100, Lukasz Majewski wrote:
> Hi Andrew,
> 
> > On Wed, Feb 19, 2025 at 12:49:36PM +0100, Lukasz Majewski wrote:
> > > The NXP's vf610 soc is equipped with L2 switch IP block from More
> > > Than IP (MTIP) vendor.
> > > 
> > > It requires special clock (VF610_CLK_ESW) to be operational.  
> > 
> > So you have a driver for this switch? It has been talked about in the
> > past, but nobody made any progress with it. Ah, it was you in 2020.
> 
> Yes, I'm going to try another time to upstream it.... :-)
> 
> > It
> > will be interesting to see what you came up with in the end, pure
> > switchdev or a DSA driver.
> 
> I think it would be:
> 
> 1. Standalone driver, which would configure the L2 switch from the very
> beginning to work (this is different from FEC on imx28/vf610 where
> switch is bypassed)
> 
> 2. It will use the in-switch registers to have two network interfaces
> separated. As a result - it may be slower than the fec_main.c in this
> use case.

Seems like a reasonable compromise. You would only load this driver if
you intend to make use of the switch...

> 3. When somebody call "bridge ..." on it - then the in-switch
> separation would be disabled. This is the "normal" state of operation
> for L2 switch, which would be a HW accelerator for bridging.
> 
> 4. The switchdev would be used to manage it
> 
> 5. This would be just a very simple driver - just bridging and startup
> of the L2 switch.
> 
> After we would have a consensus (i.e. it would be pulled to mainline) -
> I would proceed further.
> 
> I will try to not touch fec_main.c driver - just write standalone, new
> for MoreThanIP L2 switch driver.

It might make sense to refactor the MDIO code into a helper which both
can share? No point duplicating that.

> If somebody would like to use FEC, then he will insert the proper
> module. If switch, another one can be inserted, depending o the target
> use case.

This all seems like a reasonable way forward.

MoreThanIP is now part of Synopsys. I wounder if this IP now exists in
other SoCs? The press release however suggests Synopsys was
interesting in the high speed interfaces, not a two ports Fast
Ethernet switch.

	Andrew

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Lukasz Majewski]

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

Hi Andrew,

> On Wed, Feb 19, 2025 at 11:38:02PM +0100, Lukasz Majewski wrote:
> > Hi Andrew,
> >   
> > > On Wed, Feb 19, 2025 at 12:49:36PM +0100, Lukasz Majewski wrote:  
> > > > The NXP's vf610 soc is equipped with L2 switch IP block from
> > > > More Than IP (MTIP) vendor.
> > > > 
> > > > It requires special clock (VF610_CLK_ESW) to be operational.    
> > > 
> > > So you have a driver for this switch? It has been talked about in
> > > the past, but nobody made any progress with it. Ah, it was you in
> > > 2020.  
> > 
> > Yes, I'm going to try another time to upstream it.... :-)
> >   
> > > It
> > > will be interesting to see what you came up with in the end, pure
> > > switchdev or a DSA driver.  
> > 
> > I think it would be:
> > 
> > 1. Standalone driver, which would configure the L2 switch from the
> > very beginning to work (this is different from FEC on imx28/vf610
> > where switch is bypassed)
> > 
> > 2. It will use the in-switch registers to have two network
> > interfaces separated. As a result - it may be slower than the
> > fec_main.c in this use case.  
> 
> Seems like a reasonable compromise. You would only load this driver if
> you intend to make use of the switch...

Yes, the main use case would be the switch (after bridge ... command
called).

However, until then we shall? have port separation.

> 
> > 3. When somebody call "bridge ..." on it - then the in-switch
> > separation would be disabled. This is the "normal" state of
> > operation for L2 switch, which would be a HW accelerator for
> > bridging.
> > 
> > 4. The switchdev would be used to manage it
> > 
> > 5. This would be just a very simple driver - just bridging and
> > startup of the L2 switch.
> > 
> > After we would have a consensus (i.e. it would be pulled to
> > mainline) - I would proceed further.
> > 
> > I will try to not touch fec_main.c driver - just write standalone,
> > new for MoreThanIP L2 switch driver.  
> 
> It might make sense to refactor the MDIO code into a helper which both
> can share? No point duplicating that.

This is a latter step (common MDIO library code), IMHO. 

> 
> > If somebody would like to use FEC, then he will insert the proper
> > module. If switch, another one can be inserted, depending o the
> > target use case.  
> 
> This all seems like a reasonable way forward.

+1

> 
> MoreThanIP is now part of Synopsys. I wounder if this IP now exists in
> other SoCs? The press release however suggests Synopsys was
> interesting in the high speed interfaces, not a two ports Fast
> Ethernet switch.

I would need some detailed documentation....

> 
> 	Andrew




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

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

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Andrew Lunn]

> > Seems like a reasonable compromise. You would only load this driver if
> > you intend to make use of the switch...
> 
> Yes, the main use case would be the switch (after bridge ... command
> called).
> 
> However, until then we shall? have port separation.

Yes. The model Linux uses is that the ports are individual interfaces
to start with. We should keep to that model.

> > MoreThanIP is now part of Synopsys. I wounder if this IP now exists in
> > other SoCs? The press release however suggests Synopsys was
> > interesting in the high speed interfaces, not a two ports Fast
> > Ethernet switch.
> 
> I would need some detailed documentation....

Which is probably not available. You might be able to get some clues
from the Freescale datasheets, if they have kept the address spaces
separated. I don't see it as a strong requirement, given how old this
IP is, and the limited interest in supporting it over the years, My
guess is, nobody else uses it.

	Andrew

Re: [PATCH] arm: clk: Add ETH switch clock description for vf610 SoC

[Lukasz Majewski]

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

Hi Andrew,

> > > Seems like a reasonable compromise. You would only load this
> > > driver if you intend to make use of the switch...  
> > 
> > Yes, the main use case would be the switch (after bridge ... command
> > called).
> > 
> > However, until then we shall? have port separation.  
> 
> Yes. The model Linux uses is that the ports are individual interfaces
> to start with. We should keep to that model.

+1

> 
> > > MoreThanIP is now part of Synopsys. I wounder if this IP now
> > > exists in other SoCs? The press release however suggests Synopsys
> > > was interesting in the high speed interfaces, not a two ports Fast
> > > Ethernet switch.  
> > 
> > I would need some detailed documentation....  
> 
> Which is probably not available. You might be able to get some clues
> from the Freescale datasheets, if they have kept the address spaces
> separated. I don't see it as a strong requirement, given how old this
> IP is, and the limited interest in supporting it over the years, My
> guess is, nobody else uses it.

:-)

> 
> 	Andrew




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

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