[Skiboot] [PATCH 1/2] opal-prd: Disable pnor access interface on FSP system
Jeremy Kerr
jk at ozlabs.org
Thu Aug 10 16:05:16 AEST 2017
Hi Vasant,
> On FSP system host does not have access to PNOR. Hence disable PNOR
> access interfaces.
The concept looks good, but I'm not sure about the implementation:
> - rc = pnor_init(&ctx->pnor);
> - if (rc) {
> - pr_log(LOG_ERR, "PNOR: Failed to open pnor: %m");
> - goto out_close;
> + if (pnor_available(&ctx->pnor)) {
> + rc = pnor_init(&ctx->pnor);
> + if (rc) {
> + pr_log(LOG_ERR, "PNOR: Failed to open pnor: %m");
> + goto out_close;
> + }
> + } else {
> + /* Disable PNOR function pointers */
> + hinterface.pnor_read = NULL;
> + hinterface.pnor_write = NULL;
> }
>
> ipmi_init(ctx);
> diff --git a/external/opal-prd/pnor.c b/external/opal-prd/pnor.c
> index 0e7e5c0..c032421 100644
> --- a/external/opal-prd/pnor.c
> +++ b/external/opal-prd/pnor.c
> @@ -32,6 +32,25 @@
> #include "pnor.h"
> #include "opal-prd.h"
>
> +#define FDT_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
> +
> +bool pnor_available(struct pnor *pnor)
> +{
> + /* --pnor is specified */
> + if (pnor->path) {
> + if (access(pnor->path, R_OK | W_OK) == 0)
> + return true;
> +
> + pr_log(LOG_ERR, "PNOR: Does not have permission to read pnor: %m");
> + return false;
> + }
> +
> + if (access(FDT_FLASH_PATH, R_OK) == 0)
> + return true;
> +
> + return false;
> +}
> +
Here you're duplicating the check for the ibm,system-flash property in
the libflash code. Why not just use the return value of pnor_init there?
We'd want the following logic:
- if no --pnor was specified (ie., pnor->path is NULL), and pnor_init()
fails, then we continue without PNOR (and NULL-out the function
pointers). We do want a message to be printed there though.
- if --pnor *was* specified, and that specific device can't be opened,
then fail immediately.
Unless there was a specific reason to re-check ibm,system-flash, I'm
inclined to just let dev_get_mtd() do the checking instead.
Cheers,
Jeremy
More information about the Skiboot
mailing list