diff mbox

au1100fb: Delete unnecessary checks before two function calls

Message ID 54D36CCE.6050401@users.sourceforge.net (mailing list archive)
State New, archived
Headers show

Commit Message

SF Markus Elfring Feb. 5, 2015, 1:14 p.m. UTC 
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 14:10:21 +0100

The functions clk_enable() and clk_disable() test whether their argument
is NULL and then return immediately. Thus the test around the calls
is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/video/fbdev/au1100fb.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Dan Carpenter Feb. 5, 2015, 1:41 p.m. UTC | #1 
On Thu, Feb 05, 2015 at 02:14:54PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 5 Feb 2015 14:10:21 +0100
> 
> The functions clk_enable() and clk_disable() test whether their argument
> is NULL and then return immediately.

This isn't true for clk_enable().

"I find it acceptable that some of my update suggestions do not fit
to your quality expectations at the moment." --
https://lkml.org/lkml/2015/1/22/446

It's frustrating that you make the same mistake over and over and you
are fine with doing that.  If you make a mistake and you are fixing a
bug, then hopefully you fix more bugs than you introduce.  If you make a
mistake and you are doing a cleanup then you are really just introducing
bugs and that's not helpful.  I wish you would find something useful to
do instead of sending these patches.  :(

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
SF Markus Elfring Feb. 5, 2015, 2:20 p.m. UTC | #2 
>> The functions clk_enable() and clk_disable() test whether their argument
>> is NULL and then return immediately.
> 
> This isn't true for clk_enable().

I stumble on an interesting name space issue once more.


> It's frustrating that you make the same mistake over and over
> and you are fine with doing that.

I admit that there can be also risks with my update suggestions
in similar ways to contributions from other software developers.

A specific function implementation exists which performs also input
parameter validation.
http://lxr.free-electrons.com/source/drivers/sh/clk/core.c#L291

Such variants might not fit to the functions au1100fb_drv_suspend()
and au1100fb_drv_resume().


Do you want that I send a reduced patch?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Feb. 5, 2015, 5:18 p.m. UTC | #3 
On Thu, Feb 05, 2015 at 03:20:27PM +0100, SF Markus Elfring wrote:
> >> The functions clk_enable() and clk_disable() test whether their argument
> >> is NULL and then return immediately.
> > 
> > This isn't true for clk_enable().
> 
> I stumble on an interesting name space issue once more.
> 

The "once more" is the problem in this statement.  Other people would
have stopped making the mistake by now.  I am able to see your bugs so
you should be able to see your bugs as well.

> 
> > It's frustrating that you make the same mistake over and over
> > and you are fine with doing that.
> 
> I admit that there can be also risks with my update suggestions
> in similar ways to contributions from other software developers.
> 

At times, I have told other people to stop sending cleanups until they
can at least fix a bugs to balance out the bugs they have introduced.
It's unacceptable to only introduce bugs and never fix any.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
SF Markus Elfring Feb. 5, 2015, 8:15 p.m. UTC | #4 
> It's unacceptable to only introduce bugs

Such a view is generally fine.

Would you like to point any more update candidates out from my
suggestion collection?


> and never fix any.

Would you like to clarify any statistics around my update suggestions?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index 5956018..47ee023 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -605,11 +605,8 @@  int au1100fb_drv_suspend(struct platform_device *dev, pm_message_t state)
 	/* Blank the LCD */
 	au1100fb_fb_blank(VESA_POWERDOWN, &fbdev->info);
 
-	if (fbdev->lcdclk)
-		clk_disable(fbdev->lcdclk);
-
+	clk_disable(fbdev->lcdclk);
 	memcpy(&fbregs, fbdev->regs, sizeof(struct au1100fb_regs));
-
 	return 0;
 }
 
@@ -621,13 +618,10 @@  int au1100fb_drv_resume(struct platform_device *dev)
 		return 0;
 
 	memcpy(fbdev->regs, &fbregs, sizeof(struct au1100fb_regs));
-
-	if (fbdev->lcdclk)
-		clk_enable(fbdev->lcdclk);
+	clk_enable(fbdev->lcdclk);
 
 	/* Unblank the LCD */
 	au1100fb_fb_blank(VESA_NO_BLANKING, &fbdev->info);
-
 	return 0;
 }
 #else