Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature
From: Andy Shevchenko
Date: Tue Jun 24 2025 - 03:28:23 EST
On Mon, Jun 23, 2025 at 10:57:39PM +0200, Lothar Rubusch wrote:
> On Mon, Jun 23, 2025 at 11:34 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
> > On Sun, Jun 22, 2025 at 03:50:06PM +0000, Lothar Rubusch wrote:
...
> > > + case IIO_EV_TYPE_MAG:
> > > + return adxl345_read_mag_config(st, dir,
> > > + ADXL345_ACTIVITY);
> >
> > It looks like you set the editor to wrap at 72 characters, but here the single
> > line less than 80! Note that the limit is *exactly* 80 character.
> >
>
> I have my setup adjusted to 80 characters. Anyway, the cases here is
> different, it needs
> to be seen in context of the follow up patches. I tried to prepare the
> patches now in a way
> where changes are mostly "added". Is this correct and desired patch preparation?
>
> In the particular case, this patch now adds ACTIVITY. A follow up
> patch will add INACTIVITY.
> Since this is still building up, it will add yet another argument to
> those functions, i.e.
> > > + return adxl345_write_mag_config(st, dir,
> > > + ADXL345_ACTIVITY,
>
> will become, later
> > > return adxl345_write_mag_config(st, dir,
> > > ADXL345_ACTIVITY,
> > > + ADXL345_INACTIVITY,
Yeah, but with the difference that you still remove the added line in the case
above (as this example is not the same as what we are talking about).
I think you wanted more something like
return adxl345_read_mag_config(st, dir,
ADXL345_ACTIVITY);
ito become
return adxl345_read_mag_config(st, dir,
ADXL345_INACTIVITY,
ADXL345_ACTIVITY);
> To make the change more additive, I did linebreaks earlier than 80
> characters. Is this
> legitimate in this case?
I think so.
> If so, I'll keep all related formatting as is (and will only change
> the other requests).
Sure.
> Otherwise, I can do it differently and adopt all the formatting
> changes to prioritize 80 characters.
--
With Best Regards,
Andy Shevchenko