All of lore.kernel.org
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH] leds: convert symbolic permission bit macros to octal
@ 2020-12-12 15:35 Dwaipayan Ray
  2020-12-12 17:51 ` Greg KH
  0 siblings, 1 reply; 3+ messages in thread
From: Dwaipayan Ray @ 2020-12-12 15:35 UTC (permalink / raw)
  To: lukas.bulwahn; +Cc: dwaipayanray1, linux-kernel-mentees

Symbolic macro names are hard to understand and should not be
used for permission bits.

Convert all bad symbolic permission bit macro uses in led to just use
the octal numbers.

Following macros were replaced:

S_IRUGO => 0444
S_IWUSR => 0200
S_IRUGO | S_IWUSR => 0644

Link: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 drivers/leds/leds-blinkm.c        | 8 ++++----
 drivers/leds/leds-lm355x.c        | 2 +-
 drivers/leds/leds-lm3642.c        | 4 ++--
 drivers/leds/leds-lp55xx-common.h | 6 +++---
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
index e11fe1788242..6b9b13f58d8a 100644
--- a/drivers/leds/leds-blinkm.c
+++ b/drivers/leds/leds-blinkm.c
@@ -209,7 +209,7 @@ 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(red, 0644, show_red, store_red);
 
 static ssize_t show_green(struct device *dev, struct device_attribute *attr,
 			  char *buf)
@@ -229,7 +229,7 @@ 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(green, 0644, show_green, store_green);
 
 static ssize_t show_blue(struct device *dev, struct device_attribute *attr,
 			 char *buf)
@@ -248,7 +248,7 @@ 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(blue, 0644, show_blue, store_blue);
 
 static ssize_t show_test(struct device *dev, struct device_attribute *attr,
 			 char *buf)
@@ -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(test, 0644, show_test, store_test);
 
 /* TODO: HSB, fade, timeadj, script ... */
 
diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
index 1505521249b5..d15cc0808573 100644
--- a/drivers/leds/leds-lm355x.c
+++ b/drivers/leds/leds-lm355x.c
@@ -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(pattern, 0200, NULL, lm3556_indicator_pattern_store);
 
 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..b7c2edd0234a 100644
--- a/drivers/leds/leds-lm3642.c
+++ b/drivers/leds/leds-lm3642.c
@@ -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(torch_pin, 0200, NULL, lm3642_torch_pin_store);
 
 static int lm3642_torch_brightness_set(struct led_classdev *cdev,
 					enum led_brightness brightness)
@@ -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(strobe_pin, 0200, NULL, lm3642_strobe_pin_store);
 
 static int lm3642_strobe_brightness_set(struct led_classdev *cdev,
 					 enum led_brightness brightness)
diff --git a/drivers/leds/leds-lp55xx-common.h b/drivers/leds/leds-lp55xx-common.h
index 2f38c5b33830..9345ab1cf323 100644
--- a/drivers/leds/leds-lp55xx-common.h
+++ b/drivers/leds/leds-lp55xx-common.h
@@ -29,11 +29,11 @@ enum lp55xx_engine_mode {
 };
 
 #define LP55XX_DEV_ATTR_RW(name, show, store)	\
-	DEVICE_ATTR(name, S_IRUGO | S_IWUSR, show, store)
+	DEVICE_ATTR(name, 0644, show, store)
 #define LP55XX_DEV_ATTR_RO(name, show)		\
-	DEVICE_ATTR(name, S_IRUGO, show, NULL)
+	DEVICE_ATTR(name, 0444, show, NULL)
 #define LP55XX_DEV_ATTR_WO(name, store)		\
-	DEVICE_ATTR(name, S_IWUSR, NULL, store)
+	DEVICE_ATTR(name, 0200, NULL, store)
 
 #define show_mode(nr)							\
 static ssize_t show_engine##nr##_mode(struct device *dev,		\
-- 
2.27.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] leds: convert symbolic permission bit macros to octal
  2020-12-12 15:35 [Linux-kernel-mentees] [PATCH] leds: convert symbolic permission bit macros to octal Dwaipayan Ray
@ 2020-12-12 17:51 ` Greg KH
  2020-12-12 18:10   ` Dwaipayan Ray
  0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2020-12-12 17:51 UTC (permalink / raw)
  To: Dwaipayan Ray; +Cc: linux-kernel-mentees

On Sat, Dec 12, 2020 at 09:05:17PM +0530, Dwaipayan Ray wrote:
> Symbolic macro names are hard to understand and should not be
> used for permission bits.
> 
> Convert all bad symbolic permission bit macro uses in led to just use
> the octal numbers.
> 
> Following macros were replaced:
> 
> S_IRUGO => 0444
> S_IWUSR => 0200
> S_IRUGO | S_IWUSR => 0644
> 
> Link: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
> 
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---
>  drivers/leds/leds-blinkm.c        | 8 ++++----
>  drivers/leds/leds-lm355x.c        | 2 +-
>  drivers/leds/leds-lm3642.c        | 4 ++--
>  drivers/leds/leds-lp55xx-common.h | 6 +++---
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
> index e11fe1788242..6b9b13f58d8a 100644
> --- a/drivers/leds/leds-blinkm.c
> +++ b/drivers/leds/leds-blinkm.c
> @@ -209,7 +209,7 @@ 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(red, 0644, show_red, store_red);

Why not use DEVICE_ATTR_RW() instead?  THat would handle all of this
automatically for you, which is why they are recommended to be used
instead.

thanks,

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] leds: convert symbolic permission bit macros to octal
  2020-12-12 17:51 ` Greg KH
@ 2020-12-12 18:10   ` Dwaipayan Ray
  0 siblings, 0 replies; 3+ messages in thread
From: Dwaipayan Ray @ 2020-12-12 18:10 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel-mentees

On Sat, Dec 12, 2020 at 11:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sat, Dec 12, 2020 at 09:05:17PM +0530, Dwaipayan Ray wrote:
> > Symbolic macro names are hard to understand and should not be
> > used for permission bits.
> >
> > Convert all bad symbolic permission bit macro uses in led to just use
> > the octal numbers.
> >
> > Following macros were replaced:
> >
> > S_IRUGO => 0444
> > S_IWUSR => 0200
> > S_IRUGO | S_IWUSR => 0644
> >
> > Link: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
> >
> > Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> > ---
> >  drivers/leds/leds-blinkm.c        | 8 ++++----
> >  drivers/leds/leds-lm355x.c        | 2 +-
> >  drivers/leds/leds-lm3642.c        | 4 ++--
> >  drivers/leds/leds-lp55xx-common.h | 6 +++---
> >  4 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
> > index e11fe1788242..6b9b13f58d8a 100644
> > --- a/drivers/leds/leds-blinkm.c
> > +++ b/drivers/leds/leds-blinkm.c
> > @@ -209,7 +209,7 @@ 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(red, 0644, show_red, store_red);
>
> Why not use DEVICE_ATTR_RW() instead?  THat would handle all of this
> automatically for you, which is why they are recommended to be used
> instead.
>
> thanks,
>
> greg k-h

Thanks Greg, I will do that.

Seems like the led drivers are still using a lot of those old constructs.
leds$ git grep -P "DEVICE_ATTR\(" | wc -l
          31

Will correct them all.

Thank you,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-12 18:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-12 15:35 [Linux-kernel-mentees] [PATCH] leds: convert symbolic permission bit macros to octal Dwaipayan Ray
2020-12-12 17:51 ` Greg KH
2020-12-12 18:10   ` Dwaipayan Ray

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.