On Thu, Mar 28, 2024 at 2:37 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Wed, 27 Mar 2024 22:03:14 +0000 > Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > > > Replace write() data_format by regmap_update_bits(), because bus specific > > pre-configuration may have happened before on the same register. For > > further updates to the data_format register then bus pre-configuration > > needs to be masked out. > > > > Remove the data_range field from the struct adxl345_data, because it is > > not used anymore. > > > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> > > --- > > drivers/iio/accel/adxl345_core.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > > index 8bd30a23e..35df5e372 100644 > > --- a/drivers/iio/accel/adxl345_core.c > > +++ b/drivers/iio/accel/adxl345_core.c > > @@ -37,7 +37,15 @@ > > #define ADXL345_POWER_CTL_MEASURE BIT(3) > > #define ADXL345_POWER_CTL_STANDBY 0x00 > > > > +#define ADXL345_DATA_FORMAT_RANGE GENMASK(1, 0) /* Set the g range */ > > +#define ADXL345_DATA_FORMAT_JUSTIFY BIT(2) /* Left-justified (MSB) mode */ > > #define ADXL345_DATA_FORMAT_FULL_RES BIT(3) /* Up to 13-bits resolution */ > > +#define ADXL345_DATA_FORMAT_SELF_TEST BIT(7) /* Enable a self test */ > > +#define ADXL345_DATA_FORMAT_MSK (ADXL345_DATA_FORMAT_RANGE | \ > > + ADXL345_DATA_FORMAT_JUSTIFY | \ > > + ADXL345_DATA_FORMAT_FULL_RES | \ > > + ADXL345_DATA_FORMAT_SELF_TEST) > This needs renaming. It's not a mask of everything in the register, or > even just of everything related to format. > > Actually I'd just not have this definition. Use the build up value > from all the submasks at the call site. Then we are just making it clear > only a subset of fields are being cleared. > I understand this solution was not very useful. I'm not sure, I understood you correctly. Please have a look into v6 if this matches your comment. Now, I remove the mask, instead I use a local variable in core's probe() for the update mask. I added a comment. Nevertheless, I keep the used flags for FORMAT_DATA. Does this go into the direction of using the build up value from the submasks at the call site? > Jonathan > > > + > > #define ADXL345_DATA_FORMAT_2G 0 > > #define ADXL345_DATA_FORMAT_4G 1 > > #define ADXL345_DATA_FORMAT_8G 2 > > @@ -48,7 +56,6 @@ > > struct adxl345_data { > > const struct adxl345_chip_info *info; > > struct regmap *regmap; > > - u8 data_range; > > }; > > > > #define ADXL345_CHANNEL(index, axis) { \ > > @@ -218,15 +225,14 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap) > > > > data = iio_priv(indio_dev); > > data->regmap = regmap; > > - /* Enable full-resolution mode */ > > - data->data_range = ADXL345_DATA_FORMAT_FULL_RES; > > data->info = device_get_match_data(dev); > > if (!data->info) > > return -ENODEV; > > > > - ret = regmap_write(data->regmap, ADXL345_REG_DATA_FORMAT, > > - data->data_range); > > - if (ret < 0) > > + /* Enable full-resolution mode */ > > + ret = regmap_update_bits(regmap, ADXL345_REG_DATA_FORMAT, > > + ADXL345_DATA_FORMAT_MSK, ADXL345_DATA_FORMAT_FULL_RES); > > + if (ret) > > return dev_err_probe(dev, ret, "Failed to set data range\n"); > > > > indio_dev->name = data->info->name; >