build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45340 )
Change subject: /soc/amd/picasso: Generate ACPI pstate and cstate objects in cb ......................................................................
Patch Set 1:
(53 comments)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpi.h:
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 33: uint32_t p_state_enable; ///< Pstate enable please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 34: uint32_t core_freq; ///< MHz please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 35: uint32_t power; ///< milliWatts please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 36: uint32_t sw_state_number; ///< Software P-state number please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 54: int get_pstate_info( sw_pstate_values_t *pstate_values, int *max_pstate, uint32_t *thread_count); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 54: int get_pstate_info( sw_pstate_values_t *pstate_values, int *max_pstate, uint32_t *thread_count); space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 56: void write_pss_object(int pstate_count, int max_pstate, sw_pstate_values_t* new_pstate_values); "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/common/block/in... PS1, Line 58: void write_xpss_object(int pstate_count, int max_pstate, sw_pstate_values_t* new_pstate_values); "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 206: core_freq = ((200 * core_freq_mul) /(core_freq_div)); need consistent spacing around '/' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 208: printk (BIOS_WARNING, "Undefined core_freq_div %x used. Force to 1.\n", core_freq_div); line over 96 characters
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 208: printk (BIOS_WARNING, "Undefined core_freq_div %x used. Force to 1.\n", core_freq_div); space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 232: /* Voltage off for VID codes 0xF8 to 0xFF */ trailing whitespace
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 242: power_in_mw= power_in_mw / 100L; spaces required around that '=' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 258: int get_pstate_info( sw_pstate_values_t *pstate_values, int *max_pstate, uint32_t *thread_count) line over 96 characters
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 258: int get_pstate_info( sw_pstate_values_t *pstate_values, int *max_pstate, uint32_t *thread_count) space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 269: for(pstate = 0; pstate <= *max_pstate; pstate++){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 269: for(pstate = 0; pstate <= *max_pstate; pstate++){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 271: pstate_values[pstate].p_state_enable = (pstate_def.hi & PSTATE_DEF_HI_ENABLE_MASK) line over 96 characters
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 277: if(pstate_values[pstate].p_state_enable){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 277: if(pstate_values[pstate].p_state_enable){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 277: if(pstate_values[pstate].p_state_enable){ braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 285: void write_pss_object(int pstate_count, int max_pstate, sw_pstate_values_t* pstate_values) "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 291: for(pstate = 0; pstate <= max_pstate; pstate++){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 291: for(pstate = 0; pstate <= max_pstate; pstate++){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 292: if(pstate_values->p_state_enable){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 292: if(pstate_values->p_state_enable){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 294: pstate_values->power,0, 0, pstate_values->sw_state_number, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 302: void write_xpss_object(int pstate_count, int max_pstate, sw_pstate_values_t* pstate_values) "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 309: for(pstate = 0; pstate <= max_pstate; pstate++){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 309: for(pstate = 0; pstate <= max_pstate; pstate++){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 312: acpigen_write_dword( pstate_values->power); space prohibited after that open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 317: acpigen_write_byte_buffer((uint8_t*)&bytes,8); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 317: acpigen_write_byte_buffer((uint8_t*)&bytes,8); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 318: acpigen_write_byte_buffer((uint8_t*)&bytes,8); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 318: acpigen_write_byte_buffer((uint8_t*)&bytes,8); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 320: bytes=0; spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 321: acpigen_write_byte_buffer((uint8_t*)&bytes,8); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 321: acpigen_write_byte_buffer((uint8_t*)&bytes,8); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 322: acpigen_write_byte_buffer((uint8_t*)&bytes,8); "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 322: acpigen_write_byte_buffer((uint8_t*)&bytes,8); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 374: int logical_cores , pstate_count, max_pstate, cpu; space prohibited before that ',' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 383: if(cpu == 0){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 383: if(cpu == 0){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 387: }else{ space required after that close brace '}'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 393: trailing whitespace
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 398: if(CONFIG(ACPI_SSDT_PSD_INDEPENDENT)){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 398: if(CONFIG(ACPI_SSDT_PSD_INDEPENDENT)){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 398: if(CONFIG(ACPI_SSDT_PSD_INDEPENDENT)){ braces {} are not necessary for any arm of this statement
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 399: acpigen_write_PSD_package(cpu /thread_count, thread_count, HW_ALL); need consistent spacing around '/' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 400: }else{ space required after that close brace '}'
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 408: acpigen_write_CSD_package(cpu >> 1,thread_count, HW_ALL,0 ); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 408: acpigen_write_CSD_package(cpu >> 1,thread_count, HW_ALL,0 ); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/45340/1/src/soc/amd/picasso/acpi.c@... PS1, Line 408: acpigen_write_CSD_package(cpu >> 1,thread_count, HW_ALL,0 ); space prohibited before that close parenthesis ')'