[coreboot-gerrit] [M] Change in coreboot[main]: mb/google/nissa/var/rull: Match VBT with SSFC

[Subrata Banik (Code Review)]

Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Kun Liu, Nick Vaccaro, Rui Zhou.

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

The change is no longer submittable: All-Comments-Resolved is unsatisfied now.

Change subject: mb/google/nissa/var/rull: Match VBT with SSFC
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

File src/mainboard/google/brya/variants/rull/variant.c:

https://review.coreboot.org/c/coreboot/+/85875/comment/de8916d8_db10e581?usp... :
PS2, Line 47: 
            : static int get_ssfc(uint32_t *val)
            : {
            : 	static uint32_t known_value;
            : 	static enum {
            : 		SSFC_NOT_READ,
            : 		SSFC_AVAILABLE,
            : 	} ssfc_state = SSFC_NOT_READ;
            : 
            : 	if (ssfc_state == SSFC_AVAILABLE) {
            : 		*val = known_value;
            : 		return 0;
            : 	}
            : 
            : 	/*
            : 	 * If SSFC field is not in the CBI then the value of SSFC will be 0 for
            : 	 * further processing later since 0 of each bits group means default
            : 	 * component in a variant. For more detail, please refer to cbi_ssfc.h.
            : 	 */
            : 	if (google_chromeec_cbi_get_ssfc(&known_value) != 0) {
            : 		printk(BIOS_DEBUG, "SSFC not set in CBI\n");
            : 		return -1;
            : 	}
            : 
            : 	ssfc_state = SSFC_AVAILABLE;
            : 	*val = known_value;
            : 	printk(BIOS_INFO, "SSFC 0x%x.\n", known_value);
            : 	return 0;
            : }
@[e-mail]
can you please consider implementing `retrieve_ssfc_data` inside ec.c ? that way we will be able to eliminate the need to implement same `get_ssfc` in present and future across multiple variants ?

I could see same code existed inside 3 mainboard