lkml.org 
[lkml]   [2021]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC 2/3] net: Provide switchdev driver for NXP's More Than IP L2 switch
Hi Andrew,

> > > Using volatile is generally wrong. Why do you need it?
> >
> > This was the code, which I took from the legacy driver. I will
> > adjust it.
>
> It is called 'vendor crap' for a reason.

:-)

>
> > > > + for_each_available_child_of_node(p, port) {
> > > > + if (of_property_read_u32(port, "reg",
> > > > &port_num))
> > > > + continue;
> > > > +
> > > > + priv->n_ports = port_num;
> > > > +
> > > > + fep_np = of_parse_phandle(port, "phy-handle",
> > > > 0);
> > >
> > > As i said, phy-handle points to a phy. It minimum, you need to
> > > call this mac-handle. But that then makes this switch driver very
> > > different to every other switch driver.
> >
> > Other drivers (DSA for example) use "ethernet" or "link" properties.
> > Maybe those can be reused?
>
> Not really. They have well known meanings and they are nothing like
> what you are trying to do. You need a new name. Maybe 'mac-handle'?

Ok.

>
>
> > > > + pdev = of_find_device_by_node(fep_np);
> > > > + ndev = platform_get_drvdata(pdev);
> > > > + priv->fep[port_num - 1] = netdev_priv(ndev);
> > > >
> > >
> > > What happens when somebody puts reg=<42>; in DT?
> >
> > I do guess that this will break the code.
> >
> > However, DSA DT descriptions also rely on the exact numbering [1]
> > (via e.g. reg property) of the ports. I've followed this paradigm.
>
> DSA does a range check:
>
> for_each_available_child_of_node(ports, port) {
> err = of_property_read_u32(port, "reg", &reg);
> if (err)
> goto out_put_node;
>
> if (reg >= ds->num_ports) {
> dev_err(ds->dev, "port %pOF index %u exceeds
> num_ports (%zu)\n", port, reg, ds->num_ports);
> err = -EINVAL;
> goto out_put_node;
> }
>

Ok.

> > > I would say, your basic structure needs to change, to make it more
> > > like other switchdev drivers. You need to replace the two FEC
> > > device instances with one switchdev driver.
> >
> > I've used the cpsw_new.c as the example.
> >
> > > The switchdev driver will then
> > > instantiate the two netdevs for the two external MACs.
> >
> > Then there is a question - what about eth[01], which already
> > exists?
>
> They don't exist. cpsw_new is not used at the same time as cpsw, it
> replaces it. This new driver would replace the FEC driver.

I see.

> cpsw_new
> makes use of some of the code in the cpsw driver to implement two
> netdevs. This new FEC switch driver would do the same, make use of
> some of the low level code, e.g. for DMA access, MDIO bus, etc.

I'm not sure if the imx28 switch is similar to one from TI (cpsw-3g)
- it looks to me that the bypass mode for both seems to be very
different. For example, on NXP when switch is disabled we need to
handle two DMA[01]. When it is enabled, only one is used. The approach
with two DMAs is best handled with FEC driver instantiation.

>
> > To be honest - such driver for L2 switch already has been forward
> > ported by me [2] to v4.19.
>
> Which is fine, you can do whatever you want in your own fork. But for
> mainline, we need a clean architecture.

This code is a forward port of vendor's (Freescale) old driver. It uses
the _wrong_ approach, but it can (still) be used in production after
some adjustments.

> I'm not convinced your code is
> that clean,

The code from [2] needs some vendor ioctl based tool (or hardcode) to
configure the switch.

> and how well future features can be added. Do you have
> support for VLANS? Adding and removing entries to the lookup tables?
> How will IGMP snooping work? How will STP work?

This can be easily added with serving netstack hooks (as it is already
done with cpsw_new) in the new switchdev based version [3] (based on
v5.12).

>
> Andrew

Links:

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


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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2021-06-24 12:53    [W:2.126 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site