openbmc / qemu

Official QEMU mirror
Other
20 stars 22 forks source link

aspeed_smc: Calculate checksum on normal DMA #15

Closed bluecmd closed 6 years ago

bluecmd commented 6 years ago

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

amboar commented 6 years ago

Hi @bluecmd - have you sent this upstream? Ideally all of our patches would be at least on the upstream list(s), similar to how we handle kernel development. If you Cc the OpenBMC mailing list we can pick it up from there, as well as follow the conversation. Please also Cc @legoater and myself directly.

bluecmd commented 6 years ago

I have no idea how to submit kernel patches like these. If you have any documentation or guide I can follow then I can do that :-)

amboar commented 6 years ago

There's a description here of how to send qemu patches: https://github.com/qemu/qemu/blob/master/README#L52

legoater commented 6 years ago

Thanks, this is correct indeed. I recently did some changes in my patchset
and you should look at aspeed_smc_dma_checksum() :

https://github.com/openbmc/qemu/commit/034ee5d08ba73a782a180df609bf56ff445ab9e0#diff-1c5ed2b17fd2d1367fc3dd9db76216eeR722

I prefer to work with email patches, as this is how most open-source projects accept changes and do reviews. If you send patches to the QEMU mailing list, make sure they apply to the mainline tree. The DMA support is not merged yet.

bluecmd commented 6 years ago

@legoater I fully understand, no hard feelings. My goal was to raise awareness of this patch and have it merged to mainline one way or another - worst case I figured people would just redirect me to where to post it. Mission accomplished :-).

you should look at aspeed_smc_dma_checksum() :

I don't fully understand what you mean here. I did look at that function, that's how I figured what was missing from the _rw one. The patch you're referring to is on the same branch as this fix.

Btw, if your statement from your original post ("I will fix it directly in my branch if you don't mind.") is still on the table that's 100% OK.

legoater commented 6 years ago

please send you patch by email on the openbmc mailing list. I will comment there. Thanks,

bluecmd commented 6 years ago

Patch sent