linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions
@ 2021-08-20 19:03 Utkarsh Verma
  2021-08-24 13:55 ` Johan Hovold
  0 siblings, 1 reply; 6+ messages in thread
From: Utkarsh Verma @ 2021-08-20 19:03 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lukas Bulwahn,
	Utkarsh Verma

This fixed the below checkpatch issue:
WARNING: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred.
Consider using octal permissions '0644'.

Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
 drivers/usb/serial/iuu_phoenix.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c
index 19753611e7b0..0be3b5e1eaf3 100644
--- a/drivers/usb/serial/iuu_phoenix.c
+++ b/drivers/usb/serial/iuu_phoenix.c
@@ -1188,20 +1188,20 @@ MODULE_AUTHOR("Alain Degreffe eczema@ecze.com");
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-module_param(xmas, bool, S_IRUGO | S_IWUSR);
+module_param(xmas, bool, 0644);
 MODULE_PARM_DESC(xmas, "Xmas colors enabled or not");
 
-module_param(boost, int, S_IRUGO | S_IWUSR);
+module_param(boost, int, 0644);
 MODULE_PARM_DESC(boost, "Card overclock boost (in percent 100-500)");
 
-module_param(clockmode, int, S_IRUGO | S_IWUSR);
+module_param(clockmode, int, 0644);
 MODULE_PARM_DESC(clockmode, "Card clock mode (1=3.579 MHz, 2=3.680 MHz, "
 		"3=6 Mhz)");
 
-module_param(cdmode, int, S_IRUGO | S_IWUSR);
+module_param(cdmode, int, 0644);
 MODULE_PARM_DESC(cdmode, "Card detect mode (0=none, 1=CD, 2=!CD, 3=DSR, "
 		 "4=!DSR, 5=CTS, 6=!CTS, 7=RING, 8=!RING)");
 
-module_param(vcc_default, int, S_IRUGO | S_IWUSR);
+module_param(vcc_default, int, 0644);
 MODULE_PARM_DESC(vcc_default, "Set default VCC (either 3 for 3.3V or 5 "
 		"for 5V). Default to 5.");
-- 
2.17.1


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

* Re: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions
  2021-08-20 19:03 [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions Utkarsh Verma
@ 2021-08-24 13:55 ` Johan Hovold
  2021-08-24 19:15   ` Utkarsh Verma
  0 siblings, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2021-08-24 13:55 UTC (permalink / raw)
  To: Utkarsh Verma; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lukas Bulwahn

On Sat, Aug 21, 2021 at 12:33:06AM +0530, Utkarsh Verma wrote:
> This fixed the below checkpatch issue:
> WARNING: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred.
> Consider using octal permissions '0644'.

Please do not run checkpatch.pl on code that's already in the tree. Use
it for your own patches before submitting them and always use your own
judgement when considering its suggestions.

This code does not need to be changed.

Johan

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

* Re: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions
  2021-08-24 13:55 ` Johan Hovold
@ 2021-08-24 19:15   ` Utkarsh Verma
  2021-08-25  9:24     ` Johan Hovold
  0 siblings, 1 reply; 6+ messages in thread
From: Utkarsh Verma @ 2021-08-24 19:15 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lukas Bulwahn

On Tue, Aug 24, 2021 at 03:55:41PM +0200, Johan Hovold wrote:
> On Sat, Aug 21, 2021 at 12:33:06AM +0530, Utkarsh Verma wrote:
> > This fixed the below checkpatch issue:
> > WARNING: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred.
> > Consider using octal permissions '0644'.
> 
> Please do not run checkpatch.pl on code that's already in the tree. Use
> it for your own patches before submitting them and always use your own
> judgement when considering its suggestions.
> 

Okay, I will not run checkpatch on the code that's already in the tree.

> This code does not need to be changed.
> 

But using the octal permission bits makes the code more readable. So I
made the change.
But if you don't want such changes, I will refrain from doing it in the
future.
Anyway thanks for the review.


Regards,
Utkarsh Verma

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

