public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nam Cao <namcao@linutronix.de>
To: Tudor Gheorghiu <tudor.reda@gmail.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-iio@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: iio: frequency: rename macros
Date: Wed, 2 Oct 2024 00:54:26 +0200	[thread overview]
Message-ID: <20241001225426.wUBOFdMi@linutronix.de> (raw)
In-Reply-To: <20241001202430.15874-2-tudor.reda@gmail.com>

On Tue, Oct 01, 2024 at 11:24:30PM +0300, Tudor Gheorghiu wrote:
> The frequency iio drivers use custom defined macros (inside dds.h) in
> order to define sysfs attributes more easily.
> 
> However, due to their naming choice and the fact that in some of them the
> first and/or second arguments are decimal, checkpatch will throw errors
> in the source files they are used in (ad9832.c and ad9834.c).
> This is because it thinks the argument is _mode, therefore
> it expects octal notation, even if the argument itself
> does not represent file permissions. Example:
> 
> ERROR: Use 4 digit octal (0777) not decimal permissions
> +static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL, ad9834_write, AD9834_PSEL);

You probably want to elaborate what you mean by "their naming choice" (i.e.
how does the naming choice causes this false warning?)

I got curious and digged into checkpatch.pl. This script expects macros
whose names match "IIO_DEV_ATTR_[A-Z_]+" to have the first integer argument
to be octal. And this driver defines macros which "luckily" match that
pattern.

There is only IIO_DEV_ATTR_SAMP_FREQ which matches the pattern, and accepts
umode_t as its first argument.

Instead of changing code just to make checkpatch.pl happy, perhaps it's
better to fix the checkpatch script? Maybe something like the untested
patch below?

Or since checkpatch is wrong, maybe just ignore it.

Best regards,
Nam

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4427572b2477..2fb4549fede2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -817,7 +817,7 @@ our @mode_permission_funcs = (
 	["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2],
 	["proc_create(?:_data|)", 2],
 	["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2],
-	["IIO_DEV_ATTR_[A-Z_]+", 1],
+	["IIO_DEV_ATTR_SAMP_FREQ", 1],
 	["SENSOR_(?:DEVICE_|)ATTR_2", 2],
 	["SENSOR_TEMPLATE(?:_2|)", 3],
 	["__ATTR", 2],

  reply	other threads:[~2024-10-01 22:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-01 20:24 [PATCH] staging: iio: frequency: rename macros Tudor Gheorghiu
2024-10-01 22:54 ` Nam Cao [this message]
2024-10-02  2:49   ` Tudor Gheorghiu
2024-10-02  6:06     ` Nam Cao
2024-10-06 11:27     ` Jonathan Cameron
2024-10-07 15:46       ` Tudor Gheorghiu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241001225426.wUBOFdMi@linutronix.de \
    --to=namcao@linutronix.de \
    --cc=Michael.Hennerich@analog.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=tudor.reda@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox