[coreboot-gerrit] Change in coreboot[master]: sb/intel/*: Use common SMBus functions

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/19258 )

Change subject: sb/intel/*: Use common SMBus functions
......................................................................


Patch Set 15:

(6 comments)

https://review.coreboot.org/#/c/19258/15/src/southbridge/intel/common/smbus.c
File src/southbridge/intel/common/smbus.c:

PS15, Line 6:  * Copyright (C) 2013 Vladimir Serbinenko
I was going to comment on the copyright removal, but it looks like this is just an issue with git choosing which of the multiple deleted versions is the rename.


PS15, Line 96:  {
Braces aren't needed for a single line if.  Same for all the following cases.


PS15, Line 176: Setup
May as well fix it while we're here, i guess.
/* Set up transaction */


https://review.coreboot.org/#/c/19258/15/src/southbridge/intel/common/smbus.h
File src/southbridge/intel/common/smbus.h:

Line 3:  *
can we make sure this retains the correct copyrights if possible?


PS15, Line 14: SMBUS_H
INTEL_COMMON_SMBUS_H maybe?

It won't actually conflict with anything, but we have a lot of smbus.h files in the tree, and it'd be a pain if they all just used SMBUS_H for the guards.


PS15, Line 32: 
Remove some extra lines?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I93bb186e04e8c32dff04fc1abe4b5ecbc4c9c962
Gerrit-PatchSet: 15
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes