Message ID | 20170921141948.27168-1-hegdevasant@linux.vnet.ibm.com |
---|---|
State | Accepted |
Headers | show |
Series | [1/2] opal-prd: Fix occ_reset call | expand |
Seems fine (though I don't really have the context of what function this is. (I'm not fluent in patch reviews, not sure how you understand anything with so little context....) -- Dan Crowell Senior Software Engineer - Power Systems Enablement Firmware IBM Rochester: t/l 553-2987 dcrowell@us.ibm.com From: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> To: skiboot@lists.ozlabs.org Cc: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>, Jeremy Kerr <jk@ozlabs.org>, Daniel M Crowell/Rochester/IBM@IBMUS, Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> Date: 09/21/2017 09:20 AM Subject: [PATCH 1/2] opal-prd: Fix occ_reset call HBRT OCC reset interface depends on service processor type. FSP -> reset_pm_complex() BMC -> process_occ_reset() This patch adds logic to detect service processor type and then make appropriate occ reset call. CC: Jeremy Kerr <jk@ozlabs.org> CC: Daniel M Crowell <dcrowell@us.ibm.com> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> --- external/opal-prd/opal-prd.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c index a09a636..20ce5da 100644 --- a/external/opal-prd/opal-prd.c +++ b/external/opal-prd/opal-prd.c @@ -246,6 +246,21 @@ static void pr_log_daemon_init(void) } } +/* Check service processor type */ +static bool is_fsp_system(void) +{ + char *path; + int rc; + + rc = asprintf(&path, "%s/fsps", devicetree_base); + if (rc < 0) { + pr_log(LOG_ERR, "FW: error creating '/fsps' path %m"); + return false; + } + + return access(path, F_OK) ? false : true; +} + /** * ABI check that we can't perform at build-time: we want to ensure that the * layout of struct host_interfaces matches that defined in the thunk. @@ -1336,18 +1351,27 @@ static int pm_complex_reset(uint64_t chip) { int rc; - if (hservice_runtime->reset_pm_complex) { + /* + * FSP system -> reset_pm_complex + * BMC system -> process_occ_reset + */ + if (is_fsp_system()) { + if (!hservice_runtime->reset_pm_complex) { + pr_log_nocall("reset_pm_complex"); + return -1; + } + pr_debug("PM: calling pm_complex_reset(%ld)", chip); rc = call_reset_pm_complex(chip); + } else { + if (!hservice_runtime->process_occ_reset) { + pr_log_nocall("process_occ_reset"); + return -1; + } - } else if (hservice_runtime->process_occ_reset) { pr_debug("PM: calling process_occ_reset(%ld)", chip); call_process_occ_reset(chip); rc = 0; - - } else { - pr_log_nocall("reset_pm_complex/process_occ_reset"); - rc = -1; } return rc;
Daniel M Crowell <dcrowell@us.ibm.com> writes: > Seems fine (though I don't really have the context of what function this > is. (I'm not fluent in patch reviews, not sure how you understand > anything with so little context....) A bunch of code context can be just kept in the brain (it goes alongside the ego, both take up a lot of room :) - but the other half is just applying the patches and looking at the result - i'll freely pipe to 'git am -s' when doing close review - build, look, etc. Of course, that requires a sane mail client, which certainly isn't the IBM default.
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > HBRT OCC reset interface depends on service processor type. > FSP -> reset_pm_complex() > BMC -> process_occ_reset() > > This patch adds logic to detect service processor type and > then make appropriate occ reset call. > > CC: Jeremy Kerr <jk@ozlabs.org> > CC: Daniel M Crowell <dcrowell@us.ibm.com> > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> Thanks, series merged as of dceed210a2bd2c4031e41d78548e6fc1f37d8f6f
diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c index a09a636..20ce5da 100644 --- a/external/opal-prd/opal-prd.c +++ b/external/opal-prd/opal-prd.c @@ -246,6 +246,21 @@ static void pr_log_daemon_init(void) } } +/* Check service processor type */ +static bool is_fsp_system(void) +{ + char *path; + int rc; + + rc = asprintf(&path, "%s/fsps", devicetree_base); + if (rc < 0) { + pr_log(LOG_ERR, "FW: error creating '/fsps' path %m"); + return false; + } + + return access(path, F_OK) ? false : true; +} + /** * ABI check that we can't perform at build-time: we want to ensure that the * layout of struct host_interfaces matches that defined in the thunk. @@ -1336,18 +1351,27 @@ static int pm_complex_reset(uint64_t chip) { int rc; - if (hservice_runtime->reset_pm_complex) { + /* + * FSP system -> reset_pm_complex + * BMC system -> process_occ_reset + */ + if (is_fsp_system()) { + if (!hservice_runtime->reset_pm_complex) { + pr_log_nocall("reset_pm_complex"); + return -1; + } + pr_debug("PM: calling pm_complex_reset(%ld)", chip); rc = call_reset_pm_complex(chip); + } else { + if (!hservice_runtime->process_occ_reset) { + pr_log_nocall("process_occ_reset"); + return -1; + } - } else if (hservice_runtime->process_occ_reset) { pr_debug("PM: calling process_occ_reset(%ld)", chip); call_process_occ_reset(chip); rc = 0; - - } else { - pr_log_nocall("reset_pm_complex/process_occ_reset"); - rc = -1; } return rc;