Received-SPF: pass (google.com: domain of linux-kernel+bounces-60445-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sat, 10 Feb 2024 16:08:50 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: Mike Looijmans <mike.looijmans@topic.nl>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
 devicetree@vger.kernel.org, linux-iio@vger.kernel.org, Lars-Peter Clausen
 <lars@metafoo.de>, Liam Beguin <liambeguin@gmail.com>, Liam Girdwood
 <lgirdwood@gmail.com>, Maksim Kiselev <bigunclemax@gmail.com>, Marcus
 Folkesson <marcus.folkesson@gmail.com>, Marius Cristea
 <marius.cristea@microchip.com>, Mark Brown <broonie@kernel.org>, Niklas
 Schnelle <schnelle@linux.ibm.com>, Okan Sahin <okan.sahin@analog.com>,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] iio: adc: ti-ads1298: Add driver
Message-ID: <20240210160850.7495b410@jic23-huawei>
In-Reply-To: <7d7ea4e4-fcae-4966-b194-e1d328751b6b@topic.nl>
References: <ZcDo6QvoE_e5s_f1@smile.fi.intel.com>
	<7d7ea4e4-fcae-4966-b194-e1d328751b6b@topic.nl>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit


> 
> >  
> >>> +	spinlock_t irq_busy_lock; /* Handshake between SPI and DRDY irqs */
> >>> +	int rdata_xfer_busy;
> >>> +
> >>> +	/* Temporary storage for demuxing data after SPI transfer */
> >>> +	u32 bounce_buffer[ADS1298_MAX_CHANNELS];
> >>> +
> >>> +	/* For synchronous SPI exchanges (read/write registers) */
> >>> +	u8 cmd_buffer[ADS1298_SPI_CMD_BUFFER_SIZE] __aligned(IIO_DMA_MINALIGN);
> >>> +
> >>> +	/* Buffer used for incoming SPI data */
> >>> +	u8 rx_buffer[ADS1298_SPI_RDATA_BUFFER_SIZE];  
> > Cacheline aligned?
> > I see the cmd_buffer, but shouldn't this be also aligned?  
> 
> I understood from Jonathan that that wasn't needed... "My" SPI 
> controller is rather dumb and doesn't even have DMA.
> 
> Will take a closer look though.
It should be fine.  The aim here is to ensure that nothing access data in
the same cacheline from the CPU side whilst DMA is ongoing (and unwanted
write backs of stale data can occur). As long at the handling isn't
very complex, a single marking of the first buffer used for DMA (and
being sure there is nothing other than other DMA buffers after it
the same cacheline (potentially multiple lines) is enough.

So the __aligend(IIO_DMA_MINALIGN) on cmd_buffer should work for allt hese
buffers.