Change in coreboot[master]: mb/google/dedede/var/sasukette:Add fw_config probe for codec ALC5682I...

[Zhi7 Li (Code Review)]

Attention is currently required from: Paul Menzel, Tim Wawrzynczak, Karthik Ramasubramanian.
Zhi7 Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56930 )

Change subject: mb/google/dedede/var/sasukette:Add fw_config probe for codec ALC5682I-VD & VS compatibility
......................................................................

Patch Set 9:

(7 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/56930/comment/39844dfd_030b78b7 
PS6, Line 7: mb/google/dedede/var/sasukette: Codec ALC5682I-VD & VS compatibility
> Please make it a statement by using a verb (in imperative mood) [1]. […]

I have modified this place

File src/mainboard/google/dedede/variants/sasukette/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/56930/comment/a705e87f_56520357 
PS6, Line 185: 				register "name" = ""RT58""
> Thanks for your patience and guidance that help me a lot.

Done

File src/mainboard/google/dedede/variants/sasukette/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/56930/comment/cf45e55d_39c1332f 
PS7, Line 2: field AUDIO_CODEC_SOURCE 41 43
> Can you please confirm that there are no conflicts with the second source of other components in Sas […]

Yes, this is the result after confirmation.

https://review.coreboot.org/c/coreboot/+/56930/comment/240802f7_c0cd31f0 
PS7, Line 4: 3
> Why is it 3 instead of 2 - i.e. […]

Due to the historical project of HQ, different audio codec ICs are numbered uniformly, so there is sequence gaps.

I have added other related configurations.

File src/mainboard/google/dedede/variants/sasukette/ramstage.c:

https://review.coreboot.org/c/coreboot/+/56930/comment/f440d55b_232ce4e0 
PS6, Line 19: codec_path, ARRAY_SIZE(codec_path));
> Line lengths in coreboot are 96 wide, so feel free to use the whole width, or break it up into multi […]

Done

File src/mainboard/google/dedede/variants/sasukette/ramstage.c:

https://review.coreboot.org/c/coreboot/+/56930/comment/e10536f1_608ed0fa 
PS7, Line 29: 		config->hid =  "RTL5682";
> Nit: Seems there are 2 spaces between = and "RTL5682"

I have modified this place.

https://review.coreboot.org/c/coreboot/+/56930/comment/516312f3_f12acc1a 
PS7, Line 14: 	const struct device_path codec_path[] = {
            : 		{ .type = DEVICE_PATH_PCI, .pci.devfn = PCH_DEVFN_I2C4 },
            : 		{ .type = DEVICE_PATH_I2C, .i2c.device = 0x1a }
            : 	};
            : 	const struct device *codec = find_dev_nested_path(pci_root_bus(),
            : 						codec_path, ARRAY_SIZE(codec_path));
            : 	struct drivers_i2c_generic_config *config;
            : 
            : 	if (!codec || (codec->chip_ops != &drivers_i2c_generic_ops) || !codec->chip_info)
            : 		return;
            : 
            : 	config = codec->chip_info;
            : 	if (fw_config_probe(FW_CONFIG(AUDIO_CODEC_SOURCE, AUDIO_CODEC_ALC5682)))
            : 		config->hid = "10EC5682";
            : 	else if (fw_config_probe(FW_CONFIG(AUDIO_CODEC_SOURCE, AUDIO_CODEC_ALC5682I_VS)))
            : 		config->hid =  "RTL5682";
> W.r.t CB:56804, move this to a separate function - eg. […]

I have modified this place, do you think it is appropriate to modify this way?