[coreboot-gerrit] Change in coreboot[master]: amd/pi/hudson: Add GPIO get function

[Marc Jones (Code Review)]

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

Change subject: amd/pi/hudson: Add GPIO get function
......................................................................


Patch Set 8:

(5 comments)

https://review.coreboot.org/#/c/19159/8//COMMIT_MSG
Commit Message:

Line 9: Add a basic GPIO get function.
for Kern Hudson variation.


Line 13: gpio functions.
Note to add other hudson variations, also.


https://review.coreboot.org/#/c/19159/8/src/southbridge/amd/pi/hudson/Makefile.inc
File src/southbridge/amd/pi/hudson/Makefile.inc:

Line 42: ramstage-$(CONFIG_SOUTHBRIDGE_AMD_PI_KERN) += gpio.c
> Why is it conditionalized? gpio fetching doesn't work unless this Kconfig i
The patch only defined gpios for the Kern variation. I suppose the commit message should be clear about that.


https://review.coreboot.org/#/c/19159/8/src/southbridge/amd/pi/hudson/gpio.h
File src/southbridge/amd/pi/hudson/gpio.h:

Line 28: #if IS_ENABLED(CONFIG_SOUTHBRIDGE_AMD_PI_KERN)
> If this is based on a southbridge pairing shouldn't these defines be within
Kern is a variation of the hudson southbridge. For those less familiar with the AMD code, the Kconfig name might be more clear named CONFIG_SOUTHBRIDGE_AMD_PI_HUDSON_KERN.


Line 131: int gpio_get(gpio_t gpio_num);
> It's really unfortunate we're adding parallel declarations and not using th
I agree, but the assumed device/include/soc/ doesn't fit this chipset, yet. I tried to noted that in the commit message.

In src/include/gpio.h 
#include <soc/gpio.h>

I'm not sure why the /src/soc/vendor/device/include would need an additional /soc/ directory under include.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f681865715ab947b525320a6f9fc63af1334b59
Gerrit-PatchSet: 8
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marc Jones <marc at marcjonesconsulting.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
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