On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: > Hi Gabriel, > > [auto build test WARNING on v4.4-rc2] > [also build test WARNING on next-20151123] > [cannot apply to robh/for-next] > > url: https://github.com/0day-ci/linux/commits/Gabriel-L-Somlo/SysFS-driver-for-QEMU-fw_cfg-device/20151124-000402 > config: arm-allyesconfig (attached as .config) > reproduce: > wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=arm > > All warnings (new ones prefixed by >>): > > drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': > >> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects argument of type 'long long int *', but argument 3 has type 'phys_addr_t *' [-Wformat=] > &ctrl_off, &data_off, &consumed); > ^ Oh, I think I know why this happened: ... phys_addr_t base; resource_size_t size, ctrl_off, data_off; ... /* get "@<base>[:<ctrl_off>:<data_off>]" chunks */ processed = sscanf(str, "@%lli%n:%lli:%lli%n" &base, &consumed, &ctrl_off, &data_off, &consumed); ... On some architectures, phys_addr_t (and resource_size_t) are u32, rather than u64. In include/linux/types.h we have: ... #ifdef CONFIG_PHYS_ADDR_T_64BIT typedef u64 phys_addr_t; #else typedef u32 phys_addr_t; #endif ... So, I could use u64 instead of phys_addr_t and resource_size_t, and keep "%lli" (or "%Li"), but then I'd have to check if the parsed value would overflow a 32-bit address value on arches where phys_addr_t is u32, which would make things a bit more messy and awkward. I'm planning on #ifdef-ing the format string instead: #ifdef CONFIG_PHYS_ADDR_T_64BIT #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" #else #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" #endif ... processed = sscanf(str, PH_ADDR_SCAN_FMT, &base, &consumed, &ctrl_off, &data_off, &consumed); This should make the warning go away, and be correct. Does that sound like a valid plan, or is there some other stylistic reason I should stay away from doing it that way ? thanks, --Gabriel > >> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects argument of type 'long long int *', but argument 5 has type 'resource_size_t *' [-Wformat=] > drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects argument of type 'long long int *', but argument 6 has type 'resource_size_t *' [-Wformat=] > drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_get': > >> drivers/firmware/qemu_fw_cfg.c:563:5: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t' [-Wformat=] > fw_cfg_cmdline_dev->resource[0].start); > ^ > drivers/firmware/qemu_fw_cfg.c:563:5: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t' [-Wformat=] > drivers/firmware/qemu_fw_cfg.c:569:5: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t' [-Wformat=] > fw_cfg_cmdline_dev->resource[2].start); > ^ > drivers/firmware/qemu_fw_cfg.c:569:5: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t' [-Wformat=] > >> drivers/firmware/qemu_fw_cfg.c:569:5: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 6 has type 'resource_size_t' [-Wformat=] > drivers/firmware/qemu_fw_cfg.c:569:5: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 7 has type 'resource_size_t' [-Wformat=] > > vim +510 drivers/firmware/qemu_fw_cfg.c > > 504 /* consume "<size>" portion of command line argument */ > 505 size = memparse(arg, &str); > 506 > 507 /* get "@<base>[:<ctrl_off>:<data_off>]" chunks */ > 508 processed = sscanf(str, "@%lli%n:%lli:%lli%n", > 509 &base, &consumed, > > 510 &ctrl_off, &data_off, &consumed); > 511 > 512 /* sscanf() must process precisely 1 or 3 chunks: > 513 * <base> is mandatory, optionally followed by <ctrl_off> > 514 * and <data_off>; > 515 * there must be no extra characters after the last chunk, > 516 * so str[consumed] must be '\0'. > 517 */ > 518 if (str[consumed] || > 519 (processed != 1 && processed != 3)) > 520 return -EINVAL; > 521 > 522 res[0].start = base; > 523 res[0].end = base + size - 1; > 524 res[0].flags = !strcmp(kp->name, "mmio") ? IORESOURCE_MEM : > 525 IORESOURCE_IO; > 526 > 527 /* insert register offsets, if provided */ > 528 if (processed > 1) { > 529 res[1].name = "ctrl"; > 530 res[1].start = ctrl_off; > 531 res[1].flags = IORESOURCE_REG; > 532 res[2].name = "data"; > 533 res[2].start = data_off; > 534 res[2].flags = IORESOURCE_REG; > 535 } > 536 > 537 /* "processed" happens to nicely match the number of resources > 538 * we need to pass in to this platform device. > 539 */ > 540 fw_cfg_cmdline_dev = platform_device_register_simple("fw_cfg", > 541 PLATFORM_DEVID_NONE, res, processed); > 542 if (IS_ERR(fw_cfg_cmdline_dev)) > 543 return PTR_ERR(fw_cfg_cmdline_dev); > 544 > 545 return 0; > 546 } > 547 > 548 static int fw_cfg_cmdline_get(char *buf, const struct kernel_param *kp) > 549 { > 550 /* stay silent if device was not configured via the command > 551 * line, or if the parameter name (ioport/mmio) doesn't match > 552 * the device setting > 553 */ > 554 if (!fw_cfg_cmdline_dev || > 555 (!strcmp(kp->name, "mmio") ^ > 556 (fw_cfg_cmdline_dev->resource[0].flags == IORESOURCE_MEM))) > 557 return 0; > 558 > 559 switch (fw_cfg_cmdline_dev->num_resources) { > 560 case 1: > 561 return snprintf(buf, PAGE_SIZE, "0x%llx@0x%llx", > 562 resource_size(&fw_cfg_cmdline_dev->resource[0]), > > 563 fw_cfg_cmdline_dev->resource[0].start); > 564 case 3: > 565 return snprintf(buf, PAGE_SIZE, "0x%llx@0x%llx:%llu:%llu", > 566 resource_size(&fw_cfg_cmdline_dev->resource[0]), > 567 fw_cfg_cmdline_dev->resource[0].start, > 568 fw_cfg_cmdline_dev->resource[1].start, > > 569 fw_cfg_cmdline_dev->resource[2].start); > 570 } > 571 > 572 /* Should never get here */ > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
