linux-kernel - Re: [PATCH v5 3/3] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

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] [day] [month] [year] [list]

Message-ID: <20250425092637.0c019531@jic23-huawei>
Date: Fri, 25 Apr 2025 09:26:37 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Andy Shevchenko <andy@...nel.org>
Cc: "Paller, Kim Seer" <KimSeer.Paller@...log.com>, Lars-Peter Clausen
 <lars@...afoo.de>, "Hennerich, Michael" <Michael.Hennerich@...log.com>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, David Lechner <dlechner@...libre.com>, Nuno
 Sá <noname.nuno@...il.com>, "Sa, Nuno" <Nuno.Sa@...log.com>,
 "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v5 3/3] iio: dac: ad3530r: Add driver for AD3530R and
 AD3531R

On Wed, 23 Apr 2025 19:52:13 +0300
Andy Shevchenko <andy@...nel.org> wrote:

> On Wed, Apr 23, 2025 at 07:50:51AM +0000, Paller, Kim Seer wrote:
> > > From: Jonathan Cameron <jic23@...nel.org>
> > > Sent: Monday, April 21, 2025 9:48 PM
> > > To: Paller, Kim Seer <KimSeer.Paller@...log.com>
> > > On Mon, 21 Apr 2025 12:24:54 +0800
> > > Kim Seer Paller <kimseer.paller@...log.com> wrote:  
> 
> ...
> 
> > > > +	mask = GENMASK(chan->address + 1, chan->address);  
> > > 
> > > I think maybe we need a macro to get the mask from the channel number?
> > > Using address for this seems overkill given how simple that maths is.
> > > Ideally that macro could perhaps be used in the code below to avoid
> > > all the defines I suggested.  
> > 
> > The motivation for using the chan->address field was to hide the calculation a bit.
> > However, would using a macro like 
> > #define AD3530R_OP_MODE_CHAN_MSK(chan)	GENMASK(2 * chan + 1, 2 * chan) 
> > be a good approach in this case? This drops the need for the address field and
> > can also be used to explicitly set the operating mode for the 4 fields of the register.
> > What do you think?  
> 
> Please, note that doing GENMASK(foo + X, foo) is highly discouraged as it may
> give a very bad generated code (although I haven't checked recently if it's
> still the case). The preferred way is GENMASK(X, 0) << foo. Where X is a
> compile time constant.
> 

With what Andy suggested as the implementation, this sort of macro
looks like a good solution to me.

Jonathan