diff mbox series

[RFC,1/3] ARM: dts: imx28: Add description for L2 switch on XEA board

Message ID 20210622144111.19647-2-lukma@denx.de
State New
Headers show
Series net: imx: Provide support for L2 switch as switchdev accelerator | expand

Commit Message

Lukasz Majewski June 22, 2021, 2:41 p.m. UTC
The 'eth_switch' node is now extendfed to enable support for L2
switch.

Moreover, the mac[01] nodes are defined as well and linked to the
former with 'phy-handle' property.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 arch/arm/boot/dts/imx28-xea.dts | 42 +++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

Comments

Lukasz Majewski June 23, 2021, 3:26 p.m. UTC | #1
Hi Andrew,

> On Tue, Jun 22, 2021 at 10:51:34PM +0200, Lukasz Majewski wrote:
> > Hi Andrew,
> > 
> > > On Tue, Jun 22, 2021 at 04:41:09PM +0200, Lukasz Majewski wrote:
> > > > The 'eth_switch' node is now extendfed to enable support for L2
> > > > switch.
> > > > 
> > > > Moreover, the mac[01] nodes are defined as well and linked to
> > > > the former with 'phy-handle' property.  
> > > 
> > > A phy-handle points to a phy, not a MAC! Don't abuse a well known
> > > DT property like this.
> > 
> > Ach.... You are right. I will change it.
> > 
> > Probably 'ethernet' property or 'link' will fit better?
> 
> You should first work on the overall architecture. I suspect you will
> end up with something more like the DSA binding, and not have the FEC
> nodes at all. Maybe the MDIO busses will appear under the switch?
> 
> Please don't put minimal changes to the FEC driver has your first
> goal. We want an architecture which is similar to other switchdev
> drivers. Maybe look at drivers/net/ethernet/ti/cpsw_new.c.

I'm a bit confused - as I thought that with switchdev API I could just
extend the current FEC driver to add bridge offload.
This patch series shows that it is doable with little changes
introduced.

However, now it looks like I would need to replace FEC driver and
rewrite it in a way similar to cpsw_new.c, so the switchdev could be
used for both cases - with and without L2 switch offload.

This would be probably conceptually correct, but i.MX FEC driver has
several issues to tackle:

- On some SoCs (vf610, imx287, etc.) the ENET-MAC ports don't have the
  same capabilities (eth1 is a bit special)

- Without switch we need to use DMA0 and DMA1 in the "bypass" switch
  mode (default). When switch is enabled we only use DMA0. The former
  case is best fitted with FEC driver instantiation. The latter with
  DSA or switchdev.

> The cpsw
> driver has an interesting past, it did things the wrong way for a long
> time, but the new switchdev driver has an architecture similar to what
> the FEC driver could be like.
> 
> 	Andrew

Maybe somebody from NXP can provide input to this discussion - for
example to sched some light on FEC driver (near) future.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Florian Fainelli June 24, 2021, 12:36 a.m. UTC | #2
On 6/23/2021 8:26 AM, Lukasz Majewski wrote:
> Hi Andrew,

> 

>> On Tue, Jun 22, 2021 at 10:51:34PM +0200, Lukasz Majewski wrote:

>>> Hi Andrew,

>>>

>>>> On Tue, Jun 22, 2021 at 04:41:09PM +0200, Lukasz Majewski wrote:

>>>>> The 'eth_switch' node is now extendfed to enable support for L2

>>>>> switch.

>>>>>

>>>>> Moreover, the mac[01] nodes are defined as well and linked to

>>>>> the former with 'phy-handle' property.

>>>>

>>>> A phy-handle points to a phy, not a MAC! Don't abuse a well known

>>>> DT property like this.

>>>

>>> Ach.... You are right. I will change it.

>>>

>>> Probably 'ethernet' property or 'link' will fit better?

>>

>> You should first work on the overall architecture. I suspect you will

>> end up with something more like the DSA binding, and not have the FEC

>> nodes at all. Maybe the MDIO busses will appear under the switch?

>>

>> Please don't put minimal changes to the FEC driver has your first

>> goal. We want an architecture which is similar to other switchdev

>> drivers. Maybe look at drivers/net/ethernet/ti/cpsw_new.c.

> 

> I'm a bit confused - as I thought that with switchdev API I could just

> extend the current FEC driver to add bridge offload.

> This patch series shows that it is doable with little changes

> introduced.


