Re: [PATCH 3/3] tools/intel_reg: Add option to skip forcewake

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 28 Aug 2015, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Sometimes we want to inspect the hardware as it is,
> without doing the forcewake before access. Add option to skip
> the forcewake dance.

This needs a manual page update.

Some trivial comments below, otherwise lgtm.

>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  tools/intel_reg.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index b082555..c73b9ef 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -70,12 +70,16 @@ struct config {
>  	/* spread out bits for convenience */
>  	bool binary;
>  
> +	/* dont take forcewake on register access */
> +	int nofw;

IMO should be a bool.


> +
>  	/* register spec */
>  	char *specfile;
>  	struct reg *regs;
>  	ssize_t regcount;
>  
>  	int verbosity;
> +

Superfluous whitespace change.

>  };
>  
>  /* port desc must have been set */
> @@ -409,7 +413,7 @@ static int intel_reg_read(struct config *config, int argc, char *argv[])
>  	if (config->mmiofile)
>  		intel_mmio_use_dump_file(config->mmiofile);
>  	else
> -		intel_register_access_init(config->pci_dev, 0, 0);
> +		intel_register_access_init(config->pci_dev, 0, config->nofw);
>  
>  	for (i = 1; i < argc; i++) {
>  		struct reg reg;
> @@ -439,7 +443,7 @@ static int intel_reg_write(struct config *config, int argc, char *argv[])
>  		return EXIT_FAILURE;
>  	}
>  
> -	intel_register_access_init(config->pci_dev, 0, 0);
> +	intel_register_access_init(config->pci_dev, 0, config->nofw);
>  
>  	for (i = 1; i < argc; i += 2) {
>  		struct reg reg;
> @@ -477,7 +481,7 @@ static int intel_reg_dump(struct config *config, int argc, char *argv[])
>  	if (config->mmiofile)
>  		intel_mmio_use_dump_file(config->mmiofile);
>  	else
> -		intel_register_access_init(config->pci_dev, 0, 0);
> +		intel_register_access_init(config->pci_dev, 0, config->nofw);
>  
>  	for (i = 0; i < config->regcount; i++) {
>  		reg = &config->regs[i];
> @@ -637,6 +641,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>  	printf(" --all          Decode registers for all known platforms\n");
>  	printf(" --binary       Binary dump registers\n");
>  	printf(" --verbose      Increase verbosity\n");
> +	printf(" --nofw         No forcewake on register access (DANGEROUS)\n");
>  	printf(" --quiet        Reduce verbosity\n");
>  
>  	printf("\n");
> @@ -769,6 +774,7 @@ enum opt {
>  	OPT_BINARY,
>  	OPT_SPEC,
>  	OPT_VERBOSE,
> +	OPT_NOFW,
>  	OPT_QUIET,
>  	OPT_HELP,
>  };
> @@ -781,6 +787,7 @@ int main(int argc, char *argv[])
>  	const struct command *command = NULL;
>  	struct config config = {
>  		.count = 1,
> +		.nofw = 0,

No need to initialize to zero.

>  	};
>  	bool help = false;
>  
> @@ -800,6 +807,7 @@ int main(int argc, char *argv[])
>  		/* options specific to read, dump and decode */
>  		{ "all",	no_argument,		NULL,	OPT_ALL },
>  		{ "binary",	no_argument,		NULL,	OPT_BINARY },
> +		{ "nofw",	no_argument,		NULL,	OPT_NOFW },
>  		{ 0 }
>  	};
>  
> @@ -846,6 +854,9 @@ int main(int argc, char *argv[])
>  		case OPT_BINARY:
>  			config.binary = true;
>  			break;
> +		case OPT_NOFW:
> +			config.nofw = 1;

= true;

> +			break;
>  		case OPT_VERBOSE:
>  			config.verbosity++;
>  			break;
> -- 
> 2.1.4
>

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx