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

Message-ID: <aAkavQVd7Px3qPU0@smile.fi.intel.com>
Date: Wed, 23 Apr 2025 19:52:13 +0300
From: Andy Shevchenko <andy@...nel.org>
To: "Paller, Kim Seer" <KimSeer.Paller@...log.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
	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, 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 Best Regards,
Andy Shevchenko