Re: [PATCH i-g-t 04/17] tools/intel_bios_reader: add --devid parameter

Marius Vlad
2016-05-04T15:22:53Z

[-- Attachment #1.1: Type: text/plain, Size: 3671 bytes --]

On Tue, May 03, 2016 at 05:18:54PM +0300, Jani Nikula wrote:
> Not sure it's a great idea to do platform specific parsing of the BIOS,
> but at least make it possible to pass in the devid on the command line
> and not just the environment.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> tools/intel_bios_reader.c | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c
> index b424b17e4852..d74e766250df 100644
> --- a/tools/intel_bios_reader.c
> +++ b/tools/intel_bios_reader.c
> @@ -41,7 +41,7 @@
> #include "intel_chipset.h"
> #include "drmtest.h"
>
> -static uint32_t devid = -1;
> +static uint32_t devid;
>
> /* no bother to include "edid.h" */
> #define _H_ACTIVE(x) (x[2] + ((x[4] & 0xF0) << 4))
> @@ -149,8 +149,10 @@ static void dump_general_features(const struct bdb_header *bdb,
> printf("\tExternal VBT: %s\n", YESNO(features->download_ext_vbt));
> printf("\tEnable SSC: %s\n", YESNO(features->enable_ssc));
> if (features->enable_ssc) {
> - if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid) ||
> - IS_BROXTON(devid))
> + if (!devid)
> + printf("\tSSC frequency: <unknown platform>\n");
> + else if (IS_VALLEYVIEW(devid) || IS_CHERRYVIEW(devid) ||
> + IS_BROXTON(devid))
> printf("\tSSC frequency: 100 MHz\n");
> else if (HAS_PCH_SPLIT(devid))
> printf("\tSSC frequency: %s\n", features->ssc_freq ?
> @@ -1375,6 +1377,7 @@ enum opt {
> OPT_UNKNOWN = '?',
> OPT_END = -1,
> OPT_FILE,
> + OPT_DEVID,
> };
>
> int main(int argc, char **argv)
> @@ -1392,10 +1395,11 @@ int main(int argc, char **argv)
> struct bdb_block *block;
> struct bdb_header *bdb;
> char signature[17];
> - char *devid_string;
> + char *endp;
>
> static struct option options[] = {
> { "file", required_argument, NULL, OPT_FILE },
> + { "devid", required_argument, NULL, OPT_DEVID },
> { 0 }
> };
>
> @@ -1406,6 +1410,13 @@ int main(int argc, char **argv)
> case OPT_FILE:
> filename = optarg;
> break;
> + case OPT_DEVID:
> + devid = strtoul(optarg, &endp, 16);
> + if (!devid || *endp) {
> + fprintf(stderr, "invalid devid '%s'\n", optarg);
> + return EXIT_FAILURE;
> + }
> + break;
> case OPT_END:
> break;
> case OPT_UNKNOWN:
> @@ -1426,9 +1437,6 @@ int main(int argc, char **argv)
> }
> }
>
> - if ((devid_string = getenv("DEVICE")))
> - devid = strtoul(devid_string, NULL, 0);
> -
> fd = open(filename, O_RDONLY);
> if (fd == -1) {
> printf("Couldn't open \"%s\": %s\n", filename, strerror(errno));
> @@ -1506,10 +1514,15 @@ int main(int argc, char **argv)
> }
> printf("\n");
>
> - if (devid == -1)
> - devid = get_device_id(VBIOS, size);
> - if (devid == -1)
> - printf("Warning: could not find PCI device ID!\n");
> + if (!devid) {
> + const char *devid_string = getenv("DEVICE");
> + if (devid_string)
> + devid = strtoul(devid_string, NULL, 0);
Wouldn't this allow to pass either base 10, 16 or 8? (as an argument
devid seems to be always specified in base 16).
> + }
> + if (!devid)
> + devid = get_device_id(VBIOS, size);
> + if (!devid)
> + fprintf(stderr, "Warning: could not find PCI device ID!\n");
>
> dump_section(bdb, BDB_GENERAL_FEATURES, size);
> dump_section(bdb, BDB_GENERAL_DEFINITIONS, size);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]