On 07/13/2018 12:52 PM, Masahiro Yamada wrote: > 2018-07-13 19:35 GMT+09:00 Marek Vasut <ma...@denx.de>: >> On 07/13/2018 12:22 PM, Masahiro Yamada wrote: >>> 2018-07-13 19:18 GMT+09:00 Marek Vasut <ma...@denx.de>: >>>> On 07/13/2018 12:09 PM, Masahiro Yamada wrote: >>>>> Hi Marek >>>>> >>>>> 2018-07-13 17:56 GMT+09:00 Marek Vasut <ma...@denx.de>: >>>>>> On 07/13/2018 10:23 AM, Masahiro Yamada wrote: >>>>>>> Hi Marek, >>>>>>> >>>>>>> 2018-07-13 16:59 GMT+09:00 Marek Vasut <ma...@denx.de>: >>>>>>>> On 07/13/2018 07:13 AM, Masahiro Yamada wrote: >>>>>>>>> 2018-07-12 21:51 GMT+09:00 Marek Vasut <ma...@denx.de>: >>>>>>>>>> On 06/20/2018 09:14 AM, Masahiro Yamada wrote: >>>>>>>>>>> Hi Marek, >>>>>>>>>> >>>>>>>>>> Hi! >>>>>>>>>> >>>>>>>>>>> 2018-06-20 13:43 GMT+09:00 Marek Vasut <ma...@denx.de>: >>>>>>>>>>>> On 06/19/2018 08:39 AM, Masahiro Yamada wrote: >>>>>>>>>>>>> Hi Marek, >>>>>>>>>>>> >>>>>>>>>>>> Hi, >>>>>>>>>>>> >>>>>>>>>>>>> 2018-06-08 5:17 GMT+09:00 Marek Vasut <ma...@denx.de>: >>>>>>>>>>>>>> Replace the ad-hoc DMA cache management functions with common >>>>>>>>>>>>>> bouncebuf, >>>>>>>>>>>>>> since those functions are not handling cases where unaligned >>>>>>>>>>>>>> buffer is >>>>>>>>>>>>>> passed in, >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Were you hit by a problem, >>>>>>>>>>>>> or just-in-case replacement? >>>>>>>>>>>> >>>>>>>>>>>> Yes, UBI triggers unaligned cache operations on my system >>>>>>>>>>>> (SoCFPGA). >>>>>>>>>>>>> I thought I took care of the buffer alignment. >>>>>>>>>>>>> >>>>>>>>>>>>> The bounce buffer is allocated by kmalloc(): >>>>>>>>>>>>> https://github.com/u-boot/u-boot/blob/v2018.05/drivers/mtd/nand/denali.c#L1348 >>>>>>>>>>>>> >>>>>>>>>>>>> According to the lib/linux_compat.c implementation, >>>>>>>>>>>>> it returns memory aligned with ARCH_DMA_MINALIGN. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> If the buffer is passed from the upper MTD layer, >>>>>>>>>>>>> the NAND core also makes sure the enough alignment: >>>>>>>>>>>>> https://github.com/u-boot/u-boot/blob/v2018.05/drivers/mtd/nand/denali.c#L1273 >>>>>>>>>>>>> >>>>>>>>>>>>> This is how this driver works in Linux. >>>>>>>>>>>>> >>>>>>>>>>>>> I'd rather want to keep the current code >>>>>>>>>>>>> unless this is a real problem, >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> One possible clean-up is to move dma_(un)map_single to a common >>>>>>>>>>>>> place. >>>>>>>>>>>> Is there any chance you can try UBI on the denali nand on uniphier >>>>>>>>>>>> ? :) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> I tested the driver only for raw block devices. >>>>>>>>>>> >>>>>>>>>>> OK, I will test UBI on my platform. >>>>>>>>>>> >>>>>>>>>>> BTW, do you see the problem only in U-Boot? >>>>>>>>>>> Is the denali driver in Linux working fine? >>>>>>>>>> >>>>>>>>>> Bump on this one ? >>>>>>>>>> >>>>>>>>> >>>>>>>>> Sorry for delay. >>>>>>>>> >>>>>>>>> >>>>>>>>> UBI is working for me without your patch. >>>>>>>>> >>>>>>>>> Not sure what is the difference. >>>>>>>>> >>>>>>>>> I will dig into it a little more, though. >>>>>>>> >>>>>>>> Verify that you're not seeing any unaligned cache flushes. I do. >>>>>>> >>>>>>> >>>>>>> Yeah, I am testing it now, >>>>>>> but I never see 'Misaligned operation at range' when using UBI. >>>>>>> >>>>>>> However, I found a simple way to trigger the warning >>>>>>> in raw device access. >>>>>>> >>>>>>> >>>>>>> >>>>>>> => nand read 81000010 0 1000 >>>>>>> >>>>>>> NAND read: device 0 offset 0x0, size 0x1000 >>>>>>> CACHE: Misaligned operation at range [81000010, 81001010] >>>>>>> CACHE: Misaligned operation at range [81000010, 81001010] >>>>>>> CACHE: Misaligned operation at range [81000010, 81001010] >>>>>>> CACHE: Misaligned operation at range [81000010, 81001010] >>>>>>> 4096 bytes read: OK >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> I can fix it with one liner. >>>>>>> >>>>>>> >>>>>>> >>>>>>> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c >>>>>>> index 6266c8a..f391727 100644 >>>>>>> --- a/drivers/mtd/nand/denali.c >>>>>>> +++ b/drivers/mtd/nand/denali.c >>>>>>> @@ -1270,7 +1270,7 @@ int denali_init(struct denali_nand_info *denali) >>>>>>> denali->dma_avail = 1; >>>>>>> >>>>>>> if (denali->dma_avail) { >>>>>>> - chip->buf_align = 16; >>>>>>> + chip->buf_align = ARCH_DMA_MINALIGN; >>>>>>> if (denali->caps & DENALI_CAP_DMA_64BIT) >>>>>>> denali->setup_dma = denali_setup_dma64; >>>>>>> else >>>>>>> >>>>>>> >>>>>>> I guess this will work for you too. >>>>>> >>>>>> Doesn't that only apply if DMA is available ? >>>>> >>>>> Of course. >>>>> If you use PIO instead of DMA, >>>>> you do not need to perform cache operation, right? >>>>> >>>>> >>>>> >>>>>> And anyway, I'd rather use common U-Boot code than have a custom >>>>>> implementation in a driver which we need to maintain and fix separately. >>>>> >>>>> >>>>> bounce_buffer_{start,stop} does >>>>> malloc() and free() every time. >>>>> This is not efficient. >>>>> >>>>> >>>>> struct nand_chip already contains page buffers, >>>>> which guarantee alignment for DMA. >>>>> >>>>> https://github.com/u-boot/u-boot/blob/v2018.07/include/linux/mtd/rawnand.h#L640 >>>>> >>>>> >>>>> We can rely on the NAND framework >>>>> for handling DMA-capable alignment. >>>> >>>> Clearly that doesn't work, otherwise I won't need this bounce buffer patch >>>> ? >>>> >>> >>> >>> >>> >>> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c >>> index 6266c8a..f391727 100644 >>> --- a/drivers/mtd/nand/denali.c >>> +++ b/drivers/mtd/nand/denali.c >>> @@ -1270,7 +1270,7 @@ int denali_init(struct denali_nand_info *denali) >>> denali->dma_avail = 1; >>> >>> if (denali->dma_avail) { >>> - chip->buf_align = 16; >>> + chip->buf_align = ARCH_DMA_MINALIGN; >>> if (denali->caps & DENALI_CAP_DMA_64BIT) >>> denali->setup_dma = denali_setup_dma64; >>> else >>> >>> >>> Did you try this? >>> I do not see unaligned cache operation. >> >> Nope, I'll have to assemble the hardware. >> But this only works if dma_avail, right ? >> > > > So, what are you worried about?
That the denali NAND is broken in mainline on socfpga. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot