Re: [PATCH v4] Add LTC2941/LTC2943 Battery Gauge Driver

[Pavel Machek]

Hi!

> +++ b/drivers/power/ltc2941-battery-gauge.c
> @@ -0,0 +1,547 @@
> +/*
> + * I2C client/driver for the Linear Technology LTC2941 and LTC2943
> + * Battery Gas Gauge IC
> + *
> + * Copyright (C) 2014 Topic Embedded Systems
> + *

NOrmally GPL notice would go here.

> + * Author: Auryn Verwegen
> + * Author: Mike Looijmans
> + */

Do you want to add yourself as a maintainers?

> +{
> +	int ret;
> +	struct i2c_msg msgs[2] = { };
> +	u8 reg_start = reg;
> +
> +	msgs[0].addr	= client->addr;
> +	msgs[0].len	= 1;
> +	msgs[0].buf	= &reg_start;
> +
> +	msgs[1].addr	= client->addr;
> +	msgs[1].len	= num_regs;
> +	msgs[1].buf	= buf;
> +	msgs[1].flags	= I2C_M_RD;
> +
> +	ret = i2c_transfer(client->adapter, &msgs[0], 2);

Here you send byte from kernel stack to i2c device. I'd add msg[0].flags = 0.

> +		ret = ltc294x_write_regs(info->client,
> +			LTC294X_REG_CONTROL, &control, 1);
> +		if (ret < 0) {
> +			dev_err(&info->client->dev,
> +				"Could not write register\n");
> +			goto error_exit;

return ret; No need to goto when you have nothing to clean up.

> +		}
> +	}
> +
> +	return 0;
> +
> +error_exit:
> +	return ret;
> +}

> +	if (info->Qlsb < 0)
> +		value += 0xFFFF;

q_lsb?

> +	dataw[0] = I16_MSB(value);
> +	dataw[1] = I16_LSB(value);
> +	ret = ltc294x_write_regs(info->client,
> +		LTC294X_REG_ACC_CHARGE_MSB, &dataw[0], 2);
> +	if (ret < 0)
> +		goto error_exit;
> +	/* Enable analog section */
> +error_exit:

Interesting use of goto... remove?

> +	ret = of_property_read_u32(np, "prescaler-exponent", &prescaler_exp);
> +	if (ret < 0) {
> +		dev_err(&client->dev,
> +			"PrescalerExponent not in devicetree, assume max\n");

assuming?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/