| Message ID | 434028dedd35194616a4462f547ece391239e62c.1462285023.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On Wed, 04 May 2016, Marius Vlad <marius.c.vlad@intel.com> wrote: > 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). That's a change in how the DEVICE environment variable was handled previously, but indeed should be changed. Thanks, Jani. >> + } >> + 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
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); + } + 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);
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(-)