On 4/30/24 10:51, Jamin Lin wrote:
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

yes. I don't see any 1bit / 4bytes conversion when setting the DMA_LEN
register. Am I mistaking ? That's not what the specs says.

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.

ok. For that, I think you could replace aspeed_smc_dma_len() with

   return QEMU_ALIGN_UP(s->regs[R_DMA_LEN] + asc->dma_start_length, 4);

Thanks,

C.




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.




Reply via email to