Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45400 )
Change subject: soc/mediatek/mt8192: add pmic MT6315 driver ......................................................................
Patch Set 2:
(16 comments)
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/mt6315.h:
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/inc... PS1, Line 1: /* : * This file is part of the coreboot project. : * : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
SPDX
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/inc... PS1, Line 21:
just one space.
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/inc... PS1, Line 34: #define MT6315_SWCID_H_CODE 0x15 : #define MT6315_SWCID_L_E1_CODE 0x10 : #define MT6315_SWCID_L_E2_CODE 0x20 : #define MT6315_SWCID_L_E3_CODE 0x30
enum
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... File src/soc/mediatek/mt8192/mt6315.c:
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 17: #include <delay.h>
SPDX
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 23: A
lowercase 'a'
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 23: static
static const
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 118: B
b
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 118: static
static const
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 262: size_t
int
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 267: size_t
int
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 273: buck_uV
buck_uv
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 287: assert(0);
die()?
Done
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 311: assert(0);
die()?
Done
https://review.coreboot.org/c/coreboot/+/45400/2/src/soc/mediatek/mt8192/mt6... File src/soc/mediatek/mt8192/mt6315.c:
https://review.coreboot.org/c/coreboot/+/45400/2/src/soc/mediatek/mt8192/mt6... PS2, Line 11: CPU lowercase 'cpu'
https://review.coreboot.org/c/coreboot/+/45400/2/src/soc/mediatek/mt8192/mt6... PS2, Line 106: GPU lowercase 'gpu'
https://review.coreboot.org/c/coreboot/+/45400/2/src/soc/mediatek/mt8192/mt6... PS2, Line 279: return; No need for 'return'.