netdev - Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to access the GMAC register

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: <20180403095005.skflxb7m2qzbhjix@flea>
Date:   Tue, 3 Apr 2018 11:50:05 +0200
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Chen-Yu Tsai <wens@...e.org>
Cc:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Mark Brown <broonie@...nel.org>,
        Icenowy Zheng <icenowy@...c.io>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-clk <linux-clk@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        Corentin Labbe <clabbe.montjoie@...il.com>
Subject: Re: [PATCH net-next 02/12] clk: sunxi-ng: r40: export a regmap to
 access the GMAC register

On Tue, Apr 03, 2018 at 11:48:45AM +0200, Maxime Ripard wrote:
> On Tue, Mar 20, 2018 at 03:15:02PM +0800, Chen-Yu Tsai wrote:
> > On Mon, Mar 19, 2018 at 5:31 AM, Maxime Ripard
> > <maxime.ripard@...tlin.com> wrote:
> > > On Sat, Mar 17, 2018 at 05:28:47PM +0800, Chen-Yu Tsai wrote:
> > >> From: Icenowy Zheng <icenowy@...c.io>
> > >>
> > >> There's a GMAC configuration register, which exists on A64/A83T/H3/H5 in
> > >> the syscon part, in the CCU of R40 SoC.
> > >>
> > >> Export a regmap of the CCU.
> > >>
> > >> Read access is not restricted to all registers, but only the GMAC
> > >> register is allowed to be written.
> > >>
> > >> Signed-off-by: Icenowy Zheng <icenowy@...c.io>
> > >> Signed-off-by: Chen-Yu Tsai <wens@...e.org>
> > >
> > > Gah, this is crazy. I'm really starting to regret letting that syscon
> > > in in the first place...
> > 
> > IMHO syscon is really a better fit. It's part of the glue layer and
> > most other dwmac user platforms treat it as such and use a syscon.
> > Plus the controls encompass delays (phase), inverters (polarity),
> > and even signal routing. It's not really just a group of clock controls,
> > like what we poorly modeled for A20/A31. I think that was really a
> > mistake.
> > 
> > As I mentioned in the cover letter, a slightly saner approach would
> > be to let drivers add custom syscon entries, which would then require
> > less custom plumbing.
> 
> A syscon is convenient, sure, but it also bypasses any abstraction
> layer we have everywhere else, which means that we'll have to maintain
> the register layout in each and every driver that uses it.
> 
> So far, it's only be the GMAC, but it can also be others (the SRAM
> controller comes to my mind), and then, if there's any difference in
> the design in a future SoC, we'll have to maintain that in the GMAC
> driver as well.

I guess I forgot to say something, I'm fine with using a syscon we
already have.

I'm just questionning if merging any other driver using one is the
right move.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)