| Message ID | 51EF8530.90809@monstr.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday, July 24, 2013 4:42 PM, Michal Simek wrote: > On 07/24/2013 09:18 AM, Michal Simek wrote: > > On 07/24/2013 08:00 AM, Jingoo Han wrote: > >> On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote: > >>> > >>> regs_phys is phys_addr_t (u32 or u64). > >>> Lets retype it to u64. > >>> > >>> Fixes compilation warning introduced by: > >>> video: xilinxfb: Use drvdata->regs_phys instead of physaddr > >>> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81) > >> > >> CC'ed Stepan Moskovchenko > >> > >> > >> phys_addr_t is defined as below: > >> > >> #ifdef CONFIG_PHYS_ADDR_T_64BIT > >> typedef u64 phys_addr_t; > >> #else > >> typedef u32 phys_addr_t; > >> #endif > >> > >> According to 'Documentation/printk-formats.txt', > >> Physical addresses: > >> %pa 0x01234567 or 0x0123456789abcdef > >> For printing a phys_addr_t type (and its derivatives, such as > >> resource_size_t) which can vary based on build options, regardless of > >> the width of the CPU data path. Passed by reference. > >> > >> Thus, '%pa' option looks proper, instead of casting (unsigned long long). > >> > >> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys, > >> drvdata->regs); > >> > > > > Ah. Wasn't aware about that. > > Will retest. > > On ppc44x_defconfig > > $ powerpc-unknown-linux-gnu-gcc --version > powerpc-unknown-linux-gnu-gcc (crosstool-NG-svn_unknown@20110406.104745) 4.3.2 > > This fix > diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c > index 79175a6..a9a1167 100644 > --- a/drivers/video/xilinxfb.c > +++ b/drivers/video/xilinxfb.c > @@ -341,12 +341,12 @@ static int xilinxfb_assign(struct platform_device *pdev, > > if (drvdata->flags & BUS_ACCESS_FLAG) { > /* Put a banner in the log (for DEBUG) */ > - dev_dbg(dev, "regs: phys=%llx, virt=%p\n", > - (unsigned long long)drvdata->regs_phys, drvdata->regs); > + dev_dbg(dev, "regs: phys=%pa, virt=%p\n", > + drvdata->regs_phys, drvdata->regs); > } > /* Put a banner in the log (for DEBUG) */ > - dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n", > - (unsigned long long)drvdata->fb_phys, drvdata->fb_virt, fbsize); > + dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n", > + drvdata->fb_phys, drvdata->fb_virt, fbsize); > Hi Michal Simek, Just now, I tested that the same problem happens on ARM config. Also, I solved it by adding '&' operator. '&' operator is necessary as below: dev_dbg(dev, "regs: phys=%pa, virt=%p\n", &drvdata->regs_phys, drvdata->regs); ^^^ dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n", &drvdata->fb_phys, drvdata->fb_virt, fbsize); ^^^ Best regards, Jingoo Han > return 0; /* success */ > > Generates two warnings even it should be ok according to link to specification you sent. > CC [M] drivers/video/xilinxfb.o > drivers/video/xilinxfb.c: In function 'xilinxfb_assign': > drivers/video/xilinxfb.c:344: warning: format '%p' expects type 'void *', but argument 4 has type > 'phys_addr_t' > drivers/video/xilinxfb.c:348: warning: format '%p' expects type 'void *', but argument 4 has type > 'dma_addr_t' > > On microblaze toolchain I see the same warnings. (mmu_defconfig) > > I have also grepped the kernel and I see that it is used in 4 c files which seems to me > weird because phy_addr_t or dma_addr_t are used on a lot of places. > > Thanks, > Michal > > -- > Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 > w: www.monstr.eu p: +42-0-721842854 > Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ > Maintainer of Linux kernel - Xilinx Zynq ARM architecture > Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/24/2013 10:08 AM, Jingoo Han wrote: > On Wednesday, July 24, 2013 4:42 PM, Michal Simek wrote: >> On 07/24/2013 09:18 AM, Michal Simek wrote: >>> On 07/24/2013 08:00 AM, Jingoo Han wrote: >>>> On Wednesday, July 24, 2013 2:32 PM, Michal Simek wrote: >>>>> >>>>> regs_phys is phys_addr_t (u32 or u64). >>>>> Lets retype it to u64. >>>>> >>>>> Fixes compilation warning introduced by: >>>>> video: xilinxfb: Use drvdata->regs_phys instead of physaddr >>>>> (sha1: c88fafef0135e1e1c3e23c3e32ccbeeabc587f81) >>>> >>>> CC'ed Stepan Moskovchenko >>>> >>>> >>>> phys_addr_t is defined as below: >>>> >>>> #ifdef CONFIG_PHYS_ADDR_T_64BIT >>>> typedef u64 phys_addr_t; >>>> #else >>>> typedef u32 phys_addr_t; >>>> #endif >>>> >>>> According to 'Documentation/printk-formats.txt', >>>> Physical addresses: >>>> %pa 0x01234567 or 0x0123456789abcdef >>>> For printing a phys_addr_t type (and its derivatives, such as >>>> resource_size_t) which can vary based on build options, regardless of >>>> the width of the CPU data path. Passed by reference. >>>> >>>> Thus, '%pa' option looks proper, instead of casting (unsigned long long). >>>> >>>> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", drvdata->regs_phys, >>>> drvdata->regs); >>>> >>> >>> Ah. Wasn't aware about that. >>> Will retest. >> >> On ppc44x_defconfig >> >> $ powerpc-unknown-linux-gnu-gcc --version >> powerpc-unknown-linux-gnu-gcc (crosstool-NG-svn_unknown@20110406.104745) 4.3.2 >> >> This fix >> diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c >> index 79175a6..a9a1167 100644 >> --- a/drivers/video/xilinxfb.c >> +++ b/drivers/video/xilinxfb.c >> @@ -341,12 +341,12 @@ static int xilinxfb_assign(struct platform_device *pdev, >> >> if (drvdata->flags & BUS_ACCESS_FLAG) { >> /* Put a banner in the log (for DEBUG) */ >> - dev_dbg(dev, "regs: phys=%llx, virt=%p\n", >> - (unsigned long long)drvdata->regs_phys, drvdata->regs); >> + dev_dbg(dev, "regs: phys=%pa, virt=%p\n", >> + drvdata->regs_phys, drvdata->regs); >> } >> /* Put a banner in the log (for DEBUG) */ >> - dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n", >> - (unsigned long long)drvdata->fb_phys, drvdata->fb_virt, fbsize); >> + dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n", >> + drvdata->fb_phys, drvdata->fb_virt, fbsize); >> > > Hi Michal Simek, > > Just now, I tested that the same problem happens on ARM config. > Also, I solved it by adding '&' operator. > > '&' operator is necessary as below: > > dev_dbg(dev, "regs: phys=%pa, virt=%p\n", > &drvdata->regs_phys, drvdata->regs); > ^^^ > > dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n", > &drvdata->fb_phys, drvdata->fb_virt, fbsize); > ^^^ ok. The truth is as I said there are just 5 files in the whole kernel which are using %pa and if %pa is right way to go we should probably fix all off them. Thanks, Michal
On Wed, Jul 24, 2013 at 10:24 AM, Michal Simek <monstr@monstr.eu> wrote: >> Just now, I tested that the same problem happens on ARM config. >> Also, I solved it by adding '&' operator. >> >> '&' operator is necessary as below: >> >> dev_dbg(dev, "regs: phys=%pa, virt=%p\n", >> &drvdata->regs_phys, drvdata->regs); >> ^^^ >> >> dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n", >> &drvdata->fb_phys, drvdata->fb_virt, fbsize); >> ^^^ > > ok. > > The truth is as I said there are just 5 files in the whole kernel > which are using %pa and if %pa is right way to go we should > probably fix all off them. "%pa" is a quite recent addition (v3.9). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c index 79175a6..a9a1167 100644 --- a/drivers/video/xilinxfb.c +++ b/drivers/video/xilinxfb.c @@ -341,12 +341,12 @@ static int xilinxfb_assign(struct platform_device *pdev, if (drvdata->flags & BUS_ACCESS_FLAG) { /* Put a banner in the log (for DEBUG) */ - dev_dbg(dev, "regs: phys=%llx, virt=%p\n", - (unsigned long long)drvdata->regs_phys, drvdata->regs); + dev_dbg(dev, "regs: phys=%pa, virt=%p\n", + drvdata->regs_phys, drvdata->regs); } /* Put a banner in the log (for DEBUG) */ - dev_dbg(dev, "fb: phys=%llx, virt=%p, size=%x\n", - (unsigned long long)drvdata->fb_phys, drvdata->fb_virt, fbsize); + dev_dbg(dev, "fb: phys=%pa, virt=%p, size=%x\n", + drvdata->fb_phys, drvdata->fb_virt, fbsize); return 0; /* success */