[coreboot-gerrit] Change in coreboot[master]: inteltool: Add PCI IDs for the C220 PCH series

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/27168 )

Change subject: inteltool: Add PCI IDs for the C220 PCH series
......................................................................


Patch Set 1:

(5 comments)

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

https://review.coreboot.org/#/c/27168/1//COMMIT_MSG@9
PS1, Line 9: ICH8
ICH was the predecessor of the PCH. Because some features of
the GMCH (northbridge) also moved into the PCH Intel counts
the PCH series based on the GMCH numbers. i.e. after 4 series
GMCH came 5 series PCH. This is about the 8 series PCHs.

Also, please break lines before 72 characters.


https://review.coreboot.org/#/c/27168/1//COMMIT_MSG@12
PS1, Line 12: Documentation on the PCH can be found here:
Please separate paragraphs with an empty line.


https://review.coreboot.org/#/c/27168/1//COMMIT_MSG@13
PS1, Line 13: https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/8-series-chipset-pch-datasheet.pdf
No links please. Document and revision numbers are more useful
in the long run.


https://review.coreboot.org/#/c/27168/1/util/inteltool/gpio.c
File util/inteltool/gpio.c:

https://review.coreboot.org/#/c/27168/1/util/inteltool/gpio.c@866
PS1, Line 866: 	case PCI_DEVICE_ID_INTEL_H81:
According to the datasheet the non-LP version doesn't have the
registers above 0x68. Can you confirm if you see (or don't see)
valid values above that?

In case, you can still use the same register description, just
reduce the `size` below.


https://review.coreboot.org/#/c/27168/1/util/inteltool/inteltool.c
File util/inteltool/inteltool.c:

https://review.coreboot.org/#/c/27168/1/util/inteltool/inteltool.c@235
PS1, Line 235: 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_C8_MOBILE,
             : 	 "Intel(R) C8 Mobile"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_C8_DESKTOP,
             : 	 "Intel(R) C8 Desktop"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Z87,
             : 	 "Intel(R) Z87"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Z85,
             : 	 "Intel(R) Z85"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_HM86,
             : 	 "Intel(R) HM86"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_H87,
             : 	 "Intel(R) H87"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_HM87,
             : 	 "Intel(R) HM87"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q85,
             : 	 "Intel(R) Q85"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_Q87,
             : 	 "Intel(R) Q87"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_QM87,
             : 	 "Intel(R) QM87"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_B85,
             : 	 "Intel(R) B85"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_C222,
             : 	 "Intel(R) C222"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_C224,
             : 	 "Intel(R) C224"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_C226,
             : 	 "Intel(R) C226"},
             : 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_H81,
             : 	 "Intel(R) H81"},
Please don't break lines that fit into 80 chars.



-- 
To view, visit https://review.coreboot.org/27168
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: I07a8f2e9cb0ee8677c8fe2c51881147ed81c1a35
Gerrit-Change-Number: 27168
Gerrit-PatchSet: 1
Gerrit-Owner: Quan Tran <qeed.quan at gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Wed, 20 Jun 2018 11:33:36 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180620/6fbe05b6/attachment.html>