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
| ||
|
Date: Mon, 19 Sep 2022 19:58:09 -0700 From: Colin Foster <colin.foster@...advantage.com> To: Vladimir Oltean <vladimir.oltean@....com> Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Russell King <linux@...linux.org.uk>, Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, "David S. Miller" <davem@...emloft.net>, Florian Fainelli <f.fainelli@...il.com>, Vivien Didelot <vivien.didelot@...il.com>, Andrew Lunn <andrew@...n.ch>, "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>, Alexandre Belloni <alexandre.belloni@...tlin.com>, Claudiu Manoil <claudiu.manoil@....com>, Lee Jones <lee@...nel.org> Subject: Re: [RFC v1 net-next 8/8] net: dsa: ocelot: add external ocelot switch control On Mon, Sep 12, 2022 at 05:21:10PM +0000, Vladimir Oltean wrote: > On Sun, Sep 11, 2022 at 01:02:44PM -0700, Colin Foster wrote: > > +#include <linux/iopoll.h> > > +#include <linux/mfd/ocelot.h> > > +#include <linux/phylink.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > +#include <soc/mscc/ocelot_ana.h> > > +#include <soc/mscc/ocelot_dev.h> > > +#include <soc/mscc/ocelot_qsys.h> > > +#include <soc/mscc/ocelot_vcap.h> > > +#include <soc/mscc/ocelot_ptp.h> > > +#include <soc/mscc/ocelot_sys.h> > > +#include <soc/mscc/ocelot.h> > > +#include <soc/mscc/vsc7514_regs.h> > > +#include "felix.h" > > + > > +#define VSC7512_NUM_PORTS 11 > > + > > +#define OCELOT_EXT_MEM_INIT_SLEEP_US 1000 > > +#define OCELOT_EXT_MEM_INIT_TIMEOUT_US 100000 > > + > > +#define OCELOT_EXT_PORT_MODE_SERDES (OCELOT_PORT_MODE_SGMII | \ > > + OCELOT_PORT_MODE_QSGMII) > > There are places where OCELOT_EXT doesn't make too much sense, like here. > The capabilities of the SERDES ports do not change depending on whether > the switch is controlled externally or not. Same for the memory init > delays. Maybe OCELOT_MEM_INIT_*, OCELOT_PORT_MODE_SERDES etc? > > There are more places as well below in function names, I'll let you be > the judge if whether ocelot is controlled externally is relevant to what > they do in any way. > > > +static int ocelot_ext_reset(struct ocelot *ocelot) > > +{ > > + int err, val; > > + > > + ocelot_ext_reset_phys(ocelot); > > + > > + /* Initialize chip memories */ > > + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_ENA], 1); > > + if (err) > > + return err; > > + > > + err = regmap_field_write(ocelot->regfields[SYS_RESET_CFG_MEM_INIT], 1); > > + if (err) > > + return err; > > + > > + /* MEM_INIT is a self-clearing bit. Wait for it to be clear (should be > > + * 100us) before enabling the switch core > > + */ > > + err = readx_poll_timeout(ocelot_ext_mem_init_status, ocelot, val, !val, > > + OCELOT_EXT_MEM_INIT_SLEEP_US, > > + OCELOT_EXT_MEM_INIT_TIMEOUT_US); > > + > > I think you can eliminate the newline between the err assignment and > checking for it. In my upcoming v2 set, "ocelot_ext_reset" is moved to the shared "ocelot_reset" routine. As such, iopoll.h isn't needed. And all soc/mscc/ocelot_*.h includes aren't necessary either, since there are literally zero register writes in ocelot_ext.c now. I'll wait a couple days for everyone to go through their backlog. If my "clean up ocelot_reset()" and your Documentation yaml patches get approved, I'll be ready to send this out.
Powered by blists - more mailing lists