[coreboot-gerrit] Change in coreboot[master]: mainboard/amd: Add padmelon board code

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33993 )

Change subject: mainboard/amd: Add padmelon board code
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33993/5/src/mainboard/amd/padmelon/... 
File src/mainboard/amd/padmelon/fan_init.c:

https://review.coreboot.org/c/coreboot/+/33993/5/src/mainboard/amd/padmelon/... 
PS5, Line 60: static int check_status(const char *name, int status)
            : {
            : 	if (status != HWM_STATUS_SUCCESS)
            : 		printk(BIOS_DEBUG, "%s returned %d\n", name, status);
            : 	if (status < HWM_STATUS_SUCCESS)
            : 		return status;
            : 	return HWM_STATUS_SUCCESS;	/* positive values are warnings only */
            : }
            : 
            : static void set_fan(uint8_t fan, external_sensor sensor, temp_sensor_type stype,
            : 				 fan_temp_source temp_source, fan_type ftype,
            : 				 fan_mode fmode, fan_pwm_freq fan_freq,
            : 				 fan_rate_up rate_up, fan_rate_down rate_down,
            : 				u8 *boundaries, u8 *sections)
            : {
            : 	int s;
            : 	printk(BIOS_DEBUG, "Set sensor type\n");
            : 	if (sensor != IGNORE_SENSOR) {
            : 		s = set_sensor_type(CONFIG_HWM_PORT, sensor, stype);
            : 		if (check_status("set_sensor_type", s) < HWM_STATUS_SUCCESS)
            : 			return;
            : 	}
            : 
            : 	printk(BIOS_DEBUG, "Set temperature source\n");
            : 	s = set_fan_temperature_source(CONFIG_HWM_PORT, fan, temp_source);
            : 	if (check_status("set_fan_temperature_source", s) < HWM_STATUS_SUCCESS)
            : 		return;
            : 
            : 	printk(BIOS_DEBUG, "Set fan type mode\n");
            : 	s = set_fan_type_mode(CONFIG_HWM_PORT, fan, ftype, fmode);
            : 	if (check_status("fan_type_mode", s) < HWM_STATUS_SUCCESS)
            : 		return;
            : 
            : 	printk(BIOS_DEBUG, "Set PWM frequency\n");
            : 	s = set_pwm_frequency(CONFIG_HWM_PORT, fan, fan_freq);
            : 	if (check_status("set_pwm_frequency", s) < HWM_STATUS_SUCCESS)
            : 		return;
            : 
            : 	printk(BIOS_DEBUG, "Set fan speed change rate\n");
            : 	s = set_fan_speed_change_rate(CONFIG_HWM_PORT, fan, rate_up, rate_down);
            : 	if (check_status("set_fan_speed_change_rate", s) < HWM_STATUS_SUCCESS)
            : 		return;
            : 
            : 	printk(BIOS_DEBUG, "Set follow\n");
            : 	s = set_fan_follow(CONFIG_HWM_PORT, fan, FAN_FOLLOW_INTERPOLATION);
            : 	if (check_status("set_follow", s) < HWM_STATUS_SUCCESS)
            : 		return;
            : 
            : 	printk(BIOS_DEBUG, "Set sections\n");
            : 	s = set_sections(CONFIG_HWM_PORT, fan, boundaries, sections);
            : 	if (check_status("set_sections", s) < HWM_STATUS_SUCCESS)
            : 		return;
            : 
            : 	printk(BIOS_DEBUG, "Fan %d completed\n", fan);
            : }
> So other projects can use it without duplicating it... […]

yes, definitely keep the current api exposed, but move this function there as an additional slightly higher-level api