Remove a warning for drivers/edac/i82975x_edac.c

PATCH RESEND In review Harry Wei 4 participants 10 messages Jun 9, 2011 – Jun 12, 2011

Subsystems (2)

EDAC-core EDAC-I82975X

lore

Share & tools

lore

Harry Wei

author

2011-06-09 03:04 UTC #0 lore raw

PATCH RESEND 1/1 Remove a warning for drivers/edac/i82975x_edac.c

From: Harry Wei <[email protected]>

When i compile 2.6.39.1, a warning shows like below. This patch can fix the warning. And i works well with my PC.

[...]

CC [M]  drivers/edac/e752x_edac.o
CC [M]  drivers/edac/i82875p_edac.o
CC [M]  drivers/edac/i82975x_edac.o
drivers/edac/i82975x_edac.c: In function ���i82975x_process_error_info���:
drivers/edac/i82975x_edac.c:298: warning: integer constant is too large for ���unsigned long��� type
CC [M]  drivers/edac/i3000_edac.o
CC [M]  drivers/edac/i3200_edac.o
CC [M]  drivers/edac/x38_edac.o
CC [M]  drivers/edac/i82860_edac.o

[...]

Signed-off-by Harry Wei <[email protected]>

drivers/edac/i82975x_edac.c +2 -2

Index: prj/drivers/edac/i82975x_edac.c
===================================================================
@@ -293,9 +293,9 @@
 		info->errsts = info->errsts2;
 	}
 
-	page = (unsigned long) info->eap;
+	page = (unsigned long long) info->eap;
 	if (info->xeap & 1)
-		page |= 0x100000000ul;
+		page |= 0x100000000ull;
 	chan = page & 1;
 	page >>= 1;
 	offst = page & ((1 << PAGE_SHIFT) - 1);

Connor Hansen

2011-06-09 04:26 UTC #1 lore raw

When I compile 3.0.0-rc2+ I don't get this warning. please make sure this issue still exists.

Connor

Quoted 42 lines · Harry Wei

From: Harry Wei <[email protected]>
When i compile 2.6.39.1, a warning shows like below. This patch can fix the warning. And i works well with my PC.
[...]
CC [M] drivers/edac/e752x_edac.o
CC [M] drivers/edac/i82875p_edac.o
CC [M] drivers/edac/i82975x_edac.o
drivers/edac/i82975x_edac.c: In function ��i82975x_process_error_info��:
drivers/edac/i82975x_edac.c:298: warning: integer constant is too large for ��unsigned long�� type CC [M] drivers/edac/i3000_edac.o
CC [M] drivers/edac/i3200_edac.o
CC [M] drivers/edac/x38_edac.o
CC [M] drivers/edac/i82860_edac.o
[...]
Signed-off-by Harry Wei <[email protected]>
Index: prj/drivers/edac/i82975x_edac.c
===================================================================
drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800 renamed from drivers/edac/i82975x_edac.c 2011-06-04 10:09:55.351174516 +0800 +2 -3
@@ -293,9 +293,9 @@
                info->errsts = info->errsts2;
        }
 
-       page = (unsigned long) info->eap;
+       page = (unsigned long long) info->eap;
        if (info->xeap & 1)
-               page |= 0x100000000ul;
+               page |= 0x100000000ull;
        chan = page & 1;
        page >>= 1;
        offst = page & ((1 << PAGE_SHIFT) - 1);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Harry Wei

author

2011-06-09 05:26 UTC #2 lore raw

Quoted 2 lines · Connor Hansen

When I compile 3.0.0-rc2+ I don't get this warning. please make sure this issue still exists.

I have not compiled 3.0.0-rc2+, but
i fetch portions of drivers/edac/i82975x_edac.c
of 3.0.0-rc2+ like below.

[...]
296 page = (unsigned long) info->eap;
297 if (info->xeap & 1)
298 page |= 0x100000000ul;
299 chan = page & 1;
300 page >>= 1;
[...]

The following codes prove this warning
remians.

#include <stdio.h>

int main(void)

