Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 )
Change subject: soc/intel/{common,skl,cnl,icl,apl,tgl}: Make me_hfsts1 structure SoC specific ......................................................................
Patch Set 25:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35546/25/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35546/25/src/soc/intel/common/block... PS25, Line 630: cse_is_hfs1_cws_normal
I still like the helper functions, but I think those can simply be implemented by the common code if […]
The whole point of having helper functions was to mask the different notions of the hfsts1 among SoCs. see my comment on https://review.coreboot.org/c/coreboot/+/35546/25/src/soc/intel/apollolake/i...
https://review.coreboot.org/c/coreboot/+/35546/25/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35546/25/src/soc/intel/common/block... PS25, Line 54: /* ME Host Firmware Status register 1 */ : union me_hfsts1 { : u32 data; : struct { : u32 working_state: 4; : u32 mfg_mode: 1; : u32 fpt_bad: 1; : u32 operation_state: 3; : u32 fw_init_complete: 1; : u32 ft_bup_ld_flr: 1; : u32 update_in_progress: 1; : u32 error_code: 4; : u32 operation_mode: 4; : u32 reset_count: 4; : u32 boot_options_present: 1; : u32 reserved1: 1; : u32 bist_test_state: 1; : u32 bist_reset_request: 1; : u32 current_power_source: 2; : u32 d3_support_valid: 1; : u32 d0i3_support_valid: 1; : } __packed fields; : };
What do you think about this: […]
I don't understand your comment, I think this patch is doing what you say here. This union has been moved to respective soc/me.h.