Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/5] EDAC: Small cleanups and fixes
@ 2019-09-02 12:33 Robert Richter
  2019-09-02 12:33 ` [PATCH 1/5] EDAC: Prefer 'unsigned int' to bare use of 'unsigned' Robert Richter
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Robert Richter @ 2019-09-02 12:33 UTC (permalink / raw)
  To: Borislav Petkov, Mauro Carvalho Chehab, Tony Luck
  Cc: Robert Richter, James Morse, linux-edac, linux-kernel

A bunch of cleanups and fixes for issues found while working with the
code. Changes are individual and independent from each other. They can
be applied separately (only #4 depends on #3).

Also updating the reviewer's entry as I will be able to do some
reviews for edac code.

Note that patch #3 is an updated version of a patch reviewed before:

 https://lore.kernel.org/patchwork/patch/1093466/

Some references to ml discussions that are related to this series:

 https://lore.kernel.org/patchwork/patch/1093480/#1312590
 https://lore.kernel.org/patchwork/patch/1093466/#1310572

Robert Richter (5):
  EDAC: Prefer 'unsigned int' to bare use of 'unsigned'
  EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int
  EDAC, mc_sysfs: Remove pointless gotos
  EDAC, mc_sysfs: Make debug messages consistent
  MAINTAINERS: update EDAC's reviewer entry

 MAINTAINERS                  |  1 +
 drivers/edac/edac_mc.c       | 20 ++++----
 drivers/edac/edac_mc.h       |  6 +--
 drivers/edac/edac_mc_sysfs.c | 91 ++++++++++++++++--------------------
 drivers/edac/ghes_edac.c     |  2 +-
 drivers/edac/i5100_edac.c    | 16 ++++---
 include/linux/edac.h         | 10 ++--
 7 files changed, 69 insertions(+), 77 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] EDAC: Prefer 'unsigned int' to bare use of 'unsigned'
  2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
@ 2019-09-02 12:33 ` Robert Richter
  2019-09-02 12:33 ` [PATCH 2/5] EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int Robert Richter
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Robert Richter @ 2019-09-02 12:33 UTC (permalink / raw)
  To: Borislav Petkov, Mauro Carvalho Chehab, Tony Luck
  Cc: Robert Richter, James Morse, linux-edac, linux-kernel

Use of 'unsigned int' instead of bare use of 'unsigned'. Fix this for
edac_mc*, ghes and the i5100 driver.

Depending on the compiler's warning level it can throw messages like
this:

 WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
 #235: FILE: drivers/edac/i5100_edac.c:854:
 +               const unsigned chan = i5100_csrow_to_chan(mci, dimm->idx);

Signed-off-by: Robert Richter <rrichter@marvell.com>
---
 drivers/edac/edac_mc.c       | 20 ++++++++++----------
 drivers/edac/edac_mc.h       |  6 +++---
 drivers/edac/edac_mc_sysfs.c |  6 +++---
 drivers/edac/ghes_edac.c     |  2 +-
 drivers/edac/i5100_edac.c    | 16 +++++++++-------
 include/linux/edac.h         | 10 +++++-----
 6 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index c68f62ab54b0..2f1e3ec8e1cc 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -114,8 +114,8 @@ static const struct kernel_param_ops edac_report_ops = {
 
 module_param_cb(edac_report, &edac_report_ops, &edac_report, 0644);
 
-unsigned edac_dimm_info_location(struct dimm_info *dimm, char *buf,
-			         unsigned len)
+unsigned int edac_dimm_info_location(struct dimm_info *dimm, char *buf,
+				unsigned int len)
 {
 	struct mem_ctl_info *mci = dimm->mci;
 	int i, n, count = 0;
@@ -236,9 +236,9 @@ EXPORT_SYMBOL_GPL(edac_mem_types);
  * At return, the pointer 'p' will be incremented to be used on a next call
  * to this function.
  */
-void *edac_align_ptr(void **p, unsigned size, int n_elems)
+void *edac_align_ptr(void **p, unsigned int size, int n_elems)
 {
-	unsigned align, r;
+	unsigned int align, r;
 	void *ptr = *p;
 
 	*p += size * n_elems;
@@ -302,10 +302,10 @@ static void _edac_mc_free(struct mem_ctl_info *mci)
 	kfree(mci);
 }
 
-struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
-				   unsigned n_layers,
+struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
+				   unsigned int n_layers,
 				   struct edac_mc_layer *layers,
-				   unsigned sz_pvt)
+				   unsigned int sz_pvt)
 {
 	struct mem_ctl_info *mci;
 	struct edac_mc_layer *layer;
@@ -313,9 +313,9 @@ struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
 	struct rank_info *chan;
 	struct dimm_info *dimm;
 	u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
-	unsigned pos[EDAC_MAX_LAYERS];
-	unsigned size, tot_dimms = 1, count = 1;
-	unsigned tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
+	unsigned int pos[EDAC_MAX_LAYERS];
+	unsigned int size, tot_dimms = 1, count = 1;
+	unsigned int tot_csrows = 1, tot_channels = 1, tot_errcount = 0;
 	void *pvt, *p, *ptr = NULL;
 	int i, j, row, chn, n, len, off;
 	bool per_rank = false;
diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 4165e15995ad..02aac5c61d00 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -122,10 +122,10 @@ do {									\
  *	On success, return a pointer to struct mem_ctl_info pointer;
  *	%NULL otherwise
  */
-struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
-				   unsigned n_layers,
+struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
+				   unsigned int n_layers,
 				   struct edac_mc_layer *layers,
-				   unsigned sz_pvt);
+				   unsigned int sz_pvt);
 
 /**
  * edac_get_owner - Return the owner's mod_name of EDAC MC
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 4386ea4b9b5a..640b9419623e 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -200,7 +200,7 @@ static ssize_t channel_dimm_label_show(struct device *dev,
 				       char *data)
 {
 	struct csrow_info *csrow = to_csrow(dev);
-	unsigned chan = to_channel(mattr);
+	unsigned int chan = to_channel(mattr);
 	struct rank_info *rank = csrow->channels[chan];
 
 	/* if field has not been initialized, there is nothing to send */
@@ -216,7 +216,7 @@ static ssize_t channel_dimm_label_store(struct device *dev,
 					const char *data, size_t count)
 {
 	struct csrow_info *csrow = to_csrow(dev);
-	unsigned chan = to_channel(mattr);
+	unsigned int chan = to_channel(mattr);
 	struct rank_info *rank = csrow->channels[chan];
 	size_t copy_count = count;
 
@@ -240,7 +240,7 @@ static ssize_t channel_ce_count_show(struct device *dev,
 				     struct device_attribute *mattr, char *data)
 {
 	struct csrow_info *csrow = to_csrow(dev);
-	unsigned chan = to_channel(mattr);
+	unsigned int chan = to_channel(mattr);
 	struct rank_info *rank = csrow->channels[chan];
 
 	return sprintf(data, "%u\n", rank->ce_count);
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 7f19f1c672c3..d413a0bdc9ad 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -68,7 +68,7 @@ struct memdev_dmi_entry {
 
 struct ghes_edac_dimm_fill {
 	struct mem_ctl_info *mci;
-	unsigned count;
+	unsigned int count;
 };
 
 static void ghes_edac_count_dimms(const struct dmi_header *dh, void *arg)
diff --git a/drivers/edac/i5100_edac.c b/drivers/edac/i5100_edac.c
index b506eef6b146..251f2b692785 100644
--- a/drivers/edac/i5100_edac.c
+++ b/drivers/edac/i5100_edac.c
@@ -417,7 +417,8 @@ static const char *i5100_err_msg(unsigned err)
 }
 
 /* convert csrow index into a rank (per channel -- 0..5) */
-static int i5100_csrow_to_rank(const struct mem_ctl_info *mci, int csrow)
+static unsigned int i5100_csrow_to_rank(const struct mem_ctl_info *mci,
+					unsigned int csrow)
 {
 	const struct i5100_priv *priv = mci->pvt_info;
 
@@ -425,7 +426,8 @@ static int i5100_csrow_to_rank(const struct mem_ctl_info *mci, int csrow)
 }
 
 /* convert csrow index into a channel (0..1) */
-static int i5100_csrow_to_chan(const struct mem_ctl_info *mci, int csrow)
+static unsigned int i5100_csrow_to_chan(const struct mem_ctl_info *mci,
+					unsigned int csrow)
 {
 	const struct i5100_priv *priv = mci->pvt_info;
 
@@ -653,11 +655,11 @@ static struct pci_dev *pci_get_device_func(unsigned vendor,
 	return ret;
 }
 
-static unsigned long i5100_npages(struct mem_ctl_info *mci, int csrow)
+static unsigned long i5100_npages(struct mem_ctl_info *mci, unsigned int csrow)
 {
 	struct i5100_priv *priv = mci->pvt_info;
-	const unsigned chan_rank = i5100_csrow_to_rank(mci, csrow);
-	const unsigned chan = i5100_csrow_to_chan(mci, csrow);
+	const unsigned int chan_rank = i5100_csrow_to_rank(mci, csrow);
+	const unsigned int chan = i5100_csrow_to_chan(mci, csrow);
 	unsigned addr_lines;
 
 	/* dimm present? */
@@ -852,8 +854,8 @@ static void i5100_init_csrows(struct mem_ctl_info *mci)
 	for (i = 0; i < mci->tot_dimms; i++) {
 		struct dimm_info *dimm;
 		const unsigned long npages = i5100_npages(mci, i);
-		const unsigned chan = i5100_csrow_to_chan(mci, i);
-		const unsigned rank = i5100_csrow_to_rank(mci, i);
+		const unsigned int chan = i5100_csrow_to_chan(mci, i);
+		const unsigned int rank = i5100_csrow_to_rank(mci, i);
 
 		if (!npages)
 			continue;
diff --git a/include/linux/edac.h b/include/linux/edac.h
index 342dabda9c7e..c19483b90079 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -440,7 +440,7 @@ struct dimm_info {
 	char label[EDAC_MC_LABEL_LEN + 1];	/* DIMM label on motherboard */
 
 	/* Memory location data */
-	unsigned location[EDAC_MAX_LAYERS];
+	unsigned int location[EDAC_MAX_LAYERS];
 
 	struct mem_ctl_info *mci;	/* the parent */
 
@@ -451,7 +451,7 @@ struct dimm_info {
 
 	u32 nr_pages;			/* number of pages on this dimm */
 
-	unsigned csrow, cschannel;	/* Points to the old API data */
+	unsigned int csrow, cschannel;	/* Points to the old API data */
 
 	u16 smbios_handle;              /* Handle for SMBIOS type 17 */
 };
@@ -597,7 +597,7 @@ struct mem_ctl_info {
 					   unsigned long page);
 	int mc_idx;
 	struct csrow_info **csrows;
-	unsigned nr_csrows, num_cschannel;
+	unsigned int nr_csrows, num_cschannel;
 
 	/*
 	 * Memory Controller hierarchy
@@ -608,14 +608,14 @@ struct mem_ctl_info {
 	 * of the recent drivers enumerate memories per DIMM, instead.
 	 * When the memory controller is per rank, csbased is true.
 	 */
-	unsigned n_layers;
+	unsigned int n_layers;
 	struct edac_mc_layer *layers;
 	bool csbased;
 
 	/*
 	 * DIMM info. Will eventually remove the entire csrows_info some day
 	 */
-	unsigned tot_dimms;
+	unsigned int tot_dimms;
 	struct dimm_info **dimms;
 
 	/*
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/5] EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int
  2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
  2019-09-02 12:33 ` [PATCH 1/5] EDAC: Prefer 'unsigned int' to bare use of 'unsigned' Robert Richter