{
	unsigned long a;

	a = 0x100000000ul;
	printf("%lu\n", a);
	return 0;
}

jiawei@jiawei-laptop:~/GTK$ gcc 5.c -Wall -o 1
5.c: In function ‘main’:
5.c:7: warning: integer constant is too large for ‘unsigned long’ type
5.c:7: warning: large integer implicitly truncated to unsigned type

But if you program it like below, it is well for us.

#include <stdio.h>

int main(void)

{
	unsigned long long a;

	a = 0x100000000ull;
	printf("%llu\n", a);
	return 0;
}

Thanks
Harry Wei

Quoted 40 lines

Connor
Quoted 37 lines · Harry Wei
From: Harry Wei <[email protected]>
When i compile 2.6.39.1, a warning shows like below. This patch can fix the warning. And i works well with my PC.
[...]
CC [M] ??drivers/edac/e752x_edac.o
CC [M] ??drivers/edac/i82875p_edac.o
CC [M] ??drivers/edac/i82975x_edac.o
drivers/edac/i82975x_edac.c: In function ?????????i82975x_process_error_info?????????:
drivers/edac/i82975x_edac.c:298: warning: integer constant is too large for ?????????unsigned long????????? type CC [M] ??drivers/edac/i3000_edac.o
CC [M] ??drivers/edac/i3200_edac.o
CC [M] ??drivers/edac/x38_edac.o
CC [M] ??drivers/edac/i82860_edac.o
[...]
Signed-off-by Harry Wei <[email protected]>
Index: prj/drivers/edac/i82975x_edac.c
===================================================================
drivers/edac/i82975x_edac.c ?? ?? 2011-06-04 10:10:30.875168437 +0800 renamed from drivers/edac/i82975x_edac.c ?? ?? ?? ??2011-06-04 10:09:55.351174516 +0800 +2 -3
@@ -293,9 +293,9 @@
 ?? ?? ?? ?? ?? ?? ?? ??info->errsts = info->errsts2;
 ?? ?? ?? ??}
 
- ?? ?? ?? page = (unsigned long) info->eap;
+ ?? ?? ?? page = (unsigned long long) info->eap;
 ?? ?? ?? ??if (info->xeap & 1)
- ?? ?? ?? ?? ?? ?? ?? page |= 0x100000000ul;
+ ?? ?? ?? ?? ?? ?? ?? page |= 0x100000000ull;
 ?? ?? ?? ??chan = page & 1;
 ?? ?? ?? ??page >>= 1;
 ?? ?? ?? ??offst = page & ((1 << PAGE_SHIFT) - 1);
--

Connor Hansen

2011-06-09 06:17 UTC #3 lore raw

Quoted 7 lines · Harry Wei

Quoted 2 lines · Connor Hansen
When I compile 3.0.0-rc2+ I don't get this warning. please make sure this issue still exists.
I have not compiled 3.0.0-rc2+, but
i fetch portions of drivers/edac/i82975x_edac.c
of 3.0.0-rc2+ like below.

Quoted 8 lines

[...]
296 page = (unsigned long) info->eap;
297 if (info->xeap & 1)
298 page |= 0x100000000ul;
299 chan = page & 1;
300 page >>= 1;
[...]

this code is the same in 3.0.0-rc2+ but produces no warning.

Quoted 5 lines

The following codes prove this warning
remians.
#include <stdio.h>

Quoted 8 lines

int main(void)
{
unsigned long a;
a = 0x100000000ul;
printf("%lu\n", a);
return 0;
}

this is not what the code you are changing does.

the code is not assigned that value, its or'd with it.

a test would be more like.

#include <stdio.h>
int main()

{
        unsigned long page;  //page
        unsigned int b = 5;  //info->eap is u32 = unsigned int
                                       // typedef unsigned int __u32;
        page = (unsigned long) b;
        page |= 0x10000000ul;

        printf("%lu\n",page);

        return 0;
}

This produces no warning.

$ gcc -g test.c && ./a.out
268435461
$

0x10000000
10000000000000000000000000000
5
00000000000000000000000000101
0x10000000 | 5 =
10000000000000000000000000101
268435461

