[coreboot-gerrit] [L] Change in em100[main]: Add target/download-RPMC/reset

[Martin L Roth (Code Review)]

Attention is currently required from: EvyLiu, Stefan Reinauer.

Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/em100/+/75675?usp=email )

Change subject: Add target/download-RPMC/reset
......................................................................

Patch Set 5:

(5 comments)

Patchset:

PS5: 
I don't see any critical issues in the patch though it looks like there might be the possibility of a buffer overrun. There are a few nits, but I think most of it could be handled in a follow-on patch.

If this were my patch, I'd probably refactor out all of the changes to send_cmd() into one patch, maybe a second patch to add the str2hex, and another for the actual RPMC functionality. (and of course another for the whitespace changes.)

As Paul mentioned, the commit message could be improved.
At least define RPMC - Replay Protected Monotonic Counter (Jedec spec JESD260)

File em100.c:

https://review.coreboot.org/c/em100/+/75675/comment/47cc0013_6e3c7830 :
PS5, Line 119: #if 0
             : 		struct timeval curtime , nowtime;
             : 		gettimeofday(&curtime,NULL);
             : 		unsigned long milli = ( (curtime.tv_sec*1000 + curtime.tv_usec/1000) + reset_time);
             : 		unsigned long nowmilli = 0;
             : 		do {
             : 			usleep(1);
             : 			gettimeofday(&nowtime,NULL);
             : 			nowmilli = (nowtime.tv_sec*1000 + nowtime.tv_usec/1000);
             : 		} while( nowmilli < milli);
             : #else
should this be removed, changed to #if DEBUG, or something else?

https://review.coreboot.org/c/em100/+/75675/comment/73700f48_8e9a2725 :
PS5, Line 1336: have
nit: "has to be used with"

https://review.coreboot.org/c/em100/+/75675/comment/47f80932_6530d1b3 :
PS5, Line 1420: ret = 
Nit: ret is not used after any of the calls. check it or get rid of it?

File ini.c:

https://review.coreboot.org/c/em100/+/75675/comment/6c013bf1_1fa1e8a5 :
PS5, Line 6: )
This function could take the buffer length as a parameter to make sure the strcpy later doesn't go past the end of the buffer.