Received-SPF: pass (google.com: domain of linux-kernel+bounces-12141-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 27 Dec 2023 18:37:42 +0200
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Petre Rodan <petre.rodan@subdimension.ro>
Cc: Jonathan Cameron <jic23@kernel.org>, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org, Andreas Klinger <ak@it-klinger.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Angel Iglesias <ang.iglesiasg@gmail.com>,
	Matti Vaittinen <mazziesaccount@gmail.com>
Subject: Re: [PATCH v2 08/10] iio: pressure: mprls0025pa.c refactor
Message-ID: <ZYxS1o2lSlaJ6dG4@smile.fi.intel.com>
References: <20231224143500.10940-1-petre.rodan@subdimension.ro>
 <20231224143500.10940-9-petre.rodan@subdimension.ro>
 <20231226164922.6d7132c1@jic23-huawei>
 <ZYw1wbvYQqqCr5C2@sunspire>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZYw1wbvYQqqCr5C2@sunspire>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

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.


-- 
With Best Regards,
Andy Shevchenko