On Tue, 26 Nov 2024 14:51:19 +0100 Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > Dear IIO Mailing-List, Hi Jonathan! > > Thank you so much for the review. As you probably saw, most (all?) of > my commits have a huge invisible question mark attached. Most of my > questions you answered clearly. On particular topics I'd like to get > back, though. Generally I will try to apply the requested changes to > best of my understanding. > > On Sun, Nov 24, 2024 at 7:07 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > On Sun, 17 Nov 2024 18:26:34 +0000 > > Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > > > > > Make measurements right-justified, since it is the default for the > > > driver and sensor. By not setting the ADXL345_DATA_FORMAT_JUSTIFY bit, > > > the data becomes right-judstified. This was the original setting, there > > > is no reason to change it to left-justified, where right-justified > > > simplifies working on the registers. > > > > Surely this can't be changed independent of other changes as it will > > change the format of the data we are processing? > > > > Each change must stand on it's own so that I can apply up to any > > point in your patch set and have everything continue to work. > > This is probably not quite clear. Originally the driver was > right-justified. One of my last commits > (f68ebfe1501bf1110eebf5e968c4d9186cba8706) changed the driver to work > with left-justified measurements. So, I feel changing the orginal > behavior is wrong, and here I try to re-establish the original driver > behavior. > > When looking at the datasheet right-justified data seems to be easier > to handle, but I don't have any personal preference. Ok. I'm still a little confused. Can userspace see any result from either the earlier patch or this one? Jonathan > > Lothar > > [...] > > > --- > > > drivers/iio/accel/adxl345_core.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c > > > index 2b62e79248..926e397678 100644 > > > --- a/drivers/iio/accel/adxl345_core.c > > > +++ b/drivers/iio/accel/adxl345_core.c > > > @@ -184,8 +184,13 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, > > > struct adxl34x_state *st; > > > struct iio_dev *indio_dev; > > > u32 regval; > > > + > > > + /* NB: ADXL345_DATA_FORMAT_JUSTIFY or 0: > > /* > > * NB: AD... > > > > is the multiline comment style all IIO drivers use (and most of the kernel > > except for networking. > > > > > + * do right-justified: 0, then adjust resolution according to 10-bit > > > + * through 13-bit in channel - this is the default behavior, and can > > > + * be modified here by oring ADXL345_DATA_FORMAT_JUSTIFY > > > + */ > > > unsigned int data_format_mask = (ADXL345_DATA_FORMAT_RANGE | > > > - ADXL345_DATA_FORMAT_JUSTIFY | > > > ADXL345_DATA_FORMAT_FULL_RES | > > > ADXL345_DATA_FORMAT_SELF_TEST); > > > int ret; > >
