Re: [PATCH] iio:magnetometer:ak8975 move out of staging

[Lars-Peter Clausen <lars@metafoo.de>]

On 02/09/2013 05:55 PM, Jonathan Cameron wrote:
> This driver has been clean and correct for quite some time.
> It is simple and uses only straight forward standard
> interfaces.
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>

Two comments, which don't necessarily need to be addressed before moving it
out of staging, but should probably should be fixed at some point.

> ---
>  drivers/iio/magnetometer/Kconfig          |  11 +
>  drivers/iio/magnetometer/Makefile         |   1 +
>  drivers/iio/magnetometer/ak8975.c         | 520 ++++++++++++++++++++++++++++++
>  drivers/staging/iio/magnetometer/Kconfig  |  11 -
>  drivers/staging/iio/magnetometer/Makefile |   1 -
>  drivers/staging/iio/magnetometer/ak8975.c | 520 ------------------------------
>  6 files changed, 532 insertions(+), 532 deletions(-)
> 
[...]
> +struct ak8975_data {
> +	struct i2c_client	*client;
> +	struct attribute_group	attrs;
> +	struct mutex		lock;
> +	u8			asa[3];
> +	long			raw_to_gauss[3];
> +	u8			reg_cache[AK8975_MAX_REGS];
> +	int			eoc_gpio;
> +	int			eoc_irq;

eoc_irq is never really used.

> +};
> +
> +static const int ak8975_index_to_reg[] = {
> +	AK8975_REG_HXL, AK8975_REG_HYL, AK8975_REG_HZL,
> +};
> +
> +/*
> + * Helper function to write to the I2C device's registers.
> + */
> +static int ak8975_write_data(struct i2c_client *client,
> +			     u8 reg, u8 val, u8 mask, u8 shift)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct ak8975_data *data = iio_priv(indio_dev);
> +	u8 regval;
> +	int ret;
> +
> +	regval = (data->reg_cache[reg] & ~mask) | (val << shift);
> +	ret = i2c_smbus_write_byte_data(client, reg, regval);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Write to device fails status %x\n", ret);
> +		return ret;
> +	}
> +	data->reg_cache[reg] = regval;

The reg cache is written to here, but it is never read from again. It may
make sense to convert the driver to using regmap instead of open-coding the
register read/write functions.

> +
> +	return 0;
> +}
> +
[...]