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: <aAkaMp9R_ukoW-RQ@smile.fi.intel.com>
Date: Wed, 23 Apr 2025 19:49:54 +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:53:37AM +0000, Paller, Kim Seer wrote:
> > From: Andy Shevchenko <andy@...nel.org>
> > Sent: Tuesday, April 22, 2025 11:11 PM
> > On Mon, Apr 21, 2025 at 12:24:54PM +0800, Kim Seer Paller wrote:

First of all, there are a lot of comments left without replies while some of
them commented as "agree I will follow your advice". This is confusing. The
rule of thumb is to not reply with positive at all, just only for the things
that you want to clarify. And with that remove a lot of unneeded (you agree
with) context!

...

> > > +		st->vref_mv = range_multiplier * vref / 1000;
> > 
> > MILLI?
> 
> Yes this is milli, will change this also to vref_mV
> 
> 	st->vref_mV = range_multiplier * vref_mV / 1000;

Ah, I was not clear enough, MILLI in capital letters is defined constant which
you may use instead of 1000.

...

> > > +	vref = devm_regulator_get_enable_read_voltage(dev, "ref");
> > > +	if (vref < 0 && vref != -ENODEV)
> > > +		return vref;
> > > +
> > > +	has_external_vref = vref != -ENODEV;
> > 
> > Wouldn't be better just make this 0 when it's == -ENODEV and check just the
> > value without having this additional boolean variable (note, I haven't checked
> > the meaning of Vref == 0 in case it's possible in real life and hardware behaves
> > adequately)?
> 
> I think it could be simpler to set vref to 0 when it's -ENODEV and check its value directly
> without having additional boolean variable. I'll try this approach.

But double check that hardware doesn't support Vref == 0 in real life.

-- 
With Best Regards,
Andy Shevchenko