* Re: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions
  2021-08-24 19:15   ` Utkarsh Verma
@ 2021-08-25  9:24     ` Johan Hovold
  2021-08-25 16:23       ` [PATCH v2] USB: serial: " Utkarsh Verma
  0 siblings, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2021-08-25  9:24 UTC (permalink / raw)
  To: Utkarsh Verma; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lukas Bulwahn

On Wed, Aug 25, 2021 at 12:45:37AM +0530, Utkarsh Verma wrote:
> On Tue, Aug 24, 2021 at 03:55:41PM +0200, Johan Hovold wrote:
> > On Sat, Aug 21, 2021 at 12:33:06AM +0530, Utkarsh Verma wrote:
> > > This fixed the below checkpatch issue:
> > > WARNING: Symbolic permissions 'S_IRUGO | S_IWUSR' are not preferred.
> > > Consider using octal permissions '0644'.
> > 
> > Please do not run checkpatch.pl on code that's already in the tree. Use
> > it for your own patches before submitting them and always use your own
> > judgement when considering its suggestions.
> > 
> 
> Okay, I will not run checkpatch on the code that's already in the tree.
> 
> > This code does not need to be changed.
> 
> But using the octal permission bits makes the code more readable. So I
> made the change.

Then put that in the commit message since that may be a valid motivation
for the change (unlike shutting up checkpatch.pl).

But if you want to do this then do it subsystem wide in one patch rather
than change only one of the seven usb-serial drivers that use the
permission macros.

Johan

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

* [PATCH v2] USB: serial: Replace symbolic permissions by octal permissions
  2021-08-25  9:24     ` Johan Hovold
@ 2021-08-25 16:23       ` Utkarsh Verma
  2021-08-26  7:51         ` Johan Hovold
  0 siblings, 1 reply; 6+ messages in thread
From: Utkarsh Verma @ 2021-08-25 16:23 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lukas Bulwahn,
	Utkarsh Verma

Replace symbolic permission macros with octal permission numbers
because, octal permission numbers are easier to read and understand
instead of their symbolic macro names.

No functional change.

Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>
---
 drivers/usb/serial/cypress_m8.c  |  6 +++---
 drivers/usb/serial/ftdi_sio.c    |  2 +-
 drivers/usb/serial/garmin_gps.c  |  2 +-
 drivers/usb/serial/io_ti.c       |  4 ++--
 drivers/usb/serial/ipaq.c        |  4 ++--
 drivers/usb/serial/iuu_phoenix.c | 10 +++++-----
 drivers/usb/serial/sierra.c      |  2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index 6b18990258c3..6924fa95f6bd 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -1199,9 +1199,9 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-module_param(stats, bool, S_IRUGO | S_IWUSR);
+module_param(stats, bool, 0644);
 MODULE_PARM_DESC(stats, "Enable statistics or not");
-module_param(interval, int, S_IRUGO | S_IWUSR);
+module_param(interval, int, 0644);
 MODULE_PARM_DESC(interval, "Overrides interrupt interval");
-module_param(unstable_bauds, bool, S_IRUGO | S_IWUSR);
+module_param(unstable_bauds, bool, 0644);
 MODULE_PARM_DESC(unstable_bauds, "Allow unstable baud rates");
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 33bbb3470ca3..99d19828dae6 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -2938,5 +2938,5 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-module_param(ndi_latency_timer, int, S_IRUGO | S_IWUSR);
+module_param(ndi_latency_timer, int, 0644);
 MODULE_PARM_DESC(ndi_latency_timer, "NDI device latency timer override");
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index 756d1ac7e96f..e5c75944ebb7 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -1444,5 +1444,5 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-module_param(initial_mode, int, S_IRUGO);
+module_param(initial_mode, int, 0444);
 MODULE_PARM_DESC(initial_mode, "Initial mode");
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 84b848d2794e..a7b3c15957ba 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2746,9 +2746,9 @@ MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 MODULE_FIRMWARE("edgeport/down3.bin");
 
-module_param(ignore_cpu_rev, bool, S_IRUGO | S_IWUSR);
+module_param(ignore_cpu_rev, bool, 0644);
 MODULE_PARM_DESC(ignore_cpu_rev,
 			"Ignore the cpu revision when connecting to a device");
 
