* [PATCH 0/6] FRV-setup: Fine-tuning for six function implementations
@ 2016-10-21 5:03 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:03 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:44:22 +0200
Some update suggestions were taken into account
from static source code analysis.
Markus Elfring (6):
Use seq_puts() in show_cpuinfo()
Use seq_putc() in show_cpuinfo()
Add some spaces for better code readability
Move "else" closer to a brace
Fix indentation in two lines
Move statements for "case" to separate lines
arch/frv/kernel/setup.c | 62 +++++++++++++++++++++++++------------------------
1 file changed, 32 insertions(+), 30 deletions(-)
--
2.10.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/6] FRV-setup: Use seq_puts() in show_cpuinfo()
2016-10-21 5:03 ` SF Markus Elfring
@ 2016-10-21 5:05 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:05 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 21:36:43 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 9f4a9a6..de9311f 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -1053,7 +1053,7 @@ static int show_cpuinfo(struct seq_file *m, void *v)
seq_printf(m, "\n");
#ifdef CONFIG_PM
- seq_printf(m, "PM-Controls:");
+ seq_puts(m, "PM-Controls:");
sep = "\t";
if (clock_bits_settable & CLOCK_BIT_CMODE) {
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 1/6] FRV-setup: Use seq_puts() in show_cpuinfo()
@ 2016-10-21 5:05 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:05 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 21:36:43 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 9f4a9a6..de9311f 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -1053,7 +1053,7 @@ static int show_cpuinfo(struct seq_file *m, void *v)
seq_printf(m, "\n");
#ifdef CONFIG_PM
- seq_printf(m, "PM-Controls:");
+ seq_puts(m, "PM-Controls:");
sep = "\t";
if (clock_bits_settable & CLOCK_BIT_CMODE) {
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/6] FRV-setup: Use seq_putc() in show_cpuinfo()
2016-10-21 5:03 ` SF Markus Elfring
@ 2016-10-21 5:07 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:07 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 21:39:00 +0200
A single character (line break) should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index de9311f..7570ff0 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -1050,7 +1050,7 @@ static int show_cpuinfo(struct seq_file *m, void *v)
if (cpu_board2)
seq_printf(m, ", %s", cpu_board2);
- seq_printf(m, "\n");
+ seq_putc(m, '\n');
#ifdef CONFIG_PM
seq_puts(m, "PM-Controls:");
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/6] FRV-setup: Use seq_putc() in show_cpuinfo()
@ 2016-10-21 5:07 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:07 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 21:39:00 +0200
A single character (line break) should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index de9311f..7570ff0 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -1050,7 +1050,7 @@ static int show_cpuinfo(struct seq_file *m, void *v)
if (cpu_board2)
seq_printf(m, ", %s", cpu_board2);
- seq_printf(m, "\n");
+ seq_putc(m, '\n');
#ifdef CONFIG_PM
seq_puts(m, "PM-Controls:");
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/6] FRV-setup: Add some spaces for better code readability
2016-10-21 5:03 ` SF Markus Elfring
@ 2016-10-21 5:08 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:08 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:05:55 +0200
Use space characters at some source code places according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 7570ff0..577bcf5 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -121,23 +121,23 @@ struct clock_cmode {
uint8_t xbus, sdram, corebus, core, dsu;
};
-#define _frac(N,D) ((N)<<4 | (D))
-#define _x0_16 _frac(1,6)
-#define _x0_25 _frac(1,4)
-#define _x0_33 _frac(1,3)
-#define _x0_375 _frac(3,8)
-#define _x0_5 _frac(1,2)
-#define _x0_66 _frac(2,3)
-#define _x0_75 _frac(3,4)
-#define _x1 _frac(1,1)
-#define _x1_5 _frac(3,2)
-#define _x2 _frac(2,1)
-#define _x3 _frac(3,1)
-#define _x4 _frac(4,1)
-#define _x4_5 _frac(9,2)
-#define _x6 _frac(6,1)
-#define _x8 _frac(8,1)
-#define _x9 _frac(9,1)
+#define _frac(N, D) ((N) << 4 | (D))
+#define _x0_16 _frac(1, 6)
+#define _x0_25 _frac(1, 4)
+#define _x0_33 _frac(1, 3)
+#define _x0_375 _frac(3, 8)
+#define _x0_5 _frac(1, 2)
+#define _x0_66 _frac(2, 3)
+#define _x0_75 _frac(3, 4)
+#define _x1 _frac(1, 1)
+#define _x1_5 _frac(3, 2)
+#define _x2 _frac(2, 1)
+#define _x3 _frac(3, 1)
+#define _x4 _frac(4, 1)
+#define _x4_5 _frac(9, 2)
+#define _x6 _frac(6, 1)
+#define _x8 _frac(8, 1)
+#define _x9 _frac(9, 1)
int __nongprelbss clock_p0_current;
int __nongprelbss clock_cm_current;
@@ -574,7 +574,7 @@ void __pminit determine_clocks(int verbose)
mode = tmode;
}
-#define CLOCK(SRC,RATIO) ((SRC) * (((RATIO) >> 4) & 0x0f) / ((RATIO) & 0x0f))
+#define CLOCK(SRC, RATIO) ((SRC) * (((RATIO) >> 4) & 0x0f) / ((RATIO) & 0x0f))
if (clock_doubled)
__clkin_clock_speed_HZ <<= 1;
@@ -1161,8 +1161,8 @@ static void __init mb93090_display(void)
/* set up the LCD */
mb93090_sendlcdcmd(LCD_CMD_CLEAR);
- mb93090_sendlcdcmd(LCD_CMD_FUNCSET(1,1,0));
- mb93090_sendlcdcmd(LCD_CMD_ON(0,0));
+ mb93090_sendlcdcmd(LCD_CMD_FUNCSET(1, 1, 0));
+ mb93090_sendlcdcmd(LCD_CMD_ON(0, 0));
mb93090_sendlcdcmd(LCD_CMD_HOME);
mb93090_sendlcdcmd(LCD_CMD_SET_DD_ADDR(0));
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 3/6] FRV-setup: Add some spaces for better code readability
@ 2016-10-21 5:08 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:08 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:05:55 +0200
Use space characters at some source code places according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 7570ff0..577bcf5 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -121,23 +121,23 @@ struct clock_cmode {
uint8_t xbus, sdram, corebus, core, dsu;
};
-#define _frac(N,D) ((N)<<4 | (D))
-#define _x0_16 _frac(1,6)
-#define _x0_25 _frac(1,4)
-#define _x0_33 _frac(1,3)
-#define _x0_375 _frac(3,8)
-#define _x0_5 _frac(1,2)
-#define _x0_66 _frac(2,3)
-#define _x0_75 _frac(3,4)
-#define _x1 _frac(1,1)
-#define _x1_5 _frac(3,2)
-#define _x2 _frac(2,1)
-#define _x3 _frac(3,1)
-#define _x4 _frac(4,1)
-#define _x4_5 _frac(9,2)
-#define _x6 _frac(6,1)
-#define _x8 _frac(8,1)
-#define _x9 _frac(9,1)
+#define _frac(N, D) ((N) << 4 | (D))
+#define _x0_16 _frac(1, 6)
+#define _x0_25 _frac(1, 4)
+#define _x0_33 _frac(1, 3)
+#define _x0_375 _frac(3, 8)
+#define _x0_5 _frac(1, 2)
+#define _x0_66 _frac(2, 3)
+#define _x0_75 _frac(3, 4)
+#define _x1 _frac(1, 1)
+#define _x1_5 _frac(3, 2)
+#define _x2 _frac(2, 1)
+#define _x3 _frac(3, 1)
+#define _x4 _frac(4, 1)
+#define _x4_5 _frac(9, 2)
+#define _x6 _frac(6, 1)
+#define _x8 _frac(8, 1)
+#define _x9 _frac(9, 1)
int __nongprelbss clock_p0_current;
int __nongprelbss clock_cm_current;
@@ -574,7 +574,7 @@ void __pminit determine_clocks(int verbose)
mode = tmode;
}
-#define CLOCK(SRC,RATIO) ((SRC) * (((RATIO) >> 4) & 0x0f) / ((RATIO) & 0x0f))
+#define CLOCK(SRC, RATIO) ((SRC) * (((RATIO) >> 4) & 0x0f) / ((RATIO) & 0x0f))
if (clock_doubled)
__clkin_clock_speed_HZ <<= 1;
@@ -1161,8 +1161,8 @@ static void __init mb93090_display(void)
/* set up the LCD */
mb93090_sendlcdcmd(LCD_CMD_CLEAR);
- mb93090_sendlcdcmd(LCD_CMD_FUNCSET(1,1,0));
- mb93090_sendlcdcmd(LCD_CMD_ON(0,0));
+ mb93090_sendlcdcmd(LCD_CMD_FUNCSET(1, 1, 0));
+ mb93090_sendlcdcmd(LCD_CMD_ON(0, 0));
mb93090_sendlcdcmd(LCD_CMD_HOME);
mb93090_sendlcdcmd(LCD_CMD_SET_DD_ADDR(0));
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 4/6] FRV-setup: Move "else" closer to a brace
2016-10-21 5:03 ` SF Markus Elfring
@ 2016-10-21 5:09 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:09 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:12:27 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: else should follow close brace '}'
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 577bcf5..35a23fc 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -902,8 +902,7 @@ static void __init setup_linux_memory(void)
#else
max_mapnr = physpages = low_top_pfn;
#endif
- }
- else {
+ } else {
low_top_pfn = physpages;
}
@@ -934,8 +933,7 @@ static void __init setup_linux_memory(void)
BOOTMEM_DEFAULT);
initrd_start = INITRD_START + PAGE_OFFSET;
initrd_end = initrd_start + INITRD_SIZE;
- }
- else {
+ } else {
printk(KERN_ERR
"initrd extends beyond end of memory (0x%08lx > 0x%08lx)\n"
"disabling initrd\n",
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 4/6] FRV-setup: Move "else" closer to a brace
@ 2016-10-21 5:09 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:09 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:12:27 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: else should follow close brace '}'
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 577bcf5..35a23fc 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -902,8 +902,7 @@ static void __init setup_linux_memory(void)
#else
max_mapnr = physpages = low_top_pfn;
#endif
- }
- else {
+ } else {
low_top_pfn = physpages;
}
@@ -934,8 +933,7 @@ static void __init setup_linux_memory(void)
BOOTMEM_DEFAULT);
initrd_start = INITRD_START + PAGE_OFFSET;
initrd_end = initrd_start + INITRD_SIZE;
- }
- else {
+ } else {
printk(KERN_ERR
"initrd extends beyond end of memory (0x%08lx > 0x%08lx)\n"
"disabling initrd\n",
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 5/6] FRV-setup: Fix indentation in two lines
2016-10-21 5:03 ` SF Markus Elfring
@ 2016-10-21 5:10 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:10 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:25:18 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: please, no spaces at the start of a line
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 35a23fc..9a44cbe 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -818,9 +818,9 @@ void __init setup_arch(char **cmdline_p)
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
- conswitchp = &vga_con;
+ conswitchp = &vga_con;
#elif defined(CONFIG_DUMMY_CONSOLE)
- conswitchp = &dummy_con;
+ conswitchp = &dummy_con;
#endif
#endif
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 5/6] FRV-setup: Fix indentation in two lines
@ 2016-10-21 5:10 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:10 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:25:18 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: please, no spaces at the start of a line
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 35a23fc..9a44cbe 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -818,9 +818,9 @@ void __init setup_arch(char **cmdline_p)
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
- conswitchp = &vga_con;
+ conswitchp = &vga_con;
#elif defined(CONFIG_DUMMY_CONSOLE)
- conswitchp = &dummy_con;
+ conswitchp = &dummy_con;
#endif
#endif
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines
2016-10-21 5:10 ` SF Markus Elfring
@ 2016-10-21 7:24 ` Jiri Kosina
-1 siblings, 0 replies; 32+ messages in thread
From: Jiri Kosina @ 2016-10-21 7:24 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Geert Uytterhoeven, LKML, kernel-janitors
On Fri, 21 Oct 2016, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Oct 2016 22:25:18 +0200
>
> The script "checkpatch.pl" pointed information out like the following.
>
> WARNING: please, no spaces at the start of a line
>
> Thus fix the affected source code places.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
I think we really should update the documentation so that it states that
checkpatch should only be run on patches or code that's not merged yet.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines
@ 2016-10-21 7:24 ` Jiri Kosina
0 siblings, 0 replies; 32+ messages in thread
From: Jiri Kosina @ 2016-10-21 7:24 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Geert Uytterhoeven, LKML, kernel-janitors
On Fri, 21 Oct 2016, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 20 Oct 2016 22:25:18 +0200
>
> The script "checkpatch.pl" pointed information out like the following.
>
> WARNING: please, no spaces at the start of a line
>
> Thus fix the affected source code places.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
I think we really should update the documentation so that it states that
checkpatch should only be run on patches or code that's not merged yet.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines
2016-10-21 7:24 ` Jiri Kosina
@ 2016-10-21 7:51 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2016-10-21 7:51 UTC (permalink / raw)
To: Jiri Kosina; +Cc: SF Markus Elfring, Geert Uytterhoeven, LKML, kernel-janitors
I don't think changing check_patch.pl or CodingStyle will help...
I've banned Markus from sending cleanups in parts of the kernel where I
have a voice (drivers/staging). Bugfixes are welcome, but cleanups are
not. I've done this before for other people who introduce a bunch of
bugs in clean up patches.
It's all about ratios... Each clean up patch has chance of introducing
a bug. If you only send clean up patches then you only introduce bugs.
We really don't want people sending patches if they introduce more bugs
than they fix.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines
@ 2016-10-21 7:51 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2016-10-21 7:51 UTC (permalink / raw)
To: Jiri Kosina; +Cc: SF Markus Elfring, Geert Uytterhoeven, LKML, kernel-janitors
I don't think changing check_patch.pl or CodingStyle will help...
I've banned Markus from sending cleanups in parts of the kernel where I
have a voice (drivers/staging). Bugfixes are welcome, but cleanups are
not. I've done this before for other people who introduce a bunch of
bugs in clean up patches.
It's all about ratios... Each clean up patch has chance of introducing
a bug. If you only send clean up patches then you only introduce bugs.
We really don't want people sending patches if they introduce more bugs
than they fix.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
2016-10-21 7:51 ` Dan Carpenter
@ 2016-10-21 9:11 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 9:11 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
> It's all about ratios... Each clean up patch has chance of introducing
> a bug.
Each update suggestion contains a possibility for mistakes.
> If you only send clean up patches then you only introduce bugs.
I find such a conclusion questionable with the wording "only".
> We really don't want people sending patches if they introduce more bugs
> than they fix.
How do you think about to discuss corresponding software development
statistics in more detail?
Regards,
Markus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
@ 2016-10-21 9:11 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 9:11 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
> It's all about ratios... Each clean up patch has chance of introducing
> a bug.
Each update suggestion contains a possibility for mistakes.
> If you only send clean up patches then you only introduce bugs.
I find such a conclusion questionable with the wording "only".
> We really don't want people sending patches if they introduce more bugs
> than they fix.
How do you think about to discuss corresponding software development
statistics in more detail?
Regards,
Markus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
2016-10-21 9:11 ` SF Markus Elfring
@ 2016-10-21 9:27 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2016-10-21 9:27 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
On Fri, Oct 21, 2016 at 11:11:54AM +0200, SF Markus Elfring wrote:
> > It's all about ratios... Each clean up patch has chance of introducing
> > a bug.
>
> Each update suggestion contains a possibility for mistakes.
>
Yes. But bug fix patches make up for it by fixing stuff.
>
> > If you only send clean up patches then you only introduce bugs.
>
> I find such a conclusion questionable with the wording "only".
>
How are you going to fix bugs if you only send clean ups?
>
> > We really don't want people sending patches if they introduce more bugs
> > than they fix.
>
> How do you think about to discuss corresponding software development
> statistics in more detail?
I think normal developers should fix 10 bugs or add a few features for
every regression they introduce. I introduced a regression just last
week, so that's a normal part of life, but I was at least *trying* to
fix a bug when I did it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
@ 2016-10-21 9:27 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2016-10-21 9:27 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
On Fri, Oct 21, 2016 at 11:11:54AM +0200, SF Markus Elfring wrote:
> > It's all about ratios... Each clean up patch has chance of introducing
> > a bug.
>
> Each update suggestion contains a possibility for mistakes.
>
Yes. But bug fix patches make up for it by fixing stuff.
>
> > If you only send clean up patches then you only introduce bugs.
>
> I find such a conclusion questionable with the wording "only".
>
How are you going to fix bugs if you only send clean ups?
>
> > We really don't want people sending patches if they introduce more bugs
> > than they fix.
>
> How do you think about to discuss corresponding software development
> statistics in more detail?
I think normal developers should fix 10 bugs or add a few features for
every regression they introduce. I introduced a regression just last
week, so that's a normal part of life, but I was at least *trying* to
fix a bug when I did it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
2016-10-21 9:27 ` Dan Carpenter
@ 2016-10-21 9:50 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 9:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
> How are you going to fix bugs if you only send clean ups?
I would prefer an other wording than "only" in your question.
I imagine that probabilities and views will matter more for the handling
of a software error spectrum.
>> How do you think about to discuss corresponding software development
>> statistics in more detail?
>
> I think normal developers should fix 10 bugs or add a few features for
> every regression they introduce.
Are you looking for compensation possibilities?
> I introduced a regression just last week, so that's a normal part of life,
Thanks that you could also admit a programming mistake.
> but I was at least *trying* to fix a bug when I did it.
I am also trying to improve various software.
* We have got just different development opinions about the relevance
of some changes. Can such a disagreement be a useful source for
interesting ideas?
* We are used to inform further contributors about results from static
source code analysis.
These tools can find a bunch of open issues because of the automatic
application of special search patterns. The importance of such findings
will vary as usual.
Regards,
Markus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
@ 2016-10-21 9:50 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 9:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
> How are you going to fix bugs if you only send clean ups?
I would prefer an other wording than "only" in your question.
I imagine that probabilities and views will matter more for the handling
of a software error spectrum.
>> How do you think about to discuss corresponding software development
>> statistics in more detail?
>
> I think normal developers should fix 10 bugs or add a few features for
> every regression they introduce.
Are you looking for compensation possibilities?
> I introduced a regression just last week, so that's a normal part of life,
Thanks that you could also admit a programming mistake.
> but I was at least *trying* to fix a bug when I did it.
I am also trying to improve various software.
* We have got just different development opinions about the relevance
of some changes. Can such a disagreement be a useful source for
interesting ideas?
* We are used to inform further contributors about results from static
source code analysis.
These tools can find a bunch of open issues because of the automatic
application of special search patterns. The importance of such findings
will vary as usual.
Regards,
Markus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
2016-10-21 9:50 ` SF Markus Elfring
@ 2016-10-21 10:22 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2016-10-21 10:22 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
I am not going to Eliza with you.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
@ 2016-10-21 10:22 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2016-10-21 10:22 UTC (permalink / raw)
To: SF Markus Elfring; +Cc: Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
I am not going to Eliza with you.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
2016-10-21 10:22 ` Dan Carpenter
@ 2016-10-21 10:28 ` Geert Uytterhoeven
-1 siblings, 0 replies; 32+ messages in thread
From: Geert Uytterhoeven @ 2016-10-21 10:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, Jiri Kosina, Geert Uytterhoeven, LKML,
kernel-janitors
On Fri, Oct 21, 2016 at 12:22 PM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> I am not going to Eliza with you.
+1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: FRV-setup: Clarification for "source code clean-up"?
@ 2016-10-21 10:28 ` Geert Uytterhoeven
0 siblings, 0 replies; 32+ messages in thread
From: Geert Uytterhoeven @ 2016-10-21 10:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, Jiri Kosina, Geert Uytterhoeven, LKML,
kernel-janitors
On Fri, Oct 21, 2016 at 12:22 PM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> I am not going to Eliza with you.
+1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
2016-10-21 10:22 ` Dan Carpenter
@ 2016-10-21 11:03 ` Julia Lawall
-1 siblings, 0 replies; 32+ messages in thread
From: Julia Lawall @ 2016-10-21 11:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, Jiri Kosina, Geert Uytterhoeven, LKML,
kernel-janitors
Markus,
If you want to do something that might be useful, you could look into the
problem of missing checks for dma mapping failure. There is nice slide
presenatation about the issue:
http://www.slideshare.net/SamsungOSG/shuah-khan-dmamaperror
The slides are from a few years ago, but I did a small test recently, and
the problem seems to persist. This needs to be done slowly and carefully,
because when there is a failure, you need to figure out what to do to
clean up and what value to return. There is no point to clean up all of
the other checkpatch errors in the file at the same time. Just stay on
the problem at hand.
julia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: FRV-setup: Clarification for "source code clean-up"?
@ 2016-10-21 11:03 ` Julia Lawall
0 siblings, 0 replies; 32+ messages in thread
From: Julia Lawall @ 2016-10-21 11:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: SF Markus Elfring, Jiri Kosina, Geert Uytterhoeven, LKML,
kernel-janitors
Markus,
If you want to do something that might be useful, you could look into the
problem of missing checks for dma mapping failure. There is nice slide
presenatation about the issue:
http://www.slideshare.net/SamsungOSG/shuah-khan-dmamaperror
The slides are from a few years ago, but I did a small test recently, and
the problem seems to persist. This needs to be done slowly and carefully,
because when there is a failure, you need to figure out what to do to
clean up and what value to return. There is no point to clean up all of
the other checkpatch errors in the file at the same time. Just stay on
the problem at hand.
julia
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Improving software around DMA API usage?
2016-10-21 11:03 ` Julia Lawall
@ 2016-10-21 11:38 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 11:38 UTC (permalink / raw)
To: Julia Lawall
Cc: Dan Carpenter, Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
> If you want to do something that might be useful, you could look into the
> problem of missing checks for dma mapping failure. There is nice slide
> presenatation about the issue:
>
> http://www.slideshare.net/SamsungOSG/shuah-khan-dmamaperror
Thanks for your suggestion about the software development idea
"Detecting Silent Data Corruptions using Linux DMA Debug API"
by Shuah Khan (from the year 2013).
> The slides are from a few years ago, but I did a small test recently,
> and the problem seems to persist.
Does this information indicate anything about the corresponding software
development attention?
> This needs to be done slowly and carefully, because when there is a failure,
> you need to figure out what to do to clean up and what value to return.
I imagine that some tools can help here again with static source code analysis,
can't they?
> There is no point to clean up all of the other checkpatch errors in the file
> at the same time.
Do you prefer to leave them over for newcomers? ;-)
> Just stay on the problem at hand.
* Can further evolution also around software like "Coccinelle" help to improve
the described situation?
* Are the chances becoming better around advanced data flow analysis
by other analysis and design approaches?
Regards,
Markus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Improving software around DMA API usage?
@ 2016-10-21 11:38 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 11:38 UTC (permalink / raw)
To: Julia Lawall
Cc: Dan Carpenter, Jiri Kosina, Geert Uytterhoeven, LKML, kernel-janitors
> If you want to do something that might be useful, you could look into the
> problem of missing checks for dma mapping failure. There is nice slide
> presenatation about the issue:
>
> http://www.slideshare.net/SamsungOSG/shuah-khan-dmamaperror
Thanks for your suggestion about the software development idea
"Detecting Silent Data Corruptions using Linux DMA Debug API"
by Shuah Khan (from the year 2013).
> The slides are from a few years ago, but I did a small test recently,
> and the problem seems to persist.
Does this information indicate anything about the corresponding software
development attention?
> This needs to be done slowly and carefully, because when there is a failure,
> you need to figure out what to do to clean up and what value to return.
I imagine that some tools can help here again with static source code analysis,
can't they?
> There is no point to clean up all of the other checkpatch errors in the file
> at the same time.
Do you prefer to leave them over for newcomers? ;-)
> Just stay on the problem at hand.
* Can further evolution also around software like "Coccinelle" help to improve
the described situation?
* Are the chances becoming better around advanced data flow analysis
by other analysis and design approaches?
Regards,
Markus
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 6/6] FRV-setup: Move statements for "case" to separate lines
2016-10-21 5:03 ` SF Markus Elfring
@ 2016-10-21 5:11 ` SF Markus Elfring
-1 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:11 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:33:56 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: trailing statements should be on next line
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 9a44cbe..df44dbd 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -474,8 +474,12 @@ static void __init determine_cpu(void)
pdm_suspend_mode = HSR0_PDM_PLL_STOP;
switch (PSR_VERSION(psr)) {
- case PSR_VERSION_FR501_MB93501: cpu_silicon = "mb93501"; break;
- case PSR_VERSION_FR501_MB93501A: cpu_silicon = "mb93501/A"; break;
+ case PSR_VERSION_FR501_MB93501:
+ cpu_silicon = "mb93501";
+ break;
+ case PSR_VERSION_FR501_MB93501A:
+ cpu_silicon = "mb93501/A";
+ break;
default:
break;
}
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH 6/6] FRV-setup: Move statements for "case" to separate lines
@ 2016-10-21 5:11 ` SF Markus Elfring
0 siblings, 0 replies; 32+ messages in thread
From: SF Markus Elfring @ 2016-10-21 5:11 UTC (permalink / raw)
To: trivial, Geert Uytterhoeven; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 20 Oct 2016 22:33:56 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: trailing statements should be on next line
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
arch/frv/kernel/setup.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/frv/kernel/setup.c b/arch/frv/kernel/setup.c
index 9a44cbe..df44dbd 100644
--- a/arch/frv/kernel/setup.c
+++ b/arch/frv/kernel/setup.c
@@ -474,8 +474,12 @@ static void __init determine_cpu(void)
pdm_suspend_mode = HSR0_PDM_PLL_STOP;
switch (PSR_VERSION(psr)) {
- case PSR_VERSION_FR501_MB93501: cpu_silicon = "mb93501"; break;
- case PSR_VERSION_FR501_MB93501A: cpu_silicon = "mb93501/A"; break;
+ case PSR_VERSION_FR501_MB93501:
+ cpu_silicon = "mb93501";
+ break;
+ case PSR_VERSION_FR501_MB93501A:
+ cpu_silicon = "mb93501/A";
+ break;
default:
break;
}
--
2.10.1
^ permalink raw reply related [flat|nested] 32+ messages in thread