[PATCH 1/3] EDAC: mv64x60: remove unused variable

[PATCH 1/3] EDAC: mv64x60: remove unused variable

[Chris Packham]

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/edac/mv64x60_edac.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
index 14b7e7b71eaa..454e1e26ee7c 100644
--- a/drivers/edac/mv64x60_edac.c
+++ b/drivers/edac/mv64x60_edac.c
@@ -853,8 +853,6 @@ static struct platform_driver * const drivers[] = {
 
 static int __init mv64x60_edac_init(void)
 {
-	int ret = 0;
-
 	printk(KERN_INFO "Marvell MV64x60 EDAC driver " MV64x60_REVISION "\n");
 	printk(KERN_INFO "\t(C) 2006-2007 MontaVista Software\n");
 	/* make sure error reporting method is sane */
-- 
2.11.0.24.ge6920cf

[PATCH 2/3] EDAC: mv64x60: Fix pdata->name

[Chris Packham]

Change this from mpc85xx_pci_err to mv64x60_pci_err.  The former is
likely a hangover from when this driver was created.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/edac/mv64x60_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
index 454e1e26ee7c..ddc5082f7577 100644
--- a/drivers/edac/mv64x60_edac.c
+++ b/drivers/edac/mv64x60_edac.c
@@ -116,7 +116,7 @@ static int mv64x60_pci_err_probe(struct platform_device *pdev)
 	pdata = pci->pvt_info;
 
 	pdata->pci_hose = pdev->id;
-	pdata->name = "mpc85xx_pci_err";
+	pdata->name = "mv64x60_pci_err";
 	platform_set_drvdata(pdev, pci);
 	pci->dev = &pdev->dev;
 	pci->dev_name = dev_name(&pdev->dev);
-- 
2.11.0.24.ge6920cf

[PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Chris Packham]

To allow this driver to be used on non-powerpc platforms it needs to use
io accessors suitable for all platforms.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/edac/mv64x60_edac.c | 84 ++++++++++++++++++++++-----------------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
index ddc5082f7577..102ec29f864b 100644
--- a/drivers/edac/mv64x60_edac.c
+++ b/drivers/edac/mv64x60_edac.c
@@ -32,21 +32,21 @@ static void mv64x60_pci_check(struct edac_pci_ctl_info *pci)
 	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
 	u32 cause;
 
-	cause = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
+	cause = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
 	if (!cause)
 		return;
 
 	printk(KERN_ERR "Error in PCI %d Interface\n", pdata->pci_hose);
 	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
 	printk(KERN_ERR "Address Low: 0x%08x\n",
-	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
+	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
 	printk(KERN_ERR "Address High: 0x%08x\n",
-	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
+	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
 	printk(KERN_ERR "Attribute: 0x%08x\n",
-	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
+	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
 	printk(KERN_ERR "Command: 0x%08x\n",
-	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
-	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, ~cause);
+	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
+	iowrite32(~cause, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
 
 	if (cause & MV64X60_PCI_PE_MASK)
 		edac_pci_handle_pe(pci, pci->ctl_name);
@@ -61,7 +61,7 @@ static irqreturn_t mv64x60_pci_isr(int irq, void *dev_id)
 	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
 	u32 val;
 
-	val = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
+	val = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
 	if (!val)
 		return IRQ_NONE;
 
@@ -93,7 +93,7 @@ static int __init mv64x60_pci_fixup(struct platform_device *pdev)
 	if (!pci_serr)
 		return -ENOMEM;
 
-	out_le32(pci_serr, in_le32(pci_serr) & ~0x1);
+	iowrite32(ioread32(pci_serr) & ~0x1, pci_serr);
 	iounmap(pci_serr);
 
 	return 0;
@@ -161,10 +161,10 @@ static int mv64x60_pci_err_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, 0);
-	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK, 0);
-	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK,
-		 MV64X60_PCIx_ERR_MASK_VAL);
+	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
+	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
+	iowrite32(MV64X60_PCIx_ERR_MASK_VAL,
+		  pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
 
 	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
 		edac_dbg(3, "failed edac_pci_add_device()\n");
@@ -233,23 +233,23 @@ static void mv64x60_sram_check(struct edac_device_ctl_info *edac_dev)
 	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
 	u32 cause;
 
-	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
+	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
 	if (!cause)
 		return;
 
 	printk(KERN_ERR "Error in internal SRAM\n");
 	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
 	printk(KERN_ERR "Address Low: 0x%08x\n",
-	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
+	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
 	printk(KERN_ERR "Address High: 0x%08x\n",
-	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
+	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
 	printk(KERN_ERR "Data Low: 0x%08x\n",
-	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
+	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
 	printk(KERN_ERR "Data High: 0x%08x\n",
-	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
+	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
 	printk(KERN_ERR "Parity: 0x%08x\n",
-	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
-	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
+	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
+	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
 
 	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
 }
@@ -260,7 +260,7 @@ static irqreturn_t mv64x60_sram_isr(int irq, void *dev_id)
 	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
 	u32 cause;
 
-	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
+	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
 	if (!cause)
 		return IRQ_NONE;
 
@@ -322,7 +322,7 @@ static int mv64x60_sram_err_probe(struct platform_device *pdev)
 	}
 
 	/* setup SRAM err registers */
-	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
+	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
 
 	edac_dev->mod_name = EDAC_MOD_STR;
 	edac_dev->ctl_name = pdata->name;
@@ -398,7 +398,7 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
 	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
 	u32 cause;
 
-	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
+	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
 	    MV64x60_CPU_CAUSE_MASK;
 	if (!cause)
 		return;
@@ -406,16 +406,16 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
 	printk(KERN_ERR "Error on CPU interface\n");
 	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
 	printk(KERN_ERR "Address Low: 0x%08x\n",
-	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
+	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
 	printk(KERN_ERR "Address High: 0x%08x\n",
-	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
+	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
 	printk(KERN_ERR "Data Low: 0x%08x\n",
-	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
+	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
 	printk(KERN_ERR "Data High: 0x%08x\n",
-	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
+	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
 	printk(KERN_ERR "Parity: 0x%08x\n",
-	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
-	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
+	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
+	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
 
 	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
 }
@@ -426,7 +426,7 @@ static irqreturn_t mv64x60_cpu_isr(int irq, void *dev_id)
 	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
 	u32 cause;
 
-	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
+	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
 	    MV64x60_CPU_CAUSE_MASK;
 	if (!cause)
 		return IRQ_NONE;
@@ -515,9 +515,9 @@ static int mv64x60_cpu_err_probe(struct platform_device *pdev)
 	}
 
 	/* setup CPU err registers */
-	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
-	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0);
-	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0x000000ff);
+	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
+	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
+	iowrite32(0x000000ff, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
 
 	edac_dev->mod_name = EDAC_MOD_STR;
 	edac_dev->ctl_name = pdata->name;
@@ -596,13 +596,13 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
 	u32 comp_ecc;
 	u32 syndrome;
 
-	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
+	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
 	if (!reg)
 		return;
 
 	err_addr = reg & ~0x3;
-	sdram_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
-	comp_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
+	sdram_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
+	comp_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
 	syndrome = sdram_ecc ^ comp_ecc;
 
 	/* first bit clear in ECC Err Reg, 1 bit error, correctable by HW */
@@ -620,7 +620,7 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
 				     mci->ctl_name, "");
 
 	/* clear the error */
-	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
+	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
 }
 
 static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
@@ -629,7 +629,7 @@ static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
 	struct mv64x60_mc_pdata *pdata = mci->pvt_info;
 	u32 reg;
 
-	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
+	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
 	if (!reg)
 		return IRQ_NONE;
 
@@ -664,7 +664,7 @@ static void mv64x60_init_csrows(struct mem_ctl_info *mci,
 
 	get_total_mem(pdata);
 
-	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
+	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
 
 	csrow = mci->csrows[0];
 	dimm = csrow->channels[0]->dimm;
@@ -753,7 +753,7 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
+	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
 	if (!(ctl & MV64X60_SDRAM_ECC)) {
 		/* Non-ECC RAM? */
 		printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
@@ -779,10 +779,10 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
 	mv64x60_init_csrows(mci, pdata);
 
 	/* setup MC registers */
-	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
-	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
+	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
+	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
 	ctl = (ctl & 0xff00ffff) | 0x10000;
-	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL, ctl);
+	iowrite32(ctl, pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
 
 	res = edac_mc_add_mc(mci);
 	if (res) {
-- 
2.11.0.24.ge6920cf

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Borislav Petkov]

Top-posting so that the PPC list can see the whole patch below.

Since I don't know PPC, let me add PPC ML to CC for a confirmation this
change is correct.

Which brings me to the tangential: this driver is from 2006-ish and
is for some "Marvell MV64x60 Memory Controller kernel module for PPC
platforms". If you're going to touch it, then you should test on the PPC
hardware too, so that you don't break the driver there.

Unless that hardware is obsolete now and we don't care and, and ..?

Maybe someone has some insights...

On Fri, May 12, 2017 at 04:20:02PM +1200, Chris Packham wrote:
> To allow this driver to be used on non-powerpc platforms it needs to use
> io accessors suitable for all platforms.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  drivers/edac/mv64x60_edac.c | 84 ++++++++++++++++++++++-----------------------
>  1 file changed, 42 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
> index ddc5082f7577..102ec29f864b 100644
> --- a/drivers/edac/mv64x60_edac.c
> +++ b/drivers/edac/mv64x60_edac.c
> @@ -32,21 +32,21 @@ static void mv64x60_pci_check(struct edac_pci_ctl_info *pci)
>  	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
>  	u32 cause;
>  
> -	cause = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
> +	cause = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>  	if (!cause)
>  		return;
>  
>  	printk(KERN_ERR "Error in PCI %d Interface\n", pdata->pci_hose);
>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>  	printk(KERN_ERR "Address Low: 0x%08x\n",
> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
>  	printk(KERN_ERR "Address High: 0x%08x\n",
> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
>  	printk(KERN_ERR "Attribute: 0x%08x\n",
> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
>  	printk(KERN_ERR "Command: 0x%08x\n",
> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, ~cause);
> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
> +	iowrite32(~cause, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>  
>  	if (cause & MV64X60_PCI_PE_MASK)
>  		edac_pci_handle_pe(pci, pci->ctl_name);
> @@ -61,7 +61,7 @@ static irqreturn_t mv64x60_pci_isr(int irq, void *dev_id)
>  	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
>  	u32 val;
>  
> -	val = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
> +	val = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>  	if (!val)
>  		return IRQ_NONE;
>  
> @@ -93,7 +93,7 @@ static int __init mv64x60_pci_fixup(struct platform_device *pdev)
>  	if (!pci_serr)
>  		return -ENOMEM;
>  
> -	out_le32(pci_serr, in_le32(pci_serr) & ~0x1);
> +	iowrite32(ioread32(pci_serr) & ~0x1, pci_serr);
>  	iounmap(pci_serr);
>  
>  	return 0;
> @@ -161,10 +161,10 @@ static int mv64x60_pci_err_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, 0);
> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK, 0);
> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK,
> -		 MV64X60_PCIx_ERR_MASK_VAL);
> +	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
> +	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
> +	iowrite32(MV64X60_PCIx_ERR_MASK_VAL,
> +		  pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
>  
>  	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
>  		edac_dbg(3, "failed edac_pci_add_device()\n");
> @@ -233,23 +233,23 @@ static void mv64x60_sram_check(struct edac_device_ctl_info *edac_dev)
>  	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
>  	u32 cause;
>  
> -	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
> +	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>  	if (!cause)
>  		return;
>  
>  	printk(KERN_ERR "Error in internal SRAM\n");
>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>  	printk(KERN_ERR "Address Low: 0x%08x\n",
> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
>  	printk(KERN_ERR "Address High: 0x%08x\n",
> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
>  	printk(KERN_ERR "Data Low: 0x%08x\n",
> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
>  	printk(KERN_ERR "Data High: 0x%08x\n",
> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
>  	printk(KERN_ERR "Parity: 0x%08x\n",
> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
> -	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
> +	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>  
>  	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
>  }
> @@ -260,7 +260,7 @@ static irqreturn_t mv64x60_sram_isr(int irq, void *dev_id)
>  	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
>  	u32 cause;
>  
> -	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
> +	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>  	if (!cause)
>  		return IRQ_NONE;
>  
> @@ -322,7 +322,7 @@ static int mv64x60_sram_err_probe(struct platform_device *pdev)
>  	}
>  
>  	/* setup SRAM err registers */
> -	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
> +	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>  
>  	edac_dev->mod_name = EDAC_MOD_STR;
>  	edac_dev->ctl_name = pdata->name;
> @@ -398,7 +398,7 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
>  	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
>  	u32 cause;
>  
> -	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
> +	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>  	    MV64x60_CPU_CAUSE_MASK;
>  	if (!cause)
>  		return;
> @@ -406,16 +406,16 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
>  	printk(KERN_ERR "Error on CPU interface\n");
>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>  	printk(KERN_ERR "Address Low: 0x%08x\n",
> -	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
> +	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
>  	printk(KERN_ERR "Address High: 0x%08x\n",
> -	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
> +	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
>  	printk(KERN_ERR "Data Low: 0x%08x\n",
> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
>  	printk(KERN_ERR "Data High: 0x%08x\n",
> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
>  	printk(KERN_ERR "Parity: 0x%08x\n",
> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
>  
>  	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
>  }
> @@ -426,7 +426,7 @@ static irqreturn_t mv64x60_cpu_isr(int irq, void *dev_id)
>  	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
>  	u32 cause;
>  
> -	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
> +	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>  	    MV64x60_CPU_CAUSE_MASK;
>  	if (!cause)
>  		return IRQ_NONE;
> @@ -515,9 +515,9 @@ static int mv64x60_cpu_err_probe(struct platform_device *pdev)
>  	}
>  
>  	/* setup CPU err registers */
> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0);
> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0x000000ff);
> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
> +	iowrite32(0x000000ff, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
>  
>  	edac_dev->mod_name = EDAC_MOD_STR;
>  	edac_dev->ctl_name = pdata->name;
> @@ -596,13 +596,13 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
>  	u32 comp_ecc;
>  	u32 syndrome;
>  
> -	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
> +	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>  	if (!reg)
>  		return;
>  
>  	err_addr = reg & ~0x3;
> -	sdram_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
> -	comp_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
> +	sdram_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
> +	comp_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
>  	syndrome = sdram_ecc ^ comp_ecc;
>  
>  	/* first bit clear in ECC Err Reg, 1 bit error, correctable by HW */
> @@ -620,7 +620,7 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
>  				     mci->ctl_name, "");
>  
>  	/* clear the error */
> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
> +	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>  }
>  
>  static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
> @@ -629,7 +629,7 @@ static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
>  	struct mv64x60_mc_pdata *pdata = mci->pvt_info;
>  	u32 reg;
>  
> -	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
> +	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>  	if (!reg)
>  		return IRQ_NONE;
>  
> @@ -664,7 +664,7 @@ static void mv64x60_init_csrows(struct mem_ctl_info *mci,
>  
>  	get_total_mem(pdata);
>  
> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>  
>  	csrow = mci->csrows[0];
>  	dimm = csrow->channels[0]->dimm;
> @@ -753,7 +753,7 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
>  		goto err;
>  	}
>  
> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>  	if (!(ctl & MV64X60_SDRAM_ECC)) {
>  		/* Non-ECC RAM? */
>  		printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
> @@ -779,10 +779,10 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
>  	mv64x60_init_csrows(mci, pdata);
>  
>  	/* setup MC registers */
> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
> +	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>  	ctl = (ctl & 0xff00ffff) | 0x10000;
> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL, ctl);
> +	iowrite32(ctl, pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>  
>  	res = edac_mc_add_mc(mci);
>  	if (res) {
> -- 
> 2.11.0.24.ge6920cf
> 

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Chris Packham]

On 18/05/17 06:18, Borislav Petkov wrote:
> Top-posting so that the PPC list can see the whole patch below.
>
> Since I don't know PPC, let me add PPC ML to CC for a confirmation this
> change is correct.
>
> Which brings me to the tangential: this driver is from 2006-ish and
> is for some "Marvell MV64x60 Memory Controller kernel module for PPC
> platforms".

Many of the peripherals from the PPC MV64x60 were brought through to the 
ARM Orion, Kirkwood, and Armada SoCs (e.g. mv643xx_eth.c, 
i2c-mv64xxx.c). I believe the memory controller ECC is also in the same 
category.

I haven't looked into the cache/pci controller aspects of the 
mv64x60_edac.c driver yet.

> If you're going to touch it, then you should test on the PPC
> hardware too, so that you don't break the driver there.

I don't disagree. I however don't have access to any such hardware (I've 
got 2 armada variant boards that have ECC). Maybe someone on the PPC 
list can help me out?

One thing I would like confirmation on is is in_le32 -> ioread32 the 
correct change? I tossed up between ioread32 and readl. Looking at 
mv643xx_eth.c which supports both the MV643XX and Orion it's using readl 
so perhaps I should be using that.

> Unless that hardware is obsolete now and we don't care and, and ..?

MV64x60 is pretty old. I considered gutting mv64x60_edac.c to make a 
separate driver but that would just be more code to maintain.

> Maybe someone has some insights...
>
> On Fri, May 12, 2017 at 04:20:02PM +1200, Chris Packham wrote:
>> To allow this driver to be used on non-powerpc platforms it needs to use
>> io accessors suitable for all platforms.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>  drivers/edac/mv64x60_edac.c | 84 ++++++++++++++++++++++-----------------------
>>  1 file changed, 42 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
>> index ddc5082f7577..102ec29f864b 100644
>> --- a/drivers/edac/mv64x60_edac.c
>> +++ b/drivers/edac/mv64x60_edac.c
>> @@ -32,21 +32,21 @@ static void mv64x60_pci_check(struct edac_pci_ctl_info *pci)
>>  	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
>>  	u32 cause;
>>
>> -	cause = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>> +	cause = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>>  	if (!cause)
>>  		return;
>>
>>  	printk(KERN_ERR "Error in PCI %d Interface\n", pdata->pci_hose);
>>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>>  	printk(KERN_ERR "Address Low: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
>>  	printk(KERN_ERR "Address High: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
>>  	printk(KERN_ERR "Attribute: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
>>  	printk(KERN_ERR "Command: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, ~cause);
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
>> +	iowrite32(~cause, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>>
>>  	if (cause & MV64X60_PCI_PE_MASK)
>>  		edac_pci_handle_pe(pci, pci->ctl_name);
>> @@ -61,7 +61,7 @@ static irqreturn_t mv64x60_pci_isr(int irq, void *dev_id)
>>  	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
>>  	u32 val;
>>
>> -	val = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>> +	val = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>>  	if (!val)
>>  		return IRQ_NONE;
>>
>> @@ -93,7 +93,7 @@ static int __init mv64x60_pci_fixup(struct platform_device *pdev)
>>  	if (!pci_serr)
>>  		return -ENOMEM;
>>
>> -	out_le32(pci_serr, in_le32(pci_serr) & ~0x1);
>> +	iowrite32(ioread32(pci_serr) & ~0x1, pci_serr);
>>  	iounmap(pci_serr);
>>
>>  	return 0;
>> @@ -161,10 +161,10 @@ static int mv64x60_pci_err_probe(struct platform_device *pdev)
>>  		goto err;
>>  	}
>>
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, 0);
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK, 0);
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK,
>> -		 MV64X60_PCIx_ERR_MASK_VAL);
>> +	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>> +	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
>> +	iowrite32(MV64X60_PCIx_ERR_MASK_VAL,
>> +		  pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
>>
>>  	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
>>  		edac_dbg(3, "failed edac_pci_add_device()\n");
>> @@ -233,23 +233,23 @@ static void mv64x60_sram_check(struct edac_device_ctl_info *edac_dev)
>>  	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>
>> -	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>> +	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>  	if (!cause)
>>  		return;
>>
>>  	printk(KERN_ERR "Error in internal SRAM\n");
>>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>>  	printk(KERN_ERR "Address Low: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
>>  	printk(KERN_ERR "Address High: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
>>  	printk(KERN_ERR "Data Low: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
>>  	printk(KERN_ERR "Data High: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
>>  	printk(KERN_ERR "Parity: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
>> -	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
>> +	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>
>>  	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
>>  }
>> @@ -260,7 +260,7 @@ static irqreturn_t mv64x60_sram_isr(int irq, void *dev_id)
>>  	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>
>> -	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>> +	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>  	if (!cause)
>>  		return IRQ_NONE;
>>
>> @@ -322,7 +322,7 @@ static int mv64x60_sram_err_probe(struct platform_device *pdev)
>>  	}
>>
>>  	/* setup SRAM err registers */
>> -	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
>> +	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>
>>  	edac_dev->mod_name = EDAC_MOD_STR;
>>  	edac_dev->ctl_name = pdata->name;
>> @@ -398,7 +398,7 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
>>  	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>
>> -	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>> +	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>>  	    MV64x60_CPU_CAUSE_MASK;
>>  	if (!cause)
>>  		return;
>> @@ -406,16 +406,16 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
>>  	printk(KERN_ERR "Error on CPU interface\n");
>>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>>  	printk(KERN_ERR "Address Low: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
>> +	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
>>  	printk(KERN_ERR "Address High: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
>> +	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
>>  	printk(KERN_ERR "Data Low: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
>> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
>>  	printk(KERN_ERR "Data High: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
>> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
>>  	printk(KERN_ERR "Parity: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
>> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
>> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
>>
>>  	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
>>  }
>> @@ -426,7 +426,7 @@ static irqreturn_t mv64x60_cpu_isr(int irq, void *dev_id)
>>  	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>
>> -	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>> +	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>>  	    MV64x60_CPU_CAUSE_MASK;
>>  	if (!cause)
>>  		return IRQ_NONE;
>> @@ -515,9 +515,9 @@ static int mv64x60_cpu_err_probe(struct platform_device *pdev)
>>  	}
>>
>>  	/* setup CPU err registers */
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0);
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0x000000ff);
>> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
>> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
>> +	iowrite32(0x000000ff, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
>>
>>  	edac_dev->mod_name = EDAC_MOD_STR;
>>  	edac_dev->ctl_name = pdata->name;
>> @@ -596,13 +596,13 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
>>  	u32 comp_ecc;
>>  	u32 syndrome;
>>
>> -	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>> +	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>>  	if (!reg)
>>  		return;
>>
>>  	err_addr = reg & ~0x3;
>> -	sdram_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
>> -	comp_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
>> +	sdram_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
>> +	comp_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
>>  	syndrome = sdram_ecc ^ comp_ecc;
>>
>>  	/* first bit clear in ECC Err Reg, 1 bit error, correctable by HW */
>> @@ -620,7 +620,7 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
>>  				     mci->ctl_name, "");
>>
>>  	/* clear the error */
>> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
>> +	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>>  }
>>
>>  static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
>> @@ -629,7 +629,7 @@ static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
>>  	struct mv64x60_mc_pdata *pdata = mci->pvt_info;
>>  	u32 reg;
>>
>> -	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>> +	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>>  	if (!reg)
>>  		return IRQ_NONE;
>>
>> @@ -664,7 +664,7 @@ static void mv64x60_init_csrows(struct mem_ctl_info *mci,
>>
>>  	get_total_mem(pdata);
>>
>> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>>
>>  	csrow = mci->csrows[0];
>>  	dimm = csrow->channels[0]->dimm;
>> @@ -753,7 +753,7 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
>>  		goto err;
>>  	}
>>
>> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>>  	if (!(ctl & MV64X60_SDRAM_ECC)) {
>>  		/* Non-ECC RAM? */
>>  		printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
>> @@ -779,10 +779,10 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
>>  	mv64x60_init_csrows(mci, pdata);
>>
>>  	/* setup MC registers */
>> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
>> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>> +	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>>  	ctl = (ctl & 0xff00ffff) | 0x10000;
>> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL, ctl);
>> +	iowrite32(ctl, pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>>
>>  	res = edac_mc_add_mc(mci);
>>  	if (res) {
>> --
>> 2.11.0.24.ge6920cf
>>
>

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Arnd Bergmann]

On Wed, May 17, 2017 at 11:16 PM, Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
> On 18/05/17 06:18, Borislav Petkov wrote:
> One thing I would like confirmation on is is in_le32 -> ioread32 the
> correct change? I tossed up between ioread32 and readl. Looking at
> mv643xx_eth.c which supports both the MV643XX and Orion it's using readl
> so perhaps I should be using that.

There is no easy answer: on powerpc, readl is used for PCI,
while in_le32 is used for on-chip devices, so in_le32 is the
right one in principle. The main difference is that readl can
work with CONFIG_EEH on pseries, but in_le32 is cheaper.

On ARM and most other architectures, readl is used for both
PCI and on-chip devices, so that's what portable code tends
to use.

ioread32 is required to behave the same way as readl
on all __iomem pointers returned from ioremap(), but
is an extern function on powerpc and can be more
expensive when CONFIG_GENERIC_IOMAP is set.

I'd go with readl() in this case.

     Arnd

RE: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[David Laight]

From: Arnd Bergmann
> Sent: 17 May 2017 22:40
> 
> On Wed, May 17, 2017 at 11:16 PM, Chris Packham
> <Chris.Packham@alliedtelesis.co.nz> wrote:
> > On 18/05/17 06:18, Borislav Petkov wrote:
> > One thing I would like confirmation on is is in_le32 -> ioread32 the
> > correct change? I tossed up between ioread32 and readl. Looking at
> > mv643xx_eth.c which supports both the MV643XX and Orion it's using readl
> > so perhaps I should be using that.
> 
> There is no easy answer: on powerpc, readl is used for PCI,
> while in_le32 is used for on-chip devices, so in_le32 is the
> right one in principle. The main difference is that readl can
> work with CONFIG_EEH on pseries, but in_le32 is cheaper.
> 
> On ARM and most other architectures, readl is used for both
> PCI and on-chip devices, so that's what portable code tends
> to use.
> 
> ioread32 is required to behave the same way as readl
> on all __iomem pointers returned from ioremap(), but
> is an extern function on powerpc and can be more
> expensive when CONFIG_GENERIC_IOMAP is set.

What about x86?
Isn't ioread32() an extern function that checks for 'io' addresses
than need 'inb' (etc) instructions rather than memory ones.
If we know a PCI slave isn't 'io' should be be using ioread32() or readl()?
Don't some architectures have different enforced barriers in both these?

	David

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Arnd Bergmann]

On Fri, May 19, 2017 at 4:01 PM, David Laight <David.Laight@aculab.com> wrote:
> From: Arnd Bergmann
>> Sent: 17 May 2017 22:40
>>
>> On Wed, May 17, 2017 at 11:16 PM, Chris Packham
>> <Chris.Packham@alliedtelesis.co.nz> wrote:
>> > On 18/05/17 06:18, Borislav Petkov wrote:
>> > One thing I would like confirmation on is is in_le32 -> ioread32 the
>> > correct change? I tossed up between ioread32 and readl. Looking at
>> > mv643xx_eth.c which supports both the MV643XX and Orion it's using readl
>> > so perhaps I should be using that.
>>
>> There is no easy answer: on powerpc, readl is used for PCI,
>> while in_le32 is used for on-chip devices, so in_le32 is the
>> right one in principle. The main difference is that readl can
>> work with CONFIG_EEH on pseries, but in_le32 is cheaper.
>>
>> On ARM and most other architectures, readl is used for both
>> PCI and on-chip devices, so that's what portable code tends
>> to use.
>>
>> ioread32 is required to behave the same way as readl
>> on all __iomem pointers returned from ioremap(), but
>> is an extern function on powerpc and can be more
>> expensive when CONFIG_GENERIC_IOMAP is set.
>
> What about x86?
> Isn't ioread32() an extern function that checks for 'io' addresses
> than need 'inb' (etc) instructions rather than memory ones.

Right, x86 uses CONFIG_GENERIC_IOMAP and has that
extra check.

> If we know a PCI slave isn't 'io' should be be using ioread32() or readl()?

Generally speaking yes, though on most architectures that don't
use CONFIG_GENERIC_IOMAP there is no practical difference.

> Don't some architectures have different enforced barriers in both these?

I'm not aware of any architecture that always adds barriers to ioread32
compared to readl, though I guess that may be a reasonable
implementation depending on the architecture. PowerPC doesn't do
it, though one could argue that it would be required to guarantee
non-posted I/O accesses on PCI I/O space that gets mapped with
pci_iomap(). On ARM, no extra barrier is needed because the difference
is in the page table flags.

       Arnd

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Borislav Petkov]

On Wed, May 17, 2017 at 09:16:47PM +0000, Chris Packham wrote:
> MV64x60 is pretty old. I considered gutting mv64x60_edac.c to make a 
> separate driver but that would just be more code to maintain.

Well, one thing we did with layerscape and mpc85xx EDAC modules is share
the FSL memory controller IP through fsl_ddr_edac.c which both modules
link.

I dunno, perhaps you could do something similar by extracting the common
parts and this way avoid any breakage of the PPC side of things - since
apparently finding such hw is not easy - and then link those into your
driver. And your driver could just be a wrapper containing the EDAC glue
to make it a separate ARM armada_edac.c or so.

Anyway, just an idea.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Michael Ellerman]

Borislav Petkov <bp@alien8.de> writes:

> Top-posting so that the PPC list can see the whole patch below.
>
> Since I don't know PPC, let me add PPC ML to CC for a confirmation this
> change is correct.
>
> Which brings me to the tangential: this driver is from 2006-ish and
> is for some "Marvell MV64x60 Memory Controller kernel module for PPC
> platforms". If you're going to touch it, then you should test on the PPC
> hardware too, so that you don't break the driver there.
>
> Unless that hardware is obsolete now and we don't care and, and ..?
>
> Maybe someone has some insights...

Not really sorry.

I don't have one of those boards, so I can't test. Maybe someone else on
the list does?

I'd err on the side of the PPC hardware being obsolete and no one really
caring. If the driver is helpful on ARM then we may as well use it
there, if we can also avoid breaking it on PPC then great.

cheers


> On Fri, May 12, 2017 at 04:20:02PM +1200, Chris Packham wrote:
>> To allow this driver to be used on non-powerpc platforms it needs to use
>> io accessors suitable for all platforms.
>> 
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>  drivers/edac/mv64x60_edac.c | 84 ++++++++++++++++++++++-----------------------
>>  1 file changed, 42 insertions(+), 42 deletions(-)
>> 
>> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
>> index ddc5082f7577..102ec29f864b 100644
>> --- a/drivers/edac/mv64x60_edac.c
>> +++ b/drivers/edac/mv64x60_edac.c
>> @@ -32,21 +32,21 @@ static void mv64x60_pci_check(struct edac_pci_ctl_info *pci)
>>  	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
>>  	u32 cause;
>>  
>> -	cause = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>> +	cause = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>>  	if (!cause)
>>  		return;
>>  
>>  	printk(KERN_ERR "Error in PCI %d Interface\n", pdata->pci_hose);
>>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>>  	printk(KERN_ERR "Address Low: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));
>>  	printk(KERN_ERR "Address High: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
>>  	printk(KERN_ERR "Attribute: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));
>>  	printk(KERN_ERR "Command: 0x%08x\n",
>> -	       in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, ~cause);
>> +	       ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));
>> +	iowrite32(~cause, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>>  
>>  	if (cause & MV64X60_PCI_PE_MASK)
>>  		edac_pci_handle_pe(pci, pci->ctl_name);
>> @@ -61,7 +61,7 @@ static irqreturn_t mv64x60_pci_isr(int irq, void *dev_id)
>>  	struct mv64x60_pci_pdata *pdata = pci->pvt_info;
>>  	u32 val;
>>  
>> -	val = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>> +	val = ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>>  	if (!val)
>>  		return IRQ_NONE;
>>  
>> @@ -93,7 +93,7 @@ static int __init mv64x60_pci_fixup(struct platform_device *pdev)
>>  	if (!pci_serr)
>>  		return -ENOMEM;
>>  
>> -	out_le32(pci_serr, in_le32(pci_serr) & ~0x1);
>> +	iowrite32(ioread32(pci_serr) & ~0x1, pci_serr);
>>  	iounmap(pci_serr);
>>  
>>  	return 0;
>> @@ -161,10 +161,10 @@ static int mv64x60_pci_err_probe(struct platform_device *pdev)
>>  		goto err;
>>  	}
>>  
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, 0);
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK, 0);
>> -	out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK,
>> -		 MV64X60_PCIx_ERR_MASK_VAL);
>> +	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
>> +	iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
>> +	iowrite32(MV64X60_PCIx_ERR_MASK_VAL,
>> +		  pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);
>>  
>>  	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
>>  		edac_dbg(3, "failed edac_pci_add_device()\n");
>> @@ -233,23 +233,23 @@ static void mv64x60_sram_check(struct edac_device_ctl_info *edac_dev)
>>  	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>  
>> -	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>> +	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>  	if (!cause)
>>  		return;
>>  
>>  	printk(KERN_ERR "Error in internal SRAM\n");
>>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>>  	printk(KERN_ERR "Address Low: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));
>>  	printk(KERN_ERR "Address High: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));
>>  	printk(KERN_ERR "Data Low: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));
>>  	printk(KERN_ERR "Data High: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));
>>  	printk(KERN_ERR "Parity: 0x%08x\n",
>> -	       in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
>> -	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
>> +	       ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));
>> +	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>  
>>  	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
>>  }
>> @@ -260,7 +260,7 @@ static irqreturn_t mv64x60_sram_isr(int irq, void *dev_id)
>>  	struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>  
>> -	cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>> +	cause = ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>  	if (!cause)
>>  		return IRQ_NONE;
>>  
>> @@ -322,7 +322,7 @@ static int mv64x60_sram_err_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	/* setup SRAM err registers */
>> -	out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
>> +	iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
>>  
>>  	edac_dev->mod_name = EDAC_MOD_STR;
>>  	edac_dev->ctl_name = pdata->name;
>> @@ -398,7 +398,7 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
>>  	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>  
>> -	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>> +	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>>  	    MV64x60_CPU_CAUSE_MASK;
>>  	if (!cause)
>>  		return;
>> @@ -406,16 +406,16 @@ static void mv64x60_cpu_check(struct edac_device_ctl_info *edac_dev)
>>  	printk(KERN_ERR "Error on CPU interface\n");
>>  	printk(KERN_ERR "Cause register: 0x%08x\n", cause);
>>  	printk(KERN_ERR "Address Low: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
>> +	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));
>>  	printk(KERN_ERR "Address High: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
>> +	       ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));
>>  	printk(KERN_ERR "Data Low: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
>> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));
>>  	printk(KERN_ERR "Data High: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
>> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));
>>  	printk(KERN_ERR "Parity: 0x%08x\n",
>> -	       in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
>> +	       ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));
>> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
>>  
>>  	edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
>>  }
>> @@ -426,7 +426,7 @@ static irqreturn_t mv64x60_cpu_isr(int irq, void *dev_id)
>>  	struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
>>  	u32 cause;
>>  
>> -	cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>> +	cause = ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
>>  	    MV64x60_CPU_CAUSE_MASK;
>>  	if (!cause)
>>  		return IRQ_NONE;
>> @@ -515,9 +515,9 @@ static int mv64x60_cpu_err_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	/* setup CPU err registers */
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0);
>> -	out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0x000000ff);
>> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);
>> +	iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
>> +	iowrite32(0x000000ff, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);
>>  
>>  	edac_dev->mod_name = EDAC_MOD_STR;
>>  	edac_dev->ctl_name = pdata->name;
>> @@ -596,13 +596,13 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
>>  	u32 comp_ecc;
>>  	u32 syndrome;
>>  
>> -	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>> +	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>>  	if (!reg)
>>  		return;
>>  
>>  	err_addr = reg & ~0x3;
>> -	sdram_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
>> -	comp_ecc = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
>> +	sdram_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);
>> +	comp_ecc = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);
>>  	syndrome = sdram_ecc ^ comp_ecc;
>>  
>>  	/* first bit clear in ECC Err Reg, 1 bit error, correctable by HW */
>> @@ -620,7 +620,7 @@ static void mv64x60_mc_check(struct mem_ctl_info *mci)
>>  				     mci->ctl_name, "");
>>  
>>  	/* clear the error */
>> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
>> +	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>>  }
>>  
>>  static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
>> @@ -629,7 +629,7 @@ static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
>>  	struct mv64x60_mc_pdata *pdata = mci->pvt_info;
>>  	u32 reg;
>>  
>> -	reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>> +	reg = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>>  	if (!reg)
>>  		return IRQ_NONE;
>>  
>> @@ -664,7 +664,7 @@ static void mv64x60_init_csrows(struct mem_ctl_info *mci,
>>  
>>  	get_total_mem(pdata);
>>  
>> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>>  
>>  	csrow = mci->csrows[0];
>>  	dimm = csrow->channels[0]->dimm;
>> @@ -753,7 +753,7 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
>>  		goto err;
>>  	}
>>  
>> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
>>  	if (!(ctl & MV64X60_SDRAM_ECC)) {
>>  		/* Non-ECC RAM? */
>>  		printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
>> @@ -779,10 +779,10 @@ static int mv64x60_mc_err_probe(struct platform_device *pdev)
>>  	mv64x60_init_csrows(mci, pdata);
>>  
>>  	/* setup MC registers */
>> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
>> -	ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>> +	iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
>> +	ctl = ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>>  	ctl = (ctl & 0xff00ffff) | 0x10000;
>> -	out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL, ctl);
>> +	iowrite32(ctl, pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);
>>  
>>  	res = edac_mc_add_mc(mci);
>>  	if (res) {
>> -- 
>> 2.11.0.24.ge6920cf
>> 
>
> -- 
> Regards/Gruss,
>     Boris.
>
> Good mailing practices for 400: try to avoid top-posting and trim the reply.

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Arnd Bergmann]

On Thu, May 18, 2017 at 7:36 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Borislav Petkov <bp@alien8.de> writes:
>
>> Top-posting so that the PPC list can see the whole patch below.
>>
>> Since I don't know PPC, let me add PPC ML to CC for a confirmation this
>> change is correct.
>>
>> Which brings me to the tangential: this driver is from 2006-ish and
>> is for some "Marvell MV64x60 Memory Controller kernel module for PPC
>> platforms". If you're going to touch it, then you should test on the PPC
>> hardware too, so that you don't break the driver there.
>>
>> Unless that hardware is obsolete now and we don't care and, and ..?
>>
>> Maybe someone has some insights...
>
> Not really sorry.
>
> I don't have one of those boards, so I can't test. Maybe someone else on
> the list does?
>
> I'd err on the side of the PPC hardware being obsolete and no one really
> caring. If the driver is helpful on ARM then we may as well use it
> there, if we can also avoid breaking it on PPC then great.

I never had one myself, but tried to figure out what is still there to be
supported. In 2014, we removed one platform (PrPMC2800) that was
obsolete. There were eight boards that didn't make the cut from
arch/ppc32 to arch/powerpc. The C2K is the last one and it was
added in 2008 with this comment:

    Support for the C2K cPCI Single Board Computer from GEFanuc
    (PowerPC MPC7448 with a Marvell MV64460 chipset).
    All features of the board are not supported yet, but the board
    boots, flash works, all Ethernet ports are working and PCI
    devices are all found (USB and SATA on PCI1 do not work yet).

    Part 3 of 5: driver for the board.  At this time it is very generic
    and similar to its original, the driver for the prpmc2800.

The original submitter never followed up on it and neither the
board code not the DTS was ever updated to include additional
features, so I assume it only got worse from there.

According to https://www.abaco.com/products/c2ka-compactpci-sbc
the end-of-life date for the product was in 2015, presumably 10 years
after it got introduced.

This is definitely obsolete by now, and given the missing features
I would assume that nobody is running mainline kernels on it and
it can be removed.

For other related system controllers, there is a good overview on
https://www.linux-mips.org/wiki/Marvell_system_controllers
Apparently some of the older chips are still used in MIPS systems,
aside from the newer ARM SoC designs.

       Arnd

Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32

[Michael Ellerman]

Arnd Bergmann <arnd@arndb.de> writes:

> On Thu, May 18, 2017 at 7:36 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Borislav Petkov <bp@alien8.de> writes:
>>
>>> Top-posting so that the PPC list can see the whole patch below.
>>>
>>> Since I don't know PPC, let me add PPC ML to CC for a confirmation this
>>> change is correct.
>>>
>>> Which brings me to the tangential: this driver is from 2006-ish and
>>> is for some "Marvell MV64x60 Memory Controller kernel module for PPC
>>> platforms". If you're going to touch it, then you should test on the PPC
>>> hardware too, so that you don't break the driver there.
>>>
>>> Unless that hardware is obsolete now and we don't care and, and ..?
>>>
>>> Maybe someone has some insights...
>>
>> Not really sorry.
>>
>> I don't have one of those boards, so I can't test. Maybe someone else on
>> the list does?
>>
>> I'd err on the side of the PPC hardware being obsolete and no one really
>> caring. If the driver is helpful on ARM then we may as well use it
>> there, if we can also avoid breaking it on PPC then great.
>
> I never had one myself, but tried to figure out what is still there to be
> supported. In 2014, we removed one platform (PrPMC2800) that was
> obsolete. There were eight boards that didn't make the cut from
> arch/ppc32 to arch/powerpc. The C2K is the last one and it was
> added in 2008 with this comment:
>
>     Support for the C2K cPCI Single Board Computer from GEFanuc
>     (PowerPC MPC7448 with a Marvell MV64460 chipset).
>     All features of the board are not supported yet, but the board
>     boots, flash works, all Ethernet ports are working and PCI
>     devices are all found (USB and SATA on PCI1 do not work yet).
>
>     Part 3 of 5: driver for the board.  At this time it is very generic
>     and similar to its original, the driver for the prpmc2800.
>
> The original submitter never followed up on it and neither the
> board code not the DTS was ever updated to include additional
> features, so I assume it only got worse from there.
>
> According to https://www.abaco.com/products/c2ka-compactpci-sbc
> the end-of-life date for the product was in 2015, presumably 10 years
> after it got introduced.
>
> This is definitely obsolete by now, and given the missing features
> I would assume that nobody is running mainline kernels on it and
> it can be removed.

Great, thanks for doing all that digging.

I'll cook up a patch to remove c2k next week.

Then we can drop CONFIG_MV64X60 from PPC and see where that leaves
things, depending on what's useful on ARM.

cheers