linux-kernel - Re: [PATCH 3/3] iio: Add driver for Nicera D3-323-AA PIR sensor

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: <pndo6vnfrnp.fsf@axis.com>
Date: Tue, 20 May 2025 13:27:54 +0200
From: Waqar Hameed <waqar.hameed@...s.com>
To: Jonathan Cameron <jic23@...nel.org>
CC: Lars-Peter Clausen <lars@...afoo.de>, <kernel@...s.com>,
	<linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>
Subject: Re: [PATCH 3/3] iio: Add driver for Nicera D3-323-AA PIR sensor

On Sun, May 18, 2025 at 18:38 +0100 Jonathan Cameron <jic23@...nel.org> wrote:

>> >> +#define D3323AA_DRV_NAME "d3323aa"  
>> >
>> > Put that inline where used.  A define like this both implies that various values
>> > must be the same when they need not be and means that we have to go find the
>> > define to fine out what they are set to.  Just setting the strings directly
>> > tends to end up more readable.  
>> 
>> Sure, we can do that. (There are a bunch of IIO-drivers doing this, so I
>> just thought that was the "convention".)
>
> I'm sometimes in less fussy mood.  One day I might just clean those up
> so there is nothing to copy into new drivers!

A quick search tells that there are (at least) 105 of those:

  rgrep -A 30 "\.driver" drivers/iio/ | grep "\.name" | grep -v '"'
  
I was just about to write a small Python script to fix those, but just
wanted to confirm with you before spending more time on this. So if you
don't want to do this yourself, I can help your here :)

>>
>> >> +	/* Input clock or output detection signal (Vout). */  
>> >
>> > I'd rename. Vout kind of suggests a variable voltage. This seems to just
>> > be a level signal.  
>> 
>> >> +	struct gpio_desc *gpiod_clk_vout;  
>> 
>> Yeah, it's a weird pin with multiple use-cases... I just named it
>> according to what the datasheet calls it. What about
>> `gpiod_clk_detection`?
>
> That sounds like it's detecting a clock.  Hmm.  
> gpiod_clkin_detectout maybe?

No objections.

[...]