Gwendal Grignou schrieb am 05.11.2014 23:10:
> For each type of compass supported (AK8975 and AK8963),
> add a definition structure for register masks, important registers,
> raw data interpretation.
> This change will make integrating new type of devices easier.
>
> Remove i2c register cache. It is only used for one single register.
Looks basically good to me, I just put some thoughts about tweaking indentations inline.
>
> Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx>
> ---
> drivers/iio/magnetometer/ak8975.c | 279 +++++++++++++++++++++++++-------------
> 1 file changed, 184 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 12f0b00..b170654 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -86,13 +86,153 @@
> #define AK8975_MAX_CONVERSION_TIMEOUT 500
> #define AK8975_CONVERSION_DONE_POLL_TIME 10
> #define AK8975_DATA_READY_TIMEOUT ((100*HZ)/1000)
> -#define RAW_TO_GAUSS_8975(asa) ((((asa) + 128) * 3000) / 256)
> -#define RAW_TO_GAUSS_8963(asa) ((((asa) + 128) * 6000) / 256)
> +
> +/*
> + * Precalculate scale factor (in Gauss units) for each axis and
> + * store in the device data.
> + *
> + * This scale factor is axis-dependent, and is derived from 3 calibration
> + * factors ASA(x), ASA(y), and ASA(z).
> + *
> + * These ASA values are read from the sensor device at start of day, and
> + * cached in the device context struct.
> + *
> + * Adjusting the flux value with the sensitivity adjustment value should be
> + * done via the following formula:
> + *
> + * Hadj = H * ( ( ( (ASA-128)*0.5 ) / 128 ) + 1 )
> + * where H is the raw value, ASA is the sensitivity adjustment, and Hadj
> + * is the resultant adjusted value.
> + *
> + * We reduce the formula to:
> + *
> + * Hadj = H * (ASA + 128) / 256
> + *
> + * H is in the range of -4096 to 4095. The magnetometer has a range of
> + * +-1229uT. To go from the raw value to uT is:
> + *
> + * HuT = H * 1229/4096, or roughly, 3/10.
> + *
> + * Since 1uT = 0.01 gauss, our final scale factor becomes:
> + *
> + * Hadj = H * ((ASA + 128) / 256) * 3/10 * 1/100
> + * Hadj = H * ((ASA + 128) * 0.003) / 256
> + *
> + * Since ASA doesn't change, we cache the resultant scale factor into the
> + * device context in ak8975_setup().
> + *
> + * Given we use IIO_VAL_INT_PLUS_MICRO bit when displaying the scale, we
> + * multiply the stored scale value by 1e6.
> + */
> +long ak8975_raw_to_gauss(u16 data)
> +{
> + return (((long)data + 128) * 3000) / 256;
> +}
> +
> +/*
> + * For AK8963, same calculation, but the device is less sensitive:
> + *
> + * H is in the range of +-8190. The magnetometer has a range of
> + * +-4912uT. To go from the raw value to uT is:
> + *
> + * HuT = H * 4912/8190, or roughly, 6/10, instead of 3/10.
> + */
> +long ak8963_raw_to_gauss(u16 data)
> +{
> + return (((long)data + 128) * 6000) / 256;
> +}
>
> /* Compatible Asahi Kasei Compass parts */
> enum asahi_compass_chipset {
> AK8975,
> AK8963,
> + AK_MAX_TYPE
> +};
> +
> +enum ak_ctrl_reg_addr {
> + ST1,
> + ST2,
> + CNTL,
> + ASA_BASE,
> + MAX_REGS,
> + REGS_END,
> +};
> +
> +enum ak_ctrl_reg_mask {
> + ST1_DRDY,
> + ST2_HOFL,
> + ST2_DERR,
> + CNTL_MODE,
> + MASK_END,
> +};
> +
> +enum ak_ctrl_mode {
> + POWER_DOWN,
> + MODE_ONCE,
> + SELF_TEST,
> + FUSE_ROM,
> + MODE_END,
> +};
> +
> +struct ak_def {
> + enum asahi_compass_chipset type;
> + long (*raw_to_gauss)(u16 data);
> + u16 range;
> + u8 ctrl_regs[REGS_END];
> + u8 ctrl_masks[MASK_END];
> + u8 ctrl_modes[MODE_END];
> + u8 data_regs[3];
> +} ak_def_array[AK_MAX_TYPE] = {
> +{
Shouldn't this be indented one level more?
> + .type = AK8975,
> + .raw_to_gauss = ak8975_raw_to_gauss,
> + .range = 4096,
> + .ctrl_regs = {
> + AK8975_REG_ST1,
> + AK8975_REG_ST2,
> + AK8975_REG_CNTL,
> + AK8975_REG_ASAX,
> + AK8975_MAX_REGS},
> + .ctrl_masks = {
> + AK8975_REG_ST1_DRDY_MASK,
> + AK8975_REG_ST2_HOFL_MASK,
> + AK8975_REG_ST2_DERR_MASK,
> + AK8975_REG_CNTL_MODE_MASK},
> + .ctrl_modes = {
> + AK8975_REG_CNTL_MODE_POWER_DOWN,
> + AK8975_REG_CNTL_MODE_ONCE,
> + AK8975_REG_CNTL_MODE_SELF_TEST,
> + AK8975_REG_CNTL_MODE_FUSE_ROM},
> + .data_regs = {
> + AK8975_REG_HXL,
> + AK8975_REG_HYL,
> + AK8975_REG_HZL},
> +},
> +{
> + .type = AK8963,
> + .raw_to_gauss = ak8963_raw_to_gauss,
> + .range = 8190,
> + .ctrl_regs = {
> + AK8975_REG_ST1,
> + AK8975_REG_ST2,
> + AK8975_REG_CNTL,
> + AK8975_REG_ASAX,
> + AK8975_MAX_REGS},
> + .ctrl_masks = {
> + AK8975_REG_ST1_DRDY_MASK,
> + AK8975_REG_ST2_HOFL_MASK,
> + 0,
> + AK8975_REG_CNTL_MODE_MASK},
> + .ctrl_modes = {
> + AK8975_REG_CNTL_MODE_POWER_DOWN,
> + AK8975_REG_CNTL_MODE_ONCE,
> + AK8975_REG_CNTL_MODE_SELF_TEST,
> + AK8975_REG_CNTL_MODE_FUSE_ROM},
> + .data_regs = {
> + AK8975_REG_HXL,
> + AK8975_REG_HYL,
> + AK8975_REG_HZL},
> +},
> };
>
> /*
> @@ -100,40 +240,36 @@ enum asahi_compass_chipset {
> */
> struct ak8975_data {
> struct i2c_client *client;
> + struct ak_def *def;
> 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;
> wait_queue_head_t data_ready_queue;
> unsigned long flags;
> - enum asahi_compass_chipset chipset;
> -};
> -
> -static const int ak8975_index_to_reg[] = {
> - AK8975_REG_HXL, AK8975_REG_HYL, AK8975_REG_HZL,
> + u8 cntl_cache;
> };
>
> /*
> - * Helper function to write to the I2C device's registers.
> + * Helper function to write to CNTL register.
> */
> -static int ak8975_write_data(struct i2c_client *client,
> - u8 reg, u8 val, u8 mask, u8 shift)
> +static int ak8975_set_mode(struct ak8975_data *data, enum ak_ctrl_mode mode)
> {
> - 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);
> + regval = (data->cntl_cache & ~data->def->ctrl_masks[CNTL_MODE]) |
> + data->def->ctrl_modes[mode];
This could be indented by one more space.
> + ret = i2c_smbus_write_byte_data(
> + data->client, data->def->ctrl_regs[CNTL], regval);
This could be indented like this:
ret = i2c_smbus_write_byte_data(data->client,
data->def->ctrl_regs[CNTL], regval);
> if (ret < 0) {
> - dev_err(&client->dev, "Write to device fails status %x\n", ret);
> return ret;
> }
> - data->reg_cache[reg] = regval;
> + data->cntl_cache = regval;
> + /* After mode change wait atleast 100us */
> + usleep_range(100, 500);
>
> return 0;
> }
> @@ -207,18 +343,15 @@ static int ak8975_setup(struct i2c_client *client)
> }
>
> /* Write the fused rom access mode. */
> - ret = ak8975_write_data(client,
> - AK8975_REG_CNTL,
> - AK8975_REG_CNTL_MODE_FUSE_ROM,
> - AK8975_REG_CNTL_MODE_MASK,
> - AK8975_REG_CNTL_MODE_SHIFT);
> + ret = ak8975_set_mode(data, FUSE_ROM);
> if (ret < 0) {
> dev_err(&client->dev, "Error in setting fuse access mode\n");
> return ret;
> }
>
> /* Get asa data and store in the device data. */
> - ret = i2c_smbus_read_i2c_block_data(client, AK8975_REG_ASAX,
> + ret = i2c_smbus_read_i2c_block_data(client,
> + data->def->ctrl_regs[ASA_BASE],
> 3, data->asa);
> if (ret < 0) {
> dev_err(&client->dev, "Not able to read asa data\n");
> @@ -226,11 +359,7 @@ static int ak8975_setup(struct i2c_client *client)
> }
>
> /* After reading fuse ROM data set power-down mode */
> - ret = ak8975_write_data(client,
> - AK8975_REG_CNTL,
> - AK8975_REG_CNTL_MODE_POWER_DOWN,
> - AK8975_REG_CNTL_MODE_MASK,
> - AK8975_REG_CNTL_MODE_SHIFT);
> + ret = ak8975_set_mode(data, POWER_DOWN);
> if (ret < 0) {
> dev_err(&client->dev, "Error in setting power-down mode\n");
> return ret;
> @@ -245,56 +374,9 @@ static int ak8975_setup(struct i2c_client *client)
> }
> }
>
> -/*
> - * Precalculate scale factor (in Gauss units) for each axis and
> - * store in the device data.
> - *
> - * This scale factor is axis-dependent, and is derived from 3 calibration
> - * factors ASA(x), ASA(y), and ASA(z).
> - *
> - * These ASA values are read from the sensor device at start of day, and
> - * cached in the device context struct.
> - *
> - * Adjusting the flux value with the sensitivity adjustment value should be
> - * done via the following formula:
> - *
> - * Hadj = H * ( ( ( (ASA-128)*0.5 ) / 128 ) + 1 )
> - *
> - * where H is the raw value, ASA is the sensitivity adjustment, and Hadj
> - * is the resultant adjusted value.
> - *
> - * We reduce the formula to:
> - *
> - * Hadj = H * (ASA + 128) / 256
> - *
> - * H is in the range of -4096 to 4095. The magnetometer has a range of
> - * +-1229uT. To go from the raw value to uT is:
> - *
> - * HuT = H * 1229/4096, or roughly, 3/10.
> - *
> - * Since 1uT = 0.01 gauss, our final scale factor becomes:
> - *
> - * Hadj = H * ((ASA + 128) / 256) * 3/10 * 1/100
> - * Hadj = H * ((ASA + 128) * 0.003) / 256
> - *
> - * Since ASA doesn't change, we cache the resultant scale factor into the
> - * device context in ak8975_setup().
> - */
> - if (data->chipset == AK8963) {
> - /*
> - * H range is +-8190 and magnetometer range is +-4912.
> - * So HuT using the above explanation for 8975,
> - * 4912/8190 = ~ 6/10.
> - * So the Hadj should use 6/10 instead of 3/10.
> - */
> - data->raw_to_gauss[0] = RAW_TO_GAUSS_8963(data->asa[0]);
> - data->raw_to_gauss[1] = RAW_TO_GAUSS_8963(data->asa[1]);
> - data->raw_to_gauss[2] = RAW_TO_GAUSS_8963(data->asa[2]);
> - } else {
> - data->raw_to_gauss[0] = RAW_TO_GAUSS_8975(data->asa[0]);
> - data->raw_to_gauss[1] = RAW_TO_GAUSS_8975(data->asa[1]);
> - data->raw_to_gauss[2] = RAW_TO_GAUSS_8975(data->asa[2]);
> - }
> + data->raw_to_gauss[0] = data->def->raw_to_gauss(data->asa[0]);
> + data->raw_to_gauss[1] = data->def->raw_to_gauss(data->asa[1]);
> + data->raw_to_gauss[2] = data->def->raw_to_gauss(data->asa[2]);
>
> return 0;
> }
> @@ -317,7 +399,7 @@ static int wait_conversion_complete_gpio(struct ak8975_data *data)
> return -EINVAL;
> }
>
> - ret = i2c_smbus_read_byte_data(client, AK8975_REG_ST1);
> + ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST1]);
> if (ret < 0)
> dev_err(&client->dev, "Error in reading ST1\n");
>
> @@ -334,7 +416,8 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
> /* Wait for the conversion to complete. */
> while (timeout_ms) {
> msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> - ret = i2c_smbus_read_byte_data(client, AK8975_REG_ST1);
> + ret = i2c_smbus_read_byte_data(
> + client, data->def->ctrl_regs[ST1]);
Same as mentioned above.
> if (ret < 0) {
> dev_err(&client->dev, "Error in reading ST1\n");
> return ret;
> @@ -377,11 +460,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
> mutex_lock(&data->lock);
>
> /* Set up the device for taking a sample. */
> - ret = ak8975_write_data(client,
> - AK8975_REG_CNTL,
> - AK8975_REG_CNTL_MODE_ONCE,
> - AK8975_REG_CNTL_MODE_MASK,
> - AK8975_REG_CNTL_MODE_SHIFT);
> + ret = ak8975_set_mode(data, MODE_ONCE);
> if (ret < 0) {
> dev_err(&client->dev, "Error in setting operating mode\n");
> goto exit;
> @@ -398,14 +477,15 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
> goto exit;
>
> /* This will be executed only for non-interrupt based waiting case */
> - if (ret & AK8975_REG_ST1_DRDY_MASK) {
> - ret = i2c_smbus_read_byte_data(client, AK8975_REG_ST2);
> + if (ret & data->def->ctrl_masks[ST1_DRDY]) {
> + ret = i2c_smbus_read_byte_data(
> + client, data->def->ctrl_regs[ST2]);
And here as well.
> if (ret < 0) {
> dev_err(&client->dev, "Error in reading ST2\n");
> goto exit;
> }
> - if (ret & (AK8975_REG_ST2_DERR_MASK |
> - AK8975_REG_ST2_HOFL_MASK)) {
> + if (ret & (data->def->ctrl_masks[ST2_DERR] |
> + data->def->ctrl_masks[ST2_HOFL])) {
> dev_err(&client->dev, "ST2 status error 0x%x\n", ret);
> ret = -EINVAL;
> goto exit;
> @@ -414,7 +494,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
>
> /* Read the flux value from the appropriate register
> (the register is specified in the iio device attributes). */
> - ret = i2c_smbus_read_word_data(client, ak8975_index_to_reg[index]);
> + ret = i2c_smbus_read_word_data(client, data->def->data_regs[index]);
> if (ret < 0) {
> dev_err(&client->dev, "Read axis data fails\n");
> goto exit;
> @@ -423,7 +503,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
> mutex_unlock(&data->lock);
>
> /* Clamp to valid range. */
> - *val = clamp_t(s16, ret, -4096, 4095);
> + *val = clamp_t(s16, ret, -data->def->range, data->def->range);
> return IIO_VAL_INT;
>
> exit:
> @@ -497,6 +577,7 @@ static int ak8975_probe(struct i2c_client *client,
> int eoc_gpio;
> int err;
> const char *name = NULL;
> + enum asahi_compass_chipset chipset;
>
> /* Grab and set up the supplied GPIO. */
> if (client->dev.platform_data)
> @@ -536,14 +617,20 @@ static int ak8975_probe(struct i2c_client *client,
>
> /* id will be NULL when enumerated via ACPI */
> if (id) {
> - data->chipset =
> - (enum asahi_compass_chipset)(id->driver_data);
> + chipset = (enum asahi_compass_chipset)(id->driver_data);
> name = id->name;
> } else if (ACPI_HANDLE(&client->dev))
> - name = ak8975_match_acpi_device(&client->dev, &data->chipset);
> + name = ak8975_match_acpi_device(&client->dev, &chipset);
> else
> return -ENOSYS;
>
> + if (chipset >= AK_MAX_TYPE) {
> + dev_err(&client->dev, "AKM device type unsupported: %d\n",
> + chipset);
> + return -ENODEV;
> + }
> +
> + data->def = &ak_def_array[chipset];
> dev_dbg(&client->dev, "Asahi compass chip %s\n", name);
>
> /* Perform some basic start-of-day setup of the device. */
> @@ -574,7 +661,9 @@ MODULE_DEVICE_TABLE(i2c, ak8975_id);
> static const struct of_device_id ak8975_of_match[] = {
> { .compatible = "asahi-kasei,ak8975", },
> { .compatible = "ak8975", },
> - { }
> + { .compatible = "asahi-kasei,ak8963", },
> + { .compatible = "ak8963", },
> + {}
> };
> MODULE_DEVICE_TABLE(of, ak8975_of_match);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
