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

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

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.