Regardless of how you end up implementing the switching part in the 
driver, one thing that you can use is the same DT binding as what DSA 
uses as far as representing ports of the Ethernet controller. That means 
that ports should ideally be embedded into an 'ethernet-ports' container 
node, and you describe each port individually as sub-nodes and provide, 
when appropriate 'phy-handle' and 'phy-mode' properties to describe how 
the Ethernet PHYs are connected.

> 

> However, now it looks like I would need to replace FEC driver and

> rewrite it in a way similar to cpsw_new.c, so the switchdev could be

> used for both cases - with and without L2 switch offload.

> 

> This would be probably conceptually correct, but i.MX FEC driver has

> several issues to tackle:

> 

> - On some SoCs (vf610, imx287, etc.) the ENET-MAC ports don't have the

>    same capabilities (eth1 is a bit special)

> 

> - Without switch we need to use DMA0 and DMA1 in the "bypass" switch

>    mode (default). When switch is enabled we only use DMA0. The former

>    case is best fitted with FEC driver instantiation. The latter with

>    DSA or switchdev.

> 

>> The cpsw

>> driver has an interesting past, it did things the wrong way for a long

>> time, but the new switchdev driver has an architecture similar to what

>> the FEC driver could be like.

>>

>> 	Andrew

> 

> Maybe somebody from NXP can provide input to this discussion - for

> example to sched some light on FEC driver (near) future.


Seems like some folks at NXP are focusing on the STMMAC controller these 
days (dwmac from Synopsys), so maybe they have given up on having their 
own Ethernet MAC for lower end products.
-- 
Florian
Joakim Zhang June 24, 2021, 2:19 a.m. UTC | #3
Hi Lukasz, Florian, Andrew,

> > Maybe somebody from NXP can provide input to this discussion - for

> > example to sched some light on FEC driver (near) future.

> 

> Seems like some folks at NXP are focusing on the STMMAC controller these

> days (dwmac from Synopsys), so maybe they have given up on having their own

> Ethernet MAC for lower end products.


I am very happy to take participate into this topic, but now I have no experience to DSA and i.MX28 MAC,
so I may need some time to increase these knowledge, limited insight could be put to now.

Florian, Andrew could comment more and I also can learn from it :-), they are all very experienced expert.

We also want to maintain FEC driver since many SoCs implemented this IP, and as I know we would also
use it for future SoCs.
  
Best Regards,
Joakim Zhang
Lukasz Majewski June 24, 2021, 11:03 a.m. UTC | #4
On Wed, 23 Jun 2021 17:36:59 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 6/23/2021 8:26 AM, Lukasz Majewski wrote:

> > Hi Andrew,

> >   

> >> On Tue, Jun 22, 2021 at 10:51:34PM +0200, Lukasz Majewski wrote:  

> >>> Hi Andrew,

> >>>  

> >>>> On Tue, Jun 22, 2021 at 04:41:09PM +0200, Lukasz Majewski wrote:

> >>>>  

> >>>>> The 'eth_switch' node is now extendfed to enable support for L2

> >>>>> switch.

> >>>>>

> >>>>> Moreover, the mac[01] nodes are defined as well and linked to

> >>>>> the former with 'phy-handle' property.  

> >>>>

> >>>> A phy-handle points to a phy, not a MAC! Don't abuse a well known

> >>>> DT property like this.  

> >>>

> >>> Ach.... You are right. I will change it.

> >>>

> >>> Probably 'ethernet' property or 'link' will fit better?  

> >>

> >> You should first work on the overall architecture. I suspect you

> >> will end up with something more like the DSA binding, and not have

> >> the FEC nodes at all. Maybe the MDIO busses will appear under the

> >> switch?

> >>

> >> Please don't put minimal changes to the FEC driver has your first

> >> goal. We want an architecture which is similar to other switchdev

> >> drivers. Maybe look at drivers/net/ethernet/ti/cpsw_new.c.  

> > 

> > I'm a bit confused - as I thought that with switchdev API I could

> > just extend the current FEC driver to add bridge offload.

> > This patch series shows that it is doable with little changes

> > introduced.  

> 

> Regardless of how you end up implementing the switching part in the 

> driver, one thing that you can use is the same DT binding as what DSA 

> uses as far as representing ports of the Ethernet controller. That

> means that ports should ideally be embedded into an 'ethernet-ports'

> container node, and you describe each port individually as sub-nodes

> and provide, when appropriate 'phy-handle' and 'phy-mode' properties

