ipmi:ipmi_si_intf.c Fix warning: variable 'addr_space' set but not used
in [Kernel]
|
Prev: linux-next: manual merge of the device-mapper tree with the block tree
Next: [PATCH] staging: tidspbridge: gen: simplify and clean up
From: Justin P. Mattock on 6 Jul 2010 10:40 On 07/06/2010 06:57 AM, Corey Minyard wrote: > Well, this patch is incorrect because IPMI_IO_ADDR_SPACE is zero. I'm > not sure what your change is trying to accomplish, anyway. This is > actually dead code from a previous change, I believe, and just needs to > be removed. I did find that document at > http://h21007.www2.hp.com/portal/download/files/unprot/hpspmi.pdf, not > sure if there's a more reliable place to find it. > > -corey > cool thanks.. so it's dead code..I'll resend then with it removed. as for the web address I don't know how you found that.. i.e. I spent at least 45mins looking on that site for that *.pdf(searching gives nothing etc..) > On 07/05/2010 11:31 PM, Justin P. Mattock wrote: >> Remove addr_space in exchange for two symbols that represent >> mm = IPMI_MEM_ADDR_SPACE; >> io = IPMI_IO_ADDR_SPACE; >> then add a dev_warn printing information so that GCC doesn't give a >> warning when building the kernel. >> the original warning from GCC is this: >> >> CC [M] drivers/char/ipmi/ipmi_si_intf.o >> drivers/char/ipmi/ipmi_si_intf.c: In function 'try_init_spmi': >> drivers/char/ipmi/ipmi_si_intf.c:2016:8: warning: variable >> 'addr_space' set but not used >> >> And also the web address pointing to a *.pdf is no where to be >> found(or atleast I couldn't >> find it), so just use the web site itself(if somebody has the *.pdf >> let me know I can change this). >> >> Signed-off-by: Justin P. Mattock<justinmattock(a)gmail.com> >> >> --- >> drivers/char/ipmi/ipmi_si_intf.c | 14 +++++++++----- >> 1 files changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/char/ipmi/ipmi_si_intf.c >> b/drivers/char/ipmi/ipmi_si_intf.c >> index 094bdc3..969e69a 100644 >> --- a/drivers/char/ipmi/ipmi_si_intf.c >> +++ b/drivers/char/ipmi/ipmi_si_intf.c >> @@ -1965,8 +1965,7 @@ static int acpi_gpe_irq_setup(struct smi_info >> *info) >> >> /* >> * Defined at >> - * http://h21007.www2.hp.com/dspp/files/unprotected/devresource/ >> - * Docs/TechPapers/IA64/hpspmi.pdf >> + * http://h21007.www2.hp.com/portal/site/dspp >> */ >> struct SPMITable { >> s8 Signature[4]; >> @@ -2013,7 +2012,7 @@ struct SPMITable { >> static __devinit int try_init_spmi(struct SPMITable *spmi) >> { >> struct smi_info *info; >> - u8 addr_space; >> + u8 mem, io; >> >> if (spmi->IPMIlegacy != 1) { >> printk(KERN_INFO PFX "Bad SPMI legacy %d\n", spmi->IPMIlegacy); >> @@ -2021,9 +2020,14 @@ static __devinit int try_init_spmi(struct >> SPMITable *spmi) >> } >> >> if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) >> - addr_space = IPMI_MEM_ADDR_SPACE; >> + mem = IPMI_MEM_ADDR_SPACE; >> else >> - addr_space = IPMI_IO_ADDR_SPACE; >> + io = IPMI_IO_ADDR_SPACE; >> + >> + if (mem || io) { >> + dev_warn(info->dev, "spmi address space %d\n", mem ?: io); >> + return 0; >> + } >> >> info = kzalloc(sizeof(*info), GFP_KERNEL); >> if (!info) { > > Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 6 Jul 2010 14:30 > Hello Justin. > > [...] >> @@ -2021,10 +2020,6 @@ static __devinit int try_init_spmi(struct SPMITable > *spmi) >> } >> >> if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) >> - addr_space = IPMI_MEM_ADDR_SPACE; >> - else >> - addr_space = IPMI_IO_ADDR_SPACE; >> - >> info = kzalloc(sizeof(*info), GFP_KERNEL); >> if (!info) { >> printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); > > This looks like after applying this patch 'info = ...' will be part of > if statement (without indent before 'info = ...') > > if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > info = kzalloc(sizeof(*info), GFP_KERNEL); > > It's correct? > > Thanks > the original patch I dont think this is correct. a bit confusing with the if(!info) at the bottom of this. here's an updated version if it looks to be correct I'll resend if not I'll redu: --- drivers/char/ipmi/ipmi_si_intf.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 094bdc3..ef03ecd 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2013,19 +2013,12 @@ struct SPMITable { static __devinit int try_init_spmi(struct SPMITable *spmi) { struct smi_info *info; - u8 addr_space; if (spmi->IPMIlegacy != 1) { printk(KERN_INFO PFX "Bad SPMI legacy %d\n", spmi->IPMIlegacy); return -ENODEV; } - if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) - addr_space = IPMI_MEM_ADDR_SPACE; - else - addr_space = IPMI_IO_ADDR_SPACE; - - info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) { printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); return -ENOMEM; -- 1.7.1.rc1.21.gf3bd6 Thanks for the info on this.. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 6 Jul 2010 16:20 On 07/06/2010 01:14 PM, Sergey V. wrote: > On Tuesday 06 of July 2010 22:30:00 Justin P. Mattock wrote: >> >>> Hello Justin. >>> >>> [...] >>>> @@ -2021,10 +2020,6 @@ static __devinit int try_init_spmi(struct SPMITable >>> *spmi) >>>> } >>>> >>>> if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) >>>> - addr_space = IPMI_MEM_ADDR_SPACE; >>>> - else >>>> - addr_space = IPMI_IO_ADDR_SPACE; >>>> - >>>> info = kzalloc(sizeof(*info), GFP_KERNEL); >>>> if (!info) { >>>> printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); >>> >>> This looks like after applying this patch 'info = ...' will be part of >>> if statement (without indent before 'info = ...') >>> >>> if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) >>> info = kzalloc(sizeof(*info), GFP_KERNEL); >>> >>> It's correct? >>> >>> Thanks >>> >> >> the original patch I dont think this is correct. a bit confusing with >> the if(!info) at the bottom of this. >> here's an updated version if it looks to be correct I'll resend if not >> I'll redu: >> >> >> --- >> drivers/char/ipmi/ipmi_si_intf.c | 7 ------- >> 1 files changed, 0 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/char/ipmi/ipmi_si_intf.c >> b/drivers/char/ipmi/ipmi_si_intf.c >> index 094bdc3..ef03ecd 100644 >> --- a/drivers/char/ipmi/ipmi_si_intf.c >> +++ b/drivers/char/ipmi/ipmi_si_intf.c >> @@ -2013,19 +2013,12 @@ struct SPMITable { >> static __devinit int try_init_spmi(struct SPMITable *spmi) >> { >> struct smi_info *info; >> - u8 addr_space; >> >> if (spmi->IPMIlegacy != 1) { >> printk(KERN_INFO PFX "Bad SPMI legacy %d\n", spmi->IPMIlegacy); >> return -ENODEV; >> } >> >> - if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) >> - addr_space = IPMI_MEM_ADDR_SPACE; >> - else >> - addr_space = IPMI_IO_ADDR_SPACE; >> - >> - info = kzalloc(sizeof(*info), GFP_KERNEL); >> if (!info) { >> printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); >> return -ENOMEM; >> -- 1.7.1.rc1.21.gf3bd6 >> >> Thanks for the info on this.. >> >> Justin P. Mattock >> > > Sorry, but I think that new patch is not correct. > > Why do you delete this row "info = kzalloc(sizeof(*info), GFP_KERNEL)"? > There is the only one unused variable `addr_space`, so I think it should > be removed, that's all. > > The following code is correct and not need to be changed: > > info = kzalloc(sizeof(*info), GFP_KERNEL); > if (!info) { > printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); > > Please see below my patch. > Thanks. > > --- > Fix compile warning > drivers/char/ipmi/ipmi_si_intf.c: In function 'try_init_spmi': > drivers/char/ipmi/ipmi_si_intf.c:2016:8: warning: variable 'addr_space' set but > not used > > Signed-off-by: Sergey V.<sftp.mtuci(a)gmail.com> > --- > drivers/char/ipmi/ipmi_si_intf.c | 6 ------ > 1 files changed, 0 insertions(+), 6 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c > b/drivers/char/ipmi/ipmi_si_intf.c > index 094bdc3..1f5f025 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -2013,18 +2013,12 @@ struct SPMITable { > static __devinit int try_init_spmi(struct SPMITable *spmi) > { > struct smi_info *info; > - u8 addr_space; > > if (spmi->IPMIlegacy != 1) { > printk(KERN_INFO PFX "Bad SPMI legacy %d\n", spmi->IPMIlegacy); > return -ENODEV; > } > > - if (spmi->addr.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > - addr_space = IPMI_MEM_ADDR_SPACE; > - else > - addr_space = IPMI_IO_ADDR_SPACE; > - > info = kzalloc(sizeof(*info), GFP_KERNEL); > if (!info) { > printk(KERN_ERR PFX "Could not allocate SI data (3)\n"); well if everybody agrees with your change then all good... Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 8 Jul 2010 00:10 what ever came about with this? Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Corey Minyard on 8 Jul 2010 14:30
On 07/07/2010 11:05 PM, Justin P. Mattock wrote: > what ever came about with this? > > Justin P. Mattock Well, Sergey sent a patch, but I don't think it was quite in a form that can be taken. Do you want to resend per the standard rules? -corey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |