Re: [PATCH 3/8] i2c: omap: fix error checking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Oct 25, 2012 at 12:33:18PM +0200, Michael Trimarchi wrote:
> >>> @@ -587,9 +587,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> >>>  		goto err_i2c_init;
> >>>  	}
> >>>  
> >>> -	/* We have an error */
> >>> -	if (dev->cmd_err & (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
> >>> -			    OMAP_I2C_STAT_XUDF)) {
> >>> +	if ((dev->cmd_err & OMAP_I2C_STAT_AL)
> >>> +			|| (dev->cmd_err & OMAP_I2C_STAT_ROVR)
> >>> +			|| (dev->cmd_err & OMAP_I2C_STAT_XUDF)) {
> >>
> >> Sorry, what is the difference? I didn't understand the optimisation
> >> and why now is more clear. Can you just add a comment?
> > 
> > semantically they're not the same, right ? We want to check if each of
> > those bits are set, not if all of them are set together.
> > 
> > my 2 cents.
> 
> You are doing the same thing, but of course is better with just one

I never claimed the contrary. I said *semantically* they're not the
same.

> *if* as before . A general rule is: when you have logic expression you

We still have a single *if* and I'm sure compiler will optimize that
expression as much as it likes.

> can use undefined states to simplify the logic. 

don't-care is not the same as undefined states.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux