* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-17 18:10 ` Borislav Petkov
0 siblings, 0 replies; 26+ messages in thread
From: Borislav Petkov @ 2017-05-17 18:10 UTC (permalink / raw)
To: Chris Packham, linuxppc-dev; +Cc: linux-edac, mchehab, linux-kernel
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
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-17 21:16 ` Chris Packham
0 siblings, 0 replies; 26+ messages in thread
From: Chris Packham @ 2017-05-17 21:16 UTC (permalink / raw)
To: Borislav Petkov, linuxppc-dev; +Cc: linux-edac, mchehab, linux-kernel
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
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-17 21:16 ` Chris Packham
0 siblings, 0 replies; 26+ messages in thread
From: Chris Packham @ 2017-05-17 21:16 UTC (permalink / raw)
To: Borislav Petkov, linuxppc-dev; +Cc: linux-edac, mchehab, linux-kernel
On 18/05/17 06:18, Borislav Petkov wrote:=0A=
> Top-posting so that the PPC list can see the whole patch below.=0A=
>=0A=
> Since I don't know PPC, let me add PPC ML to CC for a confirmation this=
=0A=
> change is correct.=0A=
>=0A=
> Which brings me to the tangential: this driver is from 2006-ish and=0A=
> is for some "Marvell MV64x60 Memory Controller kernel module for PPC=0A=
> platforms".=0A=
=0A=
Many of the peripherals from the PPC MV64x60 were brought through to the =
=0A=
ARM Orion, Kirkwood, and Armada SoCs (e.g. mv643xx_eth.c, =0A=
i2c-mv64xxx.c). I believe the memory controller ECC is also in the same =0A=
category.=0A=
=0A=
I haven't looked into the cache/pci controller aspects of the =0A=
mv64x60_edac.c driver yet.=0A=
=0A=
> If you're going to touch it, then you should test on the PPC=0A=
> hardware too, so that you don't break the driver there.=0A=
=0A=
I don't disagree. I however don't have access to any such hardware (I've =
=0A=
got 2 armada variant boards that have ECC). Maybe someone on the PPC =0A=
list can help me out?=0A=
=0A=
One thing I would like confirmation on is is in_le32 -> ioread32 the =0A=
correct change? I tossed up between ioread32 and readl. Looking at =0A=
mv643xx_eth.c which supports both the MV643XX and Orion it's using readl =
=0A=
so perhaps I should be using that.=0A=
=0A=
> Unless that hardware is obsolete now and we don't care and, and ..?=0A=
=0A=
MV64x60 is pretty old. I considered gutting mv64x60_edac.c to make a =0A=
separate driver but that would just be more code to maintain.=0A=
=0A=
> Maybe someone has some insights...=0A=
>=0A=
> On Fri, May 12, 2017 at 04:20:02PM +1200, Chris Packham wrote:=0A=
>> To allow this driver to be used on non-powerpc platforms it needs to use=
=0A=
>> io accessors suitable for all platforms.=0A=
>>=0A=
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>=0A=
>> ---=0A=
>> drivers/edac/mv64x60_edac.c | 84 ++++++++++++++++++++++----------------=
-------=0A=
>> 1 file changed, 42 insertions(+), 42 deletions(-)=0A=
>>=0A=
>> diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c=
=0A=
>> index ddc5082f7577..102ec29f864b 100644=0A=
>> --- a/drivers/edac/mv64x60_edac.c=0A=
>> +++ b/drivers/edac/mv64x60_edac.c=0A=
>> @@ -32,21 +32,21 @@ static void mv64x60_pci_check(struct edac_pci_ctl_in=
fo *pci)=0A=
>> struct mv64x60_pci_pdata *pdata =3D pci->pvt_info;=0A=
>> u32 cause;=0A=
>>=0A=
>> - cause =3D in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);=0A=
>> + cause =3D ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);=0A=
>> if (!cause)=0A=
>> return;=0A=
>>=0A=
>> printk(KERN_ERR "Error in PCI %d Interface\n", pdata->pci_hose);=0A=
>> printk(KERN_ERR "Cause register: 0x%08x\n", cause);=0A=
>> printk(KERN_ERR "Address Low: 0x%08x\n",=0A=
>> - in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));=0A=
>> + ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_LO));=0A=
>> printk(KERN_ERR "Address High: 0x%08x\n",=0A=
>> - in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));=0A=
>> + ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));=0A=
>> printk(KERN_ERR "Attribute: 0x%08x\n",=0A=
>> - in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));=0A=
>> + ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_ATTR));=0A=
>> printk(KERN_ERR "Command: 0x%08x\n",=0A=
>> - in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));=0A=
>> - out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, ~cause);=0A=
>> + ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CMD));=0A=
>> + iowrite32(~cause, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);=0A=
>>=0A=
>> if (cause & MV64X60_PCI_PE_MASK)=0A=
>> edac_pci_handle_pe(pci, pci->ctl_name);=0A=
>> @@ -61,7 +61,7 @@ static irqreturn_t mv64x60_pci_isr(int irq, void *dev_=
id)=0A=
>> struct mv64x60_pci_pdata *pdata =3D pci->pvt_info;=0A=
>> u32 val;=0A=
>>=0A=
>> - val =3D in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);=0A=
>> + val =3D ioread32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);=0A=
>> if (!val)=0A=
>> return IRQ_NONE;=0A=
>>=0A=
>> @@ -93,7 +93,7 @@ static int __init mv64x60_pci_fixup(struct platform_de=
vice *pdev)=0A=
>> if (!pci_serr)=0A=
>> return -ENOMEM;=0A=
>>=0A=
>> - out_le32(pci_serr, in_le32(pci_serr) & ~0x1);=0A=
>> + iowrite32(ioread32(pci_serr) & ~0x1, pci_serr);=0A=
>> iounmap(pci_serr);=0A=
>>=0A=
>> return 0;=0A=
>> @@ -161,10 +161,10 @@ static int mv64x60_pci_err_probe(struct platform_d=
evice *pdev)=0A=
>> goto err;=0A=
>> }=0A=
>>=0A=
>> - out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE, 0);=0A=
>> - out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK, 0);=0A=
>> - out_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_MASK,=0A=
>> - MV64X60_PCIx_ERR_MASK_VAL);=0A=
>> + iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);=0A=
>> + iowrite32(0, pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);=0A=
>> + iowrite32(MV64X60_PCIx_ERR_MASK_VAL,=0A=
>> + pdata->pci_vbase + MV64X60_PCI_ERROR_MASK);=0A=
>>=0A=
>> if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {=0A=
>> edac_dbg(3, "failed edac_pci_add_device()\n");=0A=
>> @@ -233,23 +233,23 @@ static void mv64x60_sram_check(struct edac_device_=
ctl_info *edac_dev)=0A=
>> struct mv64x60_sram_pdata *pdata =3D edac_dev->pvt_info;=0A=
>> u32 cause;=0A=
>>=0A=
>> - cause =3D in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);=0A=
>> + cause =3D ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);=0A=
>> if (!cause)=0A=
>> return;=0A=
>>=0A=
>> printk(KERN_ERR "Error in internal SRAM\n");=0A=
>> printk(KERN_ERR "Cause register: 0x%08x\n", cause);=0A=
>> printk(KERN_ERR "Address Low: 0x%08x\n",=0A=
>> - in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));=0A=
>> + ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_LO));=0A=
>> printk(KERN_ERR "Address High: 0x%08x\n",=0A=
>> - in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));=0A=
>> + ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_ADDR_HI));=0A=
>> printk(KERN_ERR "Data Low: 0x%08x\n",=0A=
>> - in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));=0A=
>> + ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_LO));=0A=
>> printk(KERN_ERR "Data High: 0x%08x\n",=0A=
>> - in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));=0A=
>> + ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_DATA_HI));=0A=
>> printk(KERN_ERR "Parity: 0x%08x\n",=0A=
>> - in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));=0A=
>> - out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);=0A=
>> + ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_PARITY));=0A=
>> + iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);=0A=
>>=0A=
>> edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);=0A=
>> }=0A=
>> @@ -260,7 +260,7 @@ static irqreturn_t mv64x60_sram_isr(int irq, void *d=
ev_id)=0A=
>> struct mv64x60_sram_pdata *pdata =3D edac_dev->pvt_info;=0A=
>> u32 cause;=0A=
>>=0A=
>> - cause =3D in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);=0A=
>> + cause =3D ioread32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);=0A=
>> if (!cause)=0A=
>> return IRQ_NONE;=0A=
>>=0A=
>> @@ -322,7 +322,7 @@ static int mv64x60_sram_err_probe(struct platform_de=
vice *pdev)=0A=
>> }=0A=
>>=0A=
>> /* setup SRAM err registers */=0A=
>> - out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);=0A=
>> + iowrite32(0, pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);=0A=
>>=0A=
>> edac_dev->mod_name =3D EDAC_MOD_STR;=0A=
>> edac_dev->ctl_name =3D pdata->name;=0A=
>> @@ -398,7 +398,7 @@ static void mv64x60_cpu_check(struct edac_device_ctl=
_info *edac_dev)=0A=
>> struct mv64x60_cpu_pdata *pdata =3D edac_dev->pvt_info;=0A=
>> u32 cause;=0A=
>>=0A=
>> - cause =3D in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &=0A=
>> + cause =3D ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &=0A=
>> MV64x60_CPU_CAUSE_MASK;=0A=
>> if (!cause)=0A=
>> return;=0A=
>> @@ -406,16 +406,16 @@ static void mv64x60_cpu_check(struct edac_device_c=
tl_info *edac_dev)=0A=
>> printk(KERN_ERR "Error on CPU interface\n");=0A=
>> printk(KERN_ERR "Cause register: 0x%08x\n", cause);=0A=
>> printk(KERN_ERR "Address Low: 0x%08x\n",=0A=
>> - in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));=0A=
>> + ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_LO));=0A=
>> printk(KERN_ERR "Address High: 0x%08x\n",=0A=
>> - in_le32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));=0A=
>> + ioread32(pdata->cpu_vbase[0] + MV64x60_CPU_ERR_ADDR_HI));=0A=
>> printk(KERN_ERR "Data Low: 0x%08x\n",=0A=
>> - in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));=0A=
>> + ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_LO));=0A=
>> printk(KERN_ERR "Data High: 0x%08x\n",=0A=
>> - in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));=0A=
>> + ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_DATA_HI));=0A=
>> printk(KERN_ERR "Parity: 0x%08x\n",=0A=
>> - in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));=0A=
>> - out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);=0A=
>> + ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_PARITY));=0A=
>> + iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);=0A=
>>=0A=
>> edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);=0A=
>> }=0A=
>> @@ -426,7 +426,7 @@ static irqreturn_t mv64x60_cpu_isr(int irq, void *de=
v_id)=0A=
>> struct mv64x60_cpu_pdata *pdata =3D edac_dev->pvt_info;=0A=
>> u32 cause;=0A=
>>=0A=
>> - cause =3D in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &=0A=
>> + cause =3D ioread32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &=0A=
>> MV64x60_CPU_CAUSE_MASK;=0A=
>> if (!cause)=0A=
>> return IRQ_NONE;=0A=
>> @@ -515,9 +515,9 @@ static int mv64x60_cpu_err_probe(struct platform_dev=
ice *pdev)=0A=
>> }=0A=
>>=0A=
>> /* setup CPU err registers */=0A=
>> - out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE, 0);=0A=
>> - out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0);=0A=
>> - out_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK, 0x000000ff);=0A=
>> + iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE);=0A=
>> + iowrite32(0, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);=0A=
>> + iowrite32(0x000000ff, pdata->cpu_vbase[1] + MV64x60_CPU_ERR_MASK);=0A=
>>=0A=
>> edac_dev->mod_name =3D EDAC_MOD_STR;=0A=
>> edac_dev->ctl_name =3D pdata->name;=0A=
>> @@ -596,13 +596,13 @@ static void mv64x60_mc_check(struct mem_ctl_info *=
mci)=0A=
>> u32 comp_ecc;=0A=
>> u32 syndrome;=0A=
>>=0A=
>> - reg =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);=0A=
>> + reg =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);=0A=
>> if (!reg)=0A=
>> return;=0A=
>>=0A=
>> err_addr =3D reg & ~0x3;=0A=
>> - sdram_ecc =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);=
=0A=
>> - comp_ecc =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);=0A=
>> + sdram_ecc =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_RCVD);=
=0A=
>> + comp_ecc =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CALC);=
=0A=
>> syndrome =3D sdram_ecc ^ comp_ecc;=0A=
>>=0A=
>> /* first bit clear in ECC Err Reg, 1 bit error, correctable by HW */=
=0A=
>> @@ -620,7 +620,7 @@ static void mv64x60_mc_check(struct mem_ctl_info *mc=
i)=0A=
>> mci->ctl_name, "");=0A=
>>=0A=
>> /* clear the error */=0A=
>> - out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);=0A=
>> + iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);=0A=
>> }=0A=
>>=0A=
>> static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)=0A=
>> @@ -629,7 +629,7 @@ static irqreturn_t mv64x60_mc_isr(int irq, void *dev=
_id)=0A=
>> struct mv64x60_mc_pdata *pdata =3D mci->pvt_info;=0A=
>> u32 reg;=0A=
>>=0A=
>> - reg =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);=0A=
>> + reg =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);=0A=
>> if (!reg)=0A=
>> return IRQ_NONE;=0A=
>>=0A=
>> @@ -664,7 +664,7 @@ static void mv64x60_init_csrows(struct mem_ctl_info =
*mci,=0A=
>>=0A=
>> get_total_mem(pdata);=0A=
>>=0A=
>> - ctl =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);=0A=
>> + ctl =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);=0A=
>>=0A=
>> csrow =3D mci->csrows[0];=0A=
>> dimm =3D csrow->channels[0]->dimm;=0A=
>> @@ -753,7 +753,7 @@ static int mv64x60_mc_err_probe(struct platform_devi=
ce *pdev)=0A=
>> goto err;=0A=
>> }=0A=
>>=0A=
>> - ctl =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);=0A=
>> + ctl =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);=0A=
>> if (!(ctl & MV64X60_SDRAM_ECC)) {=0A=
>> /* Non-ECC RAM? */=0A=
>> printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);=0A=
>> @@ -779,10 +779,10 @@ static int mv64x60_mc_err_probe(struct platform_de=
vice *pdev)=0A=
>> mv64x60_init_csrows(mci, pdata);=0A=
>>=0A=
>> /* setup MC registers */=0A=
>> - out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);=0A=
>> - ctl =3D in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);=0A=
>> + iowrite32(0, pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);=0A=
>> + ctl =3D ioread32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);=0A=
>> ctl =3D (ctl & 0xff00ffff) | 0x10000;=0A=
>> - out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL, ctl);=0A=
>> + iowrite32(ctl, pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL);=0A=
>>=0A=
>> res =3D edac_mc_add_mc(mci);=0A=
>> if (res) {=0A=
>> --=0A=
>> 2.11.0.24.ge6920cf=0A=
>>=0A=
>=0A=
=0A=
^ permalink raw reply [flat|nested] 26+ messages in thread* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-17 21:16 ` Chris Packham
0 siblings, 0 replies; 26+ messages in thread
From: Chris Packham @ 2017-05-17 21:16 UTC (permalink / raw)
To: Borislav Petkov, linuxppc-dev; +Cc: linux-edac, mchehab, linux-kernel
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
>>
>
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-17 21:40 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2017-05-17 21:40 UTC (permalink / raw)
To: Chris Packham
Cc: Borislav Petkov, linuxppc-dev, linux-edac, mchehab, linux-kernel
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
^ permalink raw reply [flat|nested] 26+ messages in thread* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-17 21:40 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2017-05-17 21:40 UTC (permalink / raw)
To: Chris Packham
Cc: Borislav Petkov, linuxppc-dev, linux-edac, mchehab, linux-kernel
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
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 14:01 ` David Laight
0 siblings, 0 replies; 26+ messages in thread
From: David Laight @ 2017-05-19 14:01 UTC (permalink / raw)
To: 'Arnd Bergmann', Chris Packham
Cc: linuxppc-dev, mchehab, Borislav Petkov, linux-kernel, linux-edac
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 14:01 ` David Laight
0 siblings, 0 replies; 26+ messages in thread
From: David Laight @ 2017-05-19 14:01 UTC (permalink / raw)
To: 'Arnd Bergmann', Chris Packham
Cc: linuxppc-dev, mchehab, Borislav Petkov, linux-kernel, linux-edac
RnJvbTogQXJuZCBCZXJnbWFubg0KPiBTZW50OiAxNyBNYXkgMjAxNyAyMjo0MA0KPiANCj4gT24g
V2VkLCBNYXkgMTcsIDIwMTcgYXQgMTE6MTYgUE0sIENocmlzIFBhY2toYW0NCj4gPENocmlzLlBh
Y2toYW1AYWxsaWVkdGVsZXNpcy5jby5uej4gd3JvdGU6DQo+ID4gT24gMTgvMDUvMTcgMDY6MTgs
IEJvcmlzbGF2IFBldGtvdiB3cm90ZToNCj4gPiBPbmUgdGhpbmcgSSB3b3VsZCBsaWtlIGNvbmZp
cm1hdGlvbiBvbiBpcyBpcyBpbl9sZTMyIC0+IGlvcmVhZDMyIHRoZQ0KPiA+IGNvcnJlY3QgY2hh
bmdlPyBJIHRvc3NlZCB1cCBiZXR3ZWVuIGlvcmVhZDMyIGFuZCByZWFkbC4gTG9va2luZyBhdA0K
PiA+IG12NjQzeHhfZXRoLmMgd2hpY2ggc3VwcG9ydHMgYm90aCB0aGUgTVY2NDNYWCBhbmQgT3Jp
b24gaXQncyB1c2luZyByZWFkbA0KPiA+IHNvIHBlcmhhcHMgSSBzaG91bGQgYmUgdXNpbmcgdGhh
dC4NCj4gDQo+IFRoZXJlIGlzIG5vIGVhc3kgYW5zd2VyOiBvbiBwb3dlcnBjLCByZWFkbCBpcyB1
c2VkIGZvciBQQ0ksDQo+IHdoaWxlIGluX2xlMzIgaXMgdXNlZCBmb3Igb24tY2hpcCBkZXZpY2Vz
LCBzbyBpbl9sZTMyIGlzIHRoZQ0KPiByaWdodCBvbmUgaW4gcHJpbmNpcGxlLiBUaGUgbWFpbiBk
aWZmZXJlbmNlIGlzIHRoYXQgcmVhZGwgY2FuDQo+IHdvcmsgd2l0aCBDT05GSUdfRUVIIG9uIHBz
ZXJpZXMsIGJ1dCBpbl9sZTMyIGlzIGNoZWFwZXIuDQo+IA0KPiBPbiBBUk0gYW5kIG1vc3Qgb3Ro
ZXIgYXJjaGl0ZWN0dXJlcywgcmVhZGwgaXMgdXNlZCBmb3IgYm90aA0KPiBQQ0kgYW5kIG9uLWNo
aXAgZGV2aWNlcywgc28gdGhhdCdzIHdoYXQgcG9ydGFibGUgY29kZSB0ZW5kcw0KPiB0byB1c2Uu
DQo+IA0KPiBpb3JlYWQzMiBpcyByZXF1aXJlZCB0byBiZWhhdmUgdGhlIHNhbWUgd2F5IGFzIHJl
YWRsDQo+IG9uIGFsbCBfX2lvbWVtIHBvaW50ZXJzIHJldHVybmVkIGZyb20gaW9yZW1hcCgpLCBi
dXQNCj4gaXMgYW4gZXh0ZXJuIGZ1bmN0aW9uIG9uIHBvd2VycGMgYW5kIGNhbiBiZSBtb3JlDQo+
IGV4cGVuc2l2ZSB3aGVuIENPTkZJR19HRU5FUklDX0lPTUFQIGlzIHNldC4NCg0KV2hhdCBhYm91
dCB4ODY/DQpJc24ndCBpb3JlYWQzMigpIGFuIGV4dGVybiBmdW5jdGlvbiB0aGF0IGNoZWNrcyBm
b3IgJ2lvJyBhZGRyZXNzZXMNCnRoYW4gbmVlZCAnaW5iJyAoZXRjKSBpbnN0cnVjdGlvbnMgcmF0
aGVyIHRoYW4gbWVtb3J5IG9uZXMuDQpJZiB3ZSBrbm93IGEgUENJIHNsYXZlIGlzbid0ICdpbycg
c2hvdWxkIGJlIGJlIHVzaW5nIGlvcmVhZDMyKCkgb3IgcmVhZGwoKT8NCkRvbid0IHNvbWUgYXJj
aGl0ZWN0dXJlcyBoYXZlIGRpZmZlcmVudCBlbmZvcmNlZCBiYXJyaWVycyBpbiBib3RoIHRoZXNl
Pw0KDQoJRGF2aWQNCg0KDQoNCg==
^ permalink raw reply [flat|nested] 26+ messages in thread
* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 14:01 ` David Laight
0 siblings, 0 replies; 26+ messages in thread
From: David Laight @ 2017-05-19 14:01 UTC (permalink / raw)
To: 'Arnd Bergmann', Chris Packham
Cc: linuxppc-dev, mchehab, Borislav Petkov, linux-kernel, linux-edac
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 15:25 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2017-05-19 15:25 UTC (permalink / raw)
To: David Laight
Cc: Chris Packham, Borislav Petkov, mchehab, linuxppc-dev,
linux-kernel, linux-edac
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
^ permalink raw reply [flat|nested] 26+ messages in thread* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 15:25 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2017-05-19 15:25 UTC (permalink / raw)
To: David Laight
Cc: Chris Packham, Borislav Petkov, mchehab, linuxppc-dev,
linux-kernel, linux-edac
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
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-18 8:08 ` Borislav Petkov
0 siblings, 0 replies; 26+ messages in thread
From: Borislav Petkov @ 2017-05-18 8:08 UTC (permalink / raw)
To: Chris Packham; +Cc: linuxppc-dev, linux-edac, mchehab, linux-kernel
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.
^ permalink raw reply [flat|nested] 26+ messages in thread* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-18 8:08 ` Borislav Petkov
0 siblings, 0 replies; 26+ messages in thread
From: Borislav Petkov @ 2017-05-18 8:08 UTC (permalink / raw)
To: Chris Packham; +Cc: linuxppc-dev, linux-edac, mchehab, linux-kernel
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.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-18 5:36 ` Michael Ellerman
0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2017-05-18 5:36 UTC (permalink / raw)
To: Borislav Petkov, Chris Packham, linuxppc-dev
Cc: mchehab, linux-kernel, linux-edac
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.
^ permalink raw reply [flat|nested] 26+ messages in thread* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-18 5:36 ` Michael Ellerman
0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2017-05-18 5:36 UTC (permalink / raw)
To: Borislav Petkov, Chris Packham, linuxppc-dev
Cc: mchehab, linux-kernel, linux-edac
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.
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-18 11:07 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2017-05-18 11:07 UTC (permalink / raw)
To: Michael Ellerman
Cc: Borislav Petkov, Chris Packham, linuxppc-dev,
Mauro Carvalho Chehab, Linux Kernel Mailing List, linux-edac
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
^ permalink raw reply [flat|nested] 26+ messages in thread* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-18 11:07 ` Arnd Bergmann
0 siblings, 0 replies; 26+ messages in thread
From: Arnd Bergmann @ 2017-05-18 11:07 UTC (permalink / raw)
To: Michael Ellerman
Cc: Borislav Petkov, Chris Packham, linuxppc-dev,
Mauro Carvalho Chehab, Linux Kernel Mailing List, linux-edac
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
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 9:53 ` Michael Ellerman
0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2017-05-19 9:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Borislav Petkov, Chris Packham, linuxppc-dev,
Mauro Carvalho Chehab, Linux Kernel Mailing List, linux-edac
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* [3/3] EDAC: mv64x60: replace in_le32/out_le32 with ioread32/iowrite32
@ 2017-05-19 9:53 ` Michael Ellerman
0 siblings, 0 replies; 26+ messages in thread
From: Michael Ellerman @ 2017-05-19 9:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Borislav Petkov, Chris Packham, linuxppc-dev,
Mauro Carvalho Chehab, Linux Kernel Mailing List, linux-edac
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
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 26+ messages in thread