@ 2019-09-02 12:33 ` Robert Richter
  2019-09-02 13:04   ` Borislav Petkov
  2019-09-02 12:33 ` [PATCH 3/5] EDAC, mc_sysfs: Remove pointless gotos Robert Richter
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Robert Richter @ 2019-09-02 12:33 UTC (permalink / raw)
  To: Borislav Petkov, Mauro Carvalho Chehab, Tony Luck
  Cc: Robert Richter, James Morse, linux-edac, linux-kernel

Struct member dev_ch_attribute->channel is always used as unsigned
int. Change type to unsigned int to avoid type casts.

Signed-off-by: Robert Richter <rrichter@marvell.com>
---
 drivers/edac/edac_mc_sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 640b9419623e..4eb8c5ceb973 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -131,7 +131,7 @@ static const char * const edac_caps[] = {
 
 struct dev_ch_attribute {
 	struct device_attribute attr;
-	int channel;
+	unsigned int channel;
 };
 
 #define DEVICE_CHANNEL(_name, _mode, _show, _store, _var) \
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/5] EDAC, mc_sysfs: Remove pointless gotos
  2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
  2019-09-02 12:33 ` [PATCH 1/5] EDAC: Prefer 'unsigned int' to bare use of 'unsigned' Robert Richter
  2019-09-02 12:33 ` [PATCH 2/5] EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int Robert Richter
@ 2019-09-02 12:33 ` Robert Richter
  2019-09-02 12:33 ` [PATCH 4/5] EDAC, mc_sysfs: Make debug messages consistent Robert Richter
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Robert Richter @ 2019-09-02 12:33 UTC (permalink / raw)
  To: Borislav Petkov, Mauro Carvalho Chehab, Tony Luck
  Cc: Robert Richter, James Morse, linux-edac, linux-kernel

Use direct returns instead of gotos. Error handling code becomes
smaller and better readable.

Signed-off-by: Robert Richter <rrichter@marvell.com>
---
 drivers/edac/edac_mc_sysfs.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 4eb8c5ceb973..309fc24339b0 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -938,7 +938,7 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 	if (err < 0) {
 		edac_dbg(1, "failure: create device %s\n", dev_name(&mci->dev));
 		put_device(&mci->dev);
-		goto out;
+		return err;
 	}
 
 	/*
@@ -987,7 +987,6 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 	}
 	device_unregister(&mci->dev);
 
-out:
 	return err;
 }
 
@@ -1044,10 +1043,8 @@ int __init edac_mc_sysfs_init(void)
 	int err;
 
 	mci_pdev = kzalloc(sizeof(*mci_pdev), GFP_KERNEL);
-	if (!mci_pdev) {
-		err = -ENOMEM;
-		goto out;
-	}
+	if (!mci_pdev)
+		return -ENOMEM;
 
 	mci_pdev->bus = edac_get_sysfs_subsys();
 	mci_pdev->type = &mc_attr_type;
@@ -1055,17 +1052,14 @@ int __init edac_mc_sysfs_init(void)
 	dev_set_name(mci_pdev, "mc");
 
 	err = device_add(mci_pdev);
-	if (err < 0)
-		goto out_put_device;
+	if (err < 0) {
+		put_device(mci_pdev);
+		return err;
+	}
 
 	edac_dbg(0, "device %s created\n", dev_name(mci_pdev));
 
 	return 0;
-
- out_put_device:
-	put_device(mci_pdev);
- out:
-	return err;
 }
 
 void edac_mc_sysfs_exit(void)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] EDAC, mc_sysfs: Make debug messages consistent
  2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
                   ` (2 preceding siblings ...)
  2019-09-02 12:33 ` [PATCH 3/5] EDAC, mc_sysfs: Remove pointless gotos Robert Richter
@ 2019-09-02 12:33 ` Robert Richter
  2019-09-02 12:33 ` [PATCH 5/5] MAINTAINERS: update EDAC's reviewer entry Robert Richter
  2019-09-02 20:17 ` [PATCH 0/5] EDAC: Small cleanups and fixes Mauro Carvalho Chehab
  5 siblings, 0 replies; 9+ messages in thread
