arc: some allmodconfig fixup patches

PATCH In review Randy Dunlap 3 participants 9 messages Jul 24, 2018 – Jul 25, 2018 Synopsys ARC architecture

lore

Share & tools

lore

Randy Dunlap

author

2018-07-24 20:46 UTC #0 lore raw

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(-)

Randy Dunlap

author

2018-07-24 20:46 UTC #1 lore raw

PATCH 1/4 arc: fix some build issues in delay.h

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 <rdunlap at infradead.org>

Cc Vineet Gupta <vgupta at synopsys.com>

Cc Ofer Levi <oferle at mellanox.com>

Cc linux-snps-arc at lists.infradead.org

---
 arch/arc/include/asm/delay.h |    2 ++
 1 file changed, 2 insertions(+)

arch/arc/include/asm/delay.h +2 -0

@@ -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)

Vineet Gupta

2018-07-24 22:04 UTC #2 lore raw

Re: [PATCH 1/4] arc: fix some build issues in delay.h

Quoted 29 lines · Randy Dunlap

From: Randy Dunlap <rdunlap at infradead.org>
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 <rdunlap at infradead.org>
Cc Vineet Gupta <vgupta at synopsys.com>
Cc Ofer Levi <oferle at mellanox.com>
Cc linux-snps-arc at lists.infradead.org
---
 arch/arc/include/asm/delay.h |    2 ++
 1 file changed, 2 insertions(+)

arch/arc/include/asm/delay.h line 20

Quoted 4 lines

arch/arc/include/asm/delay.h +1 -0

@@ -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

Quoted 5 lines

arch/arc/include/asm/delay.h +1 -0

+#include <asm-generic/types.h>
 #include <asm/param.h>		/* HZ */
 
 static inline void __delay(unsigned long loops)

Randy Dunlap

2018-07-24 22:07 UTC #3 lore raw

Re: [PATCH 1/4] arc: fix some build issues in delay.h

Quoted 33 lines · Vineet Gupta

Quoted 32 lines · Randy Dunlap
From: Randy Dunlap <rdunlap at infradead.org>
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 <rdunlap at infradead.org>
Cc Vineet Gupta <vgupta at synopsys.com>
Cc Ofer Levi <oferle at mellanox.com>
Cc linux-snps-arc at lists.infradead.org
---
 arch/arc/include/asm/delay.h |    2 ++
 1 file changed, 2 insertions(+)
arch/arc/include/asm/delay.h +0 -0
@@ -17,6 +17,8 @@
 #ifndef __ASM_ARC_UDELAY_H
 #define __ASM_ARC_UDELAY_H
 

Quoted 5 lines

arch/arc/include/asm/delay.h line 20
Quoted 1 line
arch/arc/include/asm/delay.h +1 -0
+#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.

Quoted 6 lines

Quoted 5 lines

arch/arc/include/asm/delay.h +1 -0

+#include <asm-generic/types.h>
 #include <asm/param.h>		/* HZ */
 
 static inline void __delay(unsigned long loops)

Signature · 1 line

~Randy

Randy Dunlap

author

2018-07-24 20:46 UTC #4 lore raw

PATCH 2/4 arc: fix type warnings in arc/mm/cache.c

In reply to Randy Dunlap

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 <rdunlap at infradead.org>

Cc Vineet Gupta <vgupta at synopsys.com>

Cc Ofer Levi <oferle at mellanox.com>

Cc linux-snps-arc at lists.infradead.org

---
 arch/arc/mm/cache.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

arch/arc/mm/cache.c +6 -5

@@ -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);

Vineet Gupta

2018-07-25 00:02 UTC #5 lore raw

Re: [PATCH 2/4] arc: fix type warnings in arc/mm/cache.c

Quoted 30 lines · Randy Dunlap

From: Randy Dunlap <rdunlap at infradead.org>
../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 <rdunlap at infradead.org>
Cc Vineet Gupta <vgupta at synopsys.com>
Cc Ofer Levi <oferle at mellanox.com>
Cc linux-snps-arc at lists.infradead.org
---
 arch/arc/mm/cache.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
arch/arc/mm/cache.c +1 -1
@@ -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;
 

arch/arc/mm/cache.c void flush_anon_page(struct vm_area_stru · line 1063

Quoted 8 lines

arch/arc/mm/cache.c +3 -2

@@ -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.

arch/arc/mm/cache.c void copy_user_highpage(struct page *to, · line 1088

Quoted 9 lines

arch/arc/mm/cache.c +1 -1

@@ -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 !

arch/arc/mm/cache.c void copy_user_highpage(struct page *to, · line 1112

Quoted 13 lines

arch/arc/mm/cache.c +1 -1

 		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 !

Randy Dunlap

2018-07-25 00:46 UTC #6 lore raw

Re: [PATCH 2/4] arc: fix type warnings in arc/mm/cache.c

Quoted 48 lines · Vineet Gupta

Quoted 30 lines · Randy Dunlap
From: Randy Dunlap <rdunlap at infradead.org>
../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 <rdunlap at infradead.org>
Cc Vineet Gupta <vgupta at synopsys.com>
Cc Ofer Levi <oferle at mellanox.com>
Cc linux-snps-arc at lists.infradead.org
---
 arch/arc/mm/cache.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
arch/arc/mm/cache.c +1 -1
@@ -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;
 
arch/arc/mm/cache.c void flush_anon_page(struct vm_area_stru · line 1063
Quoted 8 lines
arch/arc/mm/cache.c +3 -2
@@ -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.
Quoted 3 lines
arch/arc/mm/cache.c +0 -0
 }
 

Quoted 8 lines

arch/arc/mm/cache.c void copy_user_highpage(struct page *to, · line 1088
Quoted 6 lines
arch/arc/mm/cache.c +1 -1
@@ -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.

Quoted 10 lines

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 !
Quoted 3 lines
arch/arc/mm/cache.c +0 -0
 		clean_src_k_mappings = 1;
 	}
 

Quoted 13 lines

arch/arc/mm/cache.c void copy_user_highpage(struct page *to, · line 1112

Quoted 10 lines

arch/arc/mm/cache.c +1 -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.

Signature · 1 line

~Randy

Randy Dunlap

author

2018-07-24 20:46 UTC #7 lore raw

PATCH 3/4 arc: fix some plat-eznps data type errors

In reply to Randy Dunlap

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 <rdunlap at infradead.org>

Cc Vineet Gupta <vgupta at synopsys.com>

Cc Ofer Levi <oferle at mellanox.com>

Cc linux-snps-arc at lists.infradead.org

---
 arch/arc/plat-eznps/include/plat/ctop.h |    1 +
 1 file changed, 1 insertion(+)

arch/arc/plat-eznps/include/plat/ctop.h +1 -0

@@ -21,6 +21,7 @@
 #error "Incorrect ctop.h include"
 #endif
 
+#include <asm-generic/types.h>
 #include <soc/nps/common.h>
 
 /* core auxiliary registers */

Randy Dunlap

author

2018-07-24 20:46 UTC #8 lore raw

PATCH 4/4 arc: fix mtm.c printk format warning

In reply to Randy Dunlap

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 <rdunlap at infradead.org>

Cc Vineet Gupta <vgupta at synopsys.com>

Cc Ofer Levi <oferle at mellanox.com>

Cc linux-snps-arc at lists.infradead.org

---
 arch/arc/plat-eznps/mtm.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

arch/arc/plat-eznps/mtm.c +4 -2

@@ -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",