[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250531161029.4010a3d6@jic23-huawei>
Date: Sat, 31 May 2025 16:10:29 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Waqar Hameed <waqar.hameed@...s.com>
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 Tue, 27 May 2025 16:48:30 +0200
Waqar Hameed <waqar.hameed@...s.com> wrote:
> On Sun, May 25, 2025 at 10:30 +0100 Jonathan Cameron <jic23@...nel.org> wrote:
>
> > On Tue, 20 May 2025 13:27:54 +0200
> > Waqar Hameed <waqar.hameed@...s.com> wrote:
> >
> >> 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 :)
> >
> > It's probably not worth the churn on the ones that have the string repeated
> > multiple times. However, perhaps any that are only using it for .name would
> > be good to tidy up? Those are less a case of it being 'taste' vs it being silly
> > to have a define!
>
> I think if you use it in multiple places, it should definitively be a
> macro definition. I just sent some patches for those that only used it
> once (I didn't include those with `KBUILD_MODNAME`. We can discuss if we
> should also address those in that thread).
I would disagree slightly. If it is used in multiple places because there
is some inherent reason they should have same string then I absolutely agree.
If it's just because it's a convenient string that is used twice in places
that could have had different string then not so much.
>
> However, there are a bunch of drivers that _only_ use a macro definition
> in `.name` and `indio_dev->name`, including this one. That _is_ more
> than one place, so we should actually leave it? Or do you still think we
> should have the same string literal in both places, as you originally
> commented above?
I'd prefer that for new code, but it is a less clear cut case than the ones
you have tidied up, so not worth the churn of tidying up unless people
are otherwise working on the relevant drivers.
Jonathan
>
Powered by blists - more mailing lists