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

[Jamie Ryu (Code Review)]

Attention is currently required from: Subrata Banik.

Jamie Ryu 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 10:

(11 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/83784/comment/208143cd_ae801d1c?usp... :
PS6, Line 7: soc/intel/common:
> soc/intel/cmn/pmc

Done. Updated to patchset#10

https://review.coreboot.org/c/coreboot/+/83784/comment/e051d281_b5d4fc06?usp... :
PS6, Line 7: Add SoC QDF read function
>

Thanks for the suggestion. Updated by patchset#10

https://review.coreboot.org/c/coreboot/+/83784/comment/c6171dfa_099cb626?usp... :
PS6, Line 13:
> what we shall do with this QDF information hasn't been mentioned in this CL. […]

thanks for comments. I added more information to the commit message by patchset#10, please let me know if more information is needed.

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

https://review.coreboot.org/c/coreboot/+/83784/comment/6d4f8a39_42df68cb?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 */
> please don't use any CPP to guard the macro. […]

Sure, done by patchset#10

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

https://review.coreboot.org/c/coreboot/+/83784/comment/035e1ade_761e631f?usp... :
PS6, Line 271: #if CONFIG(SOC_QDF_DYNAMIC_READ_PMC)
> u don't need this guard

Done. Updated by patchset#10

File src/soc/intel/common/block/pmc/Kconfig:

https://review.coreboot.org/c/coreboot/+/83784/comment/e8a2346f_2951d31e?usp... :
PS6, Line 89: 	depends on SOC_INTEL_COMMON_BLOCK_PMC
            : 	depends on PMC_IPC_ACPI_INTERFACE
>

Done. Updated by patchset#10

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

https://review.coreboot.org/c/coreboot/+/83784/comment/3f82d62d_52aef8e7?usp... :
PS6, Line 888: This function reads SoC QDF information using PMC interface.
>

Thanks for the suggestion. I updated the comments after removing the return type. Please let me know if you have further suggestion on this.

https://review.coreboot.org/c/coreboot/+/83784/comment/d4fcfb74_01097358?usp... :
PS6, Line 891: int pmc_read_qdf(void)
> looks like we are not returning anything, rather we are just dumping? […]

I removed the return value of the function and renamed the function name to pmc_dump_soc_qdf_info. Thanks.

https://review.coreboot.org/c/coreboot/+/83784/comment/c483d2b9_c2e3f821?usp... :
PS6, Line 891: int pmc_read_qdf(void)
             : {
>

Thanks. Updated by patchset#10

https://review.coreboot.org/c/coreboot/+/83784/comment/d2743adf_4ea7e0cd?usp... :
PS6, Line 908: 0
> error as in `-1`

Thanks. I removed the return value of the function by patchset#10.

https://review.coreboot.org/c/coreboot/+/83784/comment/f6c0f372_21fd7202?usp... :
PS6, Line 918: } else {
             : 		printk(BIOS_INFO, "SoC QDF not available\n");
             : 	}
> do we really care about this print?

I removed it by patchset#10.