* [PATCH] EDAC, mpc85xx: fix build warning
@ 2016-02-02 8:00 Sudip Mukherjee
2016-02-02 11:15 ` Borislav Petkov
0 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2016-02-02 8:00 UTC (permalink / raw)
To: Johannes Thumshirn, Doug Thompson, Borislav Petkov,
Mauro Carvalho Chehab
Cc: linux-kernel, linux-edac, Sudip Mukherjee
We were getting build warning about:
drivers/edac/mpc85xx_edac.c:1247:6: warning: unused variable 'pvr'
pvr is only used if CONFIG_FSL_SOC_BOOKE was defined. Declare the
variable as a local variable inside the #ifdef block.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/edac/mpc85xx_edac.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index b7139c1..951be71 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -1244,7 +1244,6 @@ static struct platform_driver * const drivers[] = {
static int __init mpc85xx_mc_init(void)
{
int res = 0;
- u32 pvr = 0;
printk(KERN_INFO "Freescale(R) MPC85xx EDAC driver, "
"(C) 2006 Montavista Software\n");
@@ -1264,16 +1263,18 @@ static int __init mpc85xx_mc_init(void)
printk(KERN_WARNING EDAC_MOD_STR "drivers fail to register\n");
#ifdef CONFIG_FSL_SOC_BOOKE
- pvr = mfspr(SPRN_PVR);
-
- if ((PVR_VER(pvr) == PVR_VER_E500V1) ||
- (PVR_VER(pvr) == PVR_VER_E500V2)) {
- /*
- * need to clear HID1[RFXE] to disable machine check int
- * so we can catch it
- */
- if (edac_op_state == EDAC_OPSTATE_INT)
- on_each_cpu(mpc85xx_mc_clear_rfxe, NULL, 0);
+ {
+ u32 pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) == PVR_VER_E500V1) ||
+ (PVR_VER(pvr) == PVR_VER_E500V2)) {
+ /*
+ * need to clear HID1[RFXE] to disable machine check int
+ * so we can catch it
+ */
+ if (edac_op_state == EDAC_OPSTATE_INT)
+ on_each_cpu(mpc85xx_mc_clear_rfxe, NULL, 0);
+ }
}
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 8:00 [PATCH] EDAC, mpc85xx: fix build warning Sudip Mukherjee
@ 2016-02-02 11:15 ` Borislav Petkov
2016-02-02 14:38 ` Guenter Roeck
2016-02-02 14:48 ` Sudip Mukherjee
0 siblings, 2 replies; 10+ messages in thread
From: Borislav Petkov @ 2016-02-02 11:15 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Johannes Thumshirn, Doug Thompson, Mauro Carvalho Chehab,
linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 01:30:21PM +0530, Sudip Mukherjee wrote:
> We were getting build warning about:
> drivers/edac/mpc85xx_edac.c:1247:6: warning: unused variable 'pvr'
>
> pvr is only used if CONFIG_FSL_SOC_BOOKE was defined. Declare the
> variable as a local variable inside the #ifdef block.
What's wrong with doing the simpler thing:
---
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index b7139c160baf..f756b6215228 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -1244,7 +1244,6 @@ static struct platform_driver * const drivers[] = {
static int __init mpc85xx_mc_init(void)
{
int res = 0;
- u32 pvr = 0;
printk(KERN_INFO "Freescale(R) MPC85xx EDAC driver, "
"(C) 2006 Montavista Software\n");
@@ -1264,10 +1263,8 @@ static int __init mpc85xx_mc_init(void)
printk(KERN_WARNING EDAC_MOD_STR "drivers fail to register\n");
#ifdef CONFIG_FSL_SOC_BOOKE
- pvr = mfspr(SPRN_PVR);
-
- if ((PVR_VER(pvr) == PVR_VER_E500V1) ||
- (PVR_VER(pvr) == PVR_VER_E500V2)) {
+ if ((PVR_VER(mfspr(SPRN_PVR)) == PVR_VER_E500V1) ||
+ (PVR_VER(mfspr(SPRN_PVR)) == PVR_VER_E500V2)) {
/*
* need to clear HID1[RFXE] to disable machine check int
* so we can catch it
---
Granted, we get MFSPR issued twice by the compiler but that's the init
path.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 11:15 ` Borislav Petkov
@ 2016-02-02 14:38 ` Guenter Roeck
2016-02-02 14:48 ` Johannes Thumshirn
2016-02-02 14:48 ` Sudip Mukherjee
1 sibling, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2016-02-02 14:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: Sudip Mukherjee, Johannes Thumshirn, Doug Thompson,
Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 12:15:45PM +0100, Borislav Petkov wrote:
> On Tue, Feb 02, 2016 at 01:30:21PM +0530, Sudip Mukherjee wrote:
> > We were getting build warning about:
> > drivers/edac/mpc85xx_edac.c:1247:6: warning: unused variable 'pvr'
> >
> > pvr is only used if CONFIG_FSL_SOC_BOOKE was defined. Declare the
> > variable as a local variable inside the #ifdef block.
>
> What's wrong with doing the simpler thing:
>
> ---
> diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
> index b7139c160baf..f756b6215228 100644
> --- a/drivers/edac/mpc85xx_edac.c
> +++ b/drivers/edac/mpc85xx_edac.c
> @@ -1244,7 +1244,6 @@ static struct platform_driver * const drivers[] = {
> static int __init mpc85xx_mc_init(void)
> {
> int res = 0;
> - u32 pvr = 0;
>
Or with just adding __maybe_unused here.
Guenter
> printk(KERN_INFO "Freescale(R) MPC85xx EDAC driver, "
> "(C) 2006 Montavista Software\n");
> @@ -1264,10 +1263,8 @@ static int __init mpc85xx_mc_init(void)
> printk(KERN_WARNING EDAC_MOD_STR "drivers fail to register\n");
>
> #ifdef CONFIG_FSL_SOC_BOOKE
> - pvr = mfspr(SPRN_PVR);
> -
> - if ((PVR_VER(pvr) == PVR_VER_E500V1) ||
> - (PVR_VER(pvr) == PVR_VER_E500V2)) {
> + if ((PVR_VER(mfspr(SPRN_PVR)) == PVR_VER_E500V1) ||
> + (PVR_VER(mfspr(SPRN_PVR)) == PVR_VER_E500V2)) {
> /*
> * need to clear HID1[RFXE] to disable machine check int
> * so we can catch it
> ---
>
> Granted, we get MFSPR issued twice by the compiler but that's the init
> path.
>
> --
> Regards/Gruss,
> Boris.
>
> ECO tip #101: Trim your mails when you reply.
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 14:38 ` Guenter Roeck
@ 2016-02-02 14:48 ` Johannes Thumshirn
2016-02-02 14:55 ` Sudip Mukherjee
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2016-02-02 14:48 UTC (permalink / raw)
To: Guenter Roeck
Cc: Borislav Petkov, Sudip Mukherjee, Johannes Thumshirn,
Doug Thompson, Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 06:38:01AM -0800, Guenter Roeck wrote:
> On Tue, Feb 02, 2016 at 12:15:45PM +0100, Borislav Petkov wrote:
> > On Tue, Feb 02, 2016 at 01:30:21PM +0530, Sudip Mukherjee wrote:
> > > We were getting build warning about:
> > > drivers/edac/mpc85xx_edac.c:1247:6: warning: unused variable 'pvr'
> > >
> > > pvr is only used if CONFIG_FSL_SOC_BOOKE was defined. Declare the
> > > variable as a local variable inside the #ifdef block.
> >
> > What's wrong with doing the simpler thing:
> >
> > ---
> > diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
> > index b7139c160baf..f756b6215228 100644
> > --- a/drivers/edac/mpc85xx_edac.c
> > +++ b/drivers/edac/mpc85xx_edac.c
> > @@ -1244,7 +1244,6 @@ static struct platform_driver * const drivers[] = {
> > static int __init mpc85xx_mc_init(void)
> > {
> > int res = 0;
> > - u32 pvr = 0;
> >
> Or with just adding __maybe_unused here.
I'd favour this one as well.
Just my 2c.
byte,
Johannes
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 11:15 ` Borislav Petkov
2016-02-02 14:38 ` Guenter Roeck
@ 2016-02-02 14:48 ` Sudip Mukherjee
1 sibling, 0 replies; 10+ messages in thread
From: Sudip Mukherjee @ 2016-02-02 14:48 UTC (permalink / raw)
To: Borislav Petkov
Cc: Johannes Thumshirn, Doug Thompson, Mauro Carvalho Chehab,
linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 12:15:45PM +0100, Borislav Petkov wrote:
> On Tue, Feb 02, 2016 at 01:30:21PM +0530, Sudip Mukherjee wrote:
> > We were getting build warning about:
> > drivers/edac/mpc85xx_edac.c:1247:6: warning: unused variable 'pvr'
> >
> > pvr is only used if CONFIG_FSL_SOC_BOOKE was defined. Declare the
> > variable as a local variable inside the #ifdef block.
>
> What's wrong with doing the simpler thing:
I hesitated because of issuing MFSPR twice. But anyway, I have sent the
v2 for your review.
regards
sudip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 14:48 ` Johannes Thumshirn
@ 2016-02-02 14:55 ` Sudip Mukherjee
2016-02-02 15:09 ` Borislav Petkov
0 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2016-02-02 14:55 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Guenter Roeck, Borislav Petkov, Johannes Thumshirn,
Doug Thompson, Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 03:48:03PM +0100, Johannes Thumshirn wrote:
> On Tue, Feb 02, 2016 at 06:38:01AM -0800, Guenter Roeck wrote:
> > On Tue, Feb 02, 2016 at 12:15:45PM +0100, Borislav Petkov wrote:
> > > On Tue, Feb 02, 2016 at 01:30:21PM +0530, Sudip Mukherjee wrote:
> > > > We were getting build warning about:
> > > > drivers/edac/mpc85xx_edac.c:1247:6: warning: unused variable 'pvr'
> > > >
> > > > pvr is only used if CONFIG_FSL_SOC_BOOKE was defined. Declare the
> > > > variable as a local variable inside the #ifdef block.
> > >
> > > What's wrong with doing the simpler thing:
> > >
> > > ---
> > > diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
> > > index b7139c160baf..f756b6215228 100644
> > > --- a/drivers/edac/mpc85xx_edac.c
> > > +++ b/drivers/edac/mpc85xx_edac.c
> > > @@ -1244,7 +1244,6 @@ static struct platform_driver * const drivers[] = {
> > > static int __init mpc85xx_mc_init(void)
> > > {
> > > int res = 0;
> > > - u32 pvr = 0;
> > >
> > Or with just adding __maybe_unused here.
>
> I'd favour this one as well.
oops... i just sent the v2 while removing pvr. Which is the better way
then?
Personally, I donot like __maybe_unused, it is telling gcc that this
variable is unused so donot give any warning for it. But in reality it
is being used in some cases.
regards
sudip
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 14:55 ` Sudip Mukherjee
@ 2016-02-02 15:09 ` Borislav Petkov
2016-02-02 15:13 ` Sudip Mukherjee
0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2016-02-02 15:09 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Johannes Thumshirn, Guenter Roeck, Johannes Thumshirn,
Doug Thompson, Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 08:25:54PM +0530, Sudip Mukherjee wrote:
> oops... i just sent the v2 while removing pvr. Which is the better way
> then?
> Personally, I donot like __maybe_unused, it is telling gcc that this
> variable is unused so donot give any warning for it. But in reality it
> is being used in some cases.
And? You want to shut up the warning, right?
Adding __maybe_unused is the simplest variant without disadvantages. Or
are there any?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 15:09 ` Borislav Petkov
@ 2016-02-02 15:13 ` Sudip Mukherjee
2016-02-02 15:15 ` Borislav Petkov
0 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2016-02-02 15:13 UTC (permalink / raw)
To: Borislav Petkov
Cc: Johannes Thumshirn, Guenter Roeck, Johannes Thumshirn,
Doug Thompson, Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 04:09:50PM +0100, Borislav Petkov wrote:
> On Tue, Feb 02, 2016 at 08:25:54PM +0530, Sudip Mukherjee wrote:
> > oops... i just sent the v2 while removing pvr. Which is the better way
> > then?
> > Personally, I donot like __maybe_unused, it is telling gcc that this
> > variable is unused so donot give any warning for it. But in reality it
> > is being used in some cases.
>
> And? You want to shut up the warning, right?
>
> Adding __maybe_unused is the simplest variant without disadvantages. Or
> are there any?
another way might be:
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index b7139c1..968c0c0 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -1244,7 +1244,9 @@ static struct platform_driver * const drivers[] = {
static int __init mpc85xx_mc_init(void)
{
int res = 0;
+#ifdef CONFIG_FSL_SOC_BOOKE
u32 pvr = 0;
+#endif
printk(KERN_INFO "Freescale(R) MPC85xx EDAC driver, "
"(C) 2006 Montavista Software\n");
regards
sudip
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 15:13 ` Sudip Mukherjee
@ 2016-02-02 15:15 ` Borislav Petkov
2016-02-02 15:19 ` Sudip Mukherjee
0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2016-02-02 15:15 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Johannes Thumshirn, Guenter Roeck, Johannes Thumshirn,
Doug Thompson, Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 08:43:57PM +0530, Sudip Mukherjee wrote:
> another way might be:
>
> diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
> index b7139c1..968c0c0 100644
> --- a/drivers/edac/mpc85xx_edac.c
> +++ b/drivers/edac/mpc85xx_edac.c
> @@ -1244,7 +1244,9 @@ static struct platform_driver * const drivers[] = {
> static int __init mpc85xx_mc_init(void)
> {
> int res = 0;
> +#ifdef CONFIG_FSL_SOC_BOOKE
> u32 pvr = 0;
> +#endif
Nah, this is the ugliest of them all. We want less ifdeffery, not more.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] EDAC, mpc85xx: fix build warning
2016-02-02 15:15 ` Borislav Petkov
@ 2016-02-02 15:19 ` Sudip Mukherjee
0 siblings, 0 replies; 10+ messages in thread
From: Sudip Mukherjee @ 2016-02-02 15:19 UTC (permalink / raw)
To: Borislav Petkov
Cc: Johannes Thumshirn, Guenter Roeck, Johannes Thumshirn,
Doug Thompson, Mauro Carvalho Chehab, linux-kernel, linux-edac
On Tue, Feb 02, 2016 at 04:15:43PM +0100, Borislav Petkov wrote:
> On Tue, Feb 02, 2016 at 08:43:57PM +0530, Sudip Mukherjee wrote:
> > another way might be:
> >
> > diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
> > index b7139c1..968c0c0 100644
> > --- a/drivers/edac/mpc85xx_edac.c
> > +++ b/drivers/edac/mpc85xx_edac.c
> > @@ -1244,7 +1244,9 @@ static struct platform_driver * const drivers[] = {
> > static int __init mpc85xx_mc_init(void)
> > {
> > int res = 0;
> > +#ifdef CONFIG_FSL_SOC_BOOKE
> > u32 pvr = 0;
> > +#endif
>
> Nah, this is the ugliest of them all. We want less ifdeffery, not more.
ok, i am resending with __maybe_unused
regards
sudip
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-02-02 15:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 8:00 [PATCH] EDAC, mpc85xx: fix build warning Sudip Mukherjee
2016-02-02 11:15 ` Borislav Petkov
2016-02-02 14:38 ` Guenter Roeck
2016-02-02 14:48 ` Johannes Thumshirn
2016-02-02 14:55 ` Sudip Mukherjee
2016-02-02 15:09 ` Borislav Petkov
2016-02-02 15:13 ` Sudip Mukherjee
2016-02-02 15:15 ` Borislav Petkov
2016-02-02 15:19 ` Sudip Mukherjee
2016-02-02 14:48 ` Sudip Mukherjee
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.