linux-kernel - Re: [PATCH v2 08/10] iio: pressure: mprls0025pa.c refactor

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]

Date: Sat, 30 Dec 2023 11:34:44 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Petre Rodan <petre.rodan@...dimension.ro>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, Andreas Klinger <ak@...klinger.de>,
 Lars-Peter Clausen <lars@...afoo.de>, Angel Iglesias
 <ang.iglesiasg@...il.com>, Matti Vaittinen <mazziesaccount@...il.com>
Subject: Re: [PATCH v2 08/10] iio: pressure: mprls0025pa.c refactor

On Wed, 27 Dec 2023 18:37:42 +0200
Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:

> On Wed, Dec 27, 2023 at 04:33:37PM +0200, Petre Rodan wrote:
> > On Tue, Dec 26, 2023 at 04:49:22PM +0000, Jonathan Cameron wrote:  
> 
> ,,,
> 
> > > >  		ret = devm_request_irq(dev, data->irq, mpr_eoc_handler,
> > > > -				IRQF_TRIGGER_RISING, client->name, data);
> > > > +				      IRQF_TRIGGER_RISING, dev_name(dev), data);  
> > > 
> > > Even though you'll change it again here, would have been nice to have
> > > the alignment fixed in the earlier patch then the code update here.  
> > 
> > I tried this, but due to the fact that the line has to be right-aligned to
> > column 80 we will still see a whitespace difference due to the length diff of
> > the name-related argument.  
> 
> You can split in the previous patch accordingly, so data comes to a new line.
> 
> ...
> 
> > > > +#include <linux/iio/iio.h>  
> > > 
> > > Why include this?  Can't see an IIO specific stuff in here.  
> > 
> > tried to remove it and
> > 
> >   CC [M]  mprls0025pa_i2c.o
> > mprls0025pa.h:89:63: error: 'IIO_DMA_MINALIGN' undeclared here (not in a function); did you mean 'ARCH_DMA_MINALIGN'?
> >    89 |         u8          buffer[MPR_MEASUREMENT_RD_SIZE] __aligned(IIO_DMA_MINALIGN);  
> 
> > I guess it makes more sense to move it to the .h file, where buffer[] is defined.  
> 
> Yes, C-code and especially headers should follow IWYI principle. The real user
> of that definition is _the header_ file, and not C in this case.

Absolutely - it is clear why this should be included from the header file.

Jonathan

> 
>