Hi Cedric, > On 4/19/24 15:41, Cédric Le Goater wrote: > > On 4/16/24 11:18, Jamin Lin wrote: > >> DMA length is from 1 byte to 32MB for AST2600 and AST10x0 and DMA > >> length is from 4 bytes to 32MB for AST2500. > >> > >> In other words, if "R_DMA_LEN" is 0, it should move at least 1 byte > >> data for AST2600 and AST10x0 and 4 bytes data for AST2500. > >>> To support all ASPEED SOCs, adds dma_start_length parameter to store > >> the start length, add helper routines function to compute the dma > >> length and update DMA_LENGTH mask to "1FFFFFF" to fix dma moving > >> incorrect data length issue. > > > > OK. There are two problems to address, the "zero" length transfer and > > the DMA length unit, which is missing today. Newer SoC use a 1 bit / > > byte and older ones, AST2400 and AST2500, use 1 bit / 4 bytes. > > > > We can introduce a AspeedSMCClass::dma_len_unit and rework the loop to : > > > > do { > > > > .... > > > > if (s->regs[R_DMA_LEN]) { > > s->regs[R_DMA_LEN] -= 4 / asc->dma_len_unit; > > } > > } while (s->regs[R_DMA_LEN]); > > > > It should fix the current implementation. > > > I checked what FW is doing on a QEMU ast2500-evb machine : > > U-Boot 2019.04-v00.04.12 (Sep 29 2022 - 10:40:37 +0000) > ... > > Loading Kernel Image ... aspeed_smc_write @0x88 size 4: > 0x80001000 > aspeed_smc_write @0x84 size 4: 0x20100130 > aspeed_smc_write @0x8c size 4: 0x3c6770 > aspeed_smc_write @0x80 size 4: 0x1 > aspeed_smc_dma_rw read flash:@0x00100130 dram:@0x1000 > size:0x003c6774 > aspeed_smc_read @0x8 size 4: 0x800 > aspeed_smc_write @0x80 size 4: 0x0 > OK > Loading Ramdisk to 8fef2000, end 8ffff604 ... aspeed_smc_write > @0x88 size 4: 0x8fef2000 > aspeed_smc_write @0x84 size 4: 0x204cdde0 > aspeed_smc_write @0x8c size 4: 0x10d604 > aspeed_smc_write @0x80 size 4: 0x1 > aspeed_smc_dma_rw read flash:@0x004cdde0 dram:@0xfef2000 > size:0x0010d608 > aspeed_smc_read @0x8 size 4: 0x800 > aspeed_smc_write @0x80 size 4: 0x0 > OK > Loading Device Tree to 8fee7000, end 8fef135e ... aspeed_smc_write > @0x88 size 4: 0x8fee7000 > aspeed_smc_write @0x84 size 4: 0x204c69b4 > aspeed_smc_write @0x8c size 4: 0x7360 > aspeed_smc_write @0x80 size 4: 0x1 > aspeed_smc_dma_rw read flash:@0x004c69b4 dram:@0xfee7000 > size:0x00007364 > aspeed_smc_read @0x8 size 4: 0x800 > aspeed_smc_write @0x80 size 4: 0x0 > OK > > Starting kernel ... > > It seems that the R_DMA_LEN register is set by FW without taking into account > the length unit ( bit / 4 bytes). Would you know why ? > https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/lib/string.c#L559 This line make user input data length 4 bytes alignment. https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/arch/arm/mach-aspeed/ast2500/utils.S#L35 This line set the value of count parameter to AST_FMC_DNA_LENGTH. AST_FMC_DMA_LENGTH is 4 bytes alignment value. Example: input 4 ((4+3)/4) * 4 --> (7/4) *4 ---> 4 If AST_FMC_DMA_LENGTH is 0, it means it should move 4 bytes data and AST_FMC_DMA_LENGTH do not need to be divided by 4.
> If I change the model to match 1 bit / 4 bytes unit of the R_DMA_LEN register. > Linux fails to boot. I didn't dig further and this is something we need to > understand before committing. > > > I don't think this is necessary to add a Fixes tag because the problem > > has been there for ages and no one reported it. Probably because the > > only place DMA transfers are used is in U-Boot and transfers have a > > non-zero length. > > > >> Currently, only supports dma length 4 bytes aligned. > > Is this 4 bytes alignment new for the AST2700 or is this something you added > because the mask of DMA_LENGTH is now relaxed to match all addresses ? > > #define DMA_LENGTH(val) ((val) & 0x01FFFFFF) AST2700, AST2600 and AST1030 is from 1 byte to 1FFFFFF, so I change this Micro to fix data lost. https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2023.10/arch/arm/mach-aspeed/ast2700/spi.c#L186 Please see this line, it decrease dma_len 1 byte first then, set to DMA_LEN register because DMA_LEN is 0 which means should move 1 byte data if DMA enables for AST2600, AST1030 and AST2700. > > Thanks, > > C. Only AST2500 need 4 bytes alignment for DMA Length. However, according to the design of sapped_smc_dma_rw function, it utilizes address_space_stl_le API and it only works data 4 bytes alignment. https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L889 For example, If users want to move 0x101 data_length, after 0x100 data has been moved and remains 1 byte data need to be moved. Please see this line program, https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L940 ``` s->regs[R_DMA_LEN] -= 4; ``` The value of s->regs[R_DMA_LEN] becomes 0xffffffffXX because it is uint32_t data type and this while loop run in the unexpected behavior, https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L864 That was, why I set 4bytes alignment for all models.