[coreboot-gerrit] [L] Change in coreboot[main]: soc/intel/common/block/imc: Add Integrated Memory Controller driver

[Jérémy Compostella (Code Review)]

Attention is currently required from: Felix Singer, Paul Menzel, Shuo Liu, [e-mail].

Jérémy Compostella has posted comments on this change by [e-mail]. ( https://review.coreboot.org/c/coreboot/+/83320?usp=email )

Change subject: soc/intel/common/block/imc: Add Integrated Memory Controller driver
......................................................................

Patch Set 23:

(7 comments)

File src/soc/intel/common/block/imc/imc.c:

https://review.coreboot.org/c/coreboot/+/83320/comment/15671360_6239805e?usp... :
PS23, Line 64: B1E:D0B:F0
should you print values from `dev` ?

https://review.coreboot.org/c/coreboot/+/83320/comment/88843f51_9ced3d95?usp... :
PS23, Line 91: {
unnecessary brackets.

https://review.coreboot.org/c/coreboot/+/83320/comment/a59f5dff_10545bc3?usp... :
PS23, Line 119: poll_ready(dev, &status);
shouldn't an error message be printed if this fails ?

https://review.coreboot.org/c/coreboot/+/83320/comment/6f51c941_2c3824cc?usp... :
PS23, Line 173: ret = -1;
`ret` is necessarily still `-1`. 
BTW, there is a mix of `-1` and `CB_ERR`, could you use `CB_ERR` everywhere ?

https://review.coreboot.org/c/coreboot/+/83320/comment/b05c9b10_34d7bd37?usp... :
PS23, Line 192: ret = 0;
If you use `CB_ERR` you should also use `CB_SUCCESS`

File src/soc/intel/common/block/imc/imclib.h:

https://review.coreboot.org/c/coreboot/+/83320/comment/a0371689_f4154869?usp... :
PS23, Line 13: = 0,
unnecessary first value. I know you are just copying over so it may sound unfair to do clean that up.

File src/soc/intel/common/block/include/intelblocks/imc.h:

https://review.coreboot.org/c/coreboot/+/83320/comment/b28f3142_6fdb248c?usp... :
PS23, Line 4: #include <stdint.h>
this one is not needed anymore.