linux-kernel - Re: [PATCH v3 6/6] iio: adc: Add support for AD4000

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]

Date: Tue, 11 Jun 2024 13:34:29 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Nuno Sá <noname.nuno@...il.com>,
	Marcelo Schmitt <marcelo.schmitt@...log.com>, broonie@...nel.org,
	lars@...afoo.de, Michael.Hennerich@...log.com, robh+dt@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	nuno.sa@...log.com, dlechner@...libre.com,
	marcelo.schmitt1@...il.com, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-spi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Linus Walleij <linus.walleij@...aro.org>,
	Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v3 6/6] iio: adc: Add support for AD4000

Sun, Jun 09, 2024 at 10:23:54AM +0100, Jonathan Cameron kirjoitti:

...

> > > +	/*
> > > +	 * In 4-wire mode, the CNV line is held high for the entire
> > > conversion
> > > +	 * and acquisition process. In other modes st->cnv_gpio is NULL and
> > > is
> > > +	 * ignored (CS is wired to CNV in those cases).
> > > +	 */
> > > +	gpiod_set_value_cansleep(st->cnv_gpio, 1);  
> > 
> > Not sure it's a good practise to assume internal details as you're going for
> > GPIO. I would prefer to have an explicit check for st->cnv_gpio being NULL or
> > not.
> 
> Hmm. I had it in my head that this was documented behaviour, but
> I can't find such in the docs, so agreed checking it makes sense.
> 
> I would be very surprised if this ever changed as it's one of the
> things that makes optional gpios easy to work with but who knows!

Not Linus and not Bart, but we have tons of drivers that call GPIO APIs
unconditionally as long as they want optional GPIO.

What I see here is the comment that should be rewritten to say something like

"if GPIO is defined blablabla, otherwise blablabla."

I.o.w. do not mention that implementation detail (being NULL, i.e. optional).

-- 
With Best Regards,
Andy Shevchenko