On 3/28/24 15:59, Yihang Li wrote: > > > On 2024/3/28 14:35, Christoph Hellwig wrote: >> On Tue, Mar 26, 2024 at 01:32:09PM +0000, John Garry wrote: >>>>> + u8 *p; >>>>> + >>>>> + size = ALIGN(size, ARCH_DMA_MINALIGN); >>> >>> >>> If this is a hisi_sas requirement, then why use ARCH_DMA_MINALIGN and not >>> 16B as minimum alignment? >>> >>> Or are we really talking about an arch requirement? >> >> One thing is that we should never allocate unaligned memory for >> anything DMA mapped, or data will be corrupted by non-coherent DMA. >> So ARCH_DMA_MINALIGN needs to be here. If specific hardware has >> further requirements we'll need to communicated it through a field >> or op vector. > > Got it. Looks like it's still going to be aligned to ARCH_DMA_MINALIGN. But I thought that the original issue was that some arch have ARCH_DMA_MINALIGN down to 8B but hisi driver needs at least 16 ? So in the end, you need something like: size = ALIGN(size, max(16, ARCH_DMA_MINALIGN)); no ? And define a macro for the "16" value to document it. Something like: #define SAS_SMP_REQ_ALIGN 16 Not sure about the name... Naming is hard :) > > Thanks, > Yihang > >> >> >> . >> -- Damien Le Moal Western Digital Research
