[PATCH v2] powerpc/powernv: Make possible for user to force a full ipl cec reboot

Vaibhav Jain vaibhav at linux.ibm.com
Tue Sep 4 12:42:43 AEST 2018


Thanks for reviewing this patch Andrew,

Andrew Donnellan <andrew.donnellan at au1.ibm.com> writes:

> The logic here is a bit hard to follow, relying on if (cmd && strcmp...) 
> failing then setting rc = OPAL_UNSUPPORTED in order to handle the normal 
> case seems a bit backwards.
Agree. With this change I am trying to make adding new reboot types in
future simpler, without resorting to major refactoring of the
function. opal_cec_reboot() should be a fallback for any failures in
doing a typed reboot. So adding a new foobar reboot type just needs
adding a new branch in the if-else ladder.

if (cmd && strcmp(cmd, "full") == 0)
    rc = opal_cec_reboot2(OPAL_REBOOT_FULL_IPL, NULL);
else if(cmd && strcmd(cmd, "foobar") == 0)
    rc = /* new code here */
else
    rc = OPAL_UNSUPPORTED;

> I think I would slightly prefer it if the !cmd case were handled first 
> to make it clear that opal_cec_reboot() is the normal case.
Fair suggestion. But then I will still have to handle the errors just
after the if-else ladder is finished to determine if I need to fallback.

> Acked-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
Thanks

-- 
Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.



More information about the Linuxppc-dev mailing list