I am green to kernel development so I will wait for someone with a bit more knowledge to
chime in, or verify this warning.
Connor

Quoted 62 lines

jiawei@jiawei-laptop:~/GTK$ gcc 5.c -Wall -o 1
5.c: In function ‘main’:
5.c:7: warning: integer constant is too large for ‘unsigned long’ type
5.c:7: warning: large integer implicitly truncated to unsigned type
But if you program it like below, it is well for us.
#include <stdio.h>
int main(void)
{
unsigned long long a;
a = 0x100000000ull;
printf("%llu\n", a);
return 0;
}
Thanks
Harry Wei
Quoted 40 lines
Connor
Quoted 37 lines · Harry Wei
From: Harry Wei <[email protected]>
When i compile 2.6.39.1, a warning shows like below. This patch can fix the warning. And i works well with my PC.
[...]
CC [M] ??drivers/edac/e752x_edac.o
CC [M] ??drivers/edac/i82875p_edac.o
CC [M] ??drivers/edac/i82975x_edac.o
drivers/edac/i82975x_edac.c: In function ?????????i82975x_process_error_info?????????:
drivers/edac/i82975x_edac.c:298: warning: integer constant is too large for ?????????unsigned long????????? type CC [M] ??drivers/edac/i3000_edac.o
CC [M] ??drivers/edac/i3200_edac.o
CC [M] ??drivers/edac/x38_edac.o
CC [M] ??drivers/edac/i82860_edac.o
[...]
Signed-off-by Harry Wei <[email protected]>
Index: prj/drivers/edac/i82975x_edac.c
===================================================================
drivers/edac/i82975x_edac.c ?? ?? 2011-06-04 10:10:30.875168437 +0800 renamed from drivers/edac/i82975x_edac.c ?? ?? ?? ??2011-06-04 10:09:55.351174516 +0800 +2 -3
@@ -293,9 +293,9 @@
 ?? ?? ?? ?? ?? ?? ?? ??info->errsts = info->errsts2;
 ?? ?? ?? ??}
 
- ?? ?? ?? page = (unsigned long) info->eap;
+ ?? ?? ?? page = (unsigned long long) info->eap;
 ?? ?? ?? ??if (info->xeap & 1)
- ?? ?? ?? ?? ?? ?? ?? page |= 0x100000000ul;
+ ?? ?? ?? ?? ?? ?? ?? page |= 0x100000000ull;
 ?? ?? ?? ??chan = page & 1;
 ?? ?? ?? ??page >>= 1;
 ?? ?? ?? ??offst = page & ((1 << PAGE_SHIFT) - 1);
--

Harry Wei

author

2011-06-10 05:08 UTC #4 lore raw

Quoted 34 lines · Connor Hansen

this is not what the code you are changing does.
the code is not assigned that value, its or'd with it.
a test would be more like.
#include <stdio.h>
int main()
{
        unsigned long page;  //page
        unsigned int b = 5;  //info->eap is u32 = unsigned int
                                       // typedef unsigned int __u32;
        page = (unsigned long) b;
        page |= 0x10000000ul;

        printf("%lu\n",page);

        return 0;
}
This produces no warning.
$ gcc -g test.c && ./a.out
268435461
$
0x10000000
10000000000000000000000000000
5
00000000000000000000000000101
0x10000000 | 5 =
10000000000000000000000000101
268435461

Quoted 4 lines

I am green to kernel development so I will wait for someone with a bit more knowledge to
chime in, or verify this warning.
Connor

You are right, but it really has warnings yet.
My PC is also 32-bits.

Thanks
Harry Wei

Randy Dunlap

2011-06-09 16:35 UTC #5 lore raw

In reply to Connor Hansen

Quoted 3 lines · Connor Hansen

When I compile 3.0.0-rc2+ I don't get this warning. please make sure this issue still exists.

It still happens for me -- but only on i386 (X86_32). Did you build for 32-bit target?

Signature · 2 lines

~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Randy Dunlap

2011-06-09 17:38 UTC #6 lore raw

