On Wed, Nov 28, 2012 at 03:31:58PM -0800, Andrew Morton wrote: > On Wed, 28 Nov 2012 20:21:26 +0100 > Thierry Reding <thierry.red...@avionic-design.de> wrote: > > > + err = i2c_transfer(client->adapter, &msg, 1); > > + if (err < 0) { > > + /* > > + * If the time cannot be set, restart the RTC anyway. Note > > + * that errors are ignored if the RTC cannot be started so > > + * that we have a chance to propagate the original error. > > + */ > > + pcf8523_start_rtc(client); > > + return err; > > + } > > + > > + return pcf8523_start_rtc(client); > > hm, well, that is of course equivalent to > > err = i2c_transfer(client->adapter, &msg, 1); > pcf8523_start_rtc(client); > return err;
At the risk of sounding pedantic, it isn't quite the same thing. There is a possibility that i2c_transfer() succeeds but pcf8523_start_rtc() still fails. So in fact we catch errors in i2c_transfer() and from starting the RTC in case the time was properly set. The only error we might loose if if the RTC fails to start after the time cannot be set but in that case there's probably not a whole lot we can do. > but I suppose the code as proposed is clear, extensible, and not our > worst-ever sin ;) Yes, this is one of the cases where the gain in clarity justifies the extra verbosity. =) Thierry
pgpPnuSxJENjJ.pgp
Description: PGP signature