From: Robert Richter @ 2019-09-02 12:33 UTC (permalink / raw)
  To: Borislav Petkov, Mauro Carvalho Chehab, Tony Luck
  Cc: Robert Richter, James Morse, linux-edac, linux-kernel

Debug messages are inconsistently used in the error handlers. Some
lack an error message, some are called regardless the return status,
messages for the same error are at different locations in the code
depending on the error code. This happens esp. near put_device()
calls. Make those debug messages more consistent. Additionally, unify
the error messages to have the same terms for the same operations of
the device.

Signed-off-by: Robert Richter <rrichter@marvell.com>
---
 drivers/edac/edac_mc_sysfs.c | 63 +++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 309fc24339b0..eaccde3fc1b8 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -278,7 +278,7 @@ static void csrow_attr_release(struct device *dev)
 {
 	struct csrow_info *csrow = container_of(dev, struct csrow_info, dev);
 
-	edac_dbg(1, "Releasing csrow device %s\n", dev_name(dev));
+	edac_dbg(1, "device %s released\n", dev_name(dev));
 	kfree(csrow);
 }
 
@@ -414,14 +414,16 @@ static int edac_create_csrow_object(struct mem_ctl_info *mci,
 	dev_set_name(&csrow->dev, "csrow%d", index);
 	dev_set_drvdata(&csrow->dev, csrow);
 
-	edac_dbg(0, "creating (virtual) csrow node %s\n",
-		 dev_name(&csrow->dev));
-
 	err = device_add(&csrow->dev);
-	if (err)
+	if (err) {
+		edac_dbg(1, "failure: create device %s\n", dev_name(&csrow->dev));
 		put_device(&csrow->dev);
+		return err;
+	}
 
-	return err;
+	edac_dbg(0, "device %s created\n", dev_name(&csrow->dev));
+
+	return 0;
 }
 
 /* Create a CSROW object under specifed edac_mc_device */
