Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1505467 > unrolled thread

[PATCH 0/6] FRV-setup: Fine-tuning for six function implementations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] FRV-setup: Fine-tuning for six function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 07:10 +0200
    [PATCH 2/6] FRV-setup: Use seq_putc() in show_cpuinfo() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 07:10 +0200
    [PATCH 3/6] FRV-setup: Add some spaces for better code readability SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 07:10 +0200
    [PATCH 5/6] FRV-setup: Fix indentation in two lines SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 07:20 +0200
      Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines Jiri Kosina <jikos@kernel.org> - 2016-10-21 09:30 +0200
        Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines Dan Carpenter <dan.carpenter@oracle.com> - 2016-10-21 10:00 +0200
          Re: FRV-setup: Clarification for "source code clean-up"? SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 11:20 +0200
            Re: FRV-setup: Clarification for "source code clean-up"? Dan Carpenter <dan.carpenter@oracle.com> - 2016-10-21 11:30 +0200
              Re: FRV-setup: Clarification for "source code clean-up"? SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 12:00 +0200
                Re: FRV-setup: Clarification for "source code clean-up"? Geert Uytterhoeven <geert@linux-m68k.org> - 2016-10-21 12:30 +0200
                Re: FRV-setup: Clarification for "source code clean-up"? Dan Carpenter <dan.carpenter@oracle.com> - 2016-10-21 12:30 +0200
                  Re: FRV-setup: Clarification for "source code clean-up"? Julia Lawall <julia.lawall@lip6.fr> - 2016-10-21 13:10 +0200
                    Re: Improving software around DMA API usage? SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 13:40 +0200
    [PATCH 6/6] FRV-setup: Move statements for "case" to separate lines SF Markus Elfring <elfring@users.sourceforge.net> - 2016-10-21 07:20 +0200

#1505467 — [PATCH 0/6] FRV-setup: Fine-tuning for six function implementations

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

[toc] | [next] | [standalone]

#1505469 — [PATCH 2/6] FRV-setup: Use seq_putc() in show_cpuinfo()

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

[toc] | [prev] | [next] | [standalone]

#1505470 — [PATCH 3/6] FRV-setup: Add some spaces for better code readability

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

[toc] | [prev] | [next] | [standalone]

#1505471 — [PATCH 5/6] FRV-setup: Fix indentation in two lines

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

[toc] | [prev] | [next] | [standalone]

#1505526 — Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines

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

[toc] | [prev] | [next] | [standalone]

#1505548 — Re: [PATCH 5/6] FRV-setup: Fix indentation in two lines

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

[toc] | [prev] | [next] | [standalone]

#1505639 — Re: FRV-setup: Clarification for "source code clean-up"?

> 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

[toc] | [prev] | [next] | [standalone]

#1505687 — Re: FRV-setup: Clarification for "source code clean-up"?

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

[toc] | [prev] | [next] | [standalone]

#1505744 — Re: FRV-setup: Clarification for "source code clean-up"?

> 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

[toc] | [prev] | [next] | [standalone]

#1505761 — Re: FRV-setup: Clarification for "source code clean-up"?

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

[toc] | [prev] | [next] | [standalone]

#1505762 — Re: FRV-setup: Clarification for "source code clean-up"?

I am not going to Eliza with you.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1505784 — Re: FRV-setup: Clarification for "source code clean-up"?

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

[toc] | [prev] | [next] | [standalone]

#1505808 — Re: Improving software around DMA API usage?

> 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

[toc] | [prev] | [next] | [standalone]

#1505472 — [PATCH 6/6] FRV-setup: Move statements for "case" to separate lines

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web