ioremaped addresses are not linearly mapped so the physical
address can not be figured out via __pa. More generally, there
is no guarantee that backing value of an ioremapped address
is a physical address at all. The value here is only used
for debugging so just drop the call to __pa on the ioremapped
address.
Signed-off-by: Laura Abbott <[email protected]>
---
Found while working on virt_to_phys debugging for arm64
---
drivers/clk/clk-xgene.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-xgene.c b/drivers/clk/clk-xgene.c
index 3433132..4697bf9 100644
--- a/drivers/clk/clk-xgene.c
+++ b/drivers/clk/clk-xgene.c
@@ -243,22 +243,20 @@ static int xgene_clk_enable(struct clk_hw *hw)
struct xgene_clk *pclk = to_xgene_clk(hw);
unsigned long flags = 0;
u32 data;
- phys_addr_t reg;
if (pclk->lock)
spin_lock_irqsave(pclk->lock, flags);
if (pclk->param.csr_reg != NULL) {
pr_debug("%s clock enabled\n", clk_hw_get_name(hw));
- reg = __pa(pclk->param.csr_reg);
/* First enable the clock */
data = xgene_clk_read(pclk->param.csr_reg +
pclk->param.reg_clk_offset);
data |= pclk->param.reg_clk_mask;
xgene_clk_write(data, pclk->param.csr_reg +
pclk->param.reg_clk_offset);
- pr_debug("%s clock PADDR base %pa clk offset 0x%08X mask 0x%08X value 0x%08X\n",
- clk_hw_get_name(hw), ®,
+ pr_debug("%s clk offset 0x%08X mask 0x%08X value 0x%08X\n",
+ clk_hw_get_name(hw),
pclk->param.reg_clk_offset, pclk->param.reg_clk_mask,
data);
@@ -269,7 +267,7 @@ static int xgene_clk_enable(struct clk_hw *hw)
xgene_clk_write(data, pclk->param.csr_reg +
pclk->param.reg_csr_offset);
pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
- clk_hw_get_name(hw), ®,
+ clk_hw_get_name(hw),
pclk->param.reg_csr_offset, pclk->param.reg_csr_mask,
data);
}
--
2.7.4
On 10/27/2016 04:21 PM, Laura Abbott wrote:
> ioremaped addresses are not linearly mapped so the physical
> address can not be figured out via __pa. More generally, there
> is no guarantee that backing value of an ioremapped address
> is a physical address at all. The value here is only used
> for debugging so just drop the call to __pa on the ioremapped
> address.
>
> Signed-off-by: Laura Abbott <[email protected]>
> ---
> Found while working on virt_to_phys debugging for arm64
> ---
> drivers/clk/clk-xgene.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/clk-xgene.c b/drivers/clk/clk-xgene.c
> index 3433132..4697bf9 100644
> --- a/drivers/clk/clk-xgene.c
> +++ b/drivers/clk/clk-xgene.c
> @@ -243,22 +243,20 @@ static int xgene_clk_enable(struct clk_hw *hw)
> struct xgene_clk *pclk = to_xgene_clk(hw);
> unsigned long flags = 0;
> u32 data;
> - phys_addr_t reg;
>
> if (pclk->lock)
> spin_lock_irqsave(pclk->lock, flags);
>
> if (pclk->param.csr_reg != NULL) {
> pr_debug("%s clock enabled\n", clk_hw_get_name(hw));
> - reg = __pa(pclk->param.csr_reg);
> /* First enable the clock */
> data = xgene_clk_read(pclk->param.csr_reg +
> pclk->param.reg_clk_offset);
> data |= pclk->param.reg_clk_mask;
> xgene_clk_write(data, pclk->param.csr_reg +
> pclk->param.reg_clk_offset);
> - pr_debug("%s clock PADDR base %pa clk offset 0x%08X mask 0x%08X value 0x%08X\n",
> - clk_hw_get_name(hw), ®,
> + pr_debug("%s clk offset 0x%08X mask 0x%08X value 0x%08X\n",
> + clk_hw_get_name(hw),
> pclk->param.reg_clk_offset, pclk->param.reg_clk_mask,
> data);
>
> @@ -269,7 +267,7 @@ static int xgene_clk_enable(struct clk_hw *hw)
> xgene_clk_write(data, pclk->param.csr_reg +
> pclk->param.reg_csr_offset);
> pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
Bah, this got fixed locally but didn't get amended. I'll remove
the format here in a v2 assuming there are no other objections.
> - clk_hw_get_name(hw), ®,
> + clk_hw_get_name(hw),
> pclk->param.reg_csr_offset, pclk->param.reg_csr_mask,
> data);
> }
>
On 10/27, Laura Abbott wrote:
> On 10/27/2016 04:21 PM, Laura Abbott wrote:
> >
> >@@ -269,7 +267,7 @@ static int xgene_clk_enable(struct clk_hw *hw)
> > xgene_clk_write(data, pclk->param.csr_reg +
> > pclk->param.reg_csr_offset);
> > pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
>
> Bah, this got fixed locally but didn't get amended. I'll remove
> the format here in a v2 assuming there are no other objections.
>
Include a Fixes: tag too? It would be nice if sparse complained
about __pa used on an __iomem pointer too.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Laura,
[auto build test WARNING on clk/clk-next]
[also build test WARNING on v4.9-rc2 next-20161027]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Laura-Abbott/clk-xgene-Don-t-call-__pa-on-ioremaped-address/20161028-072455
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:13:0,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/clk/clk-xgene.c:23:
drivers/clk/clk-xgene.c: In function 'xgene_clk_enable':
>> drivers/clk/clk-xgene.c:489:12: warning: format '%p' expects argument of type 'void *', but argument 4 has type 'u32 {aka unsigned int}' [-Wformat=]
pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
^
include/linux/printk.h:261:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/printk.h:309:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
>> drivers/clk/clk-xgene.c:489:3: note: in expansion of macro 'pr_debug'
pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
^~~~~~~~
>> drivers/clk/clk-xgene.c:489:12: warning: format '%X' expects a matching 'unsigned int' argument [-Wformat=]
pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
^
include/linux/printk.h:261:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/printk.h:309:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
>> drivers/clk/clk-xgene.c:489:3: note: in expansion of macro 'pr_debug'
pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
^~~~~~~~
vim +489 drivers/clk/clk-xgene.c
308964ca Loc Ho 2013-06-26 473 data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho 2013-06-26 474 pclk->param.reg_clk_offset);
308964ca Loc Ho 2013-06-26 475 data |= pclk->param.reg_clk_mask;
308964ca Loc Ho 2013-06-26 476 xgene_clk_write(data, pclk->param.csr_reg +
308964ca Loc Ho 2013-06-26 477 pclk->param.reg_clk_offset);
1ee1b519 Laura Abbott 2016-10-27 478 pr_debug("%s clk offset 0x%08X mask 0x%08X value 0x%08X\n",
1ee1b519 Laura Abbott 2016-10-27 479 clk_hw_get_name(hw),
308964ca Loc Ho 2013-06-26 480 pclk->param.reg_clk_offset, pclk->param.reg_clk_mask,
308964ca Loc Ho 2013-06-26 481 data);
308964ca Loc Ho 2013-06-26 482
308964ca Loc Ho 2013-06-26 483 /* Second enable the CSR */
308964ca Loc Ho 2013-06-26 484 data = xgene_clk_read(pclk->param.csr_reg +
308964ca Loc Ho 2013-06-26 485 pclk->param.reg_csr_offset);
308964ca Loc Ho 2013-06-26 486 data &= ~pclk->param.reg_csr_mask;
308964ca Loc Ho 2013-06-26 487 xgene_clk_write(data, pclk->param.csr_reg +
308964ca Loc Ho 2013-06-26 488 pclk->param.reg_csr_offset);
6ae5fd38 Stephen Boyd 2015-05-01 @489 pr_debug("%s CSR RESET PADDR base %pa csr offset 0x%08X mask 0x%08X value 0x%08X\n",
1ee1b519 Laura Abbott 2016-10-27 490 clk_hw_get_name(hw),
308964ca Loc Ho 2013-06-26 491 pclk->param.reg_csr_offset, pclk->param.reg_csr_mask,
308964ca Loc Ho 2013-06-26 492 data);
308964ca Loc Ho 2013-06-26 493 }
308964ca Loc Ho 2013-06-26 494
308964ca Loc Ho 2013-06-26 495 if (pclk->lock)
308964ca Loc Ho 2013-06-26 496 spin_unlock_irqrestore(pclk->lock, flags);
308964ca Loc Ho 2013-06-26 497
:::::: The code at line 489 was first introduced by commit
:::::: 6ae5fd381251993a650a7fad51bd9318c19a6c80 clk: xgene: Silence sparse warnings
:::::: TO: Stephen Boyd <[email protected]>
:::::: CC: Stephen Boyd <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation