[PATCH] qemu: aspeed_smc: Calculate checksum on normal DMA

[PATCH] qemu: aspeed_smc: Calculate checksum on normal DMA

[Christian Svensson]

This patch adds the missing checksum calculation on normal DMA transfer.
According to the datasheet this is how the SMC should behave.

Verified on AST1250 that the hardware matches the behaviour.

Signed-off-by: Christian Svensson <bluecmd@google.com>
---
 hw/ssi/aspeed_smc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 55ceffe100..0dc22e44cf 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -871,6 +871,7 @@ 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;
+        s->regs[R_DMA_CHECKSUM] += data;
     }
 }
 
-- 
2.19.1.930.g4563a0d9d0-goog

Re: [PATCH] qemu: aspeed_smc: Calculate checksum on normal DMA

[Cédric Le Goater]

Hello Christian,

On 11/12/18 3:22 PM, Christian Svensson wrote:
> This patch adds the missing checksum calculation on normal DMA transfer.
> According to the datasheet this is how the SMC should behave.

The datasheet is not very clear on the topic. 

Bit 2 of FMC80 says :

	CheckSum Calculation Only
	0: Normal DMA operation
	1: Checksum accumulation only
	This bit is valid only when the DMA direction = 0
 
and one could understand that under Normal DMA operation, there is 
no Checksum accumulation. 

Anyhow, I am ok for adding the change. 

> Verified on AST1250 that the hardware matches the behaviour.

Did you check under u-boot, did you add DMA support to the Linux driver ? 
do you have a firmware doing so you could share ? I would like to check
for the ast2500.

Thanks,

C. 


> Signed-off-by: Christian Svensson <bluecmd@google.com>
> ---
>  hw/ssi/aspeed_smc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 55ceffe100..0dc22e44cf 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -871,6 +871,7 @@ 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;
> +        s->regs[R_DMA_CHECKSUM] += data;
>      }
>  }
>  
> 

Re: [PATCH] qemu: aspeed_smc: Calculate checksum on normal DMA

[Christian Svensson]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

Hi,

Thanks for the review.

On Mon, Nov 12, 2018 at 3:39 PM Cédric Le Goater <clg@kaod.org> wrote:
> The datasheet is not very clear on the topic.

I won't argue there.

> and one could understand that under Normal DMA operation, there is
> no Checksum accumulation.

Indeed, that's what I thought as well. Then my question to myself was that
since that means that you can either 1) transfer data or 2) calculate the
checksum it seems like a pretty useless feature.

If you look at 8.6.2 DMA CheckSum Calculation Mode (for ast2400 at least)
you
see the sentence:
> 5. FMC80[2] is set when only checksum calculation is necessary, no data
movement

Which I take to read that you can execute the whole section and you will
get a checksum + data transfer, but if you set FMC80[2] then you get the
checksum, but no data movement.

> do you have a firmware doing so you could share ? I would like to check
> for the ast2500.

I have my own firmware here:
https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S

Cheers,

[-- Attachment #2: Type: text/html, Size: 1615 bytes --]

Re: [PATCH] qemu: aspeed_smc: Calculate checksum on normal DMA

[Christian Svensson]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

Ping. What's left to be done here?


On Mon, Nov 12, 2018 at 3:52 PM Christian Svensson <blue@cmd.nu> wrote:

> Hi,
>
> Thanks for the review.
>
> On Mon, Nov 12, 2018 at 3:39 PM Cédric Le Goater <clg@kaod.org> wrote:
> > The datasheet is not very clear on the topic.
>
> I won't argue there.
>
> > and one could understand that under Normal DMA operation, there is
> > no Checksum accumulation.
>
> Indeed, that's what I thought as well. Then my question to myself was that
> since that means that you can either 1) transfer data or 2) calculate the
> checksum it seems like a pretty useless feature.
>
> If you look at 8.6.2 DMA CheckSum Calculation Mode (for ast2400 at least)
> you
> see the sentence:
> > 5. FMC80[2] is set when only checksum calculation is necessary, no data
> movement
>
> Which I take to read that you can execute the whole section and you will
> get a checksum + data transfer, but if you set FMC80[2] then you get the
> checksum, but no data movement.
>
> > do you have a firmware doing so you could share ? I would like to check
> > for the ast2500.
>
> I have my own firmware here:
>
> https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S
>
> Cheers,
>

[-- Attachment #2: Type: text/html, Size: 2043 bytes --]

Re: [PATCH] qemu: aspeed_smc: Calculate checksum on normal DMA

[Cédric Le Goater]

On 1/11/19 1:30 PM, Christian Svensson wrote:
> Ping. What's left to be done here?

Nothing. I have merged the update in my patch.

Thanks,

C.

> On Mon, Nov 12, 2018 at 3:52 PM Christian Svensson <blue@cmd.nu <mailto:blue@cmd.nu>> wrote:
> 
>     Hi,
> 
>     Thanks for the review.
> 
>     On Mon, Nov 12, 2018 at 3:39 PM Cédric Le Goater <clg@kaod.org <mailto:clg@kaod.org>> wrote:
>     > The datasheet is not very clear on the topic.
> 
>     I won't argue there.
> 
>     > and one could understand that under Normal DMA operation, there is
>     > no Checksum accumulation.
> 
>     Indeed, that's what I thought as well. Then my question to myself was that
>     since that means that you can either 1) transfer data or 2) calculate the
>     checksum it seems like a pretty useless feature.
> 
>     If you look at 8.6.2 DMA CheckSum Calculation Mode (for ast2400 at least) you
>     see the sentence:
>     > 5. FMC80[2] is set when only checksum calculation is necessary, no data movement
> 
>     Which I take to read that you can execute the whole section and you will
>     get a checksum + data transfer, but if you set FMC80[2] then you get the
>     checksum, but no data movement.
> 
>     > do you have a firmware doing so you could share ? I would like to check
>     > for the ast2500.
> 
>     I have my own firmware here:
>     https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S
> 
>     Cheers,
>