| Message ID | 20181112142231.162425-1-bluecmd@google.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | qemu: aspeed_smc: Calculate checksum on normal DMA | expand |
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; > } > } > >
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, <div dir="ltr"><div dir="ltr"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">Thanks for the review.</div><div dir="ltr"><br>On Mon, Nov 12, 2018 at 3:39 PM Cédric Le Goater <<a href="mailto:clg@kaod.org">clg@kaod.org</a>> wrote:<br>> The datasheet is not very clear on the topic.<br><br>I won't argue there.<br><br>> and one could understand that under Normal DMA operation, there is<br>> no Checksum accumulation.<div><br></div><div>Indeed, that's what I thought as well. Then my question to myself was that</div><div>since that means that you can either 1) transfer data or 2) calculate the</div><div>checksum it seems like a pretty useless feature.</div><div><br></div><div>If you look at 8.6.2 DMA CheckSum Calculation Mode (for ast2400 at least) you</div><div>see the sentence:</div><div>> 5. FMC80[2] is set when only checksum calculation is necessary, no data movement</div><div><br></div><div>Which I take to read that you can execute the whole section and you will</div><div>get a checksum + data transfer, but if you set FMC80[2] then you get the</div><div>checksum, but no data movement.</div><div><br>> do you have a firmware doing so you could share ? I would like to check<br>> for the ast2500.</div><div><br></div><div>I have my own firmware here:</div><div><a href="https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S">https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S</a><br></div><div><br></div><div>Cheers,</div></div></div></div>
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, > <div dir="ltr">Ping. What's left to be done here?<div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 12, 2018 at 3:52 PM Christian Svensson <<a href="mailto:blue@cmd.nu" target="_blank">blue@cmd.nu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">Thanks for the review.</div><div dir="ltr"><br>On Mon, Nov 12, 2018 at 3:39 PM Cédric Le Goater <<a href="mailto:clg@kaod.org" target="_blank">clg@kaod.org</a>> wrote:<br>> The datasheet is not very clear on the topic.<br><br>I won't argue there.<br><br>> and one could understand that under Normal DMA operation, there is<br>> no Checksum accumulation.<div><br></div><div>Indeed, that's what I thought as well. Then my question to myself was that</div><div>since that means that you can either 1) transfer data or 2) calculate the</div><div>checksum it seems like a pretty useless feature.</div><div><br></div><div>If you look at 8.6.2 DMA CheckSum Calculation Mode (for ast2400 at least) you</div><div>see the sentence:</div><div>> 5. FMC80[2] is set when only checksum calculation is necessary, no data movement</div><div><br></div><div>Which I take to read that you can execute the whole section and you will</div><div>get a checksum + data transfer, but if you set FMC80[2] then you get the</div><div>checksum, but no data movement.</div><div><br>> do you have a firmware doing so you could share ? I would like to check<br>> for the ast2500.</div><div><br></div><div>I have my own firmware here:</div><div><a href="https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S" target="_blank">https://github.com/u-root/u-bmc/blob/07979a696df4351ec6dbb424ca80a5ebcc2eda1d/platform/ast2400/boot/main.S</a><br></div><div><br></div><div>Cheers,</div></div></div></div> </blockquote></div>
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, >
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; } }
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(+)