netdev - Re: [PATCH net-next 06/12] net: dsa: rzn1-a5psw: add Renesas RZ/N1 advanced 5 port switch driver

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: <20220415143349.652e9958@fixe.home>
Date:   Fri, 15 Apr 2022 14:33:49 +0200
From:   Clément Léger <clement.leger@...tlin.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Magnus Damm <magnus.damm@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Herve Codina <herve.codina@...tlin.com>,
        Miquèl Raynal <miquel.raynal@...tlin.com>,
        Milan Stevanovic <milan.stevanovic@...com>,
        Jimmy Lalande <jimmy.lalande@...com>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org, netdev@...r.kernel.org,
        Laurent Gonzales <laurent.gonzales@....se.com>,
        Jean-Pierre Geslin <jean-pierre.geslin@....se.com>,
        Phil Edworthy <phil.edworthy@...esas.com>
Subject: Re: [PATCH net-next 06/12] net: dsa: rzn1-a5psw: add Renesas RZ/N1
 advanced 5 port switch driver

Le Thu, 14 Apr 2022 19:55:55 +0200,
Andrew Lunn <andrew@...n.ch> a écrit :

>  +static int a5psw_mdio_reset(struct mii_bus *bus)
> > +{
> > +	struct a5psw *a5psw = bus->priv;
> > +	unsigned long rate;
> > +	unsigned long div;
> > +	u32 cfgstatus;
> > +
> > +	rate = clk_get_rate(a5psw->hclk);
> > +	div = ((rate / a5psw->mdio_freq) / 2);
> > +	if (div >= 511 || div <= 5) {
> > +		dev_err(a5psw->dev, "MDIO clock div %ld out of range\n", div);
> > +		return -ERANGE;
> > +	}
> > +
> > +	cfgstatus = FIELD_PREP(A5PSW_MDIO_CFG_STATUS_CLKDIV, div);
> > +
> > +	a5psw_reg_writel(a5psw, A5PSW_MDIO_CFG_STATUS, cfgstatus);  
> 
> I don't see anything here which does an actual reset. So i think this
> function has the wrong name. Please also pass the frequency as a
> parameter, because at a quick glance it was not easy to see where it
> was used. There does not seem to be any need to store it in a5psw.

Indeed, the reset callback can be removed entirely and the mdio bus
could be setup directly from a5psw_probe_mdio().

> 
> > +static int a5psw_probe_mdio(struct a5psw *a5psw)
> > +{
> > +	struct device *dev = a5psw->dev;
> > +	struct device_node *mdio_node;
> > +	struct mii_bus *bus;
> > +	int err;
> > +
> > +	if (of_property_read_u32(dev->of_node, "clock-frequency",
> > +				 &a5psw->mdio_freq))
> > +		a5psw->mdio_freq = A5PSW_MDIO_DEF_FREQ;
> > +
> > +	bus = devm_mdiobus_alloc(dev);
> > +	if (!bus)
> > +		return -ENOMEM;
> > +
> > +	bus->name = "a5psw_mdio";
> > +	bus->read = a5psw_mdio_read;
> > +	bus->write = a5psw_mdio_write;
> > +	bus->reset = a5psw_mdio_reset;  
> 
> As far as i can see, the read and write functions don't support
> C45. Please return -EOPNOTSUPP if they are passed C45 addresses.

Ok.

> 
>      Andrew



-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com