From: Randy Dunlap <[email protected]>
Hi,
Here are a few patches that fix build errors or warnings that
I encountered while doing arc "allmodconfig" builds.
These patches do not fix all of the build issues.
arch/arc/include/asm/delay.h | 2 ++
arch/arc/mm/cache.c | 11 ++++++-----
arch/arc/plat-eznps/include/plat/ctop.h | 1 +
arch/arc/plat-eznps/mtm.c | 6 ++++--
4 files changed, 13 insertions(+), 7 deletions(-)
From: Randy Dunlap <[email protected]>
Fix printk format warning in arch/arc/plat-eznps/mtm.c:
In file included from ../include/linux/printk.h:7,
from ../include/linux/kernel.h:14,
from ../include/linux/list.h:9,
from ../include/linux/smp.h:12,
from ../arch/arc/plat-eznps/mtm.c:17:
../arch/arc/plat-eznps/mtm.c: In function 'set_mtm_hs_ctr':
../include/linux/kern_levels.h:5:18: warning: format '%d' expects argument of type 'int', but argument 2 has type 'long int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
../include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
../include/linux/printk.h:308:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
../arch/arc/plat-eznps/mtm.c:166:3: note: in expansion of macro 'pr_err'
pr_err("** Invalid @nps_mtm_hs_ctr [%d] needs to be [%d:%d] (incl)\n",
^~~~~~
../arch/arc/plat-eznps/mtm.c:166:40: note: format string is defined here
pr_err("** Invalid @nps_mtm_hs_ctr [%d] needs to be [%d:%d] (incl)\n",
~^
%ld
The hs_ctr variable can just be int instead of long, so also change
kstrtol() to kstrtoint() and leave the format string as %d.
Also add 2 header files since they are used in mtm.c and we prefer
not to depend on accidental/indirect #includes.
Signed-off-by: Randy Dunlap <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: Ofer Levi <[email protected]>
Cc: [email protected]
---
arch/arc/plat-eznps/mtm.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
--- linux-next-20180723.orig/arch/arc/plat-eznps/mtm.c
+++ linux-next-20180723/arch/arc/plat-eznps/mtm.c
@@ -15,6 +15,8 @@
*/
#include <linux/smp.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
#include <linux/io.h>
#include <linux/log2.h>
#include <asm/arcregs.h>
@@ -157,10 +159,10 @@ void mtm_enable_core(unsigned int cpu)
/* Verify and set the value of the mtm hs counter */
static int __init set_mtm_hs_ctr(char *ctr_str)
{
- long hs_ctr;
+ int hs_ctr;
int ret;
- ret = kstrtol(ctr_str, 0, &hs_ctr);
+ ret = kstrtoint(ctr_str, 0, &hs_ctr);
if (ret || hs_ctr > MT_HS_CNT_MAX || hs_ctr < MT_HS_CNT_MIN) {
pr_err("** Invalid @nps_mtm_hs_ctr [%d] needs to be [%d:%d] (incl)\n",
From: Randy Dunlap <[email protected]>
../arch/arc/mm/cache.c: In function 'flush_anon_page':
../arch/arc/mm/cache.c:1062:55: warning: passing argument 2 of '__flush_dcache_page' makes integer from pointer without a cast [-Wint-conversion]
__flush_dcache_page((phys_addr_t)page_address(page), page_address(page));
^~~~~~~~~~~~~~~~~~
../arch/arc/mm/cache.c:1013:59: note: expected 'long unsigned int' but argument is of type 'void *'
void __flush_dcache_page(phys_addr_t paddr, unsigned long vaddr)
~~~~~~~~~~~~~~^~~~~
Signed-off-by: Randy Dunlap <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: Ofer Levi <[email protected]>
Cc: [email protected]
---
arch/arc/mm/cache.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
--- linux-next-20180723.orig/arch/arc/mm/cache.c
+++ linux-next-20180723/arch/arc/mm/cache.c
@@ -1038,7 +1038,7 @@ void flush_cache_mm(struct mm_struct *mm
void flush_cache_page(struct vm_area_struct *vma, unsigned long u_vaddr,
unsigned long pfn)
{
- unsigned int paddr = pfn << PAGE_SHIFT;
+ phys_addr_t paddr = pfn << PAGE_SHIFT;
u_vaddr &= PAGE_MASK;
@@ -1058,8 +1058,9 @@ void flush_anon_page(struct vm_area_stru
unsigned long u_vaddr)
{
/* TBD: do we really need to clear the kernel mapping */
- __flush_dcache_page(page_address(page), u_vaddr);
- __flush_dcache_page(page_address(page), page_address(page));
+ __flush_dcache_page((phys_addr_t)page_address(page), u_vaddr);
+ __flush_dcache_page((phys_addr_t)page_address(page),
+ (phys_addr_t)page_address(page));
}
@@ -1084,7 +1085,7 @@ void copy_user_highpage(struct page *to,
* addr_not_cache_congruent() is 0
*/
if (page_mapcount(from) && addr_not_cache_congruent(kfrom, u_vaddr)) {
- __flush_dcache_page((unsigned long)kfrom, u_vaddr);
+ __flush_dcache_page((phys_addr_t)kfrom, u_vaddr);
clean_src_k_mappings = 1;
}
@@ -1105,7 +1106,7 @@ void copy_user_highpage(struct page *to,
* sync the kernel mapping back to physical page
*/
if (clean_src_k_mappings) {
- __flush_dcache_page((unsigned long)kfrom, (unsigned long)kfrom);
+ __flush_dcache_page((phys_addr_t)kfrom, (unsigned long)kfrom);
set_bit(PG_dc_clean, &from->flags);
} else {
clear_bit(PG_dc_clean, &from->flags);
From: Randy Dunlap <[email protected]>
Fix build errors in arch/arc/'s delay.h:
- add <linux/delay.h> for loops_per_jiffy
- add <asm-generic/types.h> for "u64"
In file included from ../drivers/infiniband/hw/cxgb3/cxio_hal.c:32:
../arch/arc/include/asm/delay.h: In function '__udelay':
../arch/arc/include/asm/delay.h:61:12: error: 'u64' undeclared (first use in this function)
loops = ((u64) usecs * 4295 * HZ * loops_per_jiffy) >> 32;
^~~
In file included from ../drivers/infiniband/hw/cxgb3/cxio_hal.c:32:
../arch/arc/include/asm/delay.h: In function '__udelay':
../arch/arc/include/asm/delay.h:63:37: error: 'loops_per_jiffy' undeclared (first use in this function)
loops = ((u64) usecs * 4295 * HZ * loops_per_jiffy) >> 32;
^~~~~~~~~~~~~~~
Signed-off-by: Randy Dunlap <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: Ofer Levi <[email protected]>
Cc: [email protected]
---
arch/arc/include/asm/delay.h | 2 ++
1 file changed, 2 insertions(+)
--- linux-next-20180723.orig/arch/arc/include/asm/delay.h
+++ linux-next-20180723/arch/arc/include/asm/delay.h
@@ -17,6 +17,8 @@
#ifndef __ASM_ARC_UDELAY_H
#define __ASM_ARC_UDELAY_H
+#include <linux/delay.h>
+#include <asm-generic/types.h>
#include <asm/param.h> /* HZ */
static inline void __delay(unsigned long loops)
From: Randy Dunlap <[email protected]>
Add <asm-generic/types.h> to fix build errors.
Both ctop.h and <soc/nps/common.h> use u32 types and cause many
errors.
Examples:
../include/soc/nps/common.h:71:4: error: unknown type name 'u32'
u32 __reserved:20, cluster:4, core:4, thread:4;
../include/soc/nps/common.h:76:3: error: unknown type name 'u32'
u32 value;
../include/soc/nps/common.h:124:4: error: unknown type name 'u32'
u32 base:8, cl_x:4, cl_y:4,
../include/soc/nps/common.h:127:3: error: unknown type name 'u32'
u32 value;
../arch/arc/plat-eznps/include/plat/ctop.h:83:4: error: unknown type name 'u32'
u32 gen:1, gdis:1, clk_gate_dis:1, asb:1,
../arch/arc/plat-eznps/include/plat/ctop.h:86:3: error: unknown type name 'u32'
u32 value;
../arch/arc/plat-eznps/include/plat/ctop.h:93:4: error: unknown type name 'u32'
u32 csa:22, dmsid:6, __reserved:3, cs:1;
../arch/arc/plat-eznps/include/plat/ctop.h:95:3: error: unknown type name 'u32'
u32 value;
Signed-off-by: Randy Dunlap <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: Ofer Levi <[email protected]>
Cc: [email protected]
---
arch/arc/plat-eznps/include/plat/ctop.h | 1 +
1 file changed, 1 insertion(+)
--- linux-next-20180723.orig/arch/arc/plat-eznps/include/plat/ctop.h
+++ linux-next-20180723/arch/arc/plat-eznps/include/plat/ctop.h
@@ -21,6 +21,7 @@
#error "Incorrect ctop.h include"
#endif
+#include <asm-generic/types.h>
#include <soc/nps/common.h>
/* core auxiliary registers */
On 07/24/2018 01:46 PM, Randy Dunlap wrote:
> From: Randy Dunlap <[email protected]>
>
> Fix build errors in arch/arc/'s delay.h:
> - add <linux/delay.h> for loops_per_jiffy
> - add <asm-generic/types.h> for "u64"
>
> In file included from ../drivers/infiniband/hw/cxgb3/cxio_hal.c:32:
> ../arch/arc/include/asm/delay.h: In function '__udelay':
> ../arch/arc/include/asm/delay.h:61:12: error: 'u64' undeclared (first use in this function)
> loops = ((u64) usecs * 4295 * HZ * loops_per_jiffy) >> 32;
> ^~~
>
> In file included from ../drivers/infiniband/hw/cxgb3/cxio_hal.c:32:
> ../arch/arc/include/asm/delay.h: In function '__udelay':
> ../arch/arc/include/asm/delay.h:63:37: error: 'loops_per_jiffy' undeclared (first use in this function)
> loops = ((u64) usecs * 4295 * HZ * loops_per_jiffy) >> 32;
> ^~~~~~~~~~~~~~~
>
> Signed-off-by: Randy Dunlap <[email protected]>
> Cc: Vineet Gupta <[email protected]>
> Cc: Ofer Levi <[email protected]>
> Cc: [email protected]
> ---
> arch/arc/include/asm/delay.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-next-20180723.orig/arch/arc/include/asm/delay.h
> +++ linux-next-20180723/arch/arc/include/asm/delay.h
> @@ -17,6 +17,8 @@
> #ifndef __ASM_ARC_UDELAY_H
> #define __ASM_ARC_UDELAY_H
>
> +#include <linux/delay.h>
this file in turn includes asm/delay.h - the header guards will catch nested
inclusion still seems ugly. Should we just reference loops_per_jiffy here like
other arches do in theur delay.h
> +#include <asm-generic/types.h>
> #include <asm/param.h> /* HZ */
>
> static inline void __delay(unsigned long loops)
>
On 07/24/2018 03:04 PM, Vineet Gupta wrote:
> On 07/24/2018 01:46 PM, Randy Dunlap wrote:
>> From: Randy Dunlap <[email protected]>
>>
>> Fix build errors in arch/arc/'s delay.h:
>> - add <linux/delay.h> for loops_per_jiffy
>> - add <asm-generic/types.h> for "u64"
>>
>> In file included from ../drivers/infiniband/hw/cxgb3/cxio_hal.c:32:
>> ../arch/arc/include/asm/delay.h: In function '__udelay':
>> ../arch/arc/include/asm/delay.h:61:12: error: 'u64' undeclared (first use in this function)
>> loops = ((u64) usecs * 4295 * HZ * loops_per_jiffy) >> 32;
>> ^~~
>>
>> In file included from ../drivers/infiniband/hw/cxgb3/cxio_hal.c:32:
>> ../arch/arc/include/asm/delay.h: In function '__udelay':
>> ../arch/arc/include/asm/delay.h:63:37: error: 'loops_per_jiffy' undeclared (first use in this function)
>> loops = ((u64) usecs * 4295 * HZ * loops_per_jiffy) >> 32;
>> ^~~~~~~~~~~~~~~
>>
>> Signed-off-by: Randy Dunlap <[email protected]>
>> Cc: Vineet Gupta <[email protected]>
>> Cc: Ofer Levi <[email protected]>
>> Cc: [email protected]
>> ---
>> arch/arc/include/asm/delay.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> --- linux-next-20180723.orig/arch/arc/include/asm/delay.h
>> +++ linux-next-20180723/arch/arc/include/asm/delay.h
>> @@ -17,6 +17,8 @@
>> #ifndef __ASM_ARC_UDELAY_H
>> #define __ASM_ARC_UDELAY_H
>>
>> +#include <linux/delay.h>
>
> this file in turn includes asm/delay.h - the header guards will catch nested
> inclusion still seems ugly. Should we just reference loops_per_jiffy here like
> other arches do in theur delay.h
Yes, that's a good idea. I'll do that.
>> +#include <asm-generic/types.h>
>> #include <asm/param.h> /* HZ */
>>
>> static inline void __delay(unsigned long loops)
>>
>
--
~Randy
On 07/24/2018 01:46 PM, Randy Dunlap wrote:
> From: Randy Dunlap <[email protected]>
>
> ../arch/arc/mm/cache.c: In function 'flush_anon_page':
> ../arch/arc/mm/cache.c:1062:55: warning: passing argument 2 of '__flush_dcache_page' makes integer from pointer without a cast [-Wint-conversion]
> __flush_dcache_page((phys_addr_t)page_address(page), page_address(page));
> ^~~~~~~~~~~~~~~~~~
> ../arch/arc/mm/cache.c:1013:59: note: expected 'long unsigned int' but argument is of type 'void *'
> void __flush_dcache_page(phys_addr_t paddr, unsigned long vaddr)
> ~~~~~~~~~~~~~~^~~~~
>
> Signed-off-by: Randy Dunlap <[email protected]>
> Cc: Vineet Gupta <[email protected]>
> Cc: Ofer Levi <[email protected]>
> Cc: [email protected]
> ---
> arch/arc/mm/cache.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> --- linux-next-20180723.orig/arch/arc/mm/cache.c
> +++ linux-next-20180723/arch/arc/mm/cache.c
> @@ -1038,7 +1038,7 @@ void flush_cache_mm(struct mm_struct *mm
> void flush_cache_page(struct vm_area_struct *vma, unsigned long u_vaddr,
> unsigned long pfn)
> {
> - unsigned int paddr = pfn << PAGE_SHIFT;
> + phys_addr_t paddr = pfn << PAGE_SHIFT;
>
> u_vaddr &= PAGE_MASK;
>
> @@ -1058,8 +1058,9 @@ void flush_anon_page(struct vm_area_stru
> unsigned long u_vaddr)
> {
> /* TBD: do we really need to clear the kernel mapping */
> - __flush_dcache_page(page_address(page), u_vaddr);
> - __flush_dcache_page(page_address(page), page_address(page));
> + __flush_dcache_page((phys_addr_t)page_address(page), u_vaddr);
> + __flush_dcache_page((phys_addr_t)page_address(page),
> + (phys_addr_t)page_address(page));
CONFIG_ARC_CACHE_VIPT_ALIASING is a total bitrot and I'm considering removing it
as no sane systems should use it. There's none which I know are using it.
But for now this will do.
>
> }
>
> @@ -1084,7 +1085,7 @@ void copy_user_highpage(struct page *to,
> * addr_not_cache_congruent() is 0
> */
> if (page_mapcount(from) && addr_not_cache_congruent(kfrom, u_vaddr)) {
> - __flush_dcache_page((unsigned long)kfrom, u_vaddr);
> + __flush_dcache_page((phys_addr_t)kfrom, u_vaddr);
Was this needed really ? I can't seem to trigger any warnings here.
All of this (original code before your change) is technically incorrect due to the
combination of CONFIG_ARC_CACHE_VIPT_ALIASING and CONFIG_HIGHMEM. @kfrom is
returned by kmap_atomic() and in highmem regime it could be a kernel virtual
address, so calling the low level cache flush interface with a kvaddr, pretending
it is paddr (whethe runsigned long or phys_addr_t) is just not right. This
combinations needs to be disallowed from Kconfig !
> clean_src_k_mappings = 1;
> }
>
> @@ -1105,7 +1106,7 @@ void copy_user_highpage(struct page *to,
> * sync the kernel mapping back to physical page
> */
> if (clean_src_k_mappings) {
> - __flush_dcache_page((unsigned long)kfrom, (unsigned long)kfrom);
> + __flush_dcache_page((phys_addr_t)kfrom, (unsigned long)kfrom);
> set_bit(PG_dc_clean, &from->flags);
> } else {
> clear_bit(PG_dc_clean, &from->flags);
>
Ditto !
On 07/24/2018 05:02 PM, Vineet Gupta wrote:
> On 07/24/2018 01:46 PM, Randy Dunlap wrote:
>> From: Randy Dunlap <[email protected]>
>>
>> ../arch/arc/mm/cache.c: In function 'flush_anon_page':
>> ../arch/arc/mm/cache.c:1062:55: warning: passing argument 2 of '__flush_dcache_page' makes integer from pointer without a cast [-Wint-conversion]
>> __flush_dcache_page((phys_addr_t)page_address(page), page_address(page));
>> ^~~~~~~~~~~~~~~~~~
>> ../arch/arc/mm/cache.c:1013:59: note: expected 'long unsigned int' but argument is of type 'void *'
>> void __flush_dcache_page(phys_addr_t paddr, unsigned long vaddr)
>> ~~~~~~~~~~~~~~^~~~~
>>
>> Signed-off-by: Randy Dunlap <[email protected]>
>> Cc: Vineet Gupta <[email protected]>
>> Cc: Ofer Levi <[email protected]>
>> Cc: [email protected]
>> ---
>> arch/arc/mm/cache.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> --- linux-next-20180723.orig/arch/arc/mm/cache.c
>> +++ linux-next-20180723/arch/arc/mm/cache.c
>> @@ -1038,7 +1038,7 @@ void flush_cache_mm(struct mm_struct *mm
>> void flush_cache_page(struct vm_area_struct *vma, unsigned long u_vaddr,
>> unsigned long pfn)
>> {
>> - unsigned int paddr = pfn << PAGE_SHIFT;
>> + phys_addr_t paddr = pfn << PAGE_SHIFT;
>>
>> u_vaddr &= PAGE_MASK;
>>
>> @@ -1058,8 +1058,9 @@ void flush_anon_page(struct vm_area_stru
>> unsigned long u_vaddr)
>> {
>> /* TBD: do we really need to clear the kernel mapping */
>> - __flush_dcache_page(page_address(page), u_vaddr);
>> - __flush_dcache_page(page_address(page), page_address(page));
>> + __flush_dcache_page((phys_addr_t)page_address(page), u_vaddr);
>> + __flush_dcache_page((phys_addr_t)page_address(page),
>> + (phys_addr_t)page_address(page));
>
> CONFIG_ARC_CACHE_VIPT_ALIASING is a total bitrot and I'm considering removing it
> as no sane systems should use it. There's none which I know are using it.
> But for now this will do.
>
>
>>
>> }
>>
>> @@ -1084,7 +1085,7 @@ void copy_user_highpage(struct page *to,
>> * addr_not_cache_congruent() is 0
>> */
>> if (page_mapcount(from) && addr_not_cache_congruent(kfrom, u_vaddr)) {
>> - __flush_dcache_page((unsigned long)kfrom, u_vaddr);
>> + __flush_dcache_page((phys_addr_t)kfrom, u_vaddr);
>
> Was this needed really ? I can't seem to trigger any warnings here.
Nope.. glad you caught that. I can't explain it. :(
I can only say that if I had _meant_ to patch that, I would have also included
the warning messages from gcc.
> All of this (original code before your change) is technically incorrect due to the
> combination of CONFIG_ARC_CACHE_VIPT_ALIASING and CONFIG_HIGHMEM. @kfrom is
> returned by kmap_atomic() and in highmem regime it could be a kernel virtual
> address, so calling the low level cache flush interface with a kvaddr, pretending
> it is paddr (whethe runsigned long or phys_addr_t) is just not right. This
> combinations needs to be disallowed from Kconfig !
>
>> clean_src_k_mappings = 1;
>> }
>>
>> @@ -1105,7 +1106,7 @@ void copy_user_highpage(struct page *to,
>> * sync the kernel mapping back to physical page
>> */
>> if (clean_src_k_mappings) {
>> - __flush_dcache_page((unsigned long)kfrom, (unsigned long)kfrom);
>> + __flush_dcache_page((phys_addr_t)kfrom, (unsigned long)kfrom);
>> set_bit(PG_dc_clean, &from->flags);
>> } else {
>> clear_bit(PG_dc_clean, &from->flags);
>>
>
> Ditto !
>
Thanks.
--
~Randy