On Thu, 28 May 2009 12:22:41 +0100, Ben Dooks wrote: > Jean Delvare wrote: > > On Tue, 26 May 2009 09:07:01 +0100, Ben Dooks wrote: > >> + int val; > > > > This is an horribly vague struct member name, and undocumented at that. > > Added documentation and renamed to ret_val. > > * @ret_val: Value returned from current conversion to return to caller. This seems artificially complex. You need to wait for the conversion to complete anyway, so why bother with an asynchronous mechanism? > >> +static struct s3c_hwmon *done_adc; > > > > What is this? > > The core adc driver doesn't keep any private data, so we need > this to get back to our state when the conversion ends. This will break if you have two "s3c-hwmon" devices on the same system. Why don't you just fix the core adc driver? > >> + ret *= cfg->mult; > >> + ret /= cfg->div; > > > > Division lacks rounding. > > Which rounding should be used? DIV_ROUND_CLOSEST() -- Jean Delvare
