2021-08-20 19:06:11

by Utkarsh Verma

[permalink] [raw]
Subject: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions

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 <[email protected]>
Suggested-by: Lukas Bulwahn <[email protected]>
---
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 [email protected]");
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


2021-08-24 13:58:05

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions

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

2021-08-24 19:19:04

by Utkarsh Verma

[permalink] [raw]
Subject: Re: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions

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

2021-08-25 09:26:51

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH] USB: serial: iuu_phoenix: Replace symbolic permissions by octal permissions

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

2021-08-25 20:53:20

by Utkarsh Verma

[permalink] [raw]
Subject: [PATCH v2] USB: serial: Replace symbolic permissions by octal permissions

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 <[email protected]>
Signed-off-by: Utkarsh Verma <[email protected]>
---
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 [email protected]");
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

2021-08-26 07:52:47

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH v2] USB: serial: Replace symbolic permissions by octal permissions

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 <[email protected]>
> Signed-off-by: Utkarsh Verma <[email protected]>

Thanks for the update. Now applied.

Johan