[coreboot-gerrit] Change in coreboot[master]: google/fizz: Add new board

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/18796 )

Change subject: google/fizz: Add new board
......................................................................


Patch Set 5:

(17 comments)

https://review.coreboot.org/#/c/18796/5/src/mainboard/google/fizz/devicetree.cb
File src/mainboard/google/fizz/devicetree.cb:

Line 1: chip soc/intel/skylake
I don't see changes from this file in any follow-up CL. So commenting here. Please feel free to push a follow-up CL for the comments here.


PS5, Line 22: # Enable DPTF
            : 	register "dptf_enable" = "1"
Get rid of this for now.


PS5, Line 27: register "EnableLan" = "0"
            : 	register "EnableSata" = "0"
            : 	register "SataSalpSupport" = "0"
            : 	register "SataMode" = "0"
            : 	register "SataPortsEnable[0]" = "0"
You might need some of these.


PS5, Line 155: I2C_VOLTAGE_1V8
I2C_VOLTAGE_3V3


PS5, Line 156: register "i2c_voltage[3]" = "I2C_VOLTAGE_1V8"		# Pen
             : 	register "i2c_voltage[4]" = "I2C_VOLTAGE_1V8"		# Camera
This is not required.


PS5, Line 153: register "i2c_voltage[0]" = "I2C_VOLTAGE_3V3"		# Touchscreen
             : 	register "i2c_voltage[1]" = "I2C_VOLTAGE_3V3"		# NFC
             : 	register "i2c_voltage[2]" = "I2C_VOLTAGE_1V8"		# Camera
             : 	register "i2c_voltage[3]" = "I2C_VOLTAGE_1V8"		# Pen
             : 	register "i2c_voltage[4]" = "I2C_VOLTAGE_1V8"		# Camera
             : 	register "i2c_voltage[5]" = "I2C_VOLTAGE_1V8"		# Audio
Comments on here are not right.


PS5, Line 165: [PchSerialIoIndexI2C3]  = PchSerialIoPci,
             : 		[PchSerialIoIndexI2C4]  = PchSerialIoPci,
PchSerialIoDisabled.


PS5, Line 169: [PchSerialIoIndexSpi1]  = PchSerialIoPci,
PchSerialIoDisabled


PS5, Line 180: GPP_G7
GPP_A7


PS5, Line 192: chip drivers/i2c/generic
             : 				register "hid" = ""ATML0001""
             : 				register "desc" = ""Atmel Touchscreen""
             : 				register "irq" = "ACPI_IRQ_LEVEL_LOW(GPP_E7_IRQ)"
             : 				register "probed" = "1"
             : 				device i2c 4b on end
             : 			end
get rid of this.


PS5, Line 202: on
off


PS5, Line 203: chip drivers/i2c/hid
             : 				register "generic.hid" = ""WCOM50C1""
             : 				register "generic.desc" = ""WCOM Digitizer""
             : 				register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D1_IRQ)"
             : 				register "hid_desc_reg_offset" = "0x1"
             : 				device i2c 0x9 on end
             : 			end
get rid of this.


PS5, Line 216: off
on?


PS5, Line 219: chip drivers/i2c/max98927
             : 				register "interleave_mode" = "1"
             : 				register "uid" = "0"
             : 				register "desc" = ""SSM4567 Right Speaker Amp""
             : 				register "name" = ""MAXR""
             : 				device i2c 39 on end
             : 			end
             : 			chip drivers/i2c/max98927
             : 				register "interleave_mode" = "1"
             : 				register "uid" = "1"
             : 				register "desc" = ""SSM4567 Left Speaker Amp""
             : 				register "name" = ""MAXL""
             : 				device i2c 3A on end
             : 			end
             : 			chip drivers/i2c/generic
             : 				register "hid" = ""10EC5663""
             : 				register "name" = ""RT53""
             : 				register "desc" = ""Realtek RT5663""
             : 				register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_D9)"
             : 				register "probed" = "1"
             : 				device i2c 13 on end
             : 			end
get rid of this.


PS5, Line 242: on
off


PS5, Line 263: on
off

GSPI#1 pins are not being used for GSPI functionality. It would be good to switch this off in a later CL.


PS5, Line 264: on
Switch this to off in a later CL.


-- 
To view, visit https://review.coreboot.org/18796
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icab3639a53fef65e904e797028916fda879fff7c
Gerrit-PatchSet: 5
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Shelley Chen <shchen at google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Shelley Chen <shchen at google.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes