Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41640 )
Change subject: util/mb/google: add templates for dedede boards ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41640/1/util/mainboard/google/waddl... File util/mainboard/google/waddledee/template/include/variant/ec.h:
https://review.coreboot.org/c/coreboot/+/41640/1/util/mainboard/google/waddl... PS1, Line 1: /* : * : * : * SPDX-License-Identifier: GPL-2.0-or-later : */ I feel this can be a single line comment.
It appears this way in the existing files which have been going through some clean-ups.
https://review.coreboot.org/c/coreboot/+/41640/1/util/mainboard/google/waddl... File util/mainboard/google/waddledee/template/memory.c:
https://review.coreboot.org/c/coreboot/+/41640/1/util/mainboard/google/waddl... PS1, Line 18: This file can be removed. The concerned strap GPIO_MEM_CH_SEL will be stuffed in all the upcoming reference and variant boards. A weak override function to read the GPIO has been implemented in variants/baseboard/memory.c
Also this file can be removed in the makefile. Same comment applies for waddledoo as well.
https://review.coreboot.org/c/coreboot/+/41640/1/util/mainboard/google/waddl... File util/mainboard/google/waddledoo/template/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/41640/1/util/mainboard/google/waddl... PS1, Line 27: rise_time_ns = 66, : .fall_time_ns = 90, : .data_hold_time_ns = 350 The rise time, fall time and data hold time may require tuning based on individual variant boards. So we can remove them.