[coreboot-gerrit] Change in coreboot[master]: amd/pi/hudson: Add LPC IO decode enable function

[Marc Jones (Code Review)]

Marc Jones has posted comments on this change. ( https://review.coreboot.org/19160 )

Change subject: amd/pi/hudson: Add LPC IO decode enable function
......................................................................


Patch Set 12:

(1 comment)

https://review.coreboot.org/#/c/19160/12/src/southbridge/amd/pi/hudson/early_setup.c
File src/southbridge/amd/pi/hudson/early_setup.c:

Line 200: 		pci_write_config32(dev, LPC_WIDEIO_GENERIC_PORT, tmp);
> Also see comment on line 175 above.
I'm probably overreacting to your comments, but come on, this is getting excessive. The function is pretty straightforward. You can call this as early as you need. If something comes along and changes things from under you, you are going to have to figure that out. That could be said for anything setup and then later touched by AGESA, FSP, or other code that might change a register.

It doesn't check the window assuming the developer knows what window they need to setup. It also doesn't check to see if regions could be combined into a single space. And finally, it doesn't cut grass, do laundry, or take out the trash. So, mostly useless. I'll add a comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2bed3a99180188101e00b4431d634227e488cbda
Gerrit-PatchSet: 12
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes