linux-kernel - Re: [Patch v1 04/11]Power: DA9052 battery driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110715005749.GA2760@opensource.wolfsonmicro.com>
Date:	Fri, 15 Jul 2011 09:57:54 +0900
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	ashishj3 <ashish.jangam@...tcummins.com>
Cc:	linaro-dev@...ts.linaro.org, cbou@...l.ru, dwmw2@...radead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [Patch v1 04/11]Power: DA9052 battery driver

On Thu, Jul 14, 2011 at 02:27:08PM +0530, ashishj3 wrote:

> +static inline int volt_reg_to_mV(int value)
> +		{ return ((value*1000) / 512) + 2500; }
> +static inline int ichg_reg_to_mA(int value)
> +		{ return ((value * 3900) / 1000); }

It'd be better to use the standard coding style for this stuff.

> +static int da9052_battery_read_temperature(struct da9052_battery *battery,
> +					    int *bat_temp)
> +{
> +	int count, avg_value[DA9052_AVERAGE_SIZE];
> +
> +	for (count = 0; count < DA9052_AVERAGE_SIZE; count++)
> +		avg_value[count] = da9052_adc_temperature_read(battery->da9052);
> +		if (avg_value[count] < 0)
> +			return avg_value[count];
> +
> +	*bat_temp = da9052_average_calcuation(avg_value);
> +
> +	return 0;
> +}

Don't do this, just return the reading let something higher up the stack
figure out if it needs to take an average reading.  Probably userspace.
This isn't specific to this device, if we need to do something in the
kernel it should be a core feature but I'd expect userspace can do a
better job by looking at longer term trends.

> +static struct power_supply template_battery = {
> +	.name		= "da9052-bat",
> +	.type		= POWER_SUPPLY_TYPE_BATTERY,
> +	.properties	= da9052_bat_props,
> +	.num_properties	= ARRAY_SIZE(da9052_bat_props),
> +	.get_property	= da9052_bat_get_property,
> +	.use_for_apm	= 1,
> +};

Hrm, use_for_apm isn't typically set by embedded battery drivers so
probably shouldn't be set by this one.  We should figure out some better
way to set this, perhaps board specific, if it's needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/