In reply to Harry Wei

Quoted 25 lines · On Thu, 9 Jun 2011 11:04:13 +0800 Harry Wei wrote:

From: Harry Wei <[email protected]>
When i compile 2.6.39.1, a warning shows like below. This patch can fix the warning. And i works well with my PC.
[...]
CC [M]  drivers/edac/e752x_edac.o
CC [M]  drivers/edac/i82875p_edac.o
CC [M]  drivers/edac/i82975x_edac.o
drivers/edac/i82975x_edac.c: In function ���i82975x_process_error_info���:
drivers/edac/i82975x_edac.c:298: warning: integer constant is too large for ���unsigned long��� type
CC [M]  drivers/edac/i3000_edac.o
CC [M]  drivers/edac/i3200_edac.o
CC [M]  drivers/edac/x38_edac.o
CC [M]  drivers/edac/i82860_edac.o
[...]
Signed-off-by Harry Wei <[email protected]>
Index: prj/drivers/edac/i82975x_edac.c
===================================================================

drivers/edac/i82975x_edac.c line 296

Quoted 5 lines

drivers/edac/i82975x_edac.c +1 -1

@@ -293,9 +293,9 @@
 		info->errsts = info->errsts2;
 	}
 
-	page = (unsigned long) info->eap;
+	page = (unsigned long long) info->eap;

But page is still unsigned long, not unsigned long long...

drivers/edac/i82975x_edac.c line 298

Quoted 3 lines

drivers/edac/i82975x_edac.c +1 -1

 	if (info->xeap & 1)
-		page |= 0x100000000ul;
+		page |= 0x100000000ull;

and just making this second change fixes the warning.

I don't know what the right answer is, but this patch does not look complete to me.

Back in April there was a discussion about this warning message and the conclusion was that this driver should be replaced. See
https://lkml.org/lkml/2011/4/21/232 and its replies.
But we seem to be having problems reaching the EDAC subsystem maintainer (again cc-ed).

drivers/edac/i82975x_edac.c

Quoted 4 lines

drivers/edac/i82975x_edac.c +0 -1

 	chan = page & 1;
 	page >>= 1;
 	offst = page & ((1 << PAGE_SHIFT) - 1);
--

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Harry Wei

author

2011-06-10 05:21 UTC #7 lore raw

Quoted 6 lines · Randy Dunlap

drivers/edac/i82975x_edac.c line 296
Quoted 2 lines · On Thu, 9 Jun 2011 11:04:13 +0800 Harry Wei wrote:
drivers/edac/i82975x_edac.c +1 -1
-	page = (unsigned long) info->eap;
+	page = (unsigned long long) info->eap;
But page is still unsigned long, not unsigned long long...

Yup, but if we can define a 'unsigned long long' type?

Quoted 6 lines

drivers/edac/i82975x_edac.c line 298
Quoted 4 lines
drivers/edac/i82975x_edac.c +1 -1
 	if (info->xeap & 1)
-		page |= 0x100000000ul;
+		page |= 0x100000000ull;
and just making this second change fixes the warning.

I will try my best. Can anyone give some other comments?

Quoted 3 lines

I don't know what the right answer is, but this patch does not look complete to me.

I will try to give a v2.

Quoted 6 lines

Back in April there was a discussion about this warning message and the conclusion was that this driver should be replaced. See
https://lkml.org/lkml/2011/4/21/232 and its replies.
But we seem to be having problems reaching the EDAC subsystem maintainer (again cc-ed).

