Thanks for reviewing this patch Andrew,
Andrew Donnellan <[email protected]> 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 <[email protected]>
Thanks
--
Vaibhav Jain <[email protected]>
Linux Technology Center, IBM India Pvt. Ltd.
