linux-kernel - Re: [PATCH v2 00/14] Add Microchip ZL3073x support (part 1)

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: <20250415102851.GW372032@google.com>
Date: Tue, 15 Apr 2025 11:28:51 +0100
From: Lee Jones <lee@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: Michal Schmidt <mschmidt@...hat.com>, Ivan Vecera <ivecera@...hat.com>,
	netdev@...r.kernel.org, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Prathosh Satish <Prathosh.Satish@...rochip.com>,
	Kees Cook <kees@...nel.org>, Andy Shevchenko <andy@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 00/14] Add Microchip ZL3073x support (part 1)

On Fri, 11 Apr 2025, Rob Herring wrote:

> On Fri, Apr 11, 2025 at 04:27:08PM +0200, Michal Schmidt wrote:
> > On Fri, Apr 11, 2025 at 9:26 AM Lee Jones <lee@...nel.org> wrote:
> > > On Wed, 09 Apr 2025, Ivan Vecera wrote:
> > > > Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> > > > provides DPLL and PTP functionality. This series bring first part
> > > > that adds the common MFD driver that provides an access to the bus
> > > > that can be either I2C or SPI.
> > > > [...]
> > >
> > > Not only are all of the added abstractions and ugly MACROs hard to read
> > > and troublesome to maintain, they're also completely unnecessary at this
> > > (driver) level.  Nicely authored, easy to read / maintain code wins over
> > > clever code 95% of the time.
> > 
> > Hello Lee,
> > 
> > IMHO defining the registers with the ZL3073X_REG*_DEF macros is both
> > clever and easy to read / maintain. On one line I can see the register
> > name, size and address. For the indexed registers also their count and
> > the stride. It's almost like looking at a datasheet. And the
> > type-checking for accessing the registers using the correct size is
> > nice. I even liked the paranoid WARN_ON for checking the index
> > overflows.
> 
> If this is much better, define (and upstream) something for everyone to 
> use rather than a one-off in some driver. It doesn't matter how great it 
> is if it is different from everyone else. The last thing I want to do is 
> figure out how register accesses work when looking at an unfamilar 
> driver.

Exactly right.  The issue isn't that defining register accesses using
MACROs is a bad idea generally.  I've seen it done before in downstream
BSPs and the like.  It's that this solution isn't following the
conventions already in carved for such things in the Mainline kernel.
To engineers already used to the current conventions, this is much
harder to follow and interact with.

As Rob says, if this is truly better, discuss the idea with a much
wider audience and have it applied broadly across all areas.  We shall
not be bucking the trend or trend setting here in MFD.

-- 
Lee Jones [李琼斯]