Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754692AbbKXQ4p (ORCPT ); Tue, 24 Nov 2015 11:56:45 -0500 Received: from SMTP.ANDREW.CMU.EDU ([128.2.105.203]:35683 "EHLO smtp.andrew.cmu.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753897AbbKXQ4n (ORCPT ); Tue, 24 Nov 2015 11:56:43 -0500 Date: Tue, 24 Nov 2015 11:55:53 -0500 From: "Gabriel L. Somlo" To: kbuild test robot Cc: kbuild-all@01.org, gregkh@linuxfoundation.org, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, arnd@arndb.de, lersek@redhat.com, ralf@linux-mips.org, rmk+kernel@arm.linux.org.uk, eric@anholt.net, hanjun.guo@linaro.org, zajec5@gmail.com, sudeep.holla@arm.com, agross@codeaurora.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, qemu-devel@nongnu.org, jordan.l.justen@intel.com, mst@redhat.com, peter.maydell@linaro.org, leif.lindholm@linaro.org, ard.biesheuvel@linaro.org, pbonzini@redhat.com, kraxel@redhat.com, luto@amacapital.net, stefanha@gmail.com, revol@free.fr Subject: Re: [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device Message-ID: <20151124165553.GA22627@HEDWIG.INI.CMU.EDU> References: <1448294264-17388-2-git-send-email-somlo@cmu.edu> <201511240404.AFpczj7x%fengguang.wu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201511240404.AFpczj7x%fengguang.wu@intel.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.24 (2015-08-30) X-PMX-Version: 6.0.3.2322014, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2015.11.24.164517 X-SMTP-Spam-Clean: 8% ( MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, DATE_TZ_NA 0, FROM_EDU_TLD 0, REFERENCES 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CD 0, __CT 0, __CT_TEXT_PLAIN 0, __FORWARDED_MSG 0, __HAS_FROM 0, __HAS_MSGID 0, __HTTPS_URI 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_URI_TEXT 0, __PHISH_SPEAR_STRUCTURE_1 0, __REFERENCES 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __SUBJ_ALPHA_NEGATE 0, __TO_MALFORMED_2 0, __URI_IN_BODY 0, __URI_NO_MAILTO 0, __URI_NO_PATH 0, __URI_NO_WWW 0, __URI_NS , __USER_AGENT 0) X-SMTP-Spam-Score: 8% Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7099 Lines: 166 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 "@[::]" 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 "" portion of command line argument */ > 505 size = memparse(arg, &str); > 506 > 507 /* get "@[::]" 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 * is mandatory, optionally followed by > 514 * and ; > 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/