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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Apr 2023 18:44:20 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Arun Ramadoss <arun.ramadoss@...rochip.com>,
        kernel@...gutronix.de, UNGLinuxDriver@...rochip.com,
        netdev@...r.kernel.org
Subject: Re: FWD: Re: [PATCH net-next v1 1/1] net: dsa: microchip: ksz8: Make
 flow control, speed, and duplex on CPU port configurable

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!