You have not got a exact conclusion about this warning. Maybe Arvind knows more about this warning.(cc'ed)

Thanks
Harry Wei

Connor Hansen

2011-06-10 06:42 UTC #8 lore raw

My system is 64bit, thus not showing a warning

adding cc to maintainer, and dev list

2011/6/9 Harry Wei <[email protected]>:

Quoted 29 lines · Randy Dunlap

Quoted 6 lines · On Thu, 9 Jun 2011 11:04:13 +0800 Harry Wei wrote:
Quoted 3 lines
- page = (unsigned long) info->eap;
+ page = (unsigned long long) info->eap;
But page is still unsigned long, not unsigned long long...
Yup, but if we can define a 'unsigned long long' type?
Quoted 6 lines
Quoted 4 lines
if (info->xeap & 1)
- page |= 0x100000000ul;
+ page |= 0x100000000ull;
and just making this second change fixes the warning.
I will try my best. Can anyone give some other comments?
Quoted 3 lines
I don't know what the right answer is, but this patch does not look complete to me.
I will try to give a v2.
Quoted 6 lines
Back in April there was a discussion about this warning message and the conclusion was that this driver should be replaced. See
https://lkml.org/lkml/2011/4/21/232 and its replies.
But we seem to be having problems reaching the EDAC subsystem maintainer (again cc-ed).
You have not got a exact conclusion about this warning. Maybe Arvind knows more about this warning.(cc'ed)

Quoted 8 lines

Thanks
Harry Wei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

older convo for new cc's

Quoted 25 lines · On Thu, 9 Jun 2011 11:04:13 +0800 Harry Wei wrote:

From: Harry Wei <[email protected]>
When i compile 2.6.39.1, a warning shows like below. This patch can fix the warning. And i works well with my PC.
[...]
CC [M]  drivers/edac/e752x_edac.o
CC [M]  drivers/edac/i82875p_edac.o
CC [M]  drivers/edac/i82975x_edac.o
drivers/edac/i82975x_edac.c: In function ���i82975x_process_error_info���:
drivers/edac/i82975x_edac.c:298: warning: integer constant is too large for ���unsigned long��� type
CC [M]  drivers/edac/i3000_edac.o
CC [M]  drivers/edac/i3200_edac.o
CC [M]  drivers/edac/x38_edac.o
CC [M]  drivers/edac/i82860_edac.o
[...]
Signed-off-by Harry Wei <[email protected]>
Index: prj/drivers/edac/i82975x_edac.c
===================================================================

drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800 line 296

Quoted 5 lines

drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800 renamed from drivers/edac/i82975x_edac.c 2011-06-04 10:09:55.351174516 +0800 +1 -1
@@ -293,9 +293,9 @@
              info->errsts = info->errsts2;
      }
 
-     page = (unsigned long) info->eap;
+     page = (unsigned long long) info->eap;

But page is still unsigned long, not unsigned long long...

drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800 line 298

Quoted 3 lines

drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800 renamed from drivers/edac/i82975x_edac.c 2011-06-04 10:09:55.351174516 +0800 +1 -1
      if (info->xeap & 1)
-             page |= 0x100000000ul;
+             page |= 0x100000000ull;

and just making this second change fixes the warning.

I don't know what the right answer is, but this patch does not look complete to me.

drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800

Quoted 4 lines

drivers/edac/i82975x_edac.c 2011-06-04 10:10:30.875168437 +0800 renamed from drivers/edac/i82975x_edac.c 2011-06-04 10:09:55.351174516 +0800 +0 -1
      chan = page & 1;
      page >>= 1;
      offst = page & ((1 << PAGE_SHIFT) - 1);
--

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
- Show quoted text -

Arvind R

2011-06-12 15:50 UTC #9 lore raw

Hi,
My system is broken currently, so I'm unable to check:(

But the foll. patch should fix the problem for 64 and 32 bits AND work correct wrt.
to error-info. Would be glad if can check, please.

drivers/edac/i82975x_edac.c +4 -4

@@ -294,12 +294,12 @@
 	}
 
 	page = (unsigned long) info->eap;
-	if (info->xeap & 1)
-		page |= 0x100000000ul;
 	chan = page & 1;
-	page >>= 1;
 	offst = page & ((1 << PAGE_SHIFT) - 1);
-	page >>= PAGE_SHIFT;
+	page >>= 1;
+	if (info->xeap & 1)
+		page |= 0x80000000;
+	page >>= (PAGE_SHIFT - 1);
 	row = edac_mc_find_csrow_by_page(mci, page);
 
 	if (info->errsts & 0x0002)

- arvind