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.
Currently, only supports dma length 4 bytes aligned.
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/ssi/aspeed_smc.c | 52 ++++++++++++++++++++++++++++++++-----
include/hw/ssi/aspeed_smc.h | 1 +
2 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 8a8d77b480..71abc7a2d8 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -178,13 +178,17 @@
* DMA flash addresses should be 4 bytes aligned and the valid address
* range is 0x20000000 - 0x2FFFFFFF.
*
- * DMA length is from 4 bytes to 32MB
+ * DMA length is from 4 bytes to 32MB (AST2500)
* 0: 4 bytes
* 0x7FFFFF: 32M bytes
+ *
+ * DMA length is from 1 byte to 32MB (AST2600, AST10x0)
+ * 0: 1 byte
+ * 0x1FFFFFF: 32M bytes
*/
#define DMA_DRAM_ADDR(asc, val) ((val) & (asc)->dma_dram_mask)
#define DMA_FLASH_ADDR(asc, val) ((val) & (asc)->dma_flash_mask)
-#define DMA_LENGTH(val) ((val) & 0x01FFFFFC)
+#define DMA_LENGTH(val) ((val) & 0x01FFFFFF)
/* Flash opcodes. */
#define SPI_OP_READ 0x03 /* Read data bytes (low frequency) */
@@ -843,6 +847,24 @@ static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
}
}
+static uint32_t aspeed_smc_dma_len(AspeedSMCState *s)
+{
+ AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
+ uint32_t dma_len;
+ uint32_t extra;
+
+ dma_len = s->regs[R_DMA_LEN] + asc->dma_start_length;
+
+ /* dma length 4 bytes aligned */
+ extra = dma_len % 4;
+
+ if (extra != 0) {
+ dma_len += 4 - extra;
+ }
+
+ return dma_len;
+}
+
/*
* Accumulate the result of the reads to provide a checksum that will
* be used to validate the read timing settings.
@@ -850,6 +872,7 @@ static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
static void aspeed_smc_dma_checksum(AspeedSMCState *s)
{
MemTxResult result;
+ uint32_t dma_len;
uint32_t data;
if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
@@ -861,7 +884,9 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
aspeed_smc_dma_calibration(s);
}
- while (s->regs[R_DMA_LEN]) {
+ dma_len = aspeed_smc_dma_len(s);
+
+ while (dma_len) {
data = address_space_ldl_le(&s->flash_as, s->regs[R_DMA_FLASH_ADDR],
MEMTXATTRS_UNSPECIFIED, &result);
if (result != MEMTX_OK) {
@@ -877,7 +902,8 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
*/
s->regs[R_DMA_CHECKSUM] += data;
s->regs[R_DMA_FLASH_ADDR] += 4;
- s->regs[R_DMA_LEN] -= 4;
+ dma_len -= 4;
+ s->regs[R_DMA_LEN] = dma_len;
}
if (s->inject_failure && aspeed_smc_inject_read_failure(s)) {
@@ -889,14 +915,17 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
static void aspeed_smc_dma_rw(AspeedSMCState *s)
{
MemTxResult result;
+ uint32_t dma_len;
uint32_t data;
+ dma_len = aspeed_smc_dma_len(s);
+
trace_aspeed_smc_dma_rw(s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE ?
"write" : "read",
s->regs[R_DMA_FLASH_ADDR],
s->regs[R_DMA_DRAM_ADDR],
- s->regs[R_DMA_LEN]);
- while (s->regs[R_DMA_LEN]) {
+ dma_len);
+ while (dma_len) {
if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
data = address_space_ldl_le(&s->dram_as, s->regs[R_DMA_DRAM_ADDR],
MEMTXATTRS_UNSPECIFIED, &result);
@@ -937,7 +966,8 @@ static void aspeed_smc_dma_rw(AspeedSMCState *s)
*/
s->regs[R_DMA_FLASH_ADDR] += 4;
s->regs[R_DMA_DRAM_ADDR] += 4;
- s->regs[R_DMA_LEN] -= 4;
+ dma_len -= 4;
+ s->regs[R_DMA_LEN] = dma_len;
s->regs[R_DMA_CHECKSUM] += data;
}
}
@@ -1381,6 +1411,7 @@ static void aspeed_2400_fmc_class_init(ObjectClass *klass, void *data)
asc->features = ASPEED_SMC_FEATURE_DMA;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x1FFFFFFC;
+ asc->dma_start_length = 4;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_smc_segment_to_reg;
asc->reg_to_segment = aspeed_smc_reg_to_segment;
@@ -1464,6 +1495,7 @@ static void aspeed_2500_fmc_class_init(ObjectClass *klass, void *data)
asc->features = ASPEED_SMC_FEATURE_DMA;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x3FFFFFFC;
+ asc->dma_start_length = 4;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_smc_segment_to_reg;
asc->reg_to_segment = aspeed_smc_reg_to_segment;
@@ -1620,6 +1652,7 @@ static void aspeed_2600_fmc_class_init(ObjectClass *klass, void *data)
ASPEED_SMC_FEATURE_WDT_CONTROL;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x3FFFFFFC;
+ asc->dma_start_length = 1;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
@@ -1658,6 +1691,7 @@ static void aspeed_2600_spi1_class_init(ObjectClass *klass, void *data)
ASPEED_SMC_FEATURE_DMA_GRANT;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x3FFFFFFC;
+ asc->dma_start_length = 1;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
@@ -1697,6 +1731,7 @@ static void aspeed_2600_spi2_class_init(ObjectClass *klass, void *data)
ASPEED_SMC_FEATURE_DMA_GRANT;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x3FFFFFFC;
+ asc->dma_start_length = 1;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
@@ -1778,6 +1813,7 @@ static void aspeed_1030_fmc_class_init(ObjectClass *klass, void *data)
asc->features = ASPEED_SMC_FEATURE_DMA;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x000BFFFC;
+ asc->dma_start_length = 1;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_1030_smc_segment_to_reg;
asc->reg_to_segment = aspeed_1030_smc_reg_to_segment;
@@ -1815,6 +1851,7 @@ static void aspeed_1030_spi1_class_init(ObjectClass *klass, void *data)
asc->features = ASPEED_SMC_FEATURE_DMA;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x000BFFFC;
+ asc->dma_start_length = 1;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
@@ -1851,6 +1888,7 @@ static void aspeed_1030_spi2_class_init(ObjectClass *klass, void *data)
asc->features = ASPEED_SMC_FEATURE_DMA;
asc->dma_flash_mask = 0x0FFFFFFC;
asc->dma_dram_mask = 0x000BFFFC;
+ asc->dma_start_length = 1;
asc->nregs = ASPEED_SMC_R_MAX;
asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 8e1dda556b..f359ed22cc 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -106,6 +106,7 @@ struct AspeedSMCClass {
uint32_t features;
hwaddr dma_flash_mask;
hwaddr dma_dram_mask;
+ uint32_t dma_start_length;
uint32_t nregs;
uint32_t (*segment_to_reg)(const AspeedSMCState *s,
const AspeedSegments *seg);
--
2.25.1On 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 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.
this looks like a third topic. So the minimum value R_DMA_LEN should
have on the AST2600 SoC and above is '3'. I would opt to replace the
DMA_LENGTH macro with a dma_length_sanitize() helper to fix the software
input of R_DMA_LEN.
Thanks,
C.
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
> hw/ssi/aspeed_smc.c | 52 ++++++++++++++++++++++++++++++++-----
> include/hw/ssi/aspeed_smc.h | 1 +
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 8a8d77b480..71abc7a2d8 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -178,13 +178,17 @@
> * DMA flash addresses should be 4 bytes aligned and the valid address
> * range is 0x20000000 - 0x2FFFFFFF.
> *
> - * DMA length is from 4 bytes to 32MB
> + * DMA length is from 4 bytes to 32MB (AST2500)
> * 0: 4 bytes
> * 0x7FFFFF: 32M bytes
> + *
> + * DMA length is from 1 byte to 32MB (AST2600, AST10x0)
> + * 0: 1 byte
> + * 0x1FFFFFF: 32M bytes
> */
> #define DMA_DRAM_ADDR(asc, val) ((val) & (asc)->dma_dram_mask)
> #define DMA_FLASH_ADDR(asc, val) ((val) & (asc)->dma_flash_mask)
> -#define DMA_LENGTH(val) ((val) & 0x01FFFFFC)
> +#define DMA_LENGTH(val) ((val) & 0x01FFFFFF)
>
> /* Flash opcodes. */
> #define SPI_OP_READ 0x03 /* Read data bytes (low frequency) */
> @@ -843,6 +847,24 @@ static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
> }
> }
>
> +static uint32_t aspeed_smc_dma_len(AspeedSMCState *s)
> +{
> + AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
> + uint32_t dma_len;
> + uint32_t extra;
> +
> + dma_len = s->regs[R_DMA_LEN] + asc->dma_start_length;
> +
> + /* dma length 4 bytes aligned */
> + extra = dma_len % 4;
> +
> + if (extra != 0) {
> + dma_len += 4 - extra;
> + }
> +
> + return dma_len;
> +}
> +
> /*
> * Accumulate the result of the reads to provide a checksum that will
> * be used to validate the read timing settings.
> @@ -850,6 +872,7 @@ static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
> static void aspeed_smc_dma_checksum(AspeedSMCState *s)
> {
> MemTxResult result;
> + uint32_t dma_len;
> uint32_t data;
>
> if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
> @@ -861,7 +884,9 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
> aspeed_smc_dma_calibration(s);
> }
>
> - while (s->regs[R_DMA_LEN]) {
> + dma_len = aspeed_smc_dma_len(s);
> +
> + while (dma_len) {
> data = address_space_ldl_le(&s->flash_as, s->regs[R_DMA_FLASH_ADDR],
> MEMTXATTRS_UNSPECIFIED, &result);
> if (result != MEMTX_OK) {
> @@ -877,7 +902,8 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
> */
> s->regs[R_DMA_CHECKSUM] += data;
> s->regs[R_DMA_FLASH_ADDR] += 4;
> - s->regs[R_DMA_LEN] -= 4;
> + dma_len -= 4;
> + s->regs[R_DMA_LEN] = dma_len;
> }
>
> if (s->inject_failure && aspeed_smc_inject_read_failure(s)) {
> @@ -889,14 +915,17 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
> static void aspeed_smc_dma_rw(AspeedSMCState *s)
> {
> MemTxResult result;
> + uint32_t dma_len;
> uint32_t data;
>
> + dma_len = aspeed_smc_dma_len(s);
> +
> trace_aspeed_smc_dma_rw(s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE ?
> "write" : "read",
> s->regs[R_DMA_FLASH_ADDR],
> s->regs[R_DMA_DRAM_ADDR],
> - s->regs[R_DMA_LEN]);
> - while (s->regs[R_DMA_LEN]) {
> + dma_len);
> + while (dma_len) {
> if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
> data = address_space_ldl_le(&s->dram_as, s->regs[R_DMA_DRAM_ADDR],
> MEMTXATTRS_UNSPECIFIED, &result);
> @@ -937,7 +966,8 @@ static void aspeed_smc_dma_rw(AspeedSMCState *s)
> */
> s->regs[R_DMA_FLASH_ADDR] += 4;
> s->regs[R_DMA_DRAM_ADDR] += 4;
> - s->regs[R_DMA_LEN] -= 4;
> + dma_len -= 4;
> + s->regs[R_DMA_LEN] = dma_len;
> s->regs[R_DMA_CHECKSUM] += data;
> }
> }
> @@ -1381,6 +1411,7 @@ static void aspeed_2400_fmc_class_init(ObjectClass *klass, void *data)
> asc->features = ASPEED_SMC_FEATURE_DMA;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x1FFFFFFC;
> + asc->dma_start_length = 4;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_smc_reg_to_segment;
> @@ -1464,6 +1495,7 @@ static void aspeed_2500_fmc_class_init(ObjectClass *klass, void *data)
> asc->features = ASPEED_SMC_FEATURE_DMA;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x3FFFFFFC;
> + asc->dma_start_length = 4;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_smc_reg_to_segment;
> @@ -1620,6 +1652,7 @@ static void aspeed_2600_fmc_class_init(ObjectClass *klass, void *data)
> ASPEED_SMC_FEATURE_WDT_CONTROL;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x3FFFFFFC;
> + asc->dma_start_length = 1;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
> @@ -1658,6 +1691,7 @@ static void aspeed_2600_spi1_class_init(ObjectClass *klass, void *data)
> ASPEED_SMC_FEATURE_DMA_GRANT;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x3FFFFFFC;
> + asc->dma_start_length = 1;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
> @@ -1697,6 +1731,7 @@ static void aspeed_2600_spi2_class_init(ObjectClass *klass, void *data)
> ASPEED_SMC_FEATURE_DMA_GRANT;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x3FFFFFFC;
> + asc->dma_start_length = 1;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
> @@ -1778,6 +1813,7 @@ static void aspeed_1030_fmc_class_init(ObjectClass *klass, void *data)
> asc->features = ASPEED_SMC_FEATURE_DMA;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x000BFFFC;
> + asc->dma_start_length = 1;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_1030_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_1030_smc_reg_to_segment;
> @@ -1815,6 +1851,7 @@ static void aspeed_1030_spi1_class_init(ObjectClass *klass, void *data)
> asc->features = ASPEED_SMC_FEATURE_DMA;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x000BFFFC;
> + asc->dma_start_length = 1;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
> @@ -1851,6 +1888,7 @@ static void aspeed_1030_spi2_class_init(ObjectClass *klass, void *data)
> asc->features = ASPEED_SMC_FEATURE_DMA;
> asc->dma_flash_mask = 0x0FFFFFFC;
> asc->dma_dram_mask = 0x000BFFFC;
> + asc->dma_start_length = 1;
> asc->nregs = ASPEED_SMC_R_MAX;
> asc->segment_to_reg = aspeed_2600_smc_segment_to_reg;
> asc->reg_to_segment = aspeed_2600_smc_reg_to_segment;
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index 8e1dda556b..f359ed22cc 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -106,6 +106,7 @@ struct AspeedSMCClass {
> uint32_t features;
> hwaddr dma_flash_mask;
> hwaddr dma_dram_mask;
> + uint32_t dma_start_length;
> uint32_t nregs;
> uint32_t (*segment_to_reg)(const AspeedSMCState *s,
> const AspeedSegments *seg);
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.
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.
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.
>
>
Hi Cedric,
Sorry reply you late.
> 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 ?
> >>
After I discussed with designers, my explanation as below.
AST2500 datasheet description:
FMC8C: DMA Length Register
31:25 Reserved(0)
24:2 RW DMA_LENGTH
From 4bytes to 32MB
0: 4bytes
0x7FFFFF: 32MB
1:0 Reserved
If users set this register 0, SPI DMA controller would move 4 bytes data.
If users set this register bits[24:2] 0x7FFFFF, SPI DMC controller would move 32MB data because this register includes reserved bits [1:0].
Ex: bits 24:2 --> 0x7fffff
bits 1:0 --> 0 Reserved
bits[24:0] is (0x1FFFFFC + start length 4) --> 2000000 --> 32MB
111 1111 1111 1111 1111 1111 00
LENGTH[24:2] 7 F F F F F Reserved [1:0]
That was why AST2500 DMA length should 4 bytes alignment and you do not see handling length units because it includes reserved bits 1 and 0.
If user want to move 3 bytes data, our firmware set this register 4 ensure it 4bytes alignment and the value of register as following
bits[2:0] = "b100" Finally, SPI DMA controller move 8 bytes data(4bytes input count + start length 4bytes)
And that was why it set DMA_LENGTH 0x01FFFFFC
https://github.com/qemu/qemu/blob/master/hw/ssi/aspeed_smc.c#L187
#define DMA_LENGTH(val) ((val) & 0x01FFFFFC)
I want to change it to 0x01FFFFFF to support all ASPEED SOCs because AST2600, AST2700 and AST1030 use this register bit 0 and 1 whose unit is 1byte rather than 4 bytes.
> > https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/l
> > ib/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/a
> > rch/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 for your suggestion and will fix it.
> 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/a
> > rch/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.
> >
> >
© 2016 - 2026 Red Hat, Inc.