On Tue, Feb 15, 2011 at 1:48 PM, Grazvydas Ignotas wrote: > On Tue, Feb 15, 2011 at 12:14 AM, Lars-Peter Clausen wrote: >> On 02/14/2011 10:58 PM, Grazvydas Ignotas wrote: >>> This is wrong, as read function returns negative values for bq27500 >>> when discharging. That's why read function used to pass value through >>> argument before your series (return value was for error code). >> >> I don't think so. The register is 16bit wide and it is read as a unsigned. So >> in the non error case bq27x00_read will always return >= 0. >> The value is later reinterpreted as a signed 16bit.(See the other lines you >> quoted underneath). > > Hmh, right.. > >> Did you experience any actual problem with current being wrong? > > Yes, the returned current values were randomly jumping between -500000 > and 600000 while the device was discharging, so I thought > uninitialized values were being returned (this never happened before > the series; no errors in dmesg). I'll need to debug a bit more I > guess.. ok this series seem to trigger unrelated bug, probably due to lots of reads when cache is being updated, the attached patch seems to help. Feel free to integrate it or I'll send it separately after your patches are merged. However there is bigger problem, compiling as module and doing insmod/rmmod/insmod causes kernel OOPS: [ 882.575714] BUG: sleeping function called from invalid context at arch/arm/mm/fault.c:295 [ 882.584350] in_atomic(): 0, irqs_disabled(): 128, pid: 1154, name: insmod ... [ 882.959930] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 882.968444] pgd = c14c8000 [ 882.977905] Internal error: Oops: 817 [#1] ... [ 883.304412] [] (__queue_work+0x140/0x260) from [] (queue_work_on+0x2c/0x34) [ 883.313568] [] (queue_work_on+0x2c/0x34) from [] (bq27x00_update+0x1f8/0x220 [bq27x00_battery]) [ 883.324584] [] (bq27x00_update+0x1f8/0x220 [bq27x00_battery]) from [] (bq27x00_battery_poll+0x14/0x48 [bq27x00_battery]) full backtrace attached.