[PATCH] staging: comedi: don't override read/write subdevice if not supported

Dan Carpenter dan.carpenter at oracle.com
Wed Jan 30 07:22:45 UTC 2013


On Tue, Jan 29, 2013 at 01:51:35PM +0000, Bernd Porr wrote:
> > Force of habit because some compilers issue a warning when a bitwise
> > operator is used in a context expecting a logical result.
> indeed. Good point.
> 

Ian already redid this (Thanks!).

I understand about habbits, but I don't think I have seen any bug
where using a bitwise operation instead of a logical as a condition
caused a problem.

It's not a CodingStyle thing and I did say I was fine with the first
patch as-is.  It's not a huge deal, but that the double negative
always hurts my brain a little.

For example here is an inverted if statement:

commit 4659b7f1fa3eb33a8f9a9dd209a5823602dc6dcf
Author: Robert Lee <rob.lee at linaro.org>
Date:   Mon Apr 16 18:37:48 2012 -0500

    ARM: imx: Fix imx5 idle logic bug
    
    The imx5_idle() check of the tzic_eanble_wake() return value uses
    incorrect (inverted) logic causing all attempt to idle to fail.
    
    Signed-off-by: Robert Lee <rob.lee at linaro.org>
    Signed-off-by: Sascha Hauer <s.hauer at pengutronix.de>

diff --git a/arch/arm/mach-imx/mm-imx5.c b/arch/arm/mach-imx/mm-imx5.c
index 05250ae..e10f391 100644
--- a/arch/arm/mach-imx/mm-imx5.c
+++ b/arch/arm/mach-imx/mm-imx5.c
@@ -35,7 +35,7 @@ static void imx5_idle(void)
 	}
 	clk_enable(gpc_dvfs_clk);
 	mx5_cpu_lp_set(WAIT_UNCLOCKED_POWER_OFF);
-	if (tzic_enable_wake() != 0)
+	if (!tzic_enable_wake())
 		cpu_do_idle();
 	clk_disable(gpc_dvfs_clk);
 }

Of course, part of the problem with this function is that
tzic_enable_wake() looks like it might return bool, but it actually
returns zero on success and negative error codes.  It probably would
have been a better test like this:

	if (tzic_enable_wake() < 0) { ...  /* it failed */
	if (tzic_enable_wake() == 0) { ... /* it succeeded */
Or
	ret = tzic_enable_wake();
	if (ret)
		goto fail;

	ret = tzic_enable_wake();
	if (!ret)
		cpu_do_idle();

regards,
dan carpenter



More information about the devel mailing list