From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S933559AbcAKOr4 (ORCPT <rfc822;w@1wt.eu>);
	Mon, 11 Jan 2016 09:47:56 -0500
Received: from bear.ext.ti.com ([192.94.94.41]:35203 "EHLO bear.ext.ti.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1760326AbcAKOqD (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Mon, 11 Jan 2016 09:46:03 -0500
Subject: Re: /sys/class/power_supply/bq27200-0/capacity changed meaning
 between 4.1 and 4.4?
To: Pavel Machek <pavel@ucw.cz>, <pali.rohar@gmail.com>, <sre@debian.org>,
        <sre@ring0.de>, kernel list <linux-kernel@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        <linux-omap@vger.kernel.org>, <tony@atomide.com>, <khilman@kernel.org>,
        <aaro.koskinen@iki.fi>, <ivo.g.dimitrov.75@gmail.com>,
        <patrikbachan@gmail.com>, <serge@hallyn.com>
References: <20160109230709.GA30551@amd> <20160110080621.GA3929@amd>
CC: <a.hajda@samsung.com>, <giometti@linux.it>
From: "Andrew F. Davis" <afd@ti.com>
Message-ID: <5693BFEB.3020500@ti.com>
Date: Mon, 11 Jan 2016 08:44:59 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.4.0
MIME-Version: 1.0
In-Reply-To: <20160110080621.GA3929@amd>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 01/10/2016 02:06 AM, Pavel Machek wrote:
> Hi!
>
>> Did /sys/class/power_supply/bq27200-0/capacity change meaning between
>> 4.1 and 4.4?
>>
>> It used to report battery capacity remaining in percent.
>>
>> Not sure what it reports now, but ain't in percent....
>
>> The patch does not compile, but I should be sleeping, not trying to
>> understand crazy code. Whoever wrote it, please fix it. Maybe you can
>> just do
>
> ...and more crazy code :-(.
>
>          cache.flags = bq27xxx_read(di, BQ27XXX_REG_FLAGS, has_singe_flag);
> 		if ((cache.flags & 0xff) == 0xff)
> 		                cache.flags = -1; /* read error */
> 				        /* WTF? bq27xxx returns -ERRNO
>          on error, we mask some bits off it, and then make it -1... */
> 	

This is probably left over from when the driver was 1wire only, which seems
to fail by just reading back all ones. Not sure why the -1 though?

>
> ...and one crazy optimalization...
>
>          if (memcmp(&di->cache, &cache, sizeof(cache)) != 0)
> 	                di->cache = cache;
>

Hmmm, I think the lines above it:

	if (di->cache.capacity != cache.capacity)
		power_supply_changed(di->bat);

were at one point rolled into this as so:

	if (memcmp(&di->cache, &cache, sizeof(cache)) != 0) {
		di->cache = cache;
		power_supply_changed(di->bat);
	}

Otherwise that isn't an optimization, it probably takes more
time comparing than just doing the copy every time...

> ...are we playing obfuscated C code contest, yet?
>
>          case POWER_SUPPLY_PROP_PRESENT:
> 	                val->intval = di->cache.flags < 0 ? 0 : 1;
>
> ...to decidegree C?
>                  if (ret == 0)
> 		   	        pval->intval -= 2731; /* convert decidegree k to c */
>
> as read takes enum, make it enum like this?
> static inline int bq27xxx_read(struct bq27xxx_device_info *di, enum bq27xxx_reg_index reg_index,
> 		                         bool single)
> 					

ACK

>
> 									Pavel
>