Re: [RFC 2/3] net: Provide switchdev driver for NXP's More Than IP L2 switch

[Lukasz Majewski]

Hi Vladimir,

> On Mon, Jun 28, 2021 at 02:05:26PM +0200, Lukasz Majewski wrote:
> > Hi Andrew,
> >  
> > > > I do believe that I can just extend the L2 switch driver
> > > > (fec_mtip.c file to be precise) to provide full blown L2 switch
> > > > functionality without touching the legacy FEC more than in this
> > > > patch set.
> > > >
> > > > Would you consider applying this patch series then?  
> > >
> > > What is most important is the ABI. If something is merged now, we
> > > need to ensure it does not block later refactoring to a clean new
> > > driver. The DT binding is considered ABI. So the DT binding needs
> > > to be like a traditional switchdev driver. Florian already
> > > pointed out, you can use a binding very similar to DSA.
> > > ti,cpsw-switch.yaml is another good example.  
> >
> > The best I could get would be:
> >
> > &eth_switch {
> > 	compatible = "imx,mtip-l2switch";
> > 	reg = <0x800f8000 0x400>, <0x800fC000 0x4000>;
> >
> > 	interrupts = <100>;
> > 	status = "okay";
> >
> > 	ethernet-ports {
> > 		port1@1 {
> > 			reg = <1>;
> > 			label = "eth0";
> > 			phys = <&mac0 0>;
> > 		};
> >
> > 		port2@2 {
> > 			reg = <2>;
> > 			label = "eth1";
> > 			phys = <&mac1 1>;
> > 		};
> > 	};
> > };
> >
> > Which would abuse the "phys" properties usages - as 'mac[01]' are
> > referring to ethernet controllers.
> >
> > On TI SoCs (e.g. am33xx-l4.dtsi) phys refer to some separate driver
> > responsible for PHY management. On NXP this is integrated with FEC
> > driver itself.  
> 
> If we were really honest, the binding would need to be called
> 
> port@0 {
> 	puppet = <&mac0>;
> };
> 
> port@1 {
> 	puppet = <&mac1>;
> };
> 
> which speaks for itself as to why accepting "puppet master" drivers is
> not really very compelling. I concur with the recommendation given by
> Andrew and Florian to refactor FEC as a multi-port single driver.

Ok.

> 
> > >
> > > So before considering merging your changes, i would like to see a
> > > usable binding.
> > >
> > > I also don't remember seeing support for STP. Without that, your
> > > network has broadcast storm problems when there are loops. So i
> > > would like to see the code needed to put ports into blocking,
> > > listening, learning, and forwarding states.
> > >
> > > 	  Andrew  
> 
> I cannot stress enough how important it is for us to see STP support
> and consequently the ndo_start_xmit procedure for switch ports.

Ok.

> Let me see if I understand correctly. When the switch is enabled, eth0
> sends packets towards both physical switch ports, and eth1 sends
> packets towards none, but eth0 handles the link state of switch port
> 0, and eth1 handles the link state of switch port 1?

Exactly, this is how FEC driver is utilized for this switch. 


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@xxxxxxx

Attachment: pgpI_aZV9trO0.pgp
Description: OpenPGP digital signature