[coreboot-gerrit] Change in coreboot[master]: drivers/net/r8168: Add customized LED mode

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/21862 )

Change subject: drivers/net/r8168: Add customized LED mode
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

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

https://review.coreboot.org/#/c/21862/1//COMMIT_MSG@17
PS1, Line 17: RT8111H_CUSOMIZED_LED_VALUE in Kconfig file
            : 	under target board
Board specific settings should go into devicetree as much as possible.


https://review.coreboot.org/#/c/21862/1/src/drivers/net/r8168.c
File src/drivers/net/r8168.c:

https://review.coreboot.org/#/c/21862/1/src/drivers/net/r8168.c@218
PS1, Line 218: #if IS_ENABLED(CONFIG_RT8111H_SET_LED_MODE)
if (!IS_ENABLED(...)) return;


https://review.coreboot.org/#/c/21862/1/src/drivers/net/r8168.c@221
PS1, Line 221: 	/* check the VPD for the mac address */
             : 	setting = CONFIG_RT8111H_CUSTOMIZED_LED_VALUE;
this is a Kconfig option, not VPD.

Also if it is mainboard specific it should go into devicetree, not Kconfig.


https://review.coreboot.org/#/c/21862/1/src/drivers/net/r8168.c@228
PS1, Line 228: 	/* Refer to datasheet 7.2, starting from offset 0x18
add title please



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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib3d4f2cd98ac391e1661a891d604bdd1974d07f6
Gerrit-Change-Number: 21862
Gerrit-PatchSet: 1
Gerrit-Owner: Gaggery Tsai <gaggery.tsai at intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Kane Chen <kane.chen at intel.com>
Gerrit-Reviewer: Kevin Cheng <kevin.cheng at intel.com>
Gerrit-Reviewer: Shelley Chen <shchen at google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Tue, 03 Oct 2017 08:25:17 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171003/586bcd69/attachment.html>