@@ -435,12 +437,8 @@ static int edac_create_csrow_objects(struct mem_ctl_info *mci)
 		if (!nr_pages_per_csrow(csrow))
 			continue;
 		err = edac_create_csrow_object(mci, mci->csrows[i], i);
-		if (err < 0) {
-			edac_dbg(1,
-				 "failure: create csrow objects for csrow %d\n",
-				 i);
+		if (err < 0)
 			goto error;
-		}
 	}
 	return 0;
 
@@ -624,7 +622,7 @@ static void dimm_attr_release(struct device *dev)
 {
 	struct dimm_info *dimm = container_of(dev, struct dimm_info, dev);
 
-	edac_dbg(1, "Releasing dimm device %s\n", dev_name(dev));
+	edac_dbg(1, "device %s released\n", dev_name(dev));
 	kfree(dimm);
 }
 
@@ -653,12 +651,20 @@ static int edac_create_dimm_object(struct mem_ctl_info *mci,
 	pm_runtime_forbid(&mci->dev);
 
 	err = device_add(&dimm->dev);
-	if (err)
+	if (err) {
+		edac_dbg(1, "failure: create device %s\n", dev_name(&dimm->dev));
 		put_device(&dimm->dev);
+		return err;
+	}
 
-	edac_dbg(0, "created rank/dimm device %s\n", dev_name(&dimm->dev));
+	if (IS_ENABLED(CONFIG_EDAC_DEBUG)) {
+		char location[80];
+		edac_dimm_info_location(dimm, location, sizeof(location));
+		edac_dbg(0, "device %s created at location %s\n",
+			dev_name(&dimm->dev), location);
+	}
 
-	return err;
+	return 0;
 }
 
 /*
@@ -901,7 +907,7 @@ static void mci_attr_release(struct device *dev)
 {
 	struct mem_ctl_info *mci = container_of(dev, struct mem_ctl_info, dev);
 
-	edac_dbg(1, "Releasing csrow device %s\n", dev_name(dev));
+	edac_dbg(1, "device %s released\n", dev_name(dev));
 	kfree(mci);
 }
 
@@ -933,7 +939,6 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 	dev_set_drvdata(&mci->dev, mci);
 	pm_runtime_forbid(&mci->dev);
 
-	edac_dbg(0, "creating device %s\n", dev_name(&mci->dev));
 	err = device_add(&mci->dev);
 	if (err < 0) {
 		edac_dbg(1, "failure: create device %s\n", dev_name(&mci->dev));
@@ -941,6 +946,8 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 		return err;
 	}
 
+	edac_dbg(0, "device %s created\n", dev_name(&mci->dev));
+
 	/*
 	 * Create the dimm/rank devices
 	 */