> to describe how the Ethernet PHYs are connected.


I see. Thanks for the explanation.

> 

> > 

> > However, now it looks like I would need to replace FEC driver and

> > rewrite it in a way similar to cpsw_new.c, so the switchdev could be

> > used for both cases - with and without L2 switch offload.

> > 

> > This would be probably conceptually correct, but i.MX FEC driver has

> > several issues to tackle:

> > 

> > - On some SoCs (vf610, imx287, etc.) the ENET-MAC ports don't have

> > the same capabilities (eth1 is a bit special)

> > 

> > - Without switch we need to use DMA0 and DMA1 in the "bypass" switch

> >    mode (default). When switch is enabled we only use DMA0. The

> > former case is best fitted with FEC driver instantiation. The

> > latter with DSA or switchdev.

> >   

> >> The cpsw

> >> driver has an interesting past, it did things the wrong way for a

> >> long time, but the new switchdev driver has an architecture

> >> similar to what the FEC driver could be like.

> >>

> >> 	Andrew  

> > 

> > Maybe somebody from NXP can provide input to this discussion - for

> > example to sched some light on FEC driver (near) future.  

> 

> Seems like some folks at NXP are focusing on the STMMAC controller

> these days (dwmac from Synopsys), so maybe they have given up on

> having their own Ethernet MAC for lower end products.


For example, the imx287 SoC is still active product and is supposed
to be produced for at least 15 years after release. 

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski June 24, 2021, 11:21 a.m. UTC | #5
Hi Joakim,

> Hi Lukasz, Florian, Andrew,

> 

> > > Maybe somebody from NXP can provide input to this discussion - for

> > > example to sched some light on FEC driver (near) future.  

> > 

> > Seems like some folks at NXP are focusing on the STMMAC controller

> > these days (dwmac from Synopsys), so maybe they have given up on

> > having their own Ethernet MAC for lower end products.  

> 

> I am very happy to take participate into this topic, but now I have

> no experience to DSA and i.MX28 MAC, so I may need some time to

> increase these knowledge, limited insight could be put to now.


Ok. No problem :-)

> 

> Florian, Andrew could comment more and I also can learn from it :-),

> they are all very experienced expert.


The main purpose of several RFCs for the L2 switch drivers (for DSA [1]
and switchdev [2]) was to gain feedback from community as soon as
possible (despite that the driver lacks some features - like VLAN, FDB,
etc).

> 

> We also want to maintain FEC driver since many SoCs implemented this

> IP, and as I know we would also use it for future SoCs.

>   


Florian, Andrew, please correct me if I'm wrong, but my impression is
that upstreaming the support for L2 switch on iMX depends on FEC driver
being rewritten to support switchdev?

If yes, then unfortunately, I don't have time and resources to perform
that task - that is why I have asked if NXP has any plans to update the
FEC (fec_main.c) driver.


Joakim, do you have any plans to re-factor the legacy FEC driver
(fec_main.c) and introduce new one, which would support the switchdev?

If NXP is not planning to update the driver, then maybe it would be
worth to consider adding driver from [2] to mainline? Then I could
finish it and provide all required features.


Links:
[1] -
https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-upstream-DSA-RFC_v1
[2] -
https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-upstream-switchdev-RFC_v1

> Best Regards,

> Joakim Zhang





Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Joakim Zhang June 25, 2021, 8:28 a.m. UTC | #6
Hi Lukasz,

> -----Original Message-----

> From: Lukasz Majewski <lukma@denx.de>

> Sent: 2021年6月24日 19:21

> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Florian Fainelli

> <f.fainelli@gmail.com>; Andrew Lunn <andrew@lunn.ch>

> Cc: David S . Miller <davem@davemloft.net>; Jakub Kicinski

> <kuba@kernel.org>; Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;

> Nicolas Ferre <nicolas.ferre@microchip.com>; Vladimir Oltean

> <olteanv@gmail.com>; netdev@vger.kernel.org; Arnd Bergmann

> <arnd@arndb.de>; Mark Einon <mark.einon@gmail.com>; dl-linux-imx

> <linux-imx@nxp.com>; linux-kernel@vger.kernel.org

> Subject: Re: [RFC 1/3] ARM: dts: imx28: Add description for L2 switch on XEA

> board

> 

> Hi Joakim,

> 

> > Hi Lukasz, Florian, Andrew,

> >

> > > > Maybe somebody from NXP can provide input to this discussion - for

