external/gard: Print an error if run on an FSP system

Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
---
 external/gard/gard.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

On 2017-03-24 06:01, Cyril Bur wrote:
> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
> ---
>  external/gard/gard.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/external/gard/gard.c b/external/gard/gard.c
> index 4b26a3b8..1b3775c3 100644
> --- a/external/gard/gard.c
> +++ b/external/gard/gard.c
> @@ -43,7 +43,9 @@
> 
>  #define CLEARED_RECORD_ID 0xFFFFFFFF
> 
> -#define FDT_ACTIVE_FLASH_PATH 
> "/proc/device-tree/chosen/ibm,system-flash"
> +#define FDT_PATH "/proc/device-tree"
> +#define FDT_FSP_NODE FDT_PATH"/fsps"
> +#define FDT_ACTIVE_FLASH_PATH FDT_PATH"/chosen/ibm,system-flash"
>  #define SYSFS_MTD_PATH "/sys/class/mtd/"
>  #define FLASH_GARD_PART "GUARD"
> 
> @@ -563,6 +565,11 @@ static void usage(const char *progname)
>  	}
>  }
> 
> +static bool is_fsp(void)
> +{
> +	return access(FDT_FSP_NODE, F_OK) == 0;
> +}
> +
>  static struct option global_options[] = {
>  	{ "file", required_argument, 0, 'f' },
>  	{ "part", no_argument, 0, 'p' },
> @@ -586,6 +593,12 @@ int main(int argc, char **argv)
>  	ctx = &_ctx;
>  	memset(ctx, 0, sizeof(*ctx));
> 
> +	if (is_fsp()) {
> +		fprintf(stderr, "ERROR: This tool is not compatible with FSP 
> systems\n\n");
> +		usage(progname);

Hi
Here usage is not necessary as tool won't support on this system(FSP).
Only error message is sufficient.


Thanks
Pridhiviraj

> +		return EXIT_FAILURE;
> +	}
> +
>  	/* process global options */
>  	for (;;) {
>  		int c;

On Fri, Mar 24, 2017 at 11:31 AM, Cyril Bur <cyril.bur@au1.ibm.com> wrote:
> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
> ---
>  external/gard/gard.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/external/gard/gard.c b/external/gard/gard.c
> index 4b26a3b8..1b3775c3 100644
> --- a/external/gard/gard.c
> +++ b/external/gard/gard.c
> @@ -43,7 +43,9 @@
>
>  #define CLEARED_RECORD_ID 0xFFFFFFFF
>
> -#define FDT_ACTIVE_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
> +#define FDT_PATH "/proc/device-tree"
> +#define FDT_FSP_NODE FDT_PATH"/fsps"
> +#define FDT_ACTIVE_FLASH_PATH FDT_PATH"/chosen/ibm,system-flash"
>  #define SYSFS_MTD_PATH "/sys/class/mtd/"
>  #define FLASH_GARD_PART "GUARD"
>
> @@ -563,6 +565,11 @@ static void usage(const char *progname)
>         }
>  }
>
> +static bool is_fsp(void)
> +{
> +       return access(FDT_FSP_NODE, F_OK) == 0;
> +}
> +
>  static struct option global_options[] = {
>         { "file", required_argument, 0, 'f' },
>         { "part", no_argument, 0, 'p' },
> @@ -586,6 +593,12 @@ int main(int argc, char **argv)
>         ctx = &_ctx;
>         memset(ctx, 0, sizeof(*ctx));
>
> +       if (is_fsp()) {
> +               fprintf(stderr, "ERROR: This tool is not compatible with FSP systems\n\n");

Can you change this to be something like "This is the OpenPower gard
tool which does not support FSP systems"? I can see people getting
confused with the current message.

> +               usage(progname);
> +               return EXIT_FAILURE;
> +       }
> +
>         /* process global options */
>         for (;;) {
>                 int c;
> --
> 2.12.1
>
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot

On Fri, 2017-03-24 at 14:24 +1100, Oliver O'Halloran wrote:
> On Fri, Mar 24, 2017 at 11:31 AM, Cyril Bur <cyril.bur@au1.ibm.com> wrote:
> > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
> > ---
> >  external/gard/gard.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/external/gard/gard.c b/external/gard/gard.c
> > index 4b26a3b8..1b3775c3 100644
> > --- a/external/gard/gard.c
> > +++ b/external/gard/gard.c
> > @@ -43,7 +43,9 @@
> > 
> >  #define CLEARED_RECORD_ID 0xFFFFFFFF
> > 
> > -#define FDT_ACTIVE_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
> > +#define FDT_PATH "/proc/device-tree"
> > +#define FDT_FSP_NODE FDT_PATH"/fsps"
> > +#define FDT_ACTIVE_FLASH_PATH FDT_PATH"/chosen/ibm,system-flash"
> >  #define SYSFS_MTD_PATH "/sys/class/mtd/"
> >  #define FLASH_GARD_PART "GUARD"
> > 
> > @@ -563,6 +565,11 @@ static void usage(const char *progname)
> >         }
> >  }
> > 
> > +static bool is_fsp(void)
> > +{
> > +       return access(FDT_FSP_NODE, F_OK) == 0;
> > +}
> > +
> >  static struct option global_options[] = {
> >         { "file", required_argument, 0, 'f' },
> >         { "part", no_argument, 0, 'p' },
> > @@ -586,6 +593,12 @@ int main(int argc, char **argv)
> >         ctx = &_ctx;
> >         memset(ctx, 0, sizeof(*ctx));
> > 
> > +       if (is_fsp()) {
> > +               fprintf(stderr, "ERROR: This tool is not compatible with FSP systems\n\n");
> 
> Can you change this to be something like "This is the OpenPower gard
> tool which does not support FSP systems"? I can see people getting
> confused with the current message.
> 

Thanks for the suggestion Oliver, I'll let a few other people make
their own glorious suggestions before sending v2.

Cyril

> > +               usage(progname);
> > +               return EXIT_FAILURE;
> > +       }
> > +
> >         /* process global options */
> >         for (;;) {
> >                 int c;
> > --
> > 2.12.1
> > 
> > _______________________________________________
> > Skiboot mailing list
> > Skiboot@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/skiboot
> 
>