Re: FWD: Re: [PATCH net-next v1 1/1] net: dsa: microchip: ksz8: Make flow control, speed, and duplex on CPU port configurable

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Fri, Apr 07, 2023 at 04:25:57PM +0200, Andrew Lunn wrote:
> Hi Russell
> 
> It looks like you were not Cc:ed.

Thanks Andrew.

It really would help if people Cc'd the right folk! Because they failed
to, sorry, this isn't going to be a reply that's threaded properly...

> > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> > index d0e3f6e2db1d..8917f22f90d2 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1321,12 +1321,52 @@ void ksz8_config_cpu_port(struct dsa_switch *ds)
> >  			if (remote & KSZ8_PORT_FIBER_MODE)
> >  				p->fiber = 1;
> >  		}
> > -		if (p->fiber)
> > -			ksz_port_cfg(dev, i, regs[P_STP_CTRL],
> > -				     PORT_FORCE_FLOW_CTRL, true);
> > -		else
> > -			ksz_port_cfg(dev, i, regs[P_STP_CTRL],
> > -				     PORT_FORCE_FLOW_CTRL, false);
> > +	}
> > +}
> > +
> > +void ksz8_phylink_mac_link_up(struct ksz_device *dev, int port,
> > +			      unsigned int mode, phy_interface_t interface,
> > +			      struct phy_device *phydev, int speed, int duplex,
> > +			      bool tx_pause, bool rx_pause)
> > +{
> > +	struct dsa_switch *ds = dev->ds;
> > +	struct ksz_port *p;
> > +	u8 ctrl = 0;
> > +
> > +	p = &dev->ports[port];
> > +
> > +	if (dsa_upstream_port(ds, port)) {
> > +		u8 mask = SW_HALF_DUPLEX_FLOW_CTRL | SW_HALF_DUPLEX |
> > +			SW_FLOW_CTRL | SW_10_MBIT;
> > +
> > +		if (duplex) {
> > +			if (tx_pause && rx_pause)
> > +				ctrl |= SW_FLOW_CTRL;
> > +		} else {
> > +			ctrl |= SW_HALF_DUPLEX;
> > +			if (tx_pause && rx_pause)
> > +				ctrl |= SW_HALF_DUPLEX_FLOW_CTRL;
> > +		}
> > +
> > +		if (speed == SPEED_10)
> > +			ctrl |= SW_10_MBIT;
> > +
> > +		ksz_rmw8(dev, REG_SW_CTRL_4, mask, ctrl);
> > +
> > +		p->phydev.speed = speed;
> > +	} else {
> > +		const u16 *regs = dev->info->regs;
> > +
> > +		if (duplex) {
> > +			if (tx_pause && rx_pause)
> > +				ctrl |= PORT_FORCE_FLOW_CTRL;
> > +		} else {
> > +			if (tx_pause && rx_pause)
> > +				ctrl |= PORT_BACK_PRESSURE;
> > +		}
> > +
> > +		ksz_rmw8(dev, regs[P_STP_CTRL], PORT_FORCE_FLOW_CTRL |
> > +			 PORT_BACK_PRESSURE, ctrl);

So, I guess the idea here is to enable some form of flow control when
both tx and rx pause are enabled.

Here's a bunch of questions I would like answered before I give a tag:

1) It looks like the device only supports symmetric pause?
2) If yes, are you *not* providing MAC_ASYM_PAUSE in the MAC
   capabilities? If not, why not?
3) If yes, then please do as others do and use tx_pause || rx_pause
   here.

Lastly, a more general question for ethtool/network folk - as for half
duplex and back pressure, is that a recognised facility for the MAC
to control via the ethtool pause parameter API?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!