-module_param(default_uart_mode, int, S_IRUGO | S_IWUSR);
+module_param(default_uart_mode, int, 0644);
 MODULE_PARM_DESC(default_uart_mode, "Default uart_mode, 0=RS232, ...");
diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
index f81746c3c26c..e11441bac44f 100644
--- a/drivers/usb/serial/ipaq.c
+++ b/drivers/usb/serial/ipaq.c
@@ -599,10 +599,10 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-module_param(connect_retries, int, S_IRUGO|S_IWUSR);
+module_param(connect_retries, int, 0644);
 MODULE_PARM_DESC(connect_retries,
 		"Maximum number of connect retries (one second each)");
 
-module_param(initial_wait, int, S_IRUGO|S_IWUSR);
+module_param(initial_wait, int, 0644);
 MODULE_PARM_DESC(initial_wait,
 		"Time to wait before attempting a connection (in seconds)");
diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c
index 19753611e7b0..0be3b5e1eaf3 100644
--- a/drivers/usb/serial/iuu_phoenix.c
+++ b/drivers/usb/serial/iuu_phoenix.c
@@ -1188,20 +1188,20 @@ MODULE_AUTHOR("Alain Degreffe eczema@ecze.com");
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-module_param(xmas, bool, S_IRUGO | S_IWUSR);
+module_param(xmas, bool, 0644);
 MODULE_PARM_DESC(xmas, "Xmas colors enabled or not");
 
-module_param(boost, int, S_IRUGO | S_IWUSR);
+module_param(boost, int, 0644);
 MODULE_PARM_DESC(boost, "Card overclock boost (in percent 100-500)");
 
-module_param(clockmode, int, S_IRUGO | S_IWUSR);
+module_param(clockmode, int, 0644);
 MODULE_PARM_DESC(clockmode, "Card clock mode (1=3.579 MHz, 2=3.680 MHz, "
 		"3=6 Mhz)");
 
-module_param(cdmode, int, S_IRUGO | S_IWUSR);
+module_param(cdmode, int, 0644);
 MODULE_PARM_DESC(cdmode, "Card detect mode (0=none, 1=CD, 2=!CD, 3=DSR, "
 		 "4=!DSR, 5=CTS, 6=!CTS, 7=RING, 8=!RING)");
 
-module_param(vcc_default, int, S_IRUGO | S_IWUSR);
+module_param(vcc_default, int, 0644);
 MODULE_PARM_DESC(vcc_default, "Set default VCC (either 3 for 3.3V or 5 "
 		"for 5V). Default to 5.");
diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index 4b49b7c33364..9d56138133a9 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -1056,5 +1056,5 @@ MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL v2");
 
-module_param(nmea, bool, S_IRUGO | S_IWUSR);
+module_param(nmea, bool, 0644);
 MODULE_PARM_DESC(nmea, "NMEA streaming");
-- 
2.17.1


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

* Re: [PATCH v2] USB: serial: Replace symbolic permissions by octal permissions
  2021-08-25 16:23       ` [PATCH v2] USB: serial: " Utkarsh Verma
@ 2021-08-26  7:51         ` Johan Hovold
  0 siblings, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2021-08-26  7:51 UTC (permalink / raw)
  To: Utkarsh Verma; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lukas Bulwahn

On Wed, Aug 25, 2021 at 09:53:02PM +0530, Utkarsh Verma wrote:
> Replace symbolic permission macros with octal permission numbers
> because, octal permission numbers are easier to read and understand
> instead of their symbolic macro names.
> 
> No functional change.
> 
> Suggested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> Signed-off-by: Utkarsh Verma <utkarshverma294@gmail.com>

Thanks for the update. Now applied.

Johan

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

end of thread, other threads:[~2021-08-26  7:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 19:03 [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions Utkarsh Verma
2021-08-24 13:55 ` Johan Hovold
2021-08-24 19:15   ` Utkarsh Verma
2021-08-25  9:24     ` Johan Hovold
2021-08-25 16:23       ` [PATCH v2] USB: serial: " Utkarsh Verma
2021-08-26  7:51         ` Johan Hovold

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).