linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros
@ 2020-12-12 19:55 Dwaipayan Ray
  2020-12-13  8:00 ` Lukas Bulwahn
  0 siblings, 1 reply; 5+ messages in thread
From: Dwaipayan Ray @ 2020-12-12 19:55 UTC (permalink / raw)
  To: linux-leds
  Cc: Dwaipayan Ray, Greg Kroah-Hartman, Dan Murphy, Pavel Machek,
	Lukas Bulwahn, linux-kernel, linux-kernel-mentees

Instead of open coding DEVICE_ATTR() defines, use the
DEVICE_ATTR_RW(), DEVICE_ATTR_WO(), and DEVICE_ATTR_RO()
macros intead.

This required a few functions to be renamed, but the functionality
itself is unchanged.

Note that this is compile tested only.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: linux-leds@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 drivers/leds/leds-blinkm.c        | 24 ++++++++++++------------
 drivers/leds/leds-lm3530.c        | 10 +++++-----
 drivers/leds/leds-lm355x.c        |  8 ++++----
 drivers/leds/leds-lm3642.c        | 16 ++++++++--------
 drivers/leds/leds-max8997.c       | 12 ++++++------
 drivers/leds/leds-netxbig.c       | 12 ++++++------
 drivers/leds/leds-ss4200.c        | 12 ++++++------
 drivers/leds/leds-wm831x-status.c | 12 ++++++------
 8 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
index e11fe1788242..b4e1fdff4186 100644
--- a/drivers/leds/leds-blinkm.c
+++ b/drivers/leds/leds-blinkm.c
@@ -192,13 +192,13 @@ static int store_color_common(struct device *dev, const char *buf, int color)
 	return 0;
 }
 
-static ssize_t show_red(struct device *dev, struct device_attribute *attr,
+static ssize_t red_show(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
 	return show_color_common(dev, buf, RED);
 }
 
-static ssize_t store_red(struct device *dev, struct device_attribute *attr,
+static ssize_t red_store(struct device *dev, struct device_attribute *attr,
 			 const char *buf, size_t count)
 {
 	int ret;
@@ -209,15 +209,15 @@ static ssize_t store_red(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(red, S_IRUGO | S_IWUSR, show_red, store_red);
+static DEVICE_ATTR_RW(red);
 
-static ssize_t show_green(struct device *dev, struct device_attribute *attr,
+static ssize_t green_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
 	return show_color_common(dev, buf, GREEN);
 }
 
-static ssize_t store_green(struct device *dev, struct device_attribute *attr,
+static ssize_t green_store(struct device *dev, struct device_attribute *attr,
 			   const char *buf, size_t count)
 {
 
@@ -229,15 +229,15 @@ static ssize_t store_green(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(green, S_IRUGO | S_IWUSR, show_green, store_green);
+static DEVICE_ATTR_RW(green);
 
-static ssize_t show_blue(struct device *dev, struct device_attribute *attr,
+static ssize_t blue_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	return show_color_common(dev, buf, BLUE);
 }
 
-static ssize_t store_blue(struct device *dev, struct device_attribute *attr,
+static ssize_t blue_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
 {
 	int ret;
@@ -248,16 +248,16 @@ static ssize_t store_blue(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(blue, S_IRUGO | S_IWUSR, show_blue, store_blue);
+static DEVICE_ATTR_RW(blue);
 
-static ssize_t show_test(struct device *dev, struct device_attribute *attr,
+static ssize_t test_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	return scnprintf(buf, PAGE_SIZE,
 			 "#Write into test to start test sequence!#\n");
 }
 
-static ssize_t store_test(struct device *dev, struct device_attribute *attr,
+static ssize_t test_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
 {
 
@@ -273,7 +273,7 @@ static ssize_t store_test(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(test, S_IRUGO | S_IWUSR, show_test, store_test);
+static DEVICE_ATTR_RW(test);
 
 /* TODO: HSB, fade, timeadj, script ... */
 
diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
index 2f8362f6bf75..2db455efd4b1 100644
--- a/drivers/leds/leds-lm3530.c
+++ b/drivers/leds/leds-lm3530.c
@@ -346,8 +346,8 @@ static void lm3530_brightness_set(struct led_classdev *led_cdev,
 	}
 }
 
-static ssize_t lm3530_mode_get(struct device *dev,
-		struct device_attribute *attr, char *buf)
+static ssize_t mode_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct lm3530_data *drvdata;
@@ -365,8 +365,8 @@ static ssize_t lm3530_mode_get(struct device *dev,
 	return len;
 }
 
-static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute
-				   *attr, const char *buf, size_t size)
+static ssize_t mode_store(struct device *dev, struct device_attribute
+			  *attr, const char *buf, size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct lm3530_data *drvdata;
@@ -397,7 +397,7 @@ static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute
 
 	return sizeof(drvdata->mode);
 }
-static DEVICE_ATTR(mode, 0644, lm3530_mode_get, lm3530_mode_set);
+static DEVICE_ATTR_RW(mode);
 
 static struct attribute *lm3530_attrs[] = {
 	&dev_attr_mode.attr,
diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
index 1505521249b5..2d3e11845ba5 100644
--- a/drivers/leds/leds-lm355x.c
+++ b/drivers/leds/leds-lm355x.c
@@ -349,9 +349,9 @@ static int lm355x_indicator_brightness_set(struct led_classdev *cdev,
 }
 
 /* indicator pattern only for lm3556*/
-static ssize_t lm3556_indicator_pattern_store(struct device *dev,
-					      struct device_attribute *attr,
-					      const char *buf, size_t size)
+static ssize_t pattern_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t size)
 {
 	ssize_t ret;
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
@@ -381,7 +381,7 @@ static ssize_t lm3556_indicator_pattern_store(struct device *dev,
 	return ret;
 }
 
-static DEVICE_ATTR(pattern, S_IWUSR, NULL, lm3556_indicator_pattern_store);
+static DEVICE_ATTR_WO(pattern);
 
 static struct attribute *lm355x_indicator_attrs[] = {
 	&dev_attr_pattern.attr,
diff --git a/drivers/leds/leds-lm3642.c b/drivers/leds/leds-lm3642.c
index 62c14872caf7..8007b82985a8 100644
--- a/drivers/leds/leds-lm3642.c
+++ b/drivers/leds/leds-lm3642.c
@@ -165,9 +165,9 @@ static int lm3642_control(struct lm3642_chip_data *chip,
 /* torch */
 
 /* torch pin config for lm3642 */
-static ssize_t lm3642_torch_pin_store(struct device *dev,
-				      struct device_attribute *attr,
-				      const char *buf, size_t size)
+static ssize_t torch_pin_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t size)
 {
 	ssize_t ret;
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
@@ -193,7 +193,7 @@ static ssize_t lm3642_torch_pin_store(struct device *dev,
 	return size;
 }
 
-static DEVICE_ATTR(torch_pin, S_IWUSR, NULL, lm3642_torch_pin_store);
+static DEVICE_ATTR_WO(torch_pin);
 
 static int lm3642_torch_brightness_set(struct led_classdev *cdev,
 					enum led_brightness brightness)
@@ -212,9 +212,9 @@ static int lm3642_torch_brightness_set(struct led_classdev *cdev,
 /* flash */
 
 /* strobe pin config for lm3642*/
-static ssize_t lm3642_strobe_pin_store(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t size)
+static ssize_t strobe_pin_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t size)
 {
 	ssize_t ret;
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
@@ -240,7 +240,7 @@ static ssize_t lm3642_strobe_pin_store(struct device *dev,
 	return size;
 }
 
-static DEVICE_ATTR(strobe_pin, S_IWUSR, NULL, lm3642_strobe_pin_store);
+static DEVICE_ATTR_WO(strobe_pin);
 
 static int lm3642_strobe_brightness_set(struct led_classdev *cdev,
 					 enum led_brightness brightness)
diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
index 512a11d142d0..c0bddb33888d 100644
--- a/drivers/leds/leds-max8997.c
+++ b/drivers/leds/leds-max8997.c
@@ -160,8 +160,8 @@ static void max8997_led_brightness_set(struct led_classdev *led_cdev,
 	}
 }
 
-static ssize_t max8997_led_show_mode(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t mode_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct max8997_led *led =
@@ -193,9 +193,9 @@ static ssize_t max8997_led_show_mode(struct device *dev,
 	return ret;
 }
 
-static ssize_t max8997_led_store_mode(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf, size_t size)
+static ssize_t mode_store(struct device *dev,
+			  struct device_attribute *attr,
+			  const char *buf, size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct max8997_led *led =
@@ -222,7 +222,7 @@ static ssize_t max8997_led_store_mode(struct device *dev,
 	return size;
 }
 
-static DEVICE_ATTR(mode, 0644, max8997_led_show_mode, max8997_led_store_mode);
+static DEVICE_ATTR_RW(mode);
 
 static struct attribute *max8997_attrs[] = {
 	&dev_attr_mode.attr,
diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
index 68fbf0b66fad..77213b79f84d 100644
--- a/drivers/leds/leds-netxbig.c
+++ b/drivers/leds/leds-netxbig.c
@@ -204,9 +204,9 @@ static void netxbig_led_set(struct led_classdev *led_cdev,
 	spin_unlock_irqrestore(&led_dat->lock, flags);
 }
 
-static ssize_t netxbig_led_sata_store(struct device *dev,
-				      struct device_attribute *attr,
-				      const char *buff, size_t count)
+static ssize_t sata_store(struct device *dev,
+			  struct device_attribute *attr,
+			  const char *buff, size_t count)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct netxbig_led_data *led_dat =
@@ -255,8 +255,8 @@ static ssize_t netxbig_led_sata_store(struct device *dev,
 	return ret;
 }
 
-static ssize_t netxbig_led_sata_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
+static ssize_t sata_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct netxbig_led_data *led_dat =
@@ -265,7 +265,7 @@ static ssize_t netxbig_led_sata_show(struct device *dev,
 	return sprintf(buf, "%d\n", led_dat->sata);
 }
 
-static DEVICE_ATTR(sata, 0644, netxbig_led_sata_show, netxbig_led_sata_store);
+static DEVICE_ATTR_RW(sata);
 
 static struct attribute *netxbig_led_attrs[] = {
 	&dev_attr_sata.attr,
diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
index 245de443fe9c..a6992323d69d 100644
--- a/drivers/leds/leds-ss4200.c
+++ b/drivers/leds/leds-ss4200.c
@@ -441,8 +441,8 @@ static void set_power_light_amber_noblink(void)
 	nasgpio_led_set_brightness(&amber->led_cdev, LED_FULL);
 }
 
-static ssize_t nas_led_blink_show(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t blink_show(struct device *dev,
+			  struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led = dev_get_drvdata(dev);
 	int blinking = 0;
@@ -451,9 +451,9 @@ static ssize_t nas_led_blink_show(struct device *dev,
 	return sprintf(buf, "%u\n", blinking);
 }
 
-static ssize_t nas_led_blink_store(struct device *dev,
-				   struct device_attribute *attr,
-				   const char *buf, size_t size)
+static ssize_t blink_store(struct device *dev,
+			   struct device_attribute *attr,
+			   const char *buf, size_t size)
 {
 	int ret;
 	struct led_classdev *led = dev_get_drvdata(dev);
@@ -468,7 +468,7 @@ static ssize_t nas_led_blink_store(struct device *dev,
 	return size;
 }
 
-static DEVICE_ATTR(blink, 0644, nas_led_blink_show, nas_led_blink_store);
+static DEVICE_ATTR_RW(blink);
 
 static struct attribute *nasgpio_led_attrs[] = {
 	&dev_attr_blink.attr,
diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
index 67f4235cb28a..c48b80574f02 100644
--- a/drivers/leds/leds-wm831x-status.c
+++ b/drivers/leds/leds-wm831x-status.c
@@ -155,8 +155,8 @@ static const char * const led_src_texts[] = {
 	"soft",
 };
 
-static ssize_t wm831x_status_src_show(struct device *dev,
-				      struct device_attribute *attr, char *buf)
+static ssize_t src_show(struct device *dev,
+			struct device_attribute *attr, char *buf)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct wm831x_status *led = to_wm831x_status(led_cdev);
@@ -178,9 +178,9 @@ static ssize_t wm831x_status_src_show(struct device *dev,
 	return ret;
 }
 
-static ssize_t wm831x_status_src_store(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf, size_t size)
+static ssize_t src_store(struct device *dev,
+			 struct device_attribute *attr,
+			 const char *buf, size_t size)
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct wm831x_status *led = to_wm831x_status(led_cdev);
@@ -197,7 +197,7 @@ static ssize_t wm831x_status_src_store(struct device *dev,
 	return size;
 }
 
-static DEVICE_ATTR(src, 0644, wm831x_status_src_show, wm831x_status_src_store);
+static DEVICE_ATTR_RW(src);
 
 static struct attribute *wm831x_status_attrs[] = {
 	&dev_attr_src.attr,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros
  2020-12-12 19:55 [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros Dwaipayan Ray
@ 2020-12-13  8:00 ` Lukas Bulwahn
  2020-12-13  8:18   ` Dwaipayan Ray
  0 siblings, 1 reply; 5+ messages in thread
From: Lukas Bulwahn @ 2020-12-13  8:00 UTC (permalink / raw)
  To: Dwaipayan Ray
  Cc: linux-leds, Greg Kroah-Hartman, Dan Murphy, Pavel Machek,
	Linux Kernel Mailing List, linux-kernel-mentees

On Sat, Dec 12, 2020 at 8:56 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
>
> Instead of open coding DEVICE_ATTR() defines, use the
> DEVICE_ATTR_RW(), DEVICE_ATTR_WO(), and DEVICE_ATTR_RO()
> macros intead.

typo: s/intead/instead/

No need to use the word "instead" twice in one sentence, though, we got it :)

>
> This required a few functions to be renamed, but the functionality
> itself is unchanged.
>
> Note that this is compile tested only.
>

This note does not go in the commit message. In the future, this will
be simply not true anymore, but this below the "---" (see HERE! as
marker).

For testing, you can generate the objdump of the binary before and
after and compare if that is as expected.

Other than that, the change itself looks good to me, so:

Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>

Thanks, Dwaipayan, for fixing this up.

Will you also add a checkpatch rule to identify other DEVICE_ATTR(...)
that can be adjusted to the refined macros, so that checkpatch informs
other submitters as well?

> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Cc: linux-leds@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-kernel-mentees@lists.linuxfoundation.org

As far as I know, the maintainers will add the Cc lines---if they like
those---with script support.

> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---

HERE!

>  drivers/leds/leds-blinkm.c        | 24 ++++++++++++------------
>  drivers/leds/leds-lm3530.c        | 10 +++++-----
>  drivers/leds/leds-lm355x.c        |  8 ++++----
>  drivers/leds/leds-lm3642.c        | 16 ++++++++--------
>  drivers/leds/leds-max8997.c       | 12 ++++++------
>  drivers/leds/leds-netxbig.c       | 12 ++++++------
>  drivers/leds/leds-ss4200.c        | 12 ++++++------
>  drivers/leds/leds-wm831x-status.c | 12 ++++++------
>  8 files changed, 53 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
> index e11fe1788242..b4e1fdff4186 100644
> --- a/drivers/leds/leds-blinkm.c
> +++ b/drivers/leds/leds-blinkm.c
> @@ -192,13 +192,13 @@ static int store_color_common(struct device *dev, const char *buf, int color)
>         return 0;
>  }
>
> -static ssize_t show_red(struct device *dev, struct device_attribute *attr,
> +static ssize_t red_show(struct device *dev, struct device_attribute *attr,
>                         char *buf)
>  {
>         return show_color_common(dev, buf, RED);
>  }
>
> -static ssize_t store_red(struct device *dev, struct device_attribute *attr,
> +static ssize_t red_store(struct device *dev, struct device_attribute *attr,
>                          const char *buf, size_t count)
>  {
>         int ret;
> @@ -209,15 +209,15 @@ static ssize_t store_red(struct device *dev, struct device_attribute *attr,
>         return count;
>  }
>
> -static DEVICE_ATTR(red, S_IRUGO | S_IWUSR, show_red, store_red);
> +static DEVICE_ATTR_RW(red);
>
> -static ssize_t show_green(struct device *dev, struct device_attribute *attr,
> +static ssize_t green_show(struct device *dev, struct device_attribute *attr,
>                           char *buf)
>  {
>         return show_color_common(dev, buf, GREEN);
>  }
>
> -static ssize_t store_green(struct device *dev, struct device_attribute *attr,
> +static ssize_t green_store(struct device *dev, struct device_attribute *attr,
>                            const char *buf, size_t count)
>  {
>
> @@ -229,15 +229,15 @@ static ssize_t store_green(struct device *dev, struct device_attribute *attr,
>         return count;
>  }
>
> -static DEVICE_ATTR(green, S_IRUGO | S_IWUSR, show_green, store_green);
> +static DEVICE_ATTR_RW(green);
>
> -static ssize_t show_blue(struct device *dev, struct device_attribute *attr,
> +static ssize_t blue_show(struct device *dev, struct device_attribute *attr,
>                          char *buf)
>  {
>         return show_color_common(dev, buf, BLUE);
>  }
>
> -static ssize_t store_blue(struct device *dev, struct device_attribute *attr,
> +static ssize_t blue_store(struct device *dev, struct device_attribute *attr,
>                           const char *buf, size_t count)
>  {
>         int ret;
> @@ -248,16 +248,16 @@ static ssize_t store_blue(struct device *dev, struct device_attribute *attr,
>         return count;
>  }
>
> -static DEVICE_ATTR(blue, S_IRUGO | S_IWUSR, show_blue, store_blue);
> +static DEVICE_ATTR_RW(blue);
>
> -static ssize_t show_test(struct device *dev, struct device_attribute *attr,
> +static ssize_t test_show(struct device *dev, struct device_attribute *attr,
>                          char *buf)
>  {
>         return scnprintf(buf, PAGE_SIZE,
>                          "#Write into test to start test sequence!#\n");
>  }
>
> -static ssize_t store_test(struct device *dev, struct device_attribute *attr,
> +static ssize_t test_store(struct device *dev, struct device_attribute *attr,
>                           const char *buf, size_t count)
>  {
>
> @@ -273,7 +273,7 @@ static ssize_t store_test(struct device *dev, struct device_attribute *attr,
>         return count;
>  }
>
> -static DEVICE_ATTR(test, S_IRUGO | S_IWUSR, show_test, store_test);
> +static DEVICE_ATTR_RW(test);
>
>  /* TODO: HSB, fade, timeadj, script ... */
>
> diff --git a/drivers/leds/leds-lm3530.c b/drivers/leds/leds-lm3530.c
> index 2f8362f6bf75..2db455efd4b1 100644
> --- a/drivers/leds/leds-lm3530.c
> +++ b/drivers/leds/leds-lm3530.c
> @@ -346,8 +346,8 @@ static void lm3530_brightness_set(struct led_classdev *led_cdev,
>         }
>  }
>
> -static ssize_t lm3530_mode_get(struct device *dev,
> -               struct device_attribute *attr, char *buf)
> +static ssize_t mode_show(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct lm3530_data *drvdata;
> @@ -365,8 +365,8 @@ static ssize_t lm3530_mode_get(struct device *dev,
>         return len;
>  }
>
> -static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute
> -                                  *attr, const char *buf, size_t size)
> +static ssize_t mode_store(struct device *dev, struct device_attribute
> +                         *attr, const char *buf, size_t size)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct lm3530_data *drvdata;
> @@ -397,7 +397,7 @@ static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute
>
>         return sizeof(drvdata->mode);
>  }
> -static DEVICE_ATTR(mode, 0644, lm3530_mode_get, lm3530_mode_set);
> +static DEVICE_ATTR_RW(mode);
>
>  static struct attribute *lm3530_attrs[] = {
>         &dev_attr_mode.attr,
> diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
> index 1505521249b5..2d3e11845ba5 100644
> --- a/drivers/leds/leds-lm355x.c
> +++ b/drivers/leds/leds-lm355x.c
> @@ -349,9 +349,9 @@ static int lm355x_indicator_brightness_set(struct led_classdev *cdev,
>  }
>
>  /* indicator pattern only for lm3556*/
> -static ssize_t lm3556_indicator_pattern_store(struct device *dev,
> -                                             struct device_attribute *attr,
> -                                             const char *buf, size_t size)
> +static ssize_t pattern_store(struct device *dev,
> +                            struct device_attribute *attr,
> +                            const char *buf, size_t size)
>  {
>         ssize_t ret;
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
> @@ -381,7 +381,7 @@ static ssize_t lm3556_indicator_pattern_store(struct device *dev,
>         return ret;
>  }
>
> -static DEVICE_ATTR(pattern, S_IWUSR, NULL, lm3556_indicator_pattern_store);
> +static DEVICE_ATTR_WO(pattern);
>
>  static struct attribute *lm355x_indicator_attrs[] = {
>         &dev_attr_pattern.attr,
> diff --git a/drivers/leds/leds-lm3642.c b/drivers/leds/leds-lm3642.c
> index 62c14872caf7..8007b82985a8 100644
> --- a/drivers/leds/leds-lm3642.c
> +++ b/drivers/leds/leds-lm3642.c
> @@ -165,9 +165,9 @@ static int lm3642_control(struct lm3642_chip_data *chip,
>  /* torch */
>
>  /* torch pin config for lm3642 */
> -static ssize_t lm3642_torch_pin_store(struct device *dev,
> -                                     struct device_attribute *attr,
> -                                     const char *buf, size_t size)
> +static ssize_t torch_pin_store(struct device *dev,
> +                              struct device_attribute *attr,
> +                              const char *buf, size_t size)
>  {
>         ssize_t ret;
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
> @@ -193,7 +193,7 @@ static ssize_t lm3642_torch_pin_store(struct device *dev,
>         return size;
>  }
>
> -static DEVICE_ATTR(torch_pin, S_IWUSR, NULL, lm3642_torch_pin_store);
> +static DEVICE_ATTR_WO(torch_pin);
>
>  static int lm3642_torch_brightness_set(struct led_classdev *cdev,
>                                         enum led_brightness brightness)
> @@ -212,9 +212,9 @@ static int lm3642_torch_brightness_set(struct led_classdev *cdev,
>  /* flash */
>
>  /* strobe pin config for lm3642*/
> -static ssize_t lm3642_strobe_pin_store(struct device *dev,
> -                                      struct device_attribute *attr,
> -                                      const char *buf, size_t size)
> +static ssize_t strobe_pin_store(struct device *dev,
> +                               struct device_attribute *attr,
> +                               const char *buf, size_t size)
>  {
>         ssize_t ret;
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
> @@ -240,7 +240,7 @@ static ssize_t lm3642_strobe_pin_store(struct device *dev,
>         return size;
>  }
>
> -static DEVICE_ATTR(strobe_pin, S_IWUSR, NULL, lm3642_strobe_pin_store);
> +static DEVICE_ATTR_WO(strobe_pin);
>
>  static int lm3642_strobe_brightness_set(struct led_classdev *cdev,
>                                          enum led_brightness brightness)
> diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
> index 512a11d142d0..c0bddb33888d 100644
> --- a/drivers/leds/leds-max8997.c
> +++ b/drivers/leds/leds-max8997.c
> @@ -160,8 +160,8 @@ static void max8997_led_brightness_set(struct led_classdev *led_cdev,
>         }
>  }
>
> -static ssize_t max8997_led_show_mode(struct device *dev,
> -                               struct device_attribute *attr, char *buf)
> +static ssize_t mode_show(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct max8997_led *led =
> @@ -193,9 +193,9 @@ static ssize_t max8997_led_show_mode(struct device *dev,
>         return ret;
>  }
>
> -static ssize_t max8997_led_store_mode(struct device *dev,
> -                               struct device_attribute *attr,
> -                               const char *buf, size_t size)
> +static ssize_t mode_store(struct device *dev,
> +                         struct device_attribute *attr,
> +                         const char *buf, size_t size)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct max8997_led *led =
> @@ -222,7 +222,7 @@ static ssize_t max8997_led_store_mode(struct device *dev,
>         return size;
>  }
>
> -static DEVICE_ATTR(mode, 0644, max8997_led_show_mode, max8997_led_store_mode);
> +static DEVICE_ATTR_RW(mode);
>
>  static struct attribute *max8997_attrs[] = {
>         &dev_attr_mode.attr,
> diff --git a/drivers/leds/leds-netxbig.c b/drivers/leds/leds-netxbig.c
> index 68fbf0b66fad..77213b79f84d 100644
> --- a/drivers/leds/leds-netxbig.c
> +++ b/drivers/leds/leds-netxbig.c
> @@ -204,9 +204,9 @@ static void netxbig_led_set(struct led_classdev *led_cdev,
>         spin_unlock_irqrestore(&led_dat->lock, flags);
>  }
>
> -static ssize_t netxbig_led_sata_store(struct device *dev,
> -                                     struct device_attribute *attr,
> -                                     const char *buff, size_t count)
> +static ssize_t sata_store(struct device *dev,
> +                         struct device_attribute *attr,
> +                         const char *buff, size_t count)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct netxbig_led_data *led_dat =
> @@ -255,8 +255,8 @@ static ssize_t netxbig_led_sata_store(struct device *dev,
>         return ret;
>  }
>
> -static ssize_t netxbig_led_sata_show(struct device *dev,
> -                                    struct device_attribute *attr, char *buf)
> +static ssize_t sata_show(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct netxbig_led_data *led_dat =
> @@ -265,7 +265,7 @@ static ssize_t netxbig_led_sata_show(struct device *dev,
>         return sprintf(buf, "%d\n", led_dat->sata);
>  }
>
> -static DEVICE_ATTR(sata, 0644, netxbig_led_sata_show, netxbig_led_sata_store);
> +static DEVICE_ATTR_RW(sata);
>
>  static struct attribute *netxbig_led_attrs[] = {
>         &dev_attr_sata.attr,
> diff --git a/drivers/leds/leds-ss4200.c b/drivers/leds/leds-ss4200.c
> index 245de443fe9c..a6992323d69d 100644
> --- a/drivers/leds/leds-ss4200.c
> +++ b/drivers/leds/leds-ss4200.c
> @@ -441,8 +441,8 @@ static void set_power_light_amber_noblink(void)
>         nasgpio_led_set_brightness(&amber->led_cdev, LED_FULL);
>  }
>
> -static ssize_t nas_led_blink_show(struct device *dev,
> -                                 struct device_attribute *attr, char *buf)
> +static ssize_t blink_show(struct device *dev,
> +                         struct device_attribute *attr, char *buf)
>  {
>         struct led_classdev *led = dev_get_drvdata(dev);
>         int blinking = 0;
> @@ -451,9 +451,9 @@ static ssize_t nas_led_blink_show(struct device *dev,
>         return sprintf(buf, "%u\n", blinking);
>  }
>
> -static ssize_t nas_led_blink_store(struct device *dev,
> -                                  struct device_attribute *attr,
> -                                  const char *buf, size_t size)
> +static ssize_t blink_store(struct device *dev,
> +                          struct device_attribute *attr,
> +                          const char *buf, size_t size)
>  {
>         int ret;
>         struct led_classdev *led = dev_get_drvdata(dev);
> @@ -468,7 +468,7 @@ static ssize_t nas_led_blink_store(struct device *dev,
>         return size;
>  }
>
> -static DEVICE_ATTR(blink, 0644, nas_led_blink_show, nas_led_blink_store);
> +static DEVICE_ATTR_RW(blink);
>
>  static struct attribute *nasgpio_led_attrs[] = {
>         &dev_attr_blink.attr,
> diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
> index 67f4235cb28a..c48b80574f02 100644
> --- a/drivers/leds/leds-wm831x-status.c
> +++ b/drivers/leds/leds-wm831x-status.c
> @@ -155,8 +155,8 @@ static const char * const led_src_texts[] = {
>         "soft",
>  };
>
> -static ssize_t wm831x_status_src_show(struct device *dev,
> -                                     struct device_attribute *attr, char *buf)
> +static ssize_t src_show(struct device *dev,
> +                       struct device_attribute *attr, char *buf)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct wm831x_status *led = to_wm831x_status(led_cdev);
> @@ -178,9 +178,9 @@ static ssize_t wm831x_status_src_show(struct device *dev,
>         return ret;
>  }
>
> -static ssize_t wm831x_status_src_store(struct device *dev,
> -                                      struct device_attribute *attr,
> -                                      const char *buf, size_t size)
> +static ssize_t src_store(struct device *dev,
> +                        struct device_attribute *attr,
> +                        const char *buf, size_t size)
>  {
>         struct led_classdev *led_cdev = dev_get_drvdata(dev);
>         struct wm831x_status *led = to_wm831x_status(led_cdev);
> @@ -197,7 +197,7 @@ static ssize_t wm831x_status_src_store(struct device *dev,
>         return size;
>  }
>
> -static DEVICE_ATTR(src, 0644, wm831x_status_src_show, wm831x_status_src_store);
> +static DEVICE_ATTR_RW(src);
>
>  static struct attribute *wm831x_status_attrs[] = {
>         &dev_attr_src.attr,
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros
  2020-12-13  8:00 ` Lukas Bulwahn
