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]
Message-ID: <YJHvw3AZkzJSiqov@Ansuel-xps.localdomain>
Date:   Wed, 5 May 2021 03:07:15 +0200
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH net-next v3 13/20] net: dsa: qca8k: make rgmii delay
 configurable

On Wed, May 05, 2021 at 03:00:45AM +0200, Andrew Lunn wrote:
> On Wed, May 05, 2021 at 12:29:07AM +0200, Ansuel Smith wrote:
> > The legacy qsdk code used a different delay instead of the max value.
> > Qsdk use 1 ps for rx and 2 ps for tx. Make these values configurable
> > using the standard rx/tx-internal-delay-ps ethernet binding and apply
> > qsdk values by default. The connected gmac doesn't add any delay so no
> > additional delay is added to tx/rx.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@...il.com>
> > ---
> >  drivers/net/dsa/qca8k.c | 51 +++++++++++++++++++++++++++++++++++++++--
> >  drivers/net/dsa/qca8k.h | 11 +++++----
> >  2 files changed, 55 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> > index 22334d416f53..cb9b44769e92 100644
> > --- a/drivers/net/dsa/qca8k.c
> > +++ b/drivers/net/dsa/qca8k.c
> > @@ -779,6 +779,47 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
> >  	return 0;
> >  }
> >  
> > +static int
> > +qca8k_setup_of_rgmii_delay(struct qca8k_priv *priv)
> > +{
> > +	struct device_node *ports, *port;
> > +	u32 val;
> > +
> > +	ports = of_get_child_by_name(priv->dev->of_node, "ports");
> > +	if (!ports)
> > +		return -EINVAL;
> > +
> > +	/* Assume only one port with rgmii-id mode */
> > +	for_each_available_child_of_node(ports, port) {
> 
> Are delays global? Or per port? They really should be per port.  If it
> is global, one value that applies to all ports, i would not use
> it. Have the PHY apply the delay, not the MAC.
> 
>     Andrew

It's expected to set the delay in the port node. But yes the value is
global and assigned to every port (in reality this is only assigned to
the unique rgmii cpu port). The switch has only one rgmii port
and one sgmii, that's why I skipped any logic about handling more than
one case with internal delay. If you want I can try to add some logic to
handle this value per port. But again on the switch there is only one
reg to set the delay and the qca8k_phylink_mac_config function use this
only for the rgmii cpu port.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