@@ -950,22 +957,9 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci,
 		if (!dimm->nr_pages)
 			continue;
 
-#ifdef CONFIG_EDAC_DEBUG
-		edac_dbg(1, "creating dimm%d, located at ", i);
-		if (edac_debug_level >= 1) {
-			int lay;
-			for (lay = 0; lay < mci->n_layers; lay++)
-				printk(KERN_CONT "%s %d ",
-					edac_layer_name[mci->layers[lay].type],
-					dimm->location[lay]);
-			printk(KERN_CONT "\n");
-		}
-#endif
 		err = edac_create_dimm_object(mci, dimm, i);
-		if (err) {
-			edac_dbg(1, "failure: create dimm %d obj\n", i);
+		if (err)
 			goto fail_unregister_dimm;
-		}
 	}
 
 #ifdef CONFIG_EDAC_LEGACY_SYSFS
@@ -1010,14 +1004,14 @@ void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci)
 		struct dimm_info *dimm = mci->dimms[i];
 		if (dimm->nr_pages == 0)
 			continue;
-		edac_dbg(0, "removing device %s\n", dev_name(&dimm->dev));
+		edac_dbg(1, "unregistering device %s\n", dev_name(&dimm->dev));
 		device_unregister(&dimm->dev);
 	}
 }
 
 void edac_unregister_sysfs(struct mem_ctl_info *mci)
 {
-	edac_dbg(1, "Unregistering device %s\n", dev_name(&mci->dev));
+	edac_dbg(1, "unregistering device %s\n", dev_name(&mci->dev));
 	device_unregister(&mci->dev);
 }
 
