linux-kernel - Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aFpTgoEIkWorp_pQ@smile.fi.intel.com>
Date: Tue, 24 Jun 2025 10:28:02 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
	dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org,
	corbet@....net, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	eraretuya@...il.com
Subject: Re: [PATCH v10 3/7] iio: accel: adxl345: add activity event feature

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@...el.com> 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