linux-kernel - Re: [RFC v5 net-next 01/13] mfd: ocelot: add support for external mfd control over SPI for the VSC7512

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: <Yd1YV+eUIaCnttYd@google.com>
Date:   Tue, 11 Jan 2022 10:13:43 +0000
From:   Lee Jones <lee.jones@...aro.org>
To:     Colin Foster <colin.foster@...advantage.com>
Cc:     broonie@...nel.org, linux-gpio@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linus Walleij <linus.walleij@...aro.org>,
        Russell King <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "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,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [RFC v5 net-next 01/13] mfd: ocelot: add support for external
 mfd control over SPI for the VSC7512

> > > > No magic numbers please.
> > > 
> > > I've gotten conflicting feedback on this. Several of the ocelot drivers
> > > (drivers/net/dsa/ocelot/felix_vsc9959.c) have these ranges hard-coded.
> > > Others (Documentation/devicetree/bindings/net/mscc-ocelot.txt) have them
> > > all passed in through the device tree. 
> > > 
> > > https://lore.kernel.org/netdev/20211126213225.okrskqm26lgprxrk@skbuf/
> > 
> > Ref or quote?
> > 
> > I'm not brain grepping it searching for what you might be referring to.
> > 
> > I'm not sure what you're trying to say here.  I'm asking you to define
> > this numbers please.
> 
> I'll define the numbers as you suggest.
> 
> The quote I was referring to is this:
> 
> > The last option I haven't put much consideration toward would be to
> > move some of the decision making to the device tree. The main ocelot
> > driver appears to leave a lot of these addresses out. For instance
> > Documentation/devicetree/bindings/pinctrl/mscc,ocelot-pinctrl.txt.
> > That added DT complexity could remove needs for lines like this:
> > > > +              ocelot->map[GCB][GCB_MIIM_MII_STATUS & REG_MASK],
> > But that would probably impose DT changes on Seville and Felix, which
> > is the last thing I want to do.
> 
> The thing with putting the targets in the device tree is that you're
> inflicting yourself unnecessary pain. Take a look at
> Documentation/devicetree/bindings/net/mscc-ocelot.txt, and notice that
> they mark the "ptp" target as optional because it wasn't needed when
> they first published the device tree, and now they need to maintain
> compatibility with those old blobs.

I wasn't asking you to put it in DT, just to define the numbers.

> > > There's yet another complexity with these, and I'm not sure what the
> > > answer is. Currently all regmaps are tied to the ocelot_spi device...
> > > ocelot_spi calls devm_regmap_init. So those regmaps hang around if
> > > they're created by a module that has been removed. At least until the
> > > entire MFD module is removed. Maybe there's something I haven't seen yet
> > > where the devres or similar has a reference count. I don't know the best
> > > path forward on this one.
> > 
> > Why are you worrying about creating them 2 different ways?
> > 
> > If it's possible for them to all create and use their own regmaps,
> > what's preventing you from just do that all the time?
> 
> There isn't really any worry, there just might be efficiencies to be
> had if two children share the same regmap. But so long as any regmap is
> created with unique names, there's no reason multiple regmaps can't
> overlap the same regions. In those cases, maybe syscon would be the best
> thing to implement if it becomes needed.
> 
> I have nothing against making every child regmap be unique if that's the
> desire.

Unless something has changed or my understanding is not correct,
regmap does not support over-lapping register ranges.

However, even if that is required, I still think we can come up with
something cleaner than creating a whole API based around creating
and fetching different regmap configurations depending on how the
system was initialised.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog