On Thu, Apr 19, 2012 at 03:19:14PM +0900, Tomoya MORINAGA wrote: > On Thu, Apr 19, 2012 at 12:05 AM, Wolfram Sang <w.sang@xxxxxxxxxxxxxx> wrote: > > On Mon, Mar 26, 2012 at 02:55:24PM +0900, Tomoya MORINAGA wrote: > >> > >> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@xxxxxxxxx> > > > > Woha, that's copy&paste code all over. May I ask to refactor this? > > Something like: > > > > rtn = pch_..._wait_for_checked_xfer(); > > if (rtn) > > return rtn; > > // All the things you have to do when rtn == 0 > > > > in that new function (the name was only a suugestion), you can do all > > the checks which are now copy-pasted, e.g.: > > > > 458 rtn = pch_i2c_wait_for_xfer_complete(adap); > > 459 if (rtn == 0) { > > 460 if (pch_i2c_getack(adap)) { > > 461 pch_dbg(adap, "Receive NACK for slave address" > > 462 "setting\n"); > > 463 return -EIO; > > 464 } > > 465 } else if (rtn == -EIO) { /* Arbitration Lost */ > > 466 pch_err(adap, "Lost Arbitration\n"); > > 467 pch_clrbit(adap->pch_base_address, PCH_I2CSR, I2CMAL_BIT); > > 468 pch_clrbit(adap->pch_base_address, PCH_I2CSR, I2CMIF_BIT); > > 469 pch_i2c_init(adap); > > 470 return -EAGAIN; > > 471 } else { /* wait-event timeout */ > > 472 pch_i2c_stop(adap); > > // Your current fixes added here > > 473 return -ETIME; > > 474 } > > return 0; > > > > It is only pseudo-code, but I think it can be done and will help the driver. > > > > OK. > I'm ready to submit new patch you are saying. Applied to next, thanks to the follow up patches :) -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature