[coreboot-gerrit] Change in coreboot[master]: google/poppy: Add SX9310 support

[Furquan Shaikh (Code Review)]

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

Change subject: google/poppy: Add SX9310 support
......................................................................


Patch Set 2:

(10 comments)

Can you please split this change into two CLs:
1. For driver support
2. For mainboard changes

https://review.coreboot.org/#/c/27172/1//COMMIT_MSG
Commit Message:

https://review.coreboot.org/#/c/27172/1//COMMIT_MSG@7
PS1, Line 7: oogle/poppy
mb/google/poppy/variants/nocturne


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/Kconfig
File src/drivers/i2c/sx9310/Kconfig:

https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/Kconfig@3
PS1, Line 3: 	default n
depends on HAVE_ACPI_TABLES

Then you don't have to add #ifdef for HAVE_ACPI_TABLES in .c file


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/chip.h
File src/drivers/i2c/sx9310/chip.h:

https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/chip.h@4
PS1, Line 4: Inc
LLC


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/registers.h
File src/drivers/i2c/sx9310/registers.h:

https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/registers.h@4
PS1, Line 4: Inc
LLC


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c
File src/drivers/i2c/sx9310/sx9310.c:

https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c@4
PS1, Line 4: Inc
LLC


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c@4
PS1, Line 4: 2016
2018


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c@18
PS1, Line 18: #include <console/console.h>
Is this required?


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c@22
PS1, Line 22: #include <gpio.h>
Is this required?


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c@43
PS1, Line 43: = NULL
No need to set this to NULL. dsd is set on line 64.


https://review.coreboot.org/#/c/27172/1/src/drivers/i2c/sx9310/sx9310.c@78
PS1, Line 78: %sD%03.3X
This should be 4-character long.



-- 
To view, visit https://review.coreboot.org/27172
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7358ee34df873098a86d692cc8a909b0ec5023a8
Gerrit-Change-Number: 27172
Gerrit-PatchSet: 2
Gerrit-Owner: Enrico Granata <egranata at chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Gwendal Grignou <gwendal at chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Wed, 20 Jun 2018 19:13:57 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180620/5a9b9a58/attachment.html>