@@ -1028,7 +1022,7 @@ static void mc_attr_release(struct device *dev)
 	 * parent device, used to create the /sys/devices/mc sysfs node.
 	 * So, there are no attributes on it.
 	 */
-	edac_dbg(1, "Releasing device %s\n", dev_name(dev));
+	edac_dbg(1, "device %s released\n", dev_name(dev));
 	kfree(dev);
 }
 
@@ -1053,6 +1047,7 @@ int __init edac_mc_sysfs_init(void)
 
 	err = device_add(mci_pdev);
 	if (err < 0) {
+		edac_dbg(1, "failure: create device %s\n", dev_name(mci_pdev));
 		put_device(mci_pdev);
 		return err;
 	}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 5/5] MAINTAINERS: update EDAC's reviewer entry
  2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
                   ` (3 preceding siblings ...)
  2019-09-02 12:33 ` [PATCH 4/5] EDAC, mc_sysfs: Make debug messages consistent Robert Richter
@ 2019-09-02 12:33 ` Robert Richter
  2019-09-02 20:17 ` [PATCH 0/5] EDAC: Small cleanups and fixes Mauro Carvalho Chehab
  5 siblings, 0 replies; 9+ messages in thread
From: Robert Richter @ 2019-09-02 12:33 UTC (permalink / raw)
  To: Borislav Petkov, Mauro Carvalho Chehab, Tony Luck
  Cc: Robert Richter, James Morse, linux-edac, linux-kernel

I did some significant work with code in edac_mc.c and ghes_edac.c
already, so I guess I can probably helping out a bit as code reviewer
here.

Signed-off-by: Robert Richter <rrichter@marvell.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 97912d8333a9..79d2ae8519e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5788,6 +5788,7 @@ M:	Borislav Petkov <bp@alien8.de>
 M:	Mauro Carvalho Chehab <mchehab@kernel.org>
 M:	Tony Luck <tony.luck@intel.com>
 R:	James Morse <james.morse@arm.com>
+R:	Robert Richter <rrichter@marvell.com>
 L:	linux-edac@vger.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git edac-for-next
 S:	Supported
-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/5] EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int
  2019-09-02 12:33 ` [PATCH 2/5] EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int Robert Richter
@ 2019-09-02 13:04   ` Borislav Petkov
  0 siblings, 0 replies; 9+ messages in thread
From: Borislav Petkov @ 2019-09-02 13:04 UTC (permalink / raw)
  To: Robert Richter
  Cc: Mauro Carvalho Chehab, Tony Luck, James Morse, linux-edac, linux-kernel

On Mon, Sep 02, 2019 at 12:33:43PM +0000, Robert Richter wrote:
> Struct member dev_ch_attribute->channel is always used as unsigned
> int. Change type to unsigned int to avoid type casts.
> 
> Signed-off-by: Robert Richter <rrichter@marvell.com>
> ---
>  drivers/edac/edac_mc_sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
> index 640b9419623e..4eb8c5ceb973 100644
> --- a/drivers/edac/edac_mc_sysfs.c
> +++ b/drivers/edac/edac_mc_sysfs.c
> @@ -131,7 +131,7 @@ static const char * const edac_caps[] = {
>  
>  struct dev_ch_attribute {
>  	struct device_attribute attr;
> -	int channel;
> +	unsigned int channel;
>  };
>  
>  #define DEVICE_CHANNEL(_name, _mode, _show, _store, _var) \
> -- 

Merged this one into 1/5.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/5] EDAC: Small cleanups and fixes
  2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
                   ` (4 preceding siblings ...)
  2019-09-02 12:33 ` [PATCH 5/5] MAINTAINERS: update EDAC's reviewer entry Robert Richter
@ 2019-09-02 20:17 ` Mauro Carvalho Chehab
  2019-09-04 15:53   ` Borislav Petkov
  5 siblings, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2019-09-02 20:17 UTC (permalink / raw)
  To: Robert Richter
  Cc: Borislav Petkov, Tony Luck, James Morse, linux-edac, linux-kernel

