Re: [PATCH v3 07/16] aspeed/smc: fix dma moving incorrect data length issue

[Cédric Le Goater]

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 ?

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)

Thanks,

C.