Re: [PATCH] staging: iio: frequency: rename macros

[Nam Cao]

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],