[coreboot-gerrit] [M] Change in coreboot[main]: soc/mediatek/mt8196: Add PMIC MT6363 ADC driver

[Hope Wang (Code Review)]

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu.

Hope Wang has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85128?usp=email )

Change subject: soc/mediatek/mt8196: Add PMIC MT6363 ADC driver
......................................................................

Patch Set 14:

(8 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/85128/comment/2c36297a_2a1ec635?usp... :
PS4, Line 7: mt6363
> MT6363 ADC

Done

https://review.coreboot.org/c/coreboot/+/85128/comment/b1e39d0b_67b92a31?usp... :
PS4, Line 9: add
> Add

Done

https://review.coreboot.org/c/coreboot/+/85128/comment/d7287547_06f88e53?usp... :
PS4, Line 9: auxadc
> AUXADC

Done

File src/soc/mediatek/common/include/soc/mt6363_sdmadc.h:

https://review.coreboot.org/c/coreboot/+/85128/comment/33360c7f_2f627fca?usp... :
PS4, Line 88: enum auxadc_val_type type);
> move to the next line

Done

File src/soc/mediatek/common/mt6363_sdmadc.c:

https://review.coreboot.org/c/coreboot/+/85128/comment/034e66c4_8c51f7ab?usp... :
PS4, Line 61: static struct pmif *pmif_arb = NULL;
            : 
            : static u32 mt6363_auxadc_read_value(u32 reg)
            : {
            : 	u32 data;
            : 
            : 	assert(pmif_arb);
            : 	pmif_arb->read(pmif_arb, SPMI_SLAVE_4, reg, &data);
            : 	return data;
            : }
            : 
            : static void mt6363_auxadc_write_value(u32 reg, u8 reg_val)
            : {
            : 	assert(pmif_arb);
            : 	pmif_arb->write(pmif_arb, SPMI_SLAVE_4, reg, reg_val);
            : }
            : 
            : static u32 mt6363_auxadc_read_value16(u32 reg)
            : {
            : 	u16 rdata = 0;
            : 
            : 	assert(pmif_arb);
            : 	pmif_arb->read16(pmif_arb, SPMI_SLAVE_4, reg, &rdata);
            : 	return rdata;
            : }
> move those functions to `common/mt6363.c`.

The APIs for adc code, adc code and 6363 code has been split into two patches.

https://review.coreboot.org/c/coreboot/+/85128/comment/161ae5b0_04084518?usp... :
PS4, Line 135: 
             : void mt6363_sdmadc_init(void)
             : {
             : 	if (!pmif_arb) {
             : 		pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1);
             : 		assert(pmif_arb);
             : 	}
             : 
             : 	if (pmif_arb->is_pmif_init_done(pmif_arb))
             : 		die("ERROR - Failed to initialize pmif spi");
             : 
             : 	printk(BIOS_INFO, "[%s]CHIP ID =  0x%x\n", __func__,
             : 			   mt6363_auxadc_read_value(PMIC_SWCID1));
             : }
> Remove. We can just call `mt6363_init_pmif_arb`.

Done

File src/soc/mediatek/common/mt6363_sdmadc.c:

https://review.coreboot.org/c/coreboot/+/85128/comment/032fbac6_d6a05d06?usp... :
PS7, Line 52: SDMADC_CHAN_SPEC(AUXADC_CHAN_VIN1, 1),
> I'd prefer […]

Done

File src/soc/mediatek/mt8196/Makefile.mk:

https://review.coreboot.org/c/coreboot/+/85128/comment/aa21a7d4_cd5cff9a?usp... :
PS4, Line 32: romstage-y += ../common/mt6363_sdmadc.c
> We don't need ADC in romstage.

Done