netdev - Re: [net PATCH 1/2] net: dsa: qca8k: fix internal delay applied to the wrong PAD config

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 21 Nov 2021 19:22:00 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan McDowell <noodles@...th.li>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [net PATCH 1/2] net: dsa: qca8k: fix internal delay applied to
 the wrong PAD config

On Sun, Nov 21, 2021 at 08:18:29PM +0200, Vladimir Oltean wrote:
> On Fri, Nov 19, 2021 at 03:03:49AM +0100, Ansuel Smith wrote:
> > With SGMII phy the internal delay is always applied to the PAD0 config.
> > This is caused by the falling edge configuration that hardcode the reg
> > to PAD0 (as the falling edge bits are present only in PAD0 reg)
> > Move the delay configuration before the reg overwrite to correctly apply
> > the delay.
> > 
> > Fixes: cef08115846e ("net: dsa: qca8k: set internal delay also for sgmii")
> > Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> > ---
> 
> This removes the need for your other patch "net: dsa: qca8k: skip sgmii
> delay on double cpu conf", right?
>

Correct the problem was really with overwriting and not with some
malfunction with setting the delay to both rgmii and sgmii.
And also as you pointed out, the delay was ignored with sgmii phy mode
and delay not declared in the dts.

> >  drivers/net/dsa/qca8k.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> > index a429c9750add..d7bcecbc1c53 100644
> > --- a/drivers/net/dsa/qca8k.c
> > +++ b/drivers/net/dsa/qca8k.c
> > @@ -1433,6 +1433,12 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> >  
> >  		qca8k_write(priv, QCA8K_REG_SGMII_CTRL, val);
> >  
> > +		/* From original code is reported port instability as SGMII also
> > +		 * require delay set. Apply advised values here or take them from DT.
> > +		 */
> > +		if (state->interface == PHY_INTERFACE_MODE_SGMII)
> > +			qca8k_mac_config_setup_internal_delay(priv, cpu_port_index, reg);
> > +
> >  		/* For qca8327/qca8328/qca8334/qca8338 sgmii is unique and
> >  		 * falling edge is set writing in the PORT0 PAD reg
> >  		 */
> > @@ -1455,12 +1461,6 @@ qca8k_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> >  					QCA8K_PORT0_PAD_SGMII_TXCLK_FALLING_EDGE,
> >  					val);
> >  
> > -		/* From original code is reported port instability as SGMII also
> > -		 * require delay set. Apply advised values here or take them from DT.
> > -		 */
> > -		if (state->interface == PHY_INTERFACE_MODE_SGMII)
> > -			qca8k_mac_config_setup_internal_delay(priv, cpu_port_index, reg);
> > -
> >  		break;
> >  	default:
> >  		dev_err(ds->dev, "xMII mode %s not supported for port %d\n",
> > -- 
> > 2.32.0
> > 

-- 
	Ansuel