Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47988 )
Change subject: soc/intel/common/dmi: Add DMI driver support ......................................................................
Patch Set 2:
(13 comments)
https://review.coreboot.org/c/coreboot/+/47988/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47988/1//COMMIT_MSG@9 PS1, Line 9: is
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1//COMMIT_MSG@10 PS1, Line 10: set up the decoding of extended : BIOS region in DMI
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1//COMMIT_MSG@15 PS1, Line 15: It can ensure that the PCR base address is configured
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1//COMMIT_MSG@20 PS1, Line 20: 2
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... File src/soc/intel/common/block/dmi/dmi.c:
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 8: 2
Sure
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 11: #define DMI_PCR_GPMR_BASE_SHIFT 16 : #define DMI_PCR_GPMR_LIMIT_MASK 0xffff0000 : #define DMI_PCR_GPMR_BASE_MASK 0xffff : #define DMI_PCR_GPMR_EN BIT(31)
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 29: uint32_t
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 39: printk(BIOS_ERR, "get_available_gpmr: No available free gpmr found \n");
unnecessary whitespace before a quoted newline
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 39: printk(BIOS_ERR, "get_available_gpmr: No available free gpmr found \n");
Prefer using '"%s... […]
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 50:
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 53: size
Agree, I was initially under the impression that size was limit. […]
Done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/dmi.h:
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 9: * Takes base, size and destination ID and configures the GPMR
Ack
done
https://review.coreboot.org/c/coreboot/+/47988/1/src/soc/intel/common/block/... PS1, Line 12: void
Ack
done