Re: [PATCH v2 3/4] iio: accel: adxl345: add calibration offset support

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Mon, Jun 18, 2018 at 6:59 PM, Akinobu Mita <akinobu.mita@xxxxxxxxx> wrote:
> The ADXL345 provides the offset adjustment registers for each axis.
> This adds the iio channel information for the calibraion offsets with
> that feature.

>  #define ADXL345_REG_DEVID              0x00
> +#define ADXL345_REG_OFSX               0x1e
> +#define ADXL345_REG_OFSY               0x1f
> +#define ADXL345_REG_OFSZ               0x20

+ #define ADXL345_REG_OSF_AXIS(si) (0x1e + (si))

> -       __le16 regval;
> +       __le16 accel;

I would rather split renamint to a separate change.

>                 ret = regmap_bulk_read(data->regmap, ADXL345_REG_DATAX0 +
> -                                      sizeof(regval) * chan->scan_index,
> -                                      &regval, sizeof(regval));
> +                                      sizeof(accel) * chan->scan_index, &accel,
> +                                      sizeof(accel));

This hunk would be cleaner after addressing my proposal for patch 2.

> +       case IIO_CHAN_INFO_CALIBBIAS:
> +               ret = regmap_read(data->regmap,
> +                                 ADXL345_REG_OFSX + chan->scan_index, &regval);

So, for my opinion an axis agnostic macro would suit better above and here.

> +               return regmap_write(data->regmap,
> +                                   ADXL345_REG_OFSX + chan->scan_index,
> +                                   val / 4);

Ditto.

-- 
With Best Regards,
Andy Shevchenko
--
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