Em Mon, 2 Sep 2019 12:33:38 +0000
Robert Richter <rrichter@marvell.com> escreveu:

> A bunch of cleanups and fixes for issues found while working with the
> code. Changes are individual and independent from each other. They can
> be applied separately (only #4 depends on #3).
> 
> Also updating the reviewer's entry as I will be able to do some
> reviews for edac code.
> 
> Note that patch #3 is an updated version of a patch reviewed before:
> 
>  https://lore.kernel.org/patchwork/patch/1093466/
> 
> Some references to ml discussions that are related to this series:
> 
>  https://lore.kernel.org/patchwork/patch/1093480/#1312590
>  https://lore.kernel.org/patchwork/patch/1093466/#1310572
> 
> Robert Richter (5):
>   EDAC: Prefer 'unsigned int' to bare use of 'unsigned'
>   EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int
>   EDAC, mc_sysfs: Remove pointless gotos
>   EDAC, mc_sysfs: Make debug messages consistent
>   MAINTAINERS: update EDAC's reviewer entry

For the entire series:

Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>

> 
>  MAINTAINERS                  |  1 +
>  drivers/edac/edac_mc.c       | 20 ++++----
>  drivers/edac/edac_mc.h       |  6 +--
>  drivers/edac/edac_mc_sysfs.c | 91 ++++++++++++++++--------------------
>  drivers/edac/ghes_edac.c     |  2 +-
>  drivers/edac/i5100_edac.c    | 16 ++++---
>  include/linux/edac.h         | 10 ++--
>  7 files changed, 69 insertions(+), 77 deletions(-)
> 



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/5] EDAC: Small cleanups and fixes
  2019-09-02 20:17 ` [PATCH 0/5] EDAC: Small cleanups and fixes Mauro Carvalho Chehab
@ 2019-09-04 15:53   ` Borislav Petkov
  0 siblings, 0 replies; 9+ messages in thread
From: Borislav Petkov @ 2019-09-04 15:53 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Robert Richter
  Cc: Tony Luck, James Morse, linux-edac, linux-kernel

On Mon, Sep 02, 2019 at 05:17:16PM -0300, Mauro Carvalho Chehab wrote:
> > Robert Richter (5):
> >   EDAC: Prefer 'unsigned int' to bare use of 'unsigned'
> >   EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int
> >   EDAC, mc_sysfs: Remove pointless gotos
> >   EDAC, mc_sysfs: Make debug messages consistent
> >   MAINTAINERS: update EDAC's reviewer entry
> 
> For the entire series:
> 
> Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> > 
> >  MAINTAINERS                  |  1 +
> >  drivers/edac/edac_mc.c       | 20 ++++----
> >  drivers/edac/edac_mc.h       |  6 +--
> >  drivers/edac/edac_mc_sysfs.c | 91 ++++++++++++++++--------------------
> >  drivers/edac/ghes_edac.c     |  2 +-
> >  drivers/edac/i5100_edac.c    | 16 ++++---
> >  include/linux/edac.h         | 10 ++--
> >  7 files changed, 69 insertions(+), 77 deletions(-)

Queued, thanks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-02 12:33 [PATCH 0/5] EDAC: Small cleanups and fixes Robert Richter
2019-09-02 12:33 ` [PATCH 1/5] EDAC: Prefer 'unsigned int' to bare use of 'unsigned' Robert Richter
2019-09-02 12:33 ` [PATCH 2/5] EDAC, mc_sysfs: Change dev_ch_attribute->channel to unsigned int Robert Richter
2019-09-02 13:04   ` Borislav Petkov
2019-09-02 12:33 ` [PATCH 3/5] EDAC, mc_sysfs: Remove pointless gotos Robert Richter
2019-09-02 12:33 ` [PATCH 4/5] EDAC, mc_sysfs: Make debug messages consistent Robert Richter
2019-09-02 12:33 ` [PATCH 5/5] MAINTAINERS: update EDAC's reviewer entry Robert Richter
2019-09-02 20:17 ` [PATCH 0/5] EDAC: Small cleanups and fixes Mauro Carvalho Chehab
2019-09-04 15:53   ` Borislav Petkov

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org linux-edac@archiver.kernel.org
	public-inbox-index linux-edac


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/ public-inbox