[coreboot-gerrit] Change in coreboot[master]: drivers/net: Add deivce index for multiple NIC cards

[Furquan Shaikh (Code Review)]

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

Change subject: drivers/net: Add deivce index for multiple NIC cards
......................................................................


Patch Set 1:

(4 comments)

https://review.coreboot.org/#/c/22984/1/src/drivers/net/chip.h
File src/drivers/net/chip.h:

https://review.coreboot.org/#/c/22984/1/src/drivers/net/chip.h@20
PS1, Line 20: /* Indentify device number */
Can you please add some more details what this device number is used for? Also, specify the set of valid values that are expected for this field.


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

https://review.coreboot.org/#/c/22984/1/src/drivers/net/r8168.c@94
PS1, Line 94: u8
Why not use the same data type in drivers_net_config?


https://review.coreboot.org/#/c/22984/1/src/drivers/net/r8168.c@107
PS1, Line 107: key[12] = device_index + 0x30;
This is going to break all the older devices using key "ethernet_mac". I think we should maintain backward compatibility. e.g.: Treat device_index 0 as special i.e. no index specified by the board and thus use key --> ethernet_mac\0. Else if device_index is 1-9, use key --> ethernet_macX\0 where X is 1-9.


https://review.coreboot.org/#/c/22984/1/src/drivers/net/r8168.c@109
PS1, Line 109: printk(BIOS_DEBUG, "Requesting %s from VPD.\n", key)
Maybe combine this and the print in L129 to have something like:
printk(BIOS_DEBUG, "Locating %s from VPD...", key);

...

printk(BIOS_DEBUG, "Done\n");



-- 
To view, visit https://review.coreboot.org/22984
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: I108b9bfba39370c8906a2fa4d2b39b106e884e0c
Gerrit-Change-Number: 22984
Gerrit-PatchSet: 1
Gerrit-Owner: Gaggery Tsai <gaggery.tsai at intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Tue, 26 Dec 2017 13:25:01 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171226/1d4de8af/attachment.html>