> > > > example to sched some light on FEC driver (near) future.

> > >

> > > Seems like some folks at NXP are focusing on the STMMAC controller

> > > these days (dwmac from Synopsys), so maybe they have given up on

> > > having their own Ethernet MAC for lower end products.

> >

> > I am very happy to take participate into this topic, but now I have no

> > experience to DSA and i.MX28 MAC, so I may need some time to increase

> > these knowledge, limited insight could be put to now.

> 

> Ok. No problem :-)

> 

> >

> > Florian, Andrew could comment more and I also can learn from it :-),

> > they are all very experienced expert.

> 

> The main purpose of several RFCs for the L2 switch drivers (for DSA [1] and

> switchdev [2]) was to gain feedback from community as soon as possible

> (despite that the driver lacks some features - like VLAN, FDB, etc).

> 

> >

> > We also want to maintain FEC driver since many SoCs implemented this

> > IP, and as I know we would also use it for future SoCs.

> >

> 

> Florian, Andrew, please correct me if I'm wrong, but my impression is that

> upstreaming the support for L2 switch on iMX depends on FEC driver being

> rewritten to support switchdev?

> 

> If yes, then unfortunately, I don't have time and resources to perform that task

> - that is why I have asked if NXP has any plans to update the FEC (fec_main.c)

> driver.

> 

> 

> Joakim, do you have any plans to re-factor the legacy FEC driver

> (fec_main.c) and introduce new one, which would support the switchdev?

> 

> If NXP is not planning to update the driver, then maybe it would be worth to

> consider adding driver from [2] to mainline? Then I could finish it and provide all

> required features.


I don't have such plan now, and have no confidence to re-factor the legacy FEC driver and introduce new one,
which to support switchdev in a short time. I am not very experienced for FEC driver, since I have just maintained
it for half a year. To be honest, I have no idea in my head right now, we even don't have i.MX28 boards.
I'm so sorry about this, but I am also interested in it, I am finding time to increase related knowledge.

Best Regards,
Joakim Zhang
> 

> Links:

> [1] -

> https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-u

> pstream-DSA-RFC_v1

> [2] -

> https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-u

> pstream-switchdev-RFC_v1

> 

> > Best Regards,

> > Joakim Zhang

> 

> 

> 

> 

> Best regards,

> 

> Lukasz Majewski

> 

> --

> 

> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk

> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Lukasz Majewski June 25, 2021, 10:18 a.m. UTC | #7
Hi Joakim, Andrew,

> Hi Lukasz,

> 

> > -----Original Message-----

> > From: Lukasz Majewski <lukma@denx.de>

> > Sent: 2021年6月24日 19:21

> > To: Joakim Zhang <qiangqing.zhang@nxp.com>; Florian Fainelli

> > <f.fainelli@gmail.com>; Andrew Lunn <andrew@lunn.ch>

> > Cc: David S . Miller <davem@davemloft.net>; Jakub Kicinski

> > <kuba@kernel.org>; Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>;

> > Nicolas Ferre <nicolas.ferre@microchip.com>; Vladimir Oltean

> > <olteanv@gmail.com>; netdev@vger.kernel.org; Arnd Bergmann

> > <arnd@arndb.de>; Mark Einon <mark.einon@gmail.com>; dl-linux-imx

> > <linux-imx@nxp.com>; linux-kernel@vger.kernel.org

> > Subject: Re: [RFC 1/3] ARM: dts: imx28: Add description for L2

> > switch on XEA board

> > 

> > Hi Joakim,

> >   

> > > Hi Lukasz, Florian, Andrew,

> > >  

> > > > > Maybe somebody from NXP can provide input to this discussion

> > > > > - for example to sched some light on FEC driver (near)

> > > > > future.  

> > > >

> > > > Seems like some folks at NXP are focusing on the STMMAC

> > > > controller these days (dwmac from Synopsys), so maybe they have

> > > > given up on having their own Ethernet MAC for lower end

> > > > products.  

> > >

> > > I am very happy to take participate into this topic, but now I

> > > have no experience to DSA and i.MX28 MAC, so I may need some time

> > > to increase these knowledge, limited insight could be put to now.

> > >  

> > 

> > Ok. No problem :-)

> >   

> > >

> > > Florian, Andrew could comment more and I also can learn from it

> > > :-), they are all very experienced expert.  

> > 

> > The main purpose of several RFCs for the L2 switch drivers (for DSA

> > [1] and switchdev [2]) was to gain feedback from community as soon

