There is a recent discussion at KS ML with regards to use W=1 as default.
No idea if this will happen or not, but it doesn't hurt cleaning up W=1
warnings from the EDAC subsystem, specially since it helps to cleanup
a few things.
This patch series addresses most of such warnings. After this series,
there will be just two W=1 warnings:
1) i5100 EDAC driver:
drivers/edac/i5100_edac.c: In function ‘i5100_read_log’:
drivers/edac/i5100_edac.c:487:11: warning: variable ‘ecc_loc’ set but not used [-Wunused-but-set-variable]
487 | unsigned ecc_loc = 0;
| ^~~~~~~
The ecc_loc contents is filled from MC data, but it is not used anywere.
The i5100 MC is very old: the affected code was added in 2008. It should
probably be safe to just drop the corresponding data, but, as it may
contain some relevant info, I was a little reticent of doing that.
2) Xgene EDAC driver:
drivers/edac/xgene_edac.c: In function ‘xgene_edac_rb_report’:
drivers/edac/xgene_edac.c:1486:7: warning: variable ‘address’ set but not used [-Wunused-but-set-variable]
1486 | u32 address;
| ^~~~~~~
I suspect that the content of the address field should actually be used on
at least some of the logs.
I may eventually submit patches later to address the above cases, but let's
solve first the other cases, as they all sound trivial enough.
Mauro Carvalho Chehab (7):
EDAC: i5100_edac: get rid of an unused var
EDAC: i7300_edac: rename a kernel-doc var description
EDAC: i7300_edac: fix a kernel-doc syntax
EDAC: i5400_edac: print type at debug message
EDAC: i5400_edac: get rid of some unused vars
EDAC: sb_edac: get rid of unused vars
EDAC: skx_common: get rid of unused type var
drivers/edac/i5100_edac.c | 2 --
drivers/edac/i5400_edac.c | 15 +++------------
drivers/edac/i7300_edac.c | 4 ++--
drivers/edac/sb_edac.c | 21 ++++++++-------------
drivers/edac/skx_common.c | 5 +----
5 files changed, 14 insertions(+), 33 deletions(-)
--
2.21.0
One var was renamed, but the associated kernel-doc markup still
points to the old name.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/edac/i7300_edac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c
index 7bf910d54d11..3d4bd3bf317c 100644
--- a/drivers/edac/i7300_edac.c
+++ b/drivers/edac/i7300_edac.c
@@ -580,7 +580,7 @@ static void i7300_enable_error_reporting(struct mem_ctl_info *mci)
* @ch: Channel number within the branch (0 or 1)
* @branch: Branch number (0 or 1)
* @dinfo: Pointer to DIMM info where dimm size is stored
- * @p_csrow: Pointer to the struct csrow_info that corresponds to that element
+ * @dimm: Pointer to the struct dimm_info that corresponds to that element
*/
static int decode_mtr(struct i7300_pvt *pvt,
int slot, int ch, int branch,
--
2.21.0
There are several temporary unused vars:
drivers/edac/i5400_edac.c: In function ‘i5400_get_mc_regs’:
drivers/edac/i5400_edac.c:1058:6: warning: variable ‘maxdimmperch’ set but not used [-Wunused-but-set-variable]
1058 | int maxdimmperch;
| ^~~~~~~~~~~~
drivers/edac/i5400_edac.c:1057:6: warning: variable ‘maxch’ set but not used [-Wunused-but-set-variable]
1057 | int maxch;
| ^~~~~
drivers/edac/i5400_edac.c: In function ‘i5400_init_dimms’:
drivers/edac/i5400_edac.c:1174:6: warning: variable ‘max_dimms’ set but not used [-Wunused-but-set-variable]
1174 | int max_dimms;
| ^~~~~~~~~
drivers/edac/i5400_edac.c:1173:14: warning: variable ‘channel_count’ set but not used [-Wunused-but-set-variable]
1173 | int ndimms, channel_count;
| ^~~~~~~~~~~~~
Get rid of them.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/edac/i5400_edac.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/edac/i5400_edac.c b/drivers/edac/i5400_edac.c
index 52783b9bd0df..8c86c6fd7da7 100644
--- a/drivers/edac/i5400_edac.c
+++ b/drivers/edac/i5400_edac.c
@@ -1054,8 +1054,6 @@ static void i5400_get_mc_regs(struct mem_ctl_info *mci)
u32 actual_tolm;
u16 limit;
int slot_row;
- int maxch;
- int maxdimmperch;
int way0, way1;
pvt = mci->pvt_info;
@@ -1065,9 +1063,6 @@ static void i5400_get_mc_regs(struct mem_ctl_info *mci)
pci_read_config_dword(pvt->system_address, AMBASE + sizeof(u32),
&pvt->u.ambase_top);
- maxdimmperch = pvt->maxdimmperch;
- maxch = pvt->maxch;
-
edac_dbg(2, "AMBASE= 0x%lx MAXCH= %d MAX-DIMM-Per-CH= %d\n",
(long unsigned int)pvt->ambase, pvt->maxch, pvt->maxdimmperch);
@@ -1170,17 +1165,13 @@ static int i5400_init_dimms(struct mem_ctl_info *mci)
{
struct i5400_pvt *pvt;
struct dimm_info *dimm;
- int ndimms, channel_count;
- int max_dimms;
+ int ndimms;
int mtr;
int size_mb;
int channel, slot;
pvt = mci->pvt_info;
- channel_count = pvt->maxch;
- max_dimms = pvt->maxdimmperch;
-
ndimms = 0;
/*
--
2.21.0
On Fri, Sep 13, 2019 at 11:50:25AM -0300, Mauro Carvalho Chehab wrote:
> There is a recent discussion at KS ML with regards to use W=1 as default.
>
> No idea if this will happen or not, but it doesn't hurt cleaning up W=1
> warnings from the EDAC subsystem, specially since it helps to cleanup
> a few things.
>
> This patch series addresses most of such warnings. After this series,
> there will be just two W=1 warnings:
>
> 1) i5100 EDAC driver:
>
> drivers/edac/i5100_edac.c: In function ‘i5100_read_log’:
> drivers/edac/i5100_edac.c:487:11: warning: variable ‘ecc_loc’ set but not used [-Wunused-but-set-variable]
> 487 | unsigned ecc_loc = 0;
> | ^~~~~~~
>
>
> The ecc_loc contents is filled from MC data, but it is not used anywere.
> The i5100 MC is very old: the affected code was added in 2008. It should
> probably be safe to just drop the corresponding data, but, as it may
> contain some relevant info, I was a little reticent of doing that.
>
> 2) Xgene EDAC driver:
>
> drivers/edac/xgene_edac.c: In function ‘xgene_edac_rb_report’:
> drivers/edac/xgene_edac.c:1486:7: warning: variable ‘address’ set but not used [-Wunused-but-set-variable]
> 1486 | u32 address;
> | ^~~~~~~
>
> I suspect that the content of the address field should actually be used on
> at least some of the logs.
+ Khuong Dinh <[email protected]> for that.
> I may eventually submit patches later to address the above cases, but let's
> solve first the other cases, as they all sound trivial enough.
>
> Mauro Carvalho Chehab (7):
> EDAC: i5100_edac: get rid of an unused var
> EDAC: i7300_edac: rename a kernel-doc var description
> EDAC: i7300_edac: fix a kernel-doc syntax
> EDAC: i5400_edac: print type at debug message
> EDAC: i5400_edac: get rid of some unused vars
> EDAC: sb_edac: get rid of unused vars
> EDAC: skx_common: get rid of unused type var
>
> drivers/edac/i5100_edac.c | 2 --
> drivers/edac/i5400_edac.c | 15 +++------------
> drivers/edac/i7300_edac.c | 4 ++--
> drivers/edac/sb_edac.c | 21 ++++++++-------------
> drivers/edac/skx_common.c | 5 +----
> 5 files changed, 14 insertions(+), 33 deletions(-)
Looks ok to me at a quick glance, ACK.
I've already sent the 5.4 pull request to Linus so you could queue those
after -rc1. It's not like they're urgent or so.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
> Looks ok to me at a quick glance, ACK.
Me too. Also ACK.
-Tony
The declaration of the kerneldoc entry is wrong, causing this
warning:
drivers/edac/i7300_edac.c:824: warning: Function parameter or member 'mir_no' not described in 'decode_mir'
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/edac/i7300_edac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/edac/i7300_edac.c b/drivers/edac/i7300_edac.c
index 3d4bd3bf317c..447d357c7a67 100644
--- a/drivers/edac/i7300_edac.c
+++ b/drivers/edac/i7300_edac.c
@@ -817,7 +817,7 @@ static int i7300_init_csrows(struct mem_ctl_info *mci)
/**
* decode_mir() - Decodes Memory Interleave Register (MIR) info
- * @int mir_no: number of the MIR register to decode
+ * @mir_no: number of the MIR register to decode
* @mir: array with the MIR data cached on the driver
*/
static void decode_mir(int mir_no, u16 mir[MAX_MIR])
--
2.21.0
drivers/edac/skx_common.c: In function ‘skx_mce_output_error’:
drivers/edac/skx_common.c:478:8: warning: variable ‘type’ set but not used [-Wunused-but-set-variable]
478 | char *type, *optype;
| ^~~~
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/edac/skx_common.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/edac/skx_common.c b/drivers/edac/skx_common.c
index d8ff63d91b86..83dd5da67a28 100644
--- a/drivers/edac/skx_common.c
+++ b/drivers/edac/skx_common.c
@@ -475,7 +475,7 @@ static void skx_mce_output_error(struct mem_ctl_info *mci,
struct decoded_addr *res)
{
enum hw_event_mc_err_type tp_event;
- char *type, *optype;
+ char *optype;
bool ripv = GET_BITFIELD(m->mcgstatus, 0, 0);
bool overflow = GET_BITFIELD(m->status, 62, 62);
bool uncorrected_error = GET_BITFIELD(m->status, 61, 61);
@@ -490,14 +490,11 @@ static void skx_mce_output_error(struct mem_ctl_info *mci,
if (uncorrected_error) {
core_err_cnt = 1;
if (ripv) {
- type = "FATAL";
tp_event = HW_EVENT_ERR_FATAL;
} else {
- type = "NON_FATAL";
tp_event = HW_EVENT_ERR_UNCORRECTED;
}
} else {
- type = "CORRECTED";
tp_event = HW_EVENT_ERR_CORRECTED;
}
--
2.21.0
As reported by GCC with W=1:
drivers/edac/i5100_edac.c:714:16: warning: variable ‘et’ set but not used [-Wunused-but-set-variable]
714 | unsigned long et;
| ^~
It sounds some left over from some code before the addition of
an udelay().
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/edac/i5100_edac.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/edac/i5100_edac.c b/drivers/edac/i5100_edac.c
index 251f2b692785..12bebecb203b 100644
--- a/drivers/edac/i5100_edac.c
+++ b/drivers/edac/i5100_edac.c
@@ -713,7 +713,6 @@ static int i5100_read_spd_byte(const struct mem_ctl_info *mci,
{
struct i5100_priv *priv = mci->pvt_info;
u16 w;
- unsigned long et;
pci_read_config_word(priv->mc, I5100_SPDDATA, &w);
if (i5100_spddata_busy(w))
@@ -724,7 +723,6 @@ static int i5100_read_spd_byte(const struct mem_ctl_info *mci,
0, 0));
/* wait up to 100ms */
- et = jiffies + HZ / 10;
udelay(100);
while (1) {
pci_read_config_word(priv->mc, I5100_SPDDATA, &w);
--
2.21.0
There are several vars unused on this driver, probably because
it was a modified copy of another driver. Get rid of them.
drivers/edac/sb_edac.c: In function ‘knl_get_dimm_capacity’:
drivers/edac/sb_edac.c:1343:16: warning: variable ‘sad_size’ set but not used [-Wunused-but-set-variable]
1343 | u64 sad_base, sad_size, sad_limit = 0;
| ^~~~~~~~
drivers/edac/sb_edac.c: In function ‘sbridge_mce_output_error’:
drivers/edac/sb_edac.c:2955:8: warning: variable ‘type’ set but not used [-Wunused-but-set-variable]
2955 | char *type, *optype, msg[256];
| ^~~~
drivers/edac/sb_edac.c: In function ‘sbridge_unregister_mci’:
drivers/edac/sb_edac.c:3203:22: warning: variable ‘pvt’ set but not used [-Wunused-but-set-variable]
3203 | struct sbridge_pvt *pvt;
| ^~~
At top level:
drivers/edac/sb_edac.c:266:18: warning: ‘correrrthrsld’ defined but not used [-Wunused-const-variable=]
266 | static const u32 correrrthrsld[] = {
| ^~~~~~~~~~~~~
drivers/edac/sb_edac.c:257:18: warning: ‘correrrcnt’ defined but not used [-Wunused-const-variable=]
257 | static const u32 correrrcnt[] = {
| ^~~~~~~~~~
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/edac/sb_edac.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 37746b045e18..813f66ffc09a 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -254,18 +254,20 @@ static const u32 rir_offset[MAX_RIR_RANGES][MAX_RIR_WAY] = {
* FIXME: Implement the error count reads directly
*/
-static const u32 correrrcnt[] = {
- 0x104, 0x108, 0x10c, 0x110,
-};
-
#define RANK_ODD_OV(reg) GET_BITFIELD(reg, 31, 31)
#define RANK_ODD_ERR_CNT(reg) GET_BITFIELD(reg, 16, 30)
#define RANK_EVEN_OV(reg) GET_BITFIELD(reg, 15, 15)
#define RANK_EVEN_ERR_CNT(reg) GET_BITFIELD(reg, 0, 14)
+#if 0 /* Currently unused*/
+static const u32 correrrcnt[] = {
+ 0x104, 0x108, 0x10c, 0x110,
+};
+
static const u32 correrrthrsld[] = {
0x11c, 0x120, 0x124, 0x128,
};
+#endif
#define RANK_ODD_ERR_THRSLD(reg) GET_BITFIELD(reg, 16, 30)
#define RANK_EVEN_ERR_THRSLD(reg) GET_BITFIELD(reg, 0, 14)
@@ -1340,7 +1342,7 @@ static void knl_show_mc_route(u32 reg, char *s)
*/
static int knl_get_dimm_capacity(struct sbridge_pvt *pvt, u64 *mc_sizes)
{
- u64 sad_base, sad_size, sad_limit = 0;
+ u64 sad_base, sad_limit = 0;
u64 tad_base, tad_size, tad_limit, tad_deadspace, tad_livespace;
int sad_rule = 0;
int tad_rule = 0;
@@ -1427,7 +1429,6 @@ static int knl_get_dimm_capacity(struct sbridge_pvt *pvt, u64 *mc_sizes)
edram_only = KNL_EDRAM_ONLY(dram_rule);
sad_limit = pvt->info.sad_limit(dram_rule)+1;
- sad_size = sad_limit - sad_base;
pci_read_config_dword(pvt->pci_sad0,
pvt->info.interleave_list[sad_rule], &interleave_reg);
@@ -2952,7 +2953,7 @@ static void sbridge_mce_output_error(struct mem_ctl_info *mci,
struct mem_ctl_info *new_mci;
struct sbridge_pvt *pvt = mci->pvt_info;
enum hw_event_mc_err_type tp_event;
- char *type, *optype, msg[256];
+ char *optype, msg[256];
bool ripv = GET_BITFIELD(m->mcgstatus, 0, 0);
bool overflow = GET_BITFIELD(m->status, 62, 62);
bool uncorrected_error = GET_BITFIELD(m->status, 61, 61);
@@ -2981,14 +2982,11 @@ static void sbridge_mce_output_error(struct mem_ctl_info *mci,
if (uncorrected_error) {
core_err_cnt = 1;
if (ripv) {
- type = "FATAL";
tp_event = HW_EVENT_ERR_FATAL;
} else {
- type = "NON_FATAL";
tp_event = HW_EVENT_ERR_UNCORRECTED;
}
} else {
- type = "CORRECTED";
tp_event = HW_EVENT_ERR_CORRECTED;
}
@@ -3200,7 +3198,6 @@ static struct notifier_block sbridge_mce_dec = {
static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
{
struct mem_ctl_info *mci = sbridge_dev->mci;
- struct sbridge_pvt *pvt;
if (unlikely(!mci || !mci->pvt_info)) {
edac_dbg(0, "MC: dev = %p\n", &sbridge_dev->pdev[0]->dev);
@@ -3209,8 +3206,6 @@ static void sbridge_unregister_mci(struct sbridge_dev *sbridge_dev)
return;
}
- pvt = mci->pvt_info;
-
edac_dbg(0, "MC: mci = %p, dev = %p\n",
mci, &sbridge_dev->pdev[0]->dev);
--
2.21.0
Em Fri, 13 Sep 2019 19:53:20 +0200
Borislav Petkov <[email protected]> escreveu:
> On Fri, Sep 13, 2019 at 11:50:25AM -0300, Mauro Carvalho Chehab wrote:
> > There is a recent discussion at KS ML with regards to use W=1 as default.
> >
> > No idea if this will happen or not, but it doesn't hurt cleaning up W=1
> > warnings from the EDAC subsystem, specially since it helps to cleanup
> > a few things.
> >
> > This patch series addresses most of such warnings. After this series,
> > there will be just two W=1 warnings:
> >
> > 1) i5100 EDAC driver:
> >
> > drivers/edac/i5100_edac.c: In function ‘i5100_read_log’:
> > drivers/edac/i5100_edac.c:487:11: warning: variable ‘ecc_loc’ set but not used [-Wunused-but-set-variable]
> > 487 | unsigned ecc_loc = 0;
> > | ^~~~~~~
> >
> >
> > The ecc_loc contents is filled from MC data, but it is not used anywere.
> > The i5100 MC is very old: the affected code was added in 2008. It should
> > probably be safe to just drop the corresponding data, but, as it may
> > contain some relevant info, I was a little reticent of doing that.
> >
> > 2) Xgene EDAC driver:
> >
> > drivers/edac/xgene_edac.c: In function ‘xgene_edac_rb_report’:
> > drivers/edac/xgene_edac.c:1486:7: warning: variable ‘address’ set but not used [-Wunused-but-set-variable]
> > 1486 | u32 address;
> > | ^~~~~~~
> >
> > I suspect that the content of the address field should actually be used on
> > at least some of the logs.
>
> + Khuong Dinh <[email protected]> for that.
Thanks!
>
> > I may eventually submit patches later to address the above cases, but let's
> > solve first the other cases, as they all sound trivial enough.
> >
> > Mauro Carvalho Chehab (7):
> > EDAC: i5100_edac: get rid of an unused var
> > EDAC: i7300_edac: rename a kernel-doc var description
> > EDAC: i7300_edac: fix a kernel-doc syntax
> > EDAC: i5400_edac: print type at debug message
> > EDAC: i5400_edac: get rid of some unused vars
> > EDAC: sb_edac: get rid of unused vars
> > EDAC: skx_common: get rid of unused type var
> >
> > drivers/edac/i5100_edac.c | 2 --
> > drivers/edac/i5400_edac.c | 15 +++------------
> > drivers/edac/i7300_edac.c | 4 ++--
> > drivers/edac/sb_edac.c | 21 ++++++++-------------
> > drivers/edac/skx_common.c | 5 +----
> > 5 files changed, 14 insertions(+), 33 deletions(-)
>
> Looks ok to me at a quick glance, ACK.
>
> I've already sent the 5.4 pull request to Linus so you could queue those
> after -rc1. It's not like they're urgent or so.
Yeah, that's my plan.
Thanks,
Mauro