The driver currently defines device attributes using
symbolic permission flags (S_IRUGO and S_IWUSR).
Update these to use octal permissions (0444 and 0200) to resolve
checkpatch warnings.
Signed-off-by: bhargav <rougueprince47@gmail.com>
---
Changes in v2:
- It was suggested in v1 to convert these to
IIO_DEVICE_ATTR_RO() and _WO(). However, those helper macros
expect uniquely named callbacks (e.g. pll1_locked_show),
whereas this driver uses shared multiplexed callbacks
(eg. ad9523_show). Converting to the helpers would require
generating wrapper functions.
Therefore, this v2 keeps IIO_DEVICE_ATTR but updates the
permissions to standard octal format.
drivers/iio/frequency/ad9523.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 4d0556ff5601..2a075d09ab83 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -558,52 +558,52 @@ static ssize_t ad9523_show(struct device *dev,
return ret;
}
-static IIO_DEVICE_ATTR(pll1_locked, S_IRUGO,
+static IIO_DEVICE_ATTR(pll1_locked, 0444,
ad9523_show,
NULL,
AD9523_STAT_PLL1_LD);
-static IIO_DEVICE_ATTR(pll2_locked, S_IRUGO,
+static IIO_DEVICE_ATTR(pll2_locked, 0444,
ad9523_show,
NULL,
AD9523_STAT_PLL2_LD);
-static IIO_DEVICE_ATTR(pll1_reference_clk_a_present, S_IRUGO,
+static IIO_DEVICE_ATTR(pll1_reference_clk_a_present, 0444,
ad9523_show,
NULL,
AD9523_STAT_REFA);
-static IIO_DEVICE_ATTR(pll1_reference_clk_b_present, S_IRUGO,
+static IIO_DEVICE_ATTR(pll1_reference_clk_b_present, 0444,
ad9523_show,
NULL,
AD9523_STAT_REFB);
-static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, S_IRUGO,
+static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, 0444,
ad9523_show,
NULL,
AD9523_STAT_REF_TEST);
-static IIO_DEVICE_ATTR(vcxo_clk_present, S_IRUGO,
+static IIO_DEVICE_ATTR(vcxo_clk_present, 0444,
ad9523_show,
NULL,
AD9523_STAT_VCXO);
-static IIO_DEVICE_ATTR(pll2_feedback_clk_present, S_IRUGO,
+static IIO_DEVICE_ATTR(pll2_feedback_clk_present, 0444,
ad9523_show,
NULL,
AD9523_STAT_PLL2_FB_CLK);
-static IIO_DEVICE_ATTR(pll2_reference_clk_present, S_IRUGO,
+static IIO_DEVICE_ATTR(pll2_reference_clk_present, 0444,
ad9523_show,
NULL,
AD9523_STAT_PLL2_REF_CLK);
-static IIO_DEVICE_ATTR(sync_dividers, S_IWUSR,
+static IIO_DEVICE_ATTR(sync_dividers, 0200,
NULL,
ad9523_store,
AD9523_SYNC);
-static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR,
+static IIO_DEVICE_ATTR(store_eeprom, 0200,
NULL,
ad9523_store,
AD9523_EEPROM);
--
2.53.0On Fri, Feb 20, 2026 at 02:02:15AM +0530, bhargav wrote: ... > Changes in v2: > - It was suggested in v1 to convert these to > IIO_DEVICE_ATTR_RO() and _WO(). However, those helper macros > expect uniquely named callbacks (e.g. pll1_locked_show), > whereas this driver uses shared multiplexed callbacks > (eg. ad9523_show). Converting to the helpers would require > generating wrapper functions. > Therefore, this v2 keeps IIO_DEVICE_ATTR but updates the > permissions to standard octal format. Okay, but... > -static IIO_DEVICE_ATTR(pll1_locked, S_IRUGO, > +static IIO_DEVICE_ATTR(pll1_locked, 0444, > ad9523_show, > NULL, > AD9523_STAT_PLL1_LD); > > -static IIO_DEVICE_ATTR(pll2_locked, S_IRUGO, > +static IIO_DEVICE_ATTR(pll2_locked, 0444, > ad9523_show, > NULL, > AD9523_STAT_PLL2_LD); > > -static IIO_DEVICE_ATTR(pll1_reference_clk_a_present, S_IRUGO, > +static IIO_DEVICE_ATTR(pll1_reference_clk_a_present, 0444, > ad9523_show, > NULL, > AD9523_STAT_REFA); > > -static IIO_DEVICE_ATTR(pll1_reference_clk_b_present, S_IRUGO, > +static IIO_DEVICE_ATTR(pll1_reference_clk_b_present, 0444, > ad9523_show, > NULL, > AD9523_STAT_REFB); > > -static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, S_IRUGO, > +static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, 0444, > ad9523_show, > NULL, > AD9523_STAT_REF_TEST); ...even for the longest line you can update them to be like static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, 0444, ad9523_show, NULL, AD9523_STAT_REF_TEST); Also note, that the indentation is broken for all parameters that are on the next lines to the IIO_DEVICE_ATTR(). > -static IIO_DEVICE_ATTR(vcxo_clk_present, S_IRUGO, > +static IIO_DEVICE_ATTR(vcxo_clk_present, 0444, > ad9523_show, > NULL, > AD9523_STAT_VCXO); > > -static IIO_DEVICE_ATTR(pll2_feedback_clk_present, S_IRUGO, > +static IIO_DEVICE_ATTR(pll2_feedback_clk_present, 0444, > ad9523_show, > NULL, > AD9523_STAT_PLL2_FB_CLK); > > -static IIO_DEVICE_ATTR(pll2_reference_clk_present, S_IRUGO, > +static IIO_DEVICE_ATTR(pll2_reference_clk_present, 0444, > ad9523_show, > NULL, > AD9523_STAT_PLL2_REF_CLK); > > -static IIO_DEVICE_ATTR(sync_dividers, S_IWUSR, > +static IIO_DEVICE_ATTR(sync_dividers, 0200, > NULL, > ad9523_store, > AD9523_SYNC); > > -static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR, > +static IIO_DEVICE_ATTR(store_eeprom, 0200, > NULL, > ad9523_store, > AD9523_EEPROM); -- With Best Regards, Andy Shevchenko
Changes in v3:
-Patch 1: updated macros to use '(x)' instead of 'x'.
-Patch 2: collected reviewed-by (no code changes).
-Patch 3: fixed vertical spacing and broken indentation.
These patches address several checkpatch warnings in the ad9523 driver.
Patch 1: Updated the macros to properly use their argument x.
Patch 2: Fixed the multi-line pointer dereferences.
Patch 3: Updated symbolic permissions to octal (0444/0200).
bhargav (3):
iio: frequency: ad9523: fix implicit variable usage in macros
iio: frequency: ad9523: avoid multiple line dereferences for pdata
iio: frequency: ad9523: fix checkpatch warnings for symbolic
permissions
drivers/iio/frequency/ad9523.c | 88 +++++++++++++---------------------
1 file changed, 33 insertions(+), 55 deletions(-)
--
2.53.0
On Fri, Feb 20, 2026 at 03:05:46AM +0530, bhargav wrote: > Changes in v3: > -Patch 1: updated macros to use '(x)' instead of 'x'. > -Patch 2: collected reviewed-by (no code changes). > -Patch 3: fixed vertical spacing and broken indentation. > > These patches address several checkpatch warnings in the ad9523 driver. Do a new email thread for each new version of the series. Do not send a new version more often than a couple of days. (The very bare minimum is 24h.) -- With Best Regards, Andy Shevchenko
On Fri, 20 Feb 2026 09:22:34 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Fri, Feb 20, 2026 at 03:05:46AM +0530, bhargav wrote: > > Changes in v3: > > -Patch 1: updated macros to use '(x)' instead of 'x'. > > -Patch 2: collected reviewed-by (no code changes). > > -Patch 3: fixed vertical spacing and broken indentation. > > > > These patches address several checkpatch warnings in the ad9523 driver. > > Do a new email thread for each new version of the series. > Do not send a new version more often than a couple of days. > (The very bare minimum is 24h.) > Series is simple enough I'm not going to wait on Michael giving it a quick look (as the driver maintainer). Still time for more feedback though given I'll be rebasing the tree anyway on rc1 later this week. Applied to the testing branch of iio.git. Thanks, Jonathan >
On Sun, 22 Feb 2026 13:50:54 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 20 Feb 2026 09:22:34 +0200 > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > On Fri, Feb 20, 2026 at 03:05:46AM +0530, bhargav wrote: > > > Changes in v3: > > > -Patch 1: updated macros to use '(x)' instead of 'x'. > > > -Patch 2: collected reviewed-by (no code changes). > > > -Patch 3: fixed vertical spacing and broken indentation. > > > > > > These patches address several checkpatch warnings in the ad9523 driver. > > > > Do a new email thread for each new version of the series. > > Do not send a new version more often than a couple of days. > > (The very bare minimum is 24h.) > > > > Series is simple enough I'm not going to wait on Michael > giving it a quick look (as the driver maintainer). Still > time for more feedback though given I'll be rebasing the tree > anyway on rc1 later this week. > > Applied to the testing branch of iio.git. Pulled again. Just a small thing I need to check. For sign off we need a "known identity" - see submitting patches. Mostly that's a legal name, but if people are commonly known by other names that's acceptable as well. Given short nature of your SoB and that this is (I think?) your first upstream commit I just wanted to check that bhargav is sufficient to meet this level. What counts for this varies a lot by culture or country so if that is the right level of identification to use that that's fine, just reply to let me know. Thanks, Jonathan > > Thanks, > > Jonathan > > > >
On Sun, Feb 22, 2026 at 7:28 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sun, 22 Feb 2026 13:50:54 +0000 > Jonathan Cameron <jic23@kernel.org> wrote: > > > On Fri, 20 Feb 2026 09:22:34 +0200 > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > On Fri, Feb 20, 2026 at 03:05:46AM +0530, bhargav wrote: > > > > Changes in v3: > > > > -Patch 1: updated macros to use '(x)' instead of 'x'. > > > > -Patch 2: collected reviewed-by (no code changes). > > > > -Patch 3: fixed vertical spacing and broken indentation. > > > > > > > > These patches address several checkpatch warnings in the ad9523 driver. > > > > > > Do a new email thread for each new version of the series. > > > Do not send a new version more often than a couple of days. > > > (The very bare minimum is 24h.) > > > > > > > Series is simple enough I'm not going to wait on Michael > > giving it a quick look (as the driver maintainer). Still > > time for more feedback though given I'll be rebasing the tree > > anyway on rc1 later this week. > > > > Applied to the testing branch of iio.git. > Pulled again. Just a small thing I need to check. > > For sign off we need a "known identity" - see submitting patches. > Mostly that's a legal name, but if people are commonly known > by other names that's acceptable as well. > > Given short nature of your SoB and that this is (I think?) your > first upstream commit I just wanted to check that bhargav is > sufficient to meet this level. > > What counts for this varies a lot by culture or country so if > that is the right level of identification to use that that's > fine, just reply to let me know. > > Thanks, > > Jonathan > > > > > > Thanks, > > > > Jonathan > > > > > > > > Hi Jonathan, Thank you for checking! Yes, this is my first patch. Actually "bhargav" is fine for me , but if you need my full name on signed-off (i.e Bhargav Joshi) then please let me if i need to send a v4 with updated signature or it is possible for you to amend the Signed-off-by line to "Bhargav Joshi " rougueprince47@gmail.com" on your end while applying. Best regards, Bhargav
On Mon, Feb 23, 2026 at 02:49:12AM +0530, Rogue prince wrote: > On Sun, Feb 22, 2026 at 7:28 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sun, 22 Feb 2026 13:50:54 +0000 > > Jonathan Cameron <jic23@kernel.org> wrote: ... > > Given short nature of your SoB and that this is (I think?) your > > first upstream commit I just wanted to check that bhargav is > > sufficient to meet this level. > > > > What counts for this varies a lot by culture or country so if > > that is the right level of identification to use that that's > > fine, just reply to let me know. > Thank you for checking! Yes, this is my first patch. > Actually "bhargav" is fine for me , but if you need my full name on > signed-off (i.e Bhargav Joshi) then please let me if i need to send a > v4 with updated signature or it is possible for you to amend the > Signed-off-by line to "Bhargav Joshi " rougueprince47@gmail.com" on > your end while applying. Please, make sure you send patches with this SoB (using Bhargav Joshi). Also make sure there are no double quotes. It's all documented, please take your time to study Submitting Patches and Submitting Patches Checklist documentation. -- With Best Regards, Andy Shevchenko
On Mon, 23 Feb 2026 10:35:57 +0200 Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > On Mon, Feb 23, 2026 at 02:49:12AM +0530, Rogue prince wrote: > > On Sun, Feb 22, 2026 at 7:28 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > On Sun, 22 Feb 2026 13:50:54 +0000 > > > Jonathan Cameron <jic23@kernel.org> wrote: > > ... > > > > Given short nature of your SoB and that this is (I think?) your > > > first upstream commit I just wanted to check that bhargav is > > > sufficient to meet this level. > > > > > > What counts for this varies a lot by culture or country so if > > > that is the right level of identification to use that that's > > > fine, just reply to let me know. > > > Thank you for checking! Yes, this is my first patch. > > Actually "bhargav" is fine for me , but if you need my full name on > > signed-off (i.e Bhargav Joshi) then please let me if i need to send a > > v4 with updated signature or it is possible for you to amend the > > Signed-off-by line to "Bhargav Joshi " rougueprince47@gmail.com" on > > your end while applying. > > Please, make sure you send patches with this SoB (using Bhargav Joshi). > Also make sure there are no double quotes. It's all documented, please > take your time to study Submitting Patches and Submitting Patches Checklist > documentation. > > Just to confirm. Please send a v4. SoB have legal meaning so best to have a clean record on list. Jonathan
macros AD9523_CLK_DIST_DIV_PHASE_REV(x) and
AD9523_CLK_DIST_DIV_REV(x) implicitly relied on variable
named 'ret' instead of using passed argument '(x)'.
Update the macros to explicitly
use the argument '(x)' for their operations.
This also resolves the following checkpatch.pl warning:
WARNING: Argument '(x)' is not used in function-like macro
Signed-off-by: bhargav <rougueprince47@gmail.com>
---
Changes in v3:
-replaced 'x' with '(x)' in AD9523_CLK_DIST_DIV_PHASE_REV(x) and
AD9523_CLK_DIST_DIV_REV(x)
drivers/iio/frequency/ad9523.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 63c485e9e44c..5725ab62e0fd 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -167,9 +167,9 @@
/* AD9523_CHANNEL_CLOCK_DIST */
#define AD9523_CLK_DIST_DIV_PHASE(x) (((x) & 0x3F) << 18)
-#define AD9523_CLK_DIST_DIV_PHASE_REV(x) ((ret >> 18) & 0x3F)
+#define AD9523_CLK_DIST_DIV_PHASE_REV(x) (((x) >> 18) & 0x3F)
#define AD9523_CLK_DIST_DIV(x) ((((x) - 1) & 0x3FF) << 8)
-#define AD9523_CLK_DIST_DIV_REV(x) (((ret >> 8) & 0x3FF) + 1)
+#define AD9523_CLK_DIST_DIV_REV(x) ((((x) >> 8) & 0x3FF) + 1)
#define AD9523_CLK_DIST_INV_DIV_OUTPUT_EN (1 << 7)
#define AD9523_CLK_DIST_IGNORE_SYNC_EN (1 << 6)
#define AD9523_CLK_DIST_PWR_DOWN_EN (1 << 5)
--
2.53.0On Fri, Feb 20, 2026 at 03:05:47AM +0530, bhargav wrote: > macros AD9523_CLK_DIST_DIV_PHASE_REV(x) and The macros > AD9523_CLK_DIST_DIV_REV(x) implicitly relied on variable > named 'ret' instead of using passed argument '(x)'. > > Update the macros to explicitly > use the argument '(x)' for their operations. Something with wrapping of the lines, use up to ~72 characters per line. > This also resolves the following checkpatch.pl warning: > WARNING: Argument '(x)' is not used in function-like macro Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> -- With Best Regards, Andy Shevchenko
Platform data pointer dereferences for pll1_charge_pump_current_nA
and pll2_charge_pump_current_nA were split across multiple lines.
Bring the dereference chains onto a single line.
This resolves the following checkpatch.pl warnings:
WARNING: Avoid multiple line dereference - prefer 'pdata->pll1_charge_pump_current_nA'
WARNING: Avoid multiple line dereference - prefer 'pdata->pll2_charge_pump_current_nA'
Signed-off-by: bhargav <rougueprince47@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
drivers/iio/frequency/ad9523.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 5725ab62e0fd..6daa2ea354a8 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -797,8 +797,7 @@ static int ad9523_setup(struct iio_dev *indio_dev)
return ret;
ret = ad9523_write(indio_dev, AD9523_PLL1_CHARGE_PUMP_CTRL,
- AD9523_PLL1_CHARGE_PUMP_CURRENT_nA(pdata->
- pll1_charge_pump_current_nA) |
+ AD9523_PLL1_CHARGE_PUMP_CURRENT_nA(pdata->pll1_charge_pump_current_nA) |
AD9523_PLL1_CHARGE_PUMP_MODE_NORMAL |
AD9523_PLL1_BACKLASH_PW_MIN);
if (ret < 0)
@@ -842,8 +841,7 @@ static int ad9523_setup(struct iio_dev *indio_dev)
*/
ret = ad9523_write(indio_dev, AD9523_PLL2_CHARGE_PUMP,
- AD9523_PLL2_CHARGE_PUMP_CURRENT_nA(pdata->
- pll2_charge_pump_current_nA));
+ AD9523_PLL2_CHARGE_PUMP_CURRENT_nA(pdata->pll2_charge_pump_current_nA));
if (ret < 0)
return ret;
--
2.53.0The driver currently defines device attributes using
symbolic permission flags (S_IRUGO and S_IWUSR).
Update these to use octal permissions (0444 and 0200) to resolve
checkpatch warnings.
Signed-off-by: bhargav <rougueprince47@gmail.com>
---
Changes in v3:
-reduced unwanted vertical spacing
-fixed broken indentation
drivers/iio/frequency/ad9523.c | 78 +++++++++++++---------------------
1 file changed, 29 insertions(+), 49 deletions(-)
diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 6daa2ea354a8..ad32eb66edca 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -558,55 +558,35 @@ static ssize_t ad9523_show(struct device *dev,
return ret;
}
-static IIO_DEVICE_ATTR(pll1_locked, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_PLL1_LD);
-
-static IIO_DEVICE_ATTR(pll2_locked, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_PLL2_LD);
-
-static IIO_DEVICE_ATTR(pll1_reference_clk_a_present, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_REFA);
-
-static IIO_DEVICE_ATTR(pll1_reference_clk_b_present, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_REFB);
-
-static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_REF_TEST);
-
-static IIO_DEVICE_ATTR(vcxo_clk_present, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_VCXO);
-
-static IIO_DEVICE_ATTR(pll2_feedback_clk_present, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_PLL2_FB_CLK);
-
-static IIO_DEVICE_ATTR(pll2_reference_clk_present, S_IRUGO,
- ad9523_show,
- NULL,
- AD9523_STAT_PLL2_REF_CLK);
-
-static IIO_DEVICE_ATTR(sync_dividers, S_IWUSR,
- NULL,
- ad9523_store,
- AD9523_SYNC);
-
-static IIO_DEVICE_ATTR(store_eeprom, S_IWUSR,
- NULL,
- ad9523_store,
- AD9523_EEPROM);
+static IIO_DEVICE_ATTR(pll1_locked, 0444, ad9523_show, NULL,
+ AD9523_STAT_PLL1_LD);
+
+static IIO_DEVICE_ATTR(pll2_locked, 0444, ad9523_show, NULL,
+ AD9523_STAT_PLL2_LD);
+
+static IIO_DEVICE_ATTR(pll1_reference_clk_a_present, 0444, ad9523_show, NULL,
+ AD9523_STAT_REFA);
+
+static IIO_DEVICE_ATTR(pll1_reference_clk_b_present, 0444, ad9523_show, NULL,
+ AD9523_STAT_REFB);
+
+static IIO_DEVICE_ATTR(pll1_reference_clk_test_present, 0444, ad9523_show, NULL,
+ AD9523_STAT_REF_TEST);
+
+static IIO_DEVICE_ATTR(vcxo_clk_present, 0444, ad9523_show, NULL,
+ AD9523_STAT_VCXO);
+
+static IIO_DEVICE_ATTR(pll2_feedback_clk_present, 0444, ad9523_show, NULL,
+ AD9523_STAT_PLL2_FB_CLK);
+
+static IIO_DEVICE_ATTR(pll2_reference_clk_present, 0444, ad9523_show, NULL,
+ AD9523_STAT_PLL2_REF_CLK);
+
+static IIO_DEVICE_ATTR(sync_dividers, 0200, NULL, ad9523_store,
+ AD9523_SYNC);
+
+static IIO_DEVICE_ATTR(store_eeprom, 0200, NULL, ad9523_store,
+ AD9523_EEPROM);
static struct attribute *ad9523_attributes[] = {
&iio_dev_attr_sync_dividers.dev_attr.attr,
--
2.53.0On Fri, Feb 20, 2026 at 03:05:49AM +0530, bhargav wrote: > The driver currently defines device attributes using > symbolic permission flags (S_IRUGO and S_IWUSR). > Update these to use octal permissions (0444 and 0200) to resolve > checkpatch warnings. Same for the line lengths in the commit message, use up to ~72 characters per line. Otherwise looks good to me, Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.