[coreboot-gerrit] [M] Change in coreboot[main]: soc/intel/cmn/pmc: Add API to dump silicon QDF information

[Subrata Banik (Code Review)]

Attention is currently required from: Jamie Ryu, Saurabh Mishra.

Subrata Banik has posted comments on this change by Jamie Ryu. ( https://review.coreboot.org/c/coreboot/+/83784?usp=email )

Change subject: soc/intel/cmn/pmc: Add API to dump silicon QDF information
......................................................................

Patch Set 11:

(3 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/83784/comment/c445a419_1b1e35b1?usp... :
PS6, Line 13:
> thanks for comments. […]

Acknowledged

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

https://review.coreboot.org/c/coreboot/+/83784/comment/7de7079f_d6554ad0?usp... :
PS6, Line 45: /* IPC command for accessing SoC registers */
            : #define PMC_IPC_CMD_SOC_REG_ACC		0xAA
            : #define PMC_IPC_CMD_SUBCMD_SOC_REG_RD	0x00
            : #if CONFIG(SOC_QDF_DYNAMIC_READ_PMC)
            : #define PMC_IPC_CMD_REGID_SOC_QDF	0x03
            : #endif /* SOC_QDF_DYNAMIC_READ_PMC */
> Sure, done by patchset#10

` Also, if there is no other users of these macros, then it can go into pmclib.c file itself` still not addressed. I don't think any other code would like to use these macros ? apart from ur newly added API hence, make sense to keep those into pmclib.c

File src/soc/intel/common/block/pmc/pmclib.c:

https://review.coreboot.org/c/coreboot/+/83784/comment/b1451ef4_d1d32aec?usp... :
PS11, Line 913: 	if (rsp.buf[0]) {
```suggestion
    if (r < 0 || rsp.buf[0] == 0) {
    	printk(BIOS_ERR, "%s: pmc_send_ipc_cmd failed.\n", __func__);
    	return;
    }
```