@ 2020-12-13  8:18   ` Dwaipayan Ray
  2020-12-13  8:40     ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Dwaipayan Ray @ 2020-12-13  8:18 UTC (permalink / raw)
  To: Lukas Bulwahn
  Cc: linux-leds, Greg Kroah-Hartman, Dan Murphy, Pavel Machek,
	Linux Kernel Mailing List, linux-kernel-mentees

On Sun, Dec 13, 2020 at 1:31 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
>
> On Sat, Dec 12, 2020 at 8:56 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
> >
> > Instead of open coding DEVICE_ATTR() defines, use the
> > DEVICE_ATTR_RW(), DEVICE_ATTR_WO(), and DEVICE_ATTR_RO()
> > macros intead.
>
> typo: s/intead/instead/
>
> No need to use the word "instead" twice in one sentence, though, we got it :)
>
> >
> > This required a few functions to be renamed, but the functionality
> > itself is unchanged.
> >
> > Note that this is compile tested only.
> >
>
> This note does not go in the commit message. In the future, this will
> be simply not true anymore, but this below the "---" (see HERE! as
> marker).
>
> For testing, you can generate the objdump of the binary before and
> after and compare if that is as expected.
>
> Other than that, the change itself looks good to me, so:
>
> Reviewed-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>
> Thanks, Dwaipayan, for fixing this up.
>
> Will you also add a checkpatch rule to identify other DEVICE_ATTR(...)
> that can be adjusted to the refined macros, so that checkpatch informs
> other submitters as well?
>

I think a checkpatch rule for this already exists. But it cannot automatically
rename the function names. That might be the only drawback we got.
Probably clang-format could fix these automatically.

> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Dan Murphy <dmurphy@ti.com>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> > Cc: linux-leds@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-kernel-mentees@lists.linuxfoundation.org
>
> As far as I know, the maintainers will add the Cc lines---if they like
> those---with script support.
>

I thought I might ease the work of maintainers to add those lines :(
But nevertheless I will remove them.

> > Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> > ---
>
> HERE!
>

Thanks Lukas.
I will be sending in a v2 if the led maintainers have no problem with
this patch.

Thank you,
Dwaipayan.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros
  2020-12-13  8:18   ` Dwaipayan Ray
