From: Jonathan Cameron <jic23@kernel.org>
To: Xiaofeng Yuan <xiaofengmian@163.com>
Cc: gregkh@linuxfoundation.org, linux-iio@vger.kernel.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: iio: frequency: reorder dds.h macro parameters to match IIO convention
Date: Thu, 2 Jul 2026 01:18:31 +0100 [thread overview]
Message-ID: <20260702011831.6b014318@jic23-huawei> (raw)
In-Reply-To: <20260624021612.23808-1-xiaofengmian@163.com>
On Wed, 24 Jun 2026 02:16:12 +0000
Xiaofeng Yuan <xiaofengmian@163.com> wrote:
> The IIO subsystem convention requires that the file permission
> (_mode) parameter be the first argument of IIO_DEV_ATTR_* macros.
> The dds.h macros had _mode after _channel, causing checkpatch to
> misinterpret the channel number as a permission value.
>
> Reorder the parameters so _mode is first, matching the convention
> established by IIO_DEV_ATTR_SAMP_FREQ in include/linux/iio/sysfs.h.
>
> Update all callers in ad9834.c and ad9832.c accordingly.
>
> Compile tested.
>
> Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
Hi.
Much more substantial surgery needs doing to these and the result
of that is many (probably all!) of these macros will go away.
Hence marginal value but given it is staging and so churn isn't
as much of an issue, fair enough.
One thing that this does need to mention in the commit description
is the WAVETYPE gaining a mode parameter when it was always
0200 hard coded before. I'll add that whilst applying.
Applied to the testing branch of iio.git
Jonathan
> ---
> drivers/staging/iio/frequency/ad9832.c | 20 ++++++++++----------
> drivers/staging/iio/frequency/ad9834.c | 22 +++++++++++-----------
> drivers/staging/iio/frequency/dds.h | 24 ++++++++++++------------
> 3 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
> index 659821a1e..2dfdd0d28 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -253,22 +253,22 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
> * see dds.h for further information
> */
>
> -static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9832_write, AD9832_FREQ0HM);
> -static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9832_write, AD9832_FREQ1HM);
> -static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9832_write, AD9832_FREQ_SYM);
> +static IIO_DEV_ATTR_FREQ(0200, 0, 0, NULL, ad9832_write, AD9832_FREQ0HM);
> +static IIO_DEV_ATTR_FREQ(0200, 0, 1, NULL, ad9832_write, AD9832_FREQ1HM);
> +static IIO_DEV_ATTR_FREQSYMBOL(0200, 0, NULL, ad9832_write, AD9832_FREQ_SYM);
> static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
>
> -static IIO_DEV_ATTR_PHASE(0, 0, 0200, NULL, ad9832_write, AD9832_PHASE0H);
> -static IIO_DEV_ATTR_PHASE(0, 1, 0200, NULL, ad9832_write, AD9832_PHASE1H);
> -static IIO_DEV_ATTR_PHASE(0, 2, 0200, NULL, ad9832_write, AD9832_PHASE2H);
> -static IIO_DEV_ATTR_PHASE(0, 3, 0200, NULL, ad9832_write, AD9832_PHASE3H);
> -static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL,
> +static IIO_DEV_ATTR_PHASE(0200, 0, 0, NULL, ad9832_write, AD9832_PHASE0H);
> +static IIO_DEV_ATTR_PHASE(0200, 0, 1, NULL, ad9832_write, AD9832_PHASE1H);
> +static IIO_DEV_ATTR_PHASE(0200, 0, 2, NULL, ad9832_write, AD9832_PHASE2H);
> +static IIO_DEV_ATTR_PHASE(0200, 0, 3, NULL, ad9832_write, AD9832_PHASE3H);
> +static IIO_DEV_ATTR_PHASESYMBOL(0200, 0, NULL,
> ad9832_write, AD9832_PHASE_SYM);
> static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
>
> -static IIO_DEV_ATTR_PINCONTROL_EN(0, 0200, NULL,
> +static IIO_DEV_ATTR_PINCONTROL_EN(0200, 0, NULL,
> ad9832_write, AD9832_PINCTRL_EN);
> -static IIO_DEV_ATTR_OUT_ENABLE(0, 0200, NULL,
> +static IIO_DEV_ATTR_OUT_ENABLE(0200, 0, NULL,
> ad9832_write, AD9832_OUTPUT_EN);
>
> static struct attribute *ad9832_attributes[] = {
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index 4359b358e..b232cb4a7 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -315,21 +315,21 @@ static IIO_DEVICE_ATTR(out_altvoltage0_out1_wavetype_available, 0444,
> * see dds.h for further information
> */
>
> -static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9834_write, AD9834_REG_FREQ0);
> -static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9834_write, AD9834_REG_FREQ1);
> -static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9834_write, AD9834_FSEL);
> +static IIO_DEV_ATTR_FREQ(0200, 0, 0, NULL, ad9834_write, AD9834_REG_FREQ0);
> +static IIO_DEV_ATTR_FREQ(0200, 0, 1, NULL, ad9834_write, AD9834_REG_FREQ1);
> +static IIO_DEV_ATTR_FREQSYMBOL(0200, 0, NULL, ad9834_write, AD9834_FSEL);
> static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
>
> -static IIO_DEV_ATTR_PHASE(0, 0, 0200, NULL, ad9834_write, AD9834_REG_PHASE0);
> -static IIO_DEV_ATTR_PHASE(0, 1, 0200, NULL, ad9834_write, AD9834_REG_PHASE1);
> -static IIO_DEV_ATTR_PHASESYMBOL(0, 0200, NULL, ad9834_write, AD9834_PSEL);
> +static IIO_DEV_ATTR_PHASE(0200, 0, 0, NULL, ad9834_write, AD9834_REG_PHASE0);
> +static IIO_DEV_ATTR_PHASE(0200, 0, 1, NULL, ad9834_write, AD9834_REG_PHASE1);
> +static IIO_DEV_ATTR_PHASESYMBOL(0200, 0, NULL, ad9834_write, AD9834_PSEL);
> static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
>
> -static IIO_DEV_ATTR_PINCONTROL_EN(0, 0200, NULL, ad9834_write, AD9834_PIN_SW);
> -static IIO_DEV_ATTR_OUT_ENABLE(0, 0200, NULL, ad9834_write, AD9834_RESET);
> -static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, 0200, NULL, ad9834_write, AD9834_OPBITEN);
> -static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
> -static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
> +static IIO_DEV_ATTR_PINCONTROL_EN(0200, 0, NULL, ad9834_write, AD9834_PIN_SW);
> +static IIO_DEV_ATTR_OUT_ENABLE(0200, 0, NULL, ad9834_write, AD9834_RESET);
> +static IIO_DEV_ATTR_OUTY_ENABLE(0200, 0, 1, NULL, ad9834_write, AD9834_OPBITEN);
> +static IIO_DEV_ATTR_OUT_WAVETYPE(0200, 0, 0, ad9834_store_wavetype, 0);
> +static IIO_DEV_ATTR_OUT_WAVETYPE(0200, 0, 1, ad9834_store_wavetype, 1);
>
> static struct attribute *ad9834_attributes[] = {
> &iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
> diff --git a/drivers/staging/iio/frequency/dds.h b/drivers/staging/iio/frequency/dds.h
> index 2ebe68eb7..b2ca8bb97 100644
> --- a/drivers/staging/iio/frequency/dds.h
> +++ b/drivers/staging/iio/frequency/dds.h
> @@ -11,7 +11,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_frequencyY
> */
>
> -#define IIO_DEV_ATTR_FREQ(_channel, _num, _mode, _show, _store, _addr) \
> +#define IIO_DEV_ATTR_FREQ(_mode, _channel, _num, _show, _store, _addr) \
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_frequency##_num, \
> _mode, _show, _store, _addr)
>
> @@ -26,7 +26,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_frequencysymbol
> */
>
> -#define IIO_DEV_ATTR_FREQSYMBOL(_channel, _mode, _show, _store, _addr) \
> +#define IIO_DEV_ATTR_FREQSYMBOL(_mode, _channel, _show, _store, _addr) \
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_frequencysymbol, \
> _mode, _show, _store, _addr)
>
> @@ -34,7 +34,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_phaseY
> */
>
> -#define IIO_DEV_ATTR_PHASE(_channel, _num, _mode, _show, _store, _addr) \
> +#define IIO_DEV_ATTR_PHASE(_mode, _channel, _num, _show, _store, _addr) \
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_phase##_num, \
> _mode, _show, _store, _addr)
>
> @@ -49,7 +49,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_phasesymbol
> */
>
> -#define IIO_DEV_ATTR_PHASESYMBOL(_channel, _mode, _show, _store, _addr) \
> +#define IIO_DEV_ATTR_PHASESYMBOL(_mode, _channel, _show, _store, _addr) \
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_phasesymbol, \
> _mode, _show, _store, _addr)
>
> @@ -57,7 +57,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_pincontrol_en
> */
>
> -#define IIO_DEV_ATTR_PINCONTROL_EN(_channel, _mode, _show, _store, _addr)\
> +#define IIO_DEV_ATTR_PINCONTROL_EN(_mode, _channel, _show, _store, _addr)\
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_pincontrol_en, \
> _mode, _show, _store, _addr)
>
> @@ -65,7 +65,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_pincontrol_frequency_en
> */
>
> -#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_channel, _mode, _show, _store, _addr)\
> +#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_mode, _channel, _show, _store, _addr)\
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_pincontrol_frequency_en,\
> _mode, _show, _store, _addr)
>
> @@ -73,7 +73,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_pincontrol_phase_en
> */
>
> -#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_channel, _mode, _show, _store, _addr)\
> +#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_mode, _channel, _show, _store, _addr)\
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_pincontrol_phase_en, \
> _mode, _show, _store, _addr)
>
> @@ -81,7 +81,7 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_out_enable
> */
>
> -#define IIO_DEV_ATTR_OUT_ENABLE(_channel, _mode, _show, _store, _addr) \
> +#define IIO_DEV_ATTR_OUT_ENABLE(_mode, _channel, _show, _store, _addr) \
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_out_enable, \
> _mode, _show, _store, _addr)
>
> @@ -89,8 +89,8 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_outY_enable
> */
>
> -#define IIO_DEV_ATTR_OUTY_ENABLE(_channel, _output, \
> - _mode, _show, _store, _addr) \
> +#define IIO_DEV_ATTR_OUTY_ENABLE(_mode, _channel, _output, \
> + _show, _store, _addr) \
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_out##_output##_enable,\
> _mode, _show, _store, _addr)
>
> @@ -98,9 +98,9 @@
> * /sys/bus/iio/devices/.../out_altvoltageX_outY_wavetype
> */
>
> -#define IIO_DEV_ATTR_OUT_WAVETYPE(_channel, _output, _store, _addr) \
> +#define IIO_DEV_ATTR_OUT_WAVETYPE(_mode, _channel, _output, _store, _addr)\
> IIO_DEVICE_ATTR(out_altvoltage##_channel##_out##_output##_wavetype,\
> - 0200, NULL, _store, _addr)
> + _mode, NULL, _store, _addr)
>
> /**
> * /sys/bus/iio/devices/.../out_altvoltageX_outY_wavetype_available
prev parent reply other threads:[~2026-07-02 0:18 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-24 2:16 [PATCH] staging: iio: frequency: reorder dds.h macro parameters to match IIO convention Xiaofeng Yuan
2026-07-02 0:18 ` Jonathan Cameron [this message]
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=20260702011831.6b014318@jic23-huawei \
--to=jic23@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=xiaofengmian@163.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