> > as possible (despite that the driver lacks some features - like

> > VLAN, FDB, etc). 

> > >

> > > We also want to maintain FEC driver since many SoCs implemented

> > > this IP, and as I know we would also use it for future SoCs.

> > >  

> > 

> > Florian, Andrew, please correct me if I'm wrong, but my impression

> > is that upstreaming the support for L2 switch on iMX depends on FEC

> > driver being rewritten to support switchdev?

> > 

> > If yes, then unfortunately, I don't have time and resources to

> > perform that task

> > - that is why I have asked if NXP has any plans to update the FEC

> > (fec_main.c) driver.

> > 

> > 

> > Joakim, do you have any plans to re-factor the legacy FEC driver

> > (fec_main.c) and introduce new one, which would support the

> > switchdev?

> > 

> > If NXP is not planning to update the driver, then maybe it would be

> > worth to consider adding driver from [2] to mainline? Then I could

> > finish it and provide all required features.  

> 

> I don't have such plan now, and have no confidence to re-factor the

> legacy FEC driver and introduce new one, which to support switchdev

> in a short time. 


Thanks for the clear statement, appreciated.

> I am not very experienced for FEC driver, since I

> have just maintained it for half a year. 


Ok. No problem.

> To be honest, I have no idea

> in my head right now, we even don't have i.MX28 boards.


As fair as I remember there is still imx28-dev board available for
purchase. You can also use vf610 based board.

> I'm so sorry

> about this, but I am also interested in it, I am finding time to

> increase related knowledge.


Ok.

To sum up:

- The FEC driver (legacy one) will not be rewritten anytime soon
  (maybe any other community member will work on this sooner...)

- Considering the above, support for L2 switch on imx28, vf610 is
  blocked [*]. As a result some essential functionality for still
  actively used SoCs is going to be maintained out of tree (for example
  [1][2]). 

[*] - as I've stated in the other mail - what's about the situation
where FEC legacy driver is not going to be excessively modified (just
changes from this patch set)?

Links:

[1] -
https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-upstream-switchdev-RFC_v1

[2] -
https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-upstream-DSA-RFC_v1

> 

> Best Regards,

> Joakim Zhang

> > 

> > Links:

> > [1] -

> > https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-u

> > pstream-DSA-RFC_v1

> > [2] -

> > https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-u

> > pstream-switchdev-RFC_v1

> >   

> > > Best Regards,

> > > Joakim Zhang  

> > 

> > 

> > 

> > 

> > Best regards,

> > 

> > Lukasz Majewski

> > 

> > --

> > 

> > DENX Software Engineering GmbH,      Managing Director: Wolfgang

> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,

> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:

> > lukma@denx.de  



Best regards,

Lukasz Majewski

--

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

Patch

diff --git a/arch/arm/boot/dts/imx28-xea.dts b/arch/arm/boot/dts/imx28-xea.dts
index d649822febed..94ff801669c4 100644
--- a/arch/arm/boot/dts/imx28-xea.dts
+++ b/arch/arm/boot/dts/imx28-xea.dts
@@ -23,6 +23,48 @@ 
 	status = "okay";
 };
 
+&mac0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mac0_pins_a>;
+	phy-mode = "rmii";
+	phy-supply = <&reg_fec_3v3>;
+	phy-reset-gpios = <&gpio2 13 0>;
+	phy-reset-duration = <100>;
+	local-mac-address = [ 00 11 22 AA BB CC ];
+	status = "okay";
+};
+
+&mac1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mac1_pins_a>;
+	phy-mode = "rmii";
+	phy-supply = <&reg_fec_3v3>;
+	local-mac-address = [ 00 11 22 AA BB DD ];
+	status = "okay";
+};
+
+&eth_switch {
+	compatible = "imx,mtip-l2switch";
+	reg = <0x800f8000 0x400>, <0x800fC000 0x4000>,  <0x800f8400 0x400>;
+
+	interrupts = <100>;
+	status = "okay";
+
+	ports {
+		port1@1 {
+			reg = <1>;
+			label = "eth0";
+			phy-handle = <&mac0>;
+		};
+
+		port2@2 {
+			reg = <2>;
+			label = "eth1";
+			phy-handle = <&mac1>;
+		};
+	};
+};
+
 &pinctrl {
 	pinctrl-names = "default";
 	pinctrl-0 = <&hog_pins_a &hog_pins_tiva>;