@ 2020-12-13  8:40     ` Joe Perches
  2020-12-13 19:46       ` Miguel Ojeda
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2020-12-13  8:40 UTC (permalink / raw)
  To: Dwaipayan Ray, Lukas Bulwahn
  Cc: linux-leds, Greg Kroah-Hartman, Dan Murphy, Pavel Machek,
	Linux Kernel Mailing List, linux-kernel-mentees

On Sun, 2020-12-13 at 13:48 +0530, Dwaipayan Ray wrote:
> On Sun, Dec 13, 2020 at 1:31 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
[]
> > Will you also add a checkpatch rule to identify other DEVICE_ATTR(...)
> > that can be adjusted to the refined macros, so that checkpatch informs
> > other submitters as well?
> > 
> I think a checkpatch rule for this already exists. But it cannot automatically
> rename the function names. That might be the only drawback we got.
> Probably clang-format could fix these automatically.

clang-format is not a tool to rewrite code only neaten its layout.

coccinelle _might_ be able to do this for limited cases where the
show function is in the same compilation unit/file, but even then
it would not be a trivial script.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros
  2020-12-13  8:40     ` Joe Perches
@ 2020-12-13 19:46       ` Miguel Ojeda
  0 siblings, 0 replies; 5+ messages in thread
From: Miguel Ojeda @ 2020-12-13 19:46 UTC (permalink / raw)
  To: Joe Perches
  Cc: Dwaipayan Ray, Lukas Bulwahn, linux-leds, Greg Kroah-Hartman,
	Dan Murphy, Pavel Machek, Linux Kernel Mailing List,
	linux-kernel-mentees

On Sun, Dec 13, 2020 at 7:21 PM Joe Perches <joe@perches.com> wrote:
>
> clang-format is not a tool to rewrite code only neaten its layout.
>
> coccinelle _might_ be able to do this for limited cases where the
> show function is in the same compilation unit/file, but even then
> it would not be a trivial script.

+1 The most robust approach, but the one that is most involved, would
be a clang-tidy check.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-13 19:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-12 19:55 [PATCH] leds: Use DEVICE_ATTR_{RW, RO, WO} macros Dwaipayan Ray
2020-12-13  8:00 ` Lukas Bulwahn
2020-12-13  8:18   ` Dwaipayan Ray
2020-12-13  8:40     ` Joe Perches
2020-12-13 19:46       ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).