MarvellEmbeddedProcessors / mv-ddr-marvell

18 stars 23 forks source link

Please publish latest version of mv-ddr-marvell #27

Closed pali closed 3 years ago

pali commented 3 years ago

Marvell SDK-10.3.5.0-PR1 contains new version of mv-ddr-marvell with new patches which are not available in the publically latest branch mv-ddr-devel.

Please publish latest version of mv-ddr-marvell from Marvell SDK which is currently available only under NDA.

pali commented 3 years ago

@kostapr: Could you look at mv-ddr-marvell too?

kostapr commented 3 years ago

@pali yes, I expected this dependency. Hopefully @haklai will approve it.

kostapr commented 3 years ago

@pali FYI, I got an approval and planning to push the latest changes shortly.

kostapr commented 3 years ago

@pali, @heaterC I have created a new "master" branch containing the latest code from our development sources with patches posted in this project lately on top. Please make sure these sources work well for your current projects, so we could make the master branch the default one.

pali commented 3 years ago

Thank you! I'm looking at the changes and I see that you forgot to include into master branch commit 83c182bc7cfc39a297b122712b2cb54e7e06dfcf from mv-ddr-devel branch.

kostapr commented 3 years ago

@pali, thank you, I updated the master branch with this patch,

pali commented 3 years ago

@heaterC: can you test master branch on your espressobin board?

heaterC commented 3 years ago

@pali: Sorry I have no board for testing at the moment, but will get some more soon.

Am 08.01.2021 um 14:09 schrieb pali notifications@github.com:

@heaterC: can you test master branch on your espressobin board?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

heaterC commented 3 years ago

I tried to compile but get error from Cryptopp (I assume). Unfortunately, I have not much time to dive into this:

g++ -o cryptest.exe -DNDEBUG -g2 -O3 -fPIC -pthread -pipe adhoc.o test.o bench1.o bench2.o bench3.o datatest.o dlltest.o fipsalgt.o validat0.o validat1.o validat2.o validat3.o validat4.o validat5.o validat6.o validat7.o validat8.o validat9.o validat10.o regtest1.o regtest2.o regtest3.o regtest4.o ./libcryptopp.a  
/usr/bin/ld: validat5.o:(.data.rel.ro._ZTVN8CryptoPP13Poly1305_BaseINS_8RijndaelEEE[_ZTVN8CryptoPP13Poly1305_BaseINS_8RijndaelEEE]+0xf0): undefined reference to `non-virtual thunk to CryptoPP::Poly1305_Base<CryptoPP::Rijndael>::AlgorithmProvider[abi:cxx11]() const'
pali commented 3 years ago

@heaterC: I got similar error. Clean build tree and compile it again. Seems that this is caused when some old object files are used from previous build stage of cryptopp. After cleaning build tree and started compilation again, it passed without issue.

heaterC commented 3 years ago

@pali: Thanks, after cleaning it compiled well. The image is running fine at first glance.

heaterC commented 3 years ago

I tested it more thoroughly now and it shows no problems on my platform EspressoBin v7 (DDR4 1GB). Unfortunately, the "master" branches here and at "A3700-utils" are not yet default. It would be a benefit to all users to be directed to the working default tree. Please, @haklai, would you mind setting the default to "master"?

pali commented 3 years ago

yes, master branch as a default would be really useful!

pali commented 3 years ago

@kostapr: also, are you able to "include" these additional github patches into the Marvell SDK?

kostapr commented 3 years ago

@pali, I will try when I get some time for it, but I think some patches are not compatible with the SDK. I remember we were changing some logic to an opposite one.

pali commented 3 years ago

Ok!

IIRC there are just two changes: https://github.com/MarvellEmbeddedProcessors/mv-ddr-marvell/commit/758e16ac005f4e0cb8791728e1711bce3a0bf179 and https://github.com/MarvellEmbeddedProcessors/A3700-utils-marvell/commit/5598e150fa3a1568256c30223fd2b214d729f26a. First one changes order of enum in mv-ddr-marvell to be compatible with older SDKs and second one fixes usage of that enum in A3700-utils-marvell. So if both changes are applied there should be no issue.

kostapr commented 3 years ago

@pali, I merged everything into our development branch. These patches will be available as part of the next SDK release.

pali commented 3 years ago

thank you!