* Re: pte_offset_map for ppc assumes HIGHPTE
From: Benjamin Herrenschmidt @ 2007-07-25 23:22 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linuxppc-dev, linux-kernel, linux-mm
In-Reply-To: <jewswodqcn.fsf@sykes.suse.de>
On Thu, 2007-07-26 at 01:18 +0200, Andreas Schwab wrote:
> Satya <satyakiran@gmail.com> writes:
>
> > hello,
> > The implementation of pte_offset_map() for ppc assumes that PTEs are
> > kept in highmem (CONFIG_HIGHPTE). There is only one implmentation of
> > pte_offset_map() as follows (include/asm-ppc/pgtable.h):
> >
> > #define pte_offset_map(dir, addr) \
> > ((pte_t *) kmap_atomic(pmd_page(*(dir)), KM_PTE0) + pte_index(addr))
> >
> > Shouldn't this be made conditional according to CONFIG_HIGHPTE is
> > defined or not
>
> kmap_atomic is always defined with or without CONFIG_HIGHPTE.
>
> > (as implemented in include/asm-i386/pgtable.h) ?
>
> I don't think that needs it either.
Depends... if you have CONFIG_HIGHMEM and not CONFIG_HIGHPTE, you are wasting
time going through kmap_atomic unnecessarily no ? it will probably not do anything
because the PTE page is in lowmem but still...
Ben.
^ permalink raw reply
* Re: pte_offset_map for ppc assumes HIGHPTE
From: Andreas Schwab @ 2007-07-25 23:18 UTC (permalink / raw)
To: Satya; +Cc: linuxppc-dev, linux-kernel, linux-mm
In-Reply-To: <acbcf3840707251516w301f834cj5f6a81a494d359ed@mail.gmail.com>
Satya <satyakiran@gmail.com> writes:
> hello,
> The implementation of pte_offset_map() for ppc assumes that PTEs are
> kept in highmem (CONFIG_HIGHPTE). There is only one implmentation of
> pte_offset_map() as follows (include/asm-ppc/pgtable.h):
>
> #define pte_offset_map(dir, addr) \
> ((pte_t *) kmap_atomic(pmd_page(*(dir)), KM_PTE0) + pte_index(addr))
>
> Shouldn't this be made conditional according to CONFIG_HIGHPTE is
> defined or not
kmap_atomic is always defined with or without CONFIG_HIGHPTE.
> (as implemented in include/asm-i386/pgtable.h) ?
I don't think that needs it either.
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: pte_offset_map for ppc assumes HIGHPTE
From: Benjamin Herrenschmidt @ 2007-07-25 23:10 UTC (permalink / raw)
To: Satya; +Cc: linuxppc-dev, linux-kernel, linux-mm
In-Reply-To: <acbcf3840707251516w301f834cj5f6a81a494d359ed@mail.gmail.com>
On Wed, 2007-07-25 at 17:16 -0500, Satya wrote:
> hello,
> The implementation of pte_offset_map() for ppc assumes that PTEs are
> kept in highmem (CONFIG_HIGHPTE). There is only one implmentation of
> pte_offset_map() as follows (include/asm-ppc/pgtable.h):
>
> #define pte_offset_map(dir, addr) \
> ((pte_t *) kmap_atomic(pmd_page(*(dir)), KM_PTE0) + pte_index(addr))
>
> Shouldn't this be made conditional according to CONFIG_HIGHPTE is
> defined or not (as implemented in include/asm-i386/pgtable.h) ?
>
> the same goes for pte_offset_map_nested and the corresponding unmap functions.
Do we have CONFIG_HIGHMEM without CONFIG_HIGHPTE ? If yes, then indeed,
we should change that. Though I'm not sure I see the point of splitting
those 2 options.
Ben.
^ permalink raw reply
* pte_offset_map for ppc assumes HIGHPTE
From: Satya @ 2007-07-25 22:16 UTC (permalink / raw)
To: linuxppc-dev, linux-mm, linux-kernel
hello,
The implementation of pte_offset_map() for ppc assumes that PTEs are
kept in highmem (CONFIG_HIGHPTE). There is only one implmentation of
pte_offset_map() as follows (include/asm-ppc/pgtable.h):
#define pte_offset_map(dir, addr) \
((pte_t *) kmap_atomic(pmd_page(*(dir)), KM_PTE0) + pte_index(addr))
Shouldn't this be made conditional according to CONFIG_HIGHPTE is
defined or not (as implemented in include/asm-i386/pgtable.h) ?
the same goes for pte_offset_map_nested and the corresponding unmap functions.
thanks,
Satya Popuri
^ permalink raw reply
* Re: "do section mismatch check on full vmlinux" breaks powerpc build
From: Sam Ravnborg @ 2007-07-25 21:33 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev
In-Reply-To: <20070725211610.GJ951@localdomain>
On Wed, Jul 25, 2007 at 04:16:10PM -0500, Nathan Lynch wrote:
> Hi Sam-
>
> Sam Ravnborg wrote:
> > On Tue, Jul 24, 2007 at 05:41:05PM -0500, Nathan Lynch wrote:
> > >
> > > 2.6.23-rc1 breaks the build for 64-bit powerpc for me (using
> > > maple_defconfig):
> > >
> > > LD vmlinux.o
> > > powerpc64-unknown-linux-gnu-ld: dynreloc miscount for
> > > kernel/built-in.o, section .opd
> > > powerpc64-unknown-linux-gnu-ld: can not edit opd Bad value
> > > make: *** [vmlinux.o] Error 1
> > >
> >
> > I narrowed it down to the following change to avoid the breakage:
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index c456c3a..2ea222f 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1246,7 +1246,7 @@ void drop_slab(void);
> > extern int randomize_va_space;
> > #endif
> >
> > -__attribute__((weak)) const char *arch_vma_name(struct vm_area_struct *vma);
> > +//__attribute__((weak)) const char *arch_vma_name(struct vm_area_struct *vma);
> >
> > #endif /* __KERNEL__ */
> > #endif /* _LINUX_MM_H */
> >
> > So seems that something goes a bit fishy when using weak symbols and this trigges
> > a binutils bug.
> >
> > The above line was introdused in the following commit:
> >
> > commit f269fdd1829acc5e53bf57b145003e5733133f2b
> > Author: David Howells <dhowells@redhat.com>
> > Date: Wed Sep 27 01:50:23 2006 -0700
> >
> > [PATCH] NOMMU: move the fallback arch_vma_name() to a sensible place
>
>
> Thanks for looking into this. Removing the "__attribute__((weak))"
> from arch_vma_name's declaration in linux/mm.h unbreaks the build for
> me.
>
> Maybe it shouldn't matter, but it seems unusual to have the weak
> attribute specified at the function's declaration. I wasn't able to
> find any examples of that for other weak functions in the kernel
> (e.g. sched_clock).
Unfortunately removing the weak attribute uncovered that x86_64 has
two functions with the same name => link error.
Needs to have that sorted first out but ball is rolling.
(Se lkml for details)
Sam
^ permalink raw reply
* Re: [PATCH 1/2] [POWERPC] Add support of platforms without PHY to gianfar driver
From: Jeff Garzik @ 2007-07-25 21:21 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev, linux-kernel, netdev
In-Reply-To: <20070725174312.5818.61155.stgit@localhost.localdomain>
I'll let paulus and linuxppc merge this one (or not)...
Jeff
^ permalink raw reply
* Re: "do section mismatch check on full vmlinux" breaks powerpc build
From: Nathan Lynch @ 2007-07-25 21:16 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: linuxppc-dev
In-Reply-To: <20070725114356.GB25580@uranus.ravnborg.org>
Hi Sam-
Sam Ravnborg wrote:
> On Tue, Jul 24, 2007 at 05:41:05PM -0500, Nathan Lynch wrote:
> >
> > 2.6.23-rc1 breaks the build for 64-bit powerpc for me (using
> > maple_defconfig):
> >
> > LD vmlinux.o
> > powerpc64-unknown-linux-gnu-ld: dynreloc miscount for
> > kernel/built-in.o, section .opd
> > powerpc64-unknown-linux-gnu-ld: can not edit opd Bad value
> > make: *** [vmlinux.o] Error 1
> >
>
> I narrowed it down to the following change to avoid the breakage:
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index c456c3a..2ea222f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1246,7 +1246,7 @@ void drop_slab(void);
> extern int randomize_va_space;
> #endif
>
> -__attribute__((weak)) const char *arch_vma_name(struct vm_area_struct *vma);
> +//__attribute__((weak)) const char *arch_vma_name(struct vm_area_struct *vma);
>
> #endif /* __KERNEL__ */
> #endif /* _LINUX_MM_H */
>
> So seems that something goes a bit fishy when using weak symbols and this trigges
> a binutils bug.
>
> The above line was introdused in the following commit:
>
> commit f269fdd1829acc5e53bf57b145003e5733133f2b
> Author: David Howells <dhowells@redhat.com>
> Date: Wed Sep 27 01:50:23 2006 -0700
>
> [PATCH] NOMMU: move the fallback arch_vma_name() to a sensible place
Thanks for looking into this. Removing the "__attribute__((weak))"
from arch_vma_name's declaration in linux/mm.h unbreaks the build for
me.
Maybe it shouldn't matter, but it seems unusual to have the weak
attribute specified at the function's declaration. I wasn't able to
find any examples of that for other weak functions in the kernel
(e.g. sched_clock).
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Alan Cox @ 2007-07-25 20:39 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <Pine.LNX.4.60.0707252130090.6234@poirot.grange>
> driver to using platform-device. I got a reply, that it's not worth it now
> that IDE is slowly becoming obsolete, and the pata_platform serves the
> perpose perfectly well. I found this argument reasonable, I had the same
> doubt, just wanted to double-check. So, why do we now need a new legacy
> (a/drivers/ide/legacy/ide_platform.c) driver when a "modern" driver
> exists?
We don't *need* it but some people still want to use old IDE and the
author was willing to make it neatly compatible so that anything that
works with the pata_platform should be able to use the ide_platform
driver and vice versa. For the shorter term that can only be a good thing
- arch code doesn't need to care about which driver is used, end users
can pick and it doesn't end up adding new ties between code and old IDE.
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Guennadi Liakhovetski @ 2007-07-25 20:09 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-kernel, Alan Cox, linux-ide
In-Reply-To: <46A7AC17.9060309@ru.mvista.com>
On Thu, 26 Jul 2007, Sergei Shtylyov wrote:
> Guennadi Liakhovetski wrote:
>
> > > Wrong list to submit sych stuff, post to linux-ide.
>
> > Not entirely. The patch (or other patches in the series) would also touch
> > ARM platforms in the mainline, currently using that driver. As I didn't
>
> Was worth cross-posting to linux-ide anyway to get the IDE experts'
> feedback. ;-)
linux-arm* mailing lists do not allow cross-posting.
> > have a chance to test them due to lack of hardware, I posted on arm, asking
> > if anyone would test those platforms for me.
>
> ... and they laughed at you? ;-)
No, noone had that hardware either:-) Those who had preferred to forget
about it, I guess.
> > It was largely in accordance with my own opinion, so, I chose to accept
> > it:-)
>
> It's not clear why you decided to waste time on it then. :-)
Because I myself was in the situation where my local version of the driver
was filling with #ifdef's supporting all possible variations of our
hardware, so, I switched it to platform_driver to clean up that mess. And
then decided to ask if others would consider it useful. Just in case. I
hoped they wouldn't.
> > > > doubt, just wanted to double-check. So, why do we now need a new legacy
> > > > (a/drivers/ide/legacy/ide_platform.c) driver when a "modern" driver
> > > > exists?
>
> > > Good question (I know the answer but won't tell ;-).
>
> > You've been very cooperative, thanks.
>
> In fact, I also highly doubt that we need it. What we'd need is an OF
> driver.
Great, thanks. Now we have to find out why Alan acked it (added to cc).
Thanks
Guennadi
---
Guennadi Liakhovetski
^ permalink raw reply
* [PATCH 2/2] powerpc: Marvell MV64x60 EDAC driver
From: Dave Jiang @ 2007-07-25 20:08 UTC (permalink / raw)
To: paulus; +Cc: linuxppc-dev, bluesmoke-devel, norsk5
Marvell mv64x60 SoC support for EDAC. Used on PPC and MIPS platforms.
Development and testing done on PPC Motorola prpmc2800 ATCA board.
The driver provides error reporting for the CPU error registers, the
memory controller error registers, the SRAM error registers, and the
PCI error registers. The error reporting can be done two ways, via
interrupts, or polling.
Signed-off-by: Dave Jiang <djiang@mvista.com>
Signed-off-by: Douglas Thompson <dougthompson@xmission.com>
---
drivers/edac/Kconfig | 6
drivers/edac/Makefile | 2
drivers/edac/mv64x60_edac.c | 846 +++++++++++++++++++++++++++++++++++++++++++
drivers/edac/mv64x60_edac.h | 114 ++++++
4 files changed, 967 insertions(+), 1 deletions(-)
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 1724c41..2dff779 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -131,5 +131,11 @@ config EDAC_PASEMI
Support for error detection and correction on PA Semi
PWRficient.
+config EDAC_MV64X60
+ tristate "Marvell MV64x60"
+ depends on EDAC_MM_EDAC && MV64X60
+ help
+ Support for error detection and correction on the Marvell
+ MV64360 and MV64460 chipsets.
endif # EDAC
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 02c09f0..3cbee8c 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -28,4 +28,4 @@ obj-$(CONFIG_EDAC_I3000) += i3000_edac.o
obj-$(CONFIG_EDAC_I82860) += i82860_edac.o
obj-$(CONFIG_EDAC_R82600) += r82600_edac.o
obj-$(CONFIG_EDAC_PASEMI) += pasemi_edac.o
-
+obj-$(CONFIG_EDAC_MV64X60) += mv64x60_edac.o
diff --git a/drivers/edac/mv64x60_edac.c b/drivers/edac/mv64x60_edac.c
new file mode 100644
index 0000000..06848ce
--- /dev/null
+++ b/drivers/edac/mv64x60_edac.c
@@ -0,0 +1,846 @@
+/*
+ * Marvell MV64x60 Memory Controller kernel module for PPC platforms
+ *
+ * Author: Dave Jiang <djiang@mvista.com>
+ *
+ * 2006-2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/edac.h>
+
+#include "edac_core.h"
+#include "edac_module.h"
+#include "mv64x60_edac.h"
+
+const char *mv64x60_ctl_name = "MV64x60";
+static int edac_dev_idx;
+static int edac_pci_idx;
+static int edac_mc_idx;
+
+/*********************** PCI err device **********************************/
+#ifdef CONFIG_PCI
+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);
+ 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));
+ printk(KERN_ERR "Address High: 0x%08x\n",
+ in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_ADDR_HI));
+ printk(KERN_ERR "Attribute: 0x%08x\n",
+ in_le32(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);
+
+ if (cause & MV64X60_PCI_PE_MASK)
+ edac_pci_handle_pe(pci, pci->ctl_name);
+
+ if (!(cause & MV64X60_PCI_PE_MASK))
+ edac_pci_handle_npe(pci, pci->ctl_name);
+}
+
+static irqreturn_t mv64x60_pci_isr(int irq, void *dev_id)
+{
+ struct edac_pci_ctl_info *pci = dev_id;
+ struct mv64x60_pci_pdata *pdata = pci->pvt_info;
+ u32 val;
+
+ val = in_le32(pdata->pci_vbase + MV64X60_PCI_ERROR_CAUSE);
+ if (!val)
+ return IRQ_NONE;
+
+ mv64x60_pci_check(pci);
+
+ return IRQ_HANDLED;
+}
+
+static int __devinit mv64x60_pci_err_probe(struct platform_device *pdev)
+{
+ struct edac_pci_ctl_info *pci;
+ struct mv64x60_pci_pdata *pdata;
+ struct resource *r;
+ int res = 0;
+
+ if (!devres_open_group(&pdev->dev, mv64x60_pci_err_probe, GFP_KERNEL))
+ return -ENOMEM;
+
+ pci = edac_pci_alloc_ctl_info(sizeof(*pdata), "mv64x60_pci_err");
+ if (!pci)
+ return -ENOMEM;
+
+ pdata = pci->pvt_info;
+
+ pdata->pci_hose = pdev->id;
+ pdata->name = "mpc85xx_pci_err";
+ pdata->irq = NO_IRQ;
+ platform_set_drvdata(pdev, pdata);
+ pci->dev = &pdev->dev;
+ pci->dev_name = pdev->dev.bus_id;
+ pci->mod_name = EDAC_MOD_STR;
+ pci->ctl_name = pdata->name;
+
+ if (edac_op_state == EDAC_OPSTATE_POLL)
+ pci->edac_check = mv64x60_pci_check;
+
+ pdata->edac_idx = edac_pci_idx++;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ printk(KERN_ERR "%s: Unable to get resource for "
+ "PCI err regs\n", __func__);
+ res = -ENOENT;
+ goto err;
+ }
+
+ if (!devm_request_mem_region(&pdev->dev,
+ r->start,
+ r->end - r->start + 1,
+ pdata->name)) {
+ printk(KERN_ERR "%s: Error while requesting mem region\n",
+ __func__);
+ res = -EBUSY;
+ goto err;
+ }
+
+ pdata->pci_vbase = devm_ioremap(&pdev->dev,
+ r->start,
+ r->end - r->start + 1);
+ if (!pdata->pci_vbase) {
+ printk(KERN_ERR "%s: Unable to setup PCI err regs\n", __func__);
+ res = -ENOMEM;
+ 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);
+
+ if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
+ debugf3("%s(): failed edac_pci_add_device()\n", __func__);
+ goto err;
+ }
+
+ if (edac_op_state == EDAC_OPSTATE_INT) {
+ pdata->irq = platform_get_irq(pdev, 0);
+ res = devm_request_irq(&pdev->dev,
+ pdata->irq,
+ mv64x60_pci_isr,
+ IRQF_DISABLED,
+ "[EDAC] PCI err",
+ pci);
+ if (res < 0) {
+ printk(KERN_ERR "%s: Unable to request irq %d for "
+ "MV64x60 PCI ERR\n", __func__, pdata->irq);
+ res = -ENODEV;
+ goto err2;
+ }
+ printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for PCI Err\n",
+ pdata->irq);
+ }
+
+ devres_remove_group(&pdev->dev, mv64x60_pci_err_probe);
+
+ /* get this far and it's successful */
+ debugf3("%s(): success\n", __func__);
+
+ return 0;
+
+err2:
+ edac_pci_del_device(&pdev->dev);
+err:
+ edac_pci_free_ctl_info(pci);
+ devres_release_group(&pdev->dev, mv64x60_pci_err_probe);
+ return res;
+}
+
+static int mv64x60_pci_err_remove(struct platform_device *pdev)
+{
+ struct edac_pci_ctl_info *pci = platform_get_drvdata(pdev);
+
+ debugf0("%s()\n", __func__);
+
+ edac_pci_del_device(&pdev->dev);
+
+ edac_pci_free_ctl_info(pci);
+
+ return 0;
+}
+
+static struct platform_driver mv64x60_pci_err_driver = {
+ .probe = mv64x60_pci_err_probe,
+ .remove = __devexit_p(mv64x60_pci_err_remove),
+ .driver = {
+ .name = "mv64x60_pci_err",
+ }
+};
+
+#endif /* CONFIG_PCI */
+
+/*********************** SRAM err device **********************************/
+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);
+ 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));
+ printk(KERN_ERR "Address High: 0x%08x\n",
+ in_le32(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));
+ printk(KERN_ERR "Data High: 0x%08x\n",
+ in_le32(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);
+
+ edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
+}
+
+static irqreturn_t mv64x60_sram_isr(int irq, void *dev_id)
+{
+ struct edac_device_ctl_info *edac_dev = dev_id;
+ struct mv64x60_sram_pdata *pdata = edac_dev->pvt_info;
+ u32 cause;
+
+ cause = in_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE);
+ if (!cause)
+ return IRQ_NONE;
+
+ mv64x60_sram_check(edac_dev);
+
+ return IRQ_HANDLED;
+}
+
+static int __devinit mv64x60_sram_err_probe(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *edac_dev;
+ struct mv64x60_sram_pdata *pdata;
+ struct resource *r;
+ int res = 0;
+
+ if (!devres_open_group(&pdev->dev, mv64x60_sram_err_probe, GFP_KERNEL))
+ return -ENOMEM;
+
+ edac_dev = edac_device_alloc_ctl_info(sizeof(*pdata),
+ "sram", 1, NULL, 0, 0, NULL, 0,
+ edac_dev_idx);
+ if (!edac_dev) {
+ devres_release_group(&pdev->dev, mv64x60_sram_err_probe);
+ return -ENOMEM;
+ }
+
+ pdata = edac_dev->pvt_info;
+ pdata->name = "mv64x60_sram_err";
+ pdata->irq = NO_IRQ;
+ edac_dev->dev = &pdev->dev;
+ platform_set_drvdata(pdev, pdata);
+ edac_dev->dev_name = pdev->dev.bus_id;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ printk(KERN_ERR "%s: Unable to get resource for "
+ "SRAM err regs\n", __func__);
+ res = -ENOENT;
+ goto err;
+ }
+
+ if (!devm_request_mem_region(&pdev->dev,
+ r->start,
+ r->end - r->start + 1,
+ pdata->name)) {
+ printk(KERN_ERR "%s: Error while request mem region\n",
+ __func__);
+ res = -EBUSY;
+ goto err;
+ }
+
+ pdata->sram_vbase = devm_ioremap(&pdev->dev,
+ r->start,
+ r->end - r->start + 1);
+ if (!pdata->sram_vbase) {
+ printk(KERN_ERR "%s: Unable to setup SRAM err regs\n",
+ __func__);
+ res = -ENOMEM;
+ goto err;
+ }
+
+ /* setup SRAM err registers */
+ out_le32(pdata->sram_vbase + MV64X60_SRAM_ERR_CAUSE, 0);
+
+ edac_dev->mod_name = EDAC_MOD_STR;
+ edac_dev->ctl_name = pdata->name;
+
+ if (edac_op_state == EDAC_OPSTATE_POLL)
+ edac_dev->edac_check = mv64x60_sram_check;
+
+ pdata->edac_idx = edac_dev_idx++;
+
+ if (edac_device_add_device(edac_dev) > 0) {
+ debugf3("%s(): failed edac_device_add_device()\n", __func__);
+ goto err;
+ }
+
+ if (edac_op_state == EDAC_OPSTATE_INT) {
+ pdata->irq = platform_get_irq(pdev, 0);
+ res = devm_request_irq(&pdev->dev,
+ pdata->irq,
+ mv64x60_sram_isr,
+ IRQF_DISABLED,
+ "[EDAC] SRAM err",
+ edac_dev);
+ if (res < 0) {
+ printk(KERN_ERR
+ "%s: Unable to request irq %d for "
+ "MV64x60 SRAM ERR\n", __func__, pdata->irq);
+ res = -ENODEV;
+ goto err2;
+ }
+
+ printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for SRAM Err\n",
+ pdata->irq);
+ }
+
+ devres_remove_group(&pdev->dev, mv64x60_sram_err_probe);
+
+ /* get this far and it's successful */
+ debugf3("%s(): success\n", __func__);
+
+ return 0;
+
+err2:
+ edac_device_del_device(&pdev->dev);
+err:
+ devres_release_group(&pdev->dev, mv64x60_sram_err_probe);
+ edac_device_free_ctl_info(edac_dev);
+ return res;
+}
+
+static int mv64x60_sram_err_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *edac_dev = platform_get_drvdata(pdev);
+
+ debugf0("%s()\n", __func__);
+
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(edac_dev);
+
+ return 0;
+}
+
+static struct platform_driver mv64x60_sram_err_driver = {
+ .probe = mv64x60_sram_err_probe,
+ .remove = mv64x60_sram_err_remove,
+ .driver = {
+ .name = "mv64x60_sram_err",
+ }
+};
+
+/*********************** CPU err device **********************************/
+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) &
+ MV64x60_CPU_CAUSE_MASK;
+ if (!cause)
+ return;
+
+ 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));
+ printk(KERN_ERR "Address High: 0x%08x\n",
+ in_le32(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));
+ printk(KERN_ERR "Data High: 0x%08x\n",
+ in_le32(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);
+
+ edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
+}
+
+static irqreturn_t mv64x60_cpu_isr(int irq, void *dev_id)
+{
+ struct edac_device_ctl_info *edac_dev = dev_id;
+ struct mv64x60_cpu_pdata *pdata = edac_dev->pvt_info;
+ u32 cause;
+
+ cause = in_le32(pdata->cpu_vbase[1] + MV64x60_CPU_ERR_CAUSE) &
+ MV64x60_CPU_CAUSE_MASK;
+ if (!cause)
+ return IRQ_NONE;
+
+ mv64x60_cpu_check(edac_dev);
+
+ return IRQ_HANDLED;
+}
+
+static int __devinit mv64x60_cpu_err_probe(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *edac_dev;
+ struct resource *r;
+ struct mv64x60_cpu_pdata *pdata;
+ int res = 0;
+
+ if (!devres_open_group(&pdev->dev, mv64x60_cpu_err_probe, GFP_KERNEL))
+ return -ENOMEM;
+
+ edac_dev = edac_device_alloc_ctl_info(sizeof(*pdata),
+ "cpu", 1, NULL, 0, 0, NULL, 0,
+ edac_dev_idx);
+ if (!edac_dev) {
+ devres_release_group(&pdev->dev, mv64x60_cpu_err_probe);
+ return -ENOMEM;
+ }
+
+ pdata = edac_dev->pvt_info;
+ pdata->name = "mv64x60_cpu_err";
+ pdata->irq = NO_IRQ;
+ edac_dev->dev = &pdev->dev;
+ platform_set_drvdata(pdev, edac_dev);
+ edac_dev->dev_name = pdev->dev.bus_id;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ printk(KERN_ERR "%s: Unable to get resource for "
+ "CPU err regs\n", __func__);
+ res = -ENOENT;
+ goto err;
+ }
+
+ if (!devm_request_mem_region(&pdev->dev,
+ r->start,
+ r->end - r->start + 1,
+ pdata->name)) {
+ printk(KERN_ERR "%s: Error while requesting mem region\n",
+ __func__);
+ res = -EBUSY;
+ goto err;
+ }
+
+ pdata->cpu_vbase[0] = devm_ioremap(&pdev->dev,
+ r->start,
+ r->end - r->start + 1);
+ if (!pdata->cpu_vbase[0]) {
+ printk(KERN_ERR "%s: Unable to setup CPU err regs\n", __func__);
+ res = -ENOMEM;
+ goto err;
+ }
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!r) {
+ printk(KERN_ERR "%s: Unable to get resource for "
+ "CPU err regs\n", __func__);
+ res = -ENOENT;
+ goto err;
+ }
+
+ if (!devm_request_mem_region(&pdev->dev,
+ r->start,
+ r->end - r->start + 1,
+ pdata->name)) {
+ printk(KERN_ERR "%s: Error while requesting mem region\n",
+ __func__);
+ res = -EBUSY;
+ goto err;
+ }
+
+ pdata->cpu_vbase[1] = devm_ioremap(&pdev->dev,
+ r->start,
+ r->end - r->start + 1);
+ if (!pdata->cpu_vbase[1]) {
+ printk(KERN_ERR "%s: Unable to setup CPU err regs\n", __func__);
+ res = -ENOMEM;
+ goto err;
+ }
+
+ /* 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);
+
+ edac_dev->mod_name = EDAC_MOD_STR;
+ edac_dev->ctl_name = pdata->name;
+ if (edac_op_state == EDAC_OPSTATE_POLL)
+ edac_dev->edac_check = mv64x60_cpu_check;
+
+ pdata->edac_idx = edac_dev_idx++;
+
+ if (edac_device_add_device(edac_dev) > 0) {
+ debugf3("%s(): failed edac_device_add_device()\n", __func__);
+ goto err;
+ }
+
+ if (edac_op_state == EDAC_OPSTATE_INT) {
+ pdata->irq = platform_get_irq(pdev, 0);
+ res = devm_request_irq(&pdev->dev,
+ pdata->irq,
+ mv64x60_cpu_isr,
+ IRQF_DISABLED,
+ "[EDAC] CPU err",
+ edac_dev);
+ if (res < 0) {
+ printk(KERN_ERR
+ "%s: Unable to request irq %d for MV64x60 "
+ "CPU ERR\n", __func__, pdata->irq);
+ res = -ENODEV;
+ goto err2;
+ }
+
+ printk(KERN_INFO EDAC_MOD_STR
+ " acquired irq %d for CPU Err\n", pdata->irq);
+ }
+
+ devres_remove_group(&pdev->dev, mv64x60_cpu_err_probe);
+
+ /* get this far and it's successful */
+ debugf3("%s(): success\n", __func__);
+
+ return 0;
+
+err2:
+ edac_device_del_device(&pdev->dev);
+err:
+ devres_release_group(&pdev->dev, mv64x60_cpu_err_probe);
+ edac_device_free_ctl_info(edac_dev);
+ return res;
+}
+
+static int mv64x60_cpu_err_remove(struct platform_device *pdev)
+{
+ struct edac_device_ctl_info *edac_dev = platform_get_drvdata(pdev);
+
+ debugf0("%s()\n", __func__);
+
+ edac_device_del_device(&pdev->dev);
+ edac_device_free_ctl_info(edac_dev);
+ return 0;
+}
+
+static struct platform_driver mv64x60_cpu_err_driver = {
+ .probe = mv64x60_cpu_err_probe,
+ .remove = mv64x60_cpu_err_remove,
+ .driver = {
+ .name = "mv64x60_cpu_err",
+ }
+};
+
+/*********************** DRAM err device **********************************/
+
+static void mv64x60_mc_check(struct mem_ctl_info *mci)
+{
+ struct mv64x60_mc_pdata *pdata = mci->pvt_info;
+ u32 reg;
+ u32 err_addr;
+ u32 sdram_ecc;
+ u32 comp_ecc;
+ u32 syndrome;
+
+ reg = in_le32(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);
+ syndrome = sdram_ecc ^ comp_ecc;
+
+ /* first bit clear in ECC Err Reg, 1 bit error, correctable by HW */
+ if (!(reg & 0x1))
+ edac_mc_handle_ce(mci, err_addr >> PAGE_SHIFT,
+ err_addr & PAGE_MASK, syndrome, 0, 0,
+ mci->ctl_name);
+ else /* 2 bit error, UE */
+ edac_mc_handle_ue(mci, err_addr >> PAGE_SHIFT,
+ err_addr & PAGE_MASK, 0, mci->ctl_name);
+
+ /* clear the error */
+ out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR, 0);
+}
+
+static irqreturn_t mv64x60_mc_isr(int irq, void *dev_id)
+{
+ struct mem_ctl_info *mci = dev_id;
+ struct mv64x60_mc_pdata *pdata = mci->pvt_info;
+ u32 reg;
+
+ reg = in_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ADDR);
+ if (!reg)
+ return IRQ_NONE;
+
+ /* writing 0's to the ECC err addr in check function clears irq */
+ mv64x60_mc_check(mci);
+
+ return IRQ_HANDLED;
+}
+
+static void get_total_mem(struct mv64x60_mc_pdata *pdata)
+{
+ struct device_node *np = NULL;
+ const unsigned int *reg;
+
+ np = of_find_node_by_type(NULL, "memory");
+ if (!np)
+ return;
+
+ reg = get_property(np, "reg", NULL);
+
+ pdata->total_mem = reg[1];
+}
+
+static void mv64x60_init_csrows(struct mem_ctl_info *mci,
+ struct mv64x60_mc_pdata *pdata)
+{
+ struct csrow_info *csrow;
+ u32 devtype;
+ u32 ctl;
+
+ get_total_mem(pdata);
+
+ ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
+
+ csrow = &mci->csrows[0];
+ csrow->first_page = 0;
+ csrow->nr_pages = pdata->total_mem >> PAGE_SHIFT;
+ csrow->last_page = csrow->first_page + csrow->nr_pages - 1;
+ csrow->grain = 8;
+
+ csrow->mtype = (ctl & MV64X60_SDRAM_REGISTERED) ? MEM_RDDR : MEM_DDR;
+
+ devtype = (ctl >> 20) & 0x3;
+ switch (devtype) {
+ case 0x0:
+ csrow->dtype = DEV_X32;
+ break;
+ case 0x2: /* could be X8 too, but no way to tell */
+ csrow->dtype = DEV_X16;
+ break;
+ case 0x3:
+ csrow->dtype = DEV_X4;
+ break;
+ default:
+ csrow->dtype = DEV_UNKNOWN;
+ break;
+ }
+
+ csrow->edac_mode = EDAC_SECDED;
+}
+
+static int __devinit mv64x60_mc_err_probe(struct platform_device *pdev)
+{
+ struct mem_ctl_info *mci;
+ struct mv64x60_mc_pdata *pdata;
+ struct resource *r;
+ u32 ctl;
+ int res = 0;
+
+ if (!devres_open_group(&pdev->dev, mv64x60_mc_err_probe, GFP_KERNEL))
+ return -ENOMEM;
+
+ mci = edac_mc_alloc(sizeof(struct mv64x60_mc_pdata), 1, 1, edac_mc_idx);
+ if (!mci) {
+ printk(KERN_ERR "%s: No memory for CPU err\n", __func__);
+ devres_release_group(&pdev->dev, mv64x60_mc_err_probe);
+ return -ENOMEM;
+ }
+
+ pdata = mci->pvt_info;
+ mci->dev = &pdev->dev;
+ platform_set_drvdata(pdev, mci);
+ pdata->name = "mv64x60_mc_err";
+ pdata->irq = NO_IRQ;
+ mci->dev_name = pdev->dev.bus_id;
+ pdata->edac_idx = edac_mc_idx++;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ printk(KERN_ERR "%s: Unable to get resource for "
+ "MC err regs\n", __func__);
+ res = -ENOENT;
+ goto err;
+ }
+
+ if (!devm_request_mem_region(&pdev->dev,
+ r->start,
+ r->end - r->start + 1,
+ pdata->name)) {
+ printk(KERN_ERR "%s: Error while requesting mem region\n",
+ __func__);
+ res = -EBUSY;
+ goto err;
+ }
+
+ pdata->mc_vbase = devm_ioremap(&pdev->dev,
+ r->start,
+ r->end - r->start + 1);
+ if (!pdata->mc_vbase) {
+ printk(KERN_ERR "%s: Unable to setup MC err regs\n", __func__);
+ res = -ENOMEM;
+ goto err;
+ }
+
+ ctl = in_le32(pdata->mc_vbase + MV64X60_SDRAM_CONFIG);
+ if (!(ctl & MV64X60_SDRAM_ECC)) {
+ /* Non-ECC RAM? */
+ printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
+ res = -ENODEV;
+ goto err2;
+ }
+
+ debugf3("%s(): init mci\n", __func__);
+ mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_DDR;
+ mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
+ mci->edac_cap = EDAC_FLAG_SECDED;
+ mci->mod_name = EDAC_MOD_STR;
+ mci->mod_ver = MV64x60_REVISION;
+ mci->ctl_name = mv64x60_ctl_name;
+
+ if (edac_op_state == EDAC_OPSTATE_POLL)
+ mci->edac_check = mv64x60_mc_check;
+
+ mci->ctl_page_to_phys = NULL;
+
+ mci->scrub_mode = SCRUB_SW_SRC;
+
+ 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);
+ ctl = (ctl & 0xff00ffff) | 0x10000;
+ out_le32(pdata->mc_vbase + MV64X60_SDRAM_ERR_ECC_CNTL, ctl);
+
+ if (edac_mc_add_mc(mci)) {
+ debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
+ goto err;
+ }
+
+ if (edac_op_state == EDAC_OPSTATE_INT) {
+ /* acquire interrupt that reports errors */
+ pdata->irq = platform_get_irq(pdev, 0);
+ res = devm_request_irq(&pdev->dev,
+ pdata->irq,
+ mv64x60_mc_isr,
+ IRQF_DISABLED,
+ "[EDAC] MC err",
+ mci);
+ if (res < 0) {
+ printk(KERN_ERR "%s: Unable to request irq %d for "
+ "MV64x60 DRAM ERR\n", __func__, pdata->irq);
+ res = -ENODEV;
+ goto err2;
+ }
+
+ printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for MC Err\n",
+ pdata->irq);
+ }
+
+ /* get this far and it's successful */
+ debugf3("%s(): success\n", __func__);
+
+ return 0;
+
+err2:
+ edac_mc_del_mc(&pdev->dev);
+err:
+ devres_release_group(&pdev->dev, mv64x60_mc_err_probe);
+ edac_mc_free(mci);
+ return res;
+}
+
+static int mv64x60_mc_err_remove(struct platform_device *pdev)
+{
+ struct mem_ctl_info *mci = platform_get_drvdata(pdev);
+
+ debugf0("%s()\n", __func__);
+
+ edac_mc_del_mc(&pdev->dev);
+ edac_mc_free(mci);
+ return 0;
+}
+
+static struct platform_driver mv64x60_mc_err_driver = {
+ .probe = mv64x60_mc_err_probe,
+ .remove = mv64x60_mc_err_remove,
+ .driver = {
+ .name = "mv64x60_mc_err",
+ }
+};
+
+static int __init mv64x60_edac_init(void)
+{
+ int ret = 0;
+
+ printk(KERN_INFO "Marvell MV64x60 EDAC driver " MV64x60_REVISION "\n");
+ printk(KERN_INFO "\t(C) 2006-2007 MontaVista Software\n");
+ /* make sure error reporting method is sane */
+ switch (edac_op_state) {
+ case EDAC_OPSTATE_POLL:
+ case EDAC_OPSTATE_INT:
+ break;
+ default:
+ edac_op_state = EDAC_OPSTATE_INT;
+ break;
+ }
+
+ ret = platform_driver_register(&mv64x60_mc_err_driver) ? : ret;
+
+ ret = platform_driver_register(&mv64x60_cpu_err_driver) ? : ret;
+
+ ret = platform_driver_register(&mv64x60_sram_err_driver) ? : ret;
+
+#ifdef CONFIG_PCI
+ ret = platform_driver_register(&mv64x60_pci_err_driver) ? : ret;
+#endif
+
+ return ret;
+}
+
+module_init(mv64x60_edac_init);
+
+static void __exit mv64x60_edac_exit(void)
+{
+#ifdef CONFIG_PCI
+ platform_driver_unregister(&mv64x60_pci_err_driver);
+#endif
+ platform_driver_unregister(&mv64x60_sram_err_driver);
+ platform_driver_unregister(&mv64x60_cpu_err_driver);
+ platform_driver_unregister(&mv64x60_mc_err_driver);
+}
+
+module_exit(mv64x60_edac_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Montavista Software, Inc.");
+module_param(edac_op_state, int, 0444);
+MODULE_PARM_DESC(edac_op_state,
+ "EDAC Error Reporting state: 0=Poll, 2=Interrupt");
diff --git a/drivers/edac/mv64x60_edac.h b/drivers/edac/mv64x60_edac.h
new file mode 100644
index 0000000..e042e2d
--- /dev/null
+++ b/drivers/edac/mv64x60_edac.h
@@ -0,0 +1,114 @@
+/*
+ * EDAC defs for Marvell MV64x60 bridge chip
+ *
+ * Author: Dave Jiang <djiang@mvista.com>
+ *
+ * 2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ */
+#ifndef _MV64X60_EDAC_H_
+#define _MV64X60_EDAC_H_
+
+#define MV64x60_REVISION " Ver: 2.0.0 " __DATE__
+#define EDAC_MOD_STR "MV64x60_edac"
+
+#define mv64x60_printk(level, fmt, arg...) \
+ edac_printk(level, "MV64x60", fmt, ##arg)
+
+#define mv64x60_mc_printk(mci, level, fmt, arg...) \
+ edac_mc_chipset_printk(mci, level, "MV64x60", fmt, ##arg)
+
+/* CPU Error Report Registers */
+#define MV64x60_CPU_ERR_ADDR_LO 0x00 /* 0x0070 */
+#define MV64x60_CPU_ERR_ADDR_HI 0x08 /* 0x0078 */
+#define MV64x60_CPU_ERR_DATA_LO 0x00 /* 0x0128 */
+#define MV64x60_CPU_ERR_DATA_HI 0x08 /* 0x0130 */
+#define MV64x60_CPU_ERR_PARITY 0x10 /* 0x0138 */
+#define MV64x60_CPU_ERR_CAUSE 0x18 /* 0x0140 */
+#define MV64x60_CPU_ERR_MASK 0x20 /* 0x0148 */
+
+#define MV64x60_CPU_CAUSE_MASK 0x07ffffff
+
+/* SRAM Error Report Registers */
+#define MV64X60_SRAM_ERR_CAUSE 0x08 /* 0x0388 */
+#define MV64X60_SRAM_ERR_ADDR_LO 0x10 /* 0x0390 */
+#define MV64X60_SRAM_ERR_ADDR_HI 0x78 /* 0x03f8 */
+#define MV64X60_SRAM_ERR_DATA_LO 0x18 /* 0x0398 */
+#define MV64X60_SRAM_ERR_DATA_HI 0x20 /* 0x03a0 */
+#define MV64X60_SRAM_ERR_PARITY 0x28 /* 0x03a8 */
+
+/* SDRAM Controller Registers */
+#define MV64X60_SDRAM_CONFIG 0x00 /* 0x1400 */
+#define MV64X60_SDRAM_ERR_DATA_HI 0x40 /* 0x1440 */
+#define MV64X60_SDRAM_ERR_DATA_LO 0x44 /* 0x1444 */
+#define MV64X60_SDRAM_ERR_ECC_RCVD 0x48 /* 0x1448 */
+#define MV64X60_SDRAM_ERR_ECC_CALC 0x4c /* 0x144c */
+#define MV64X60_SDRAM_ERR_ADDR 0x50 /* 0x1450 */
+#define MV64X60_SDRAM_ERR_ECC_CNTL 0x54 /* 0x1454 */
+#define MV64X60_SDRAM_ERR_ECC_ERR_CNT 0x58 /* 0x1458 */
+
+#define MV64X60_SDRAM_REGISTERED 0x20000
+#define MV64X60_SDRAM_ECC 0x40000
+
+#ifdef CONFIG_PCI
+/*
+ * Bit 0 of MV64x60_PCIx_ERR_MASK does not exist on the 64360 and because of
+ * errata FEr-#11 and FEr-##16 for the 64460, it should be 0 on that chip as
+ * well. IOW, don't set bit 0.
+ */
+#define MV64X60_PCIx_ERR_MASK_VAL 0x00a50c24
+
+/* Register offsets from PCIx error address low register */
+#define MV64X60_PCI_ERROR_ADDR_LO 0x00
+#define MV64X60_PCI_ERROR_ADDR_HI 0x04
+#define MV64X60_PCI_ERROR_ATTR 0x08
+#define MV64X60_PCI_ERROR_CMD 0x10
+#define MV64X60_PCI_ERROR_CAUSE 0x18
+#define MV64X60_PCI_ERROR_MASK 0x1c
+
+#define MV64X60_PCI_ERR_SWrPerr 0x0002
+#define MV64X60_PCI_ERR_SRdPerr 0x0004
+#define MV64X60_PCI_ERR_MWrPerr 0x0020
+#define MV64X60_PCI_ERR_MRdPerr 0x0040
+
+#define MV64X60_PCI_PE_MASK (MV64X60_PCI_ERR_SWrPerr | \
+ MV64X60_PCI_ERR_SRdPerr | \
+ MV64X60_PCI_ERR_MWrPerr | \
+ MV64X60_PCI_ERR_MRdPerr)
+
+struct mv64x60_pci_pdata {
+ int pci_hose;
+ void __iomem *pci_vbase;
+ char *name;
+ int irq;
+ int edac_idx;
+};
+
+#endif /* CONFIG_PCI */
+
+struct mv64x60_mc_pdata {
+ void __iomem *mc_vbase;
+ int total_mem;
+ char *name;
+ int irq;
+ int edac_idx;
+};
+
+struct mv64x60_cpu_pdata {
+ void __iomem *cpu_vbase[2];
+ char *name;
+ int irq;
+ int edac_idx;
+};
+
+struct mv64x60_sram_pdata {
+ void __iomem *sram_vbase;
+ char *name;
+ int irq;
+ int edac_idx;
+};
+
+#endif
^ permalink raw reply related
* [PATCH 1/2] powerpc: Marvell 64x60 EDAC platform devices setup
From: Dave Jiang @ 2007-07-25 20:06 UTC (permalink / raw)
To: paulus; +Cc: linuxppc-dev, bluesmoke-devel, norsk5
Creating platform devices (memory controller, sram error registers, cpu
error registers, PCI error registers) for Error Detection and Correction
(EDAC) driver.
The platform devices allow the mv64x60 EDAC driver to detect errors from
the memory controller (ECC erorrs), SRAM controller, CPU data path error
registers, and PCI error registers. The errors are reported to syslog.
Software ECC scrubbing is provided. These replace the mv64x60 error handlers
in the ppc branch. They are being moved to EDAC subsystem in order to
centralize error reporting.
The error reporting can be triggered via interrupts from the mv64x60 bridge
chip or via polling mechanism provided by the EDAC core code.
Signed-off-by: Dave Jiang <djiang@mvista.com>
Acked-by: Dale Farnsworth <dale@farnsworth.org>
---
arch/powerpc/sysdev/mv64x60_dev.c | 115 +++++++++++++++++++++++++++++++++----
1 files changed, 103 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index b618fa6..bc8cd5a 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -17,6 +17,7 @@
#include <linux/platform_device.h>
#include <asm/prom.h>
+#include <asm/io.h>
/*
* These functions provide the necessary setup for the mv64x60 drivers.
@@ -390,30 +391,120 @@ error:
return err;
}
+static int __init mv64x60_edac_pdev_init(struct device_node *np,
+ int id,
+ int num_addr,
+ char *pdev_name)
+{
+ struct resource *r;
+ struct platform_device *pdev;
+ int i, ret;
+
+ r = kzalloc(num_addr * sizeof(*r) + sizeof(*r), GFP_KERNEL);
+ if (!r)
+ return -ENOMEM;
+
+ for (i = 0; i < num_addr; i++) {
+ ret = of_address_to_resource(np, i, &r[i]);
+ if (ret) {
+ kfree(r);
+ return ret;
+ }
+ }
+
+ of_irq_to_resource(np, 0, &r[i]);
+
+ pdev = platform_device_register_simple(pdev_name, id, r, num_addr + 1);
+
+ kfree(r);
+
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PCI
+/*
+ * Bit 0 of MV64x60_PCIx_ERR_MASK does not exist on the 64360 and because of
+ * errata FEr-#11 and FEr-##16 for the 64460, it should be 0 on that chip as
+ * well. IOW, don't set bit 0.
+ */
+#define MV64X60_PCIx_ERR_MASK_VAL 0x00a50c24
+
+/* Erratum FEr PCI-#16: clear bit 0 of PCI SERRn Mask reg. */
+static int __init mv64x60_pci_fixup(struct device_node *np)
+{
+ struct resource res;
+ void __iomem *pci_serr;
+ int ret;
+
+ ret = of_address_to_resource(np, 1, &res);
+ if (ret)
+ return ret;
+
+ pci_serr = ioremap(res.start, res.end - res.start + 1);
+ if (!pci_serr)
+ return -ENOMEM;
+
+ out_le32(pci_serr, in_le32(pci_serr) & ~0x1);
+ iounmap(pci_serr);
+
+ return 0;
+}
+#endif /* CONFIG_PCI */
+
static int __init mv64x60_device_setup(void)
{
struct device_node *np = NULL;
int id;
int err;
- for (id = 0;
- (np = of_find_compatible_node(np, "serial", "marvell,mpsc")); id++)
- if ((err = mv64x60_mpsc_device_setup(np, id)))
+ id = 0;
+ for_each_compatible_node(np, "serial", "marvell,mpsc")
+ if ((err = mv64x60_mpsc_device_setup(np, id++)))
+ goto error;
+
+ id = 0;
+ for_each_compatible_node(np, "network", "marvell,mv64x60-eth")
+ if ((err = mv64x60_eth_device_setup(np, id++)))
goto error;
- for (id = 0;
- (np = of_find_compatible_node(np, "network",
- "marvell,mv64x60-eth"));
- id++)
- if ((err = mv64x60_eth_device_setup(np, id)))
+ id = 0;
+ for_each_compatible_node(np, "i2c", "marvell,mv64x60-i2c")
+ if ((err = mv64x60_i2c_device_setup(np, id++)))
goto error;
- for (id = 0;
- (np = of_find_compatible_node(np, "i2c", "marvell,mv64x60-i2c"));
- id++)
- if ((err = mv64x60_i2c_device_setup(np, id)))
+ id = 0;
+ for_each_compatible_node(np, NULL, "marvell,mv64x60-mem-ctrl")
+ if ((err = mv64x60_edac_pdev_init(np, id++, 1,
+ "mv64x60_mc_err")))
goto error;
+ id = 0;
+ for_each_compatible_node(np, NULL, "marvell,mv64x60-cpu-error")
+ if ((err = mv64x60_edac_pdev_init(np, id++, 2,
+ "mv64x60_cpu_err")))
+ goto error;
+
+ id = 0;
+ for_each_compatible_node(np, NULL, "marvell,mv64x60-sram-ctrl")
+ if ((err = mv64x60_edac_pdev_init(np, id++, 1,
+ "mv64x60_sram_err")))
+ goto error;
+
+#ifdef CONFIG_PCI
+ id = 0;
+ for_each_compatible_node(np, NULL, "marvell,mv64x60-pci-error") {
+ if ((err = mv64x60_pci_fixup(np)))
+ goto error;
+
+ if ((err = mv64x60_edac_pdev_init(np, id++, 1,
+ "mv64x60_pci_err")))
+ goto error;
+ }
+#endif
+
return 0;
error:
^ permalink raw reply related
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Sergei Shtylyov @ 2007-07-25 20:01 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <Pine.LNX.4.60.0707252150540.6234@poirot.grange>
Guennadi Liakhovetski wrote:
>>>>This is now very similar to pata_platform.c, they both use
>>>>same platform data structure and same resources.
>>>>To achieve that, byte_lanes_swapping platform data variable
>>>>and platform specified iops removed from that driver. It's fine,
>>>>since those were never used anyway.
>>>>pata_platform and ide_platform are carrying same driver names,
>>>>to easily switch between these drivers, without need to touch
>>>>platform code.
>>>Why? There's a drivers/ide/arm/ide_arm.c IDe driver that some platforms (not
>>>in the mainline) hack to access, e.g., CF cards in true-IDE mode. About a
>>>month ago I submitted a patch to arm-linux-kernel switching that
>> Wrong list to submit sych stuff, post to linux-ide.
> Not entirely. The patch (or other patches in the series) would also touch
> ARM platforms in the mainline, currently using that driver. As I didn't
Was worth cross-posting to linux-ide anyway to get the IDE experts'
feedback. ;-)
> have a chance to test them due to lack of hardware, I posted on arm,
> asking if anyone would test those platforms for me.
... and they laughed at you? ;-)
>>>driver to using platform-device. I got a reply, that it's not worth it now
>>>that IDE is slowly becoming obsolete, and the pata_platform serves the
>>>perpose perfectly well. I found this argument reasonable, I had the same
>> Ignore such replies in the future. ;-)
> It was largely in accordance with my own opinion, so, I chose to accept
> it:-)
It's not clear why you decided to waste time on it then. :-)
>>>doubt, just wanted to double-check. So, why do we now need a new legacy
>>>(a/drivers/ide/legacy/ide_platform.c) driver when a "modern" driver exists?
>> Good question (I know the answer but won't tell ;-).
> You've been very cooperative, thanks.
In fact, I also highly doubt that we need it. What we'd need is an OF driver.
> Thanks
> Guennadi
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Guennadi Liakhovetski @ 2007-07-25 19:54 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <46A7A8FB.5090909@ru.mvista.com>
On Wed, 25 Jul 2007, Sergei Shtylyov wrote:
> Guennadi Liakhovetski wrote:
>
> > > This is now very similar to pata_platform.c, they both use
> > > same platform data structure and same resources.
>
> > > To achieve that, byte_lanes_swapping platform data variable
> > > and platform specified iops removed from that driver. It's fine,
> > > since those were never used anyway.
>
> > > pata_platform and ide_platform are carrying same driver names,
> > > to easily switch between these drivers, without need to touch
> > > platform code.
>
> > Why? There's a drivers/ide/arm/ide_arm.c IDe driver that some platforms (not
> > in the mainline) hack to access, e.g., CF cards in true-IDE mode. About a
> > month ago I submitted a patch to arm-linux-kernel switching that
>
> Wrong list to submit sych stuff, post to linux-ide.
Not entirely. The patch (or other patches in the series) would also touch
ARM platforms in the mainline, currently using that driver. As I didn't
have a chance to test them due to lack of hardware, I posted on arm,
asking if anyone would test those platforms for me.
> > driver to using platform-device. I got a reply, that it's not worth it now
> > that IDE is slowly becoming obsolete, and the pata_platform serves the
> > perpose perfectly well. I found this argument reasonable, I had the same
>
> Ignore such replies in the future. ;-)
It was largely in accordance with my own opinion, so, I chose to accept
it:-)
> > doubt, just wanted to double-check. So, why do we now need a new legacy
> > (a/drivers/ide/legacy/ide_platform.c) driver when a "modern" driver exists?
>
> Good question (I know the answer but won't tell ;-).
You've been very cooperative, thanks.
Thanks
Guennadi
---
Guennadi Liakhovetski
^ permalink raw reply
* Re: [PATCH] mpx5200_uart: drop port lock across tty_flip_buffer() call
From: Grant Likely @ 2007-07-25 19:54 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Paul Mackerras, linuxppc-embedded
In-Reply-To: <1185392838.3227.13.camel@chaos>
On 7/25/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Wed, 2007-07-25 at 13:42 -0600, Grant Likely wrote:
> > On 7/25/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> > > The port lock needs to be dropped across the tty_flip_buffer call, as it
> > > would lead to a deadlock with the spin_lock(&port->lock) in uart_start()
> > >
> > > Uncovered by lockdep / preempt-rt
> > >
> > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> >
> > Instead of dropping the lock and reclaiming it, would it be better for
> > me to rework the driver to only grab the lock in the 'meat' of
> > mpc52xx_uart_int_rx_chars() and mpc52xx_uart_int_tx_chars()? (As
> > opposed to holding the lock for the entirety of mpc52xx_uart_int())
>
> No, it's not worth the trouble. You need to protect the hardware access.
>
> > What convention is used in other drivers?
>
> The same.
Okay, thanks.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Kumar, this is a bug fix, can you pick it up please?
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH] mpx5200_uart: drop port lock across tty_flip_buffer() call
From: Thomas Gleixner @ 2007-07-25 19:47 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-embedded
In-Reply-To: <fa686aa40707251242i6be0e30pd3b4019686308b0@mail.gmail.com>
On Wed, 2007-07-25 at 13:42 -0600, Grant Likely wrote:
> On 7/25/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> > The port lock needs to be dropped across the tty_flip_buffer call, as it
> > would lead to a deadlock with the spin_lock(&port->lock) in uart_start()
> >
> > Uncovered by lockdep / preempt-rt
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>
> Instead of dropping the lock and reclaiming it, would it be better for
> me to rework the driver to only grab the lock in the 'meat' of
> mpc52xx_uart_int_rx_chars() and mpc52xx_uart_int_tx_chars()? (As
> opposed to holding the lock for the entirety of mpc52xx_uart_int())
No, it's not worth the trouble. You need to protect the hardware access.
> What convention is used in other drivers?
The same.
tglx
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Sergei Shtylyov @ 2007-07-25 19:48 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <Pine.LNX.4.60.0707252130090.6234@poirot.grange>
Guennadi Liakhovetski wrote:
>>This is now very similar to pata_platform.c, they both use
>>same platform data structure and same resources.
>>To achieve that, byte_lanes_swapping platform data variable
>>and platform specified iops removed from that driver. It's fine,
>>since those were never used anyway.
>>pata_platform and ide_platform are carrying same driver names,
>>to easily switch between these drivers, without need to touch
>>platform code.
> Why? There's a drivers/ide/arm/ide_arm.c IDe driver that some platforms
> (not in the mainline) hack to access, e.g., CF cards in true-IDE mode.
> About a month ago I submitted a patch to arm-linux-kernel switching that
Wrong list to submit sych stuff, post to linux-ide.
> driver to using platform-device. I got a reply, that it's not worth it now
> that IDE is slowly becoming obsolete, and the pata_platform serves the
> perpose perfectly well. I found this argument reasonable, I had the same
Ignore such replies in the future. ;-)
> doubt, just wanted to double-check. So, why do we now need a new legacy
> (a/drivers/ide/legacy/ide_platform.c) driver when a "modern" driver
> exists?
Good question (I know the answer but won't tell ;-).
> Thanks
> Guennadi
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] mpx5200_uart: drop port lock across tty_flip_buffer() call
From: Grant Likely @ 2007-07-25 19:42 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linuxppc-embedded
In-Reply-To: <1185390656.3227.12.camel@chaos>
On 7/25/07, Thomas Gleixner <tglx@linutronix.de> wrote:
> The port lock needs to be dropped across the tty_flip_buffer call, as it
> would lead to a deadlock with the spin_lock(&port->lock) in uart_start()
>
> Uncovered by lockdep / preempt-rt
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Instead of dropping the lock and reclaiming it, would it be better for
me to rework the driver to only grab the lock in the 'meat' of
mpc52xx_uart_int_rx_chars() and mpc52xx_uart_int_tx_chars()? (As
opposed to holding the lock for the entirety of mpc52xx_uart_int())
What convention is used in other drivers?
Cheers,
g.
>
>
> Index: linux-2.6.22/drivers/serial/mpc52xx_uart.c
> ===================================================================
> --- linux-2.6.22.orig/drivers/serial/mpc52xx_uart.c 2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22/drivers/serial/mpc52xx_uart.c 2007-07-25 21:06:11.000000000 +0200
> @@ -501,7 +501,9 @@ mpc52xx_uart_int_rx_chars(struct uart_po
> }
> }
>
> + spin_unlock(&port->lock);
> tty_flip_buffer_push(tty);
> + spin_lock(&port->lock);
>
> return in_be16(&PSC(port)->mpc52xx_psc_status) & MPC52xx_PSC_SR_RXRDY;
> }
>
>
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Guennadi Liakhovetski @ 2007-07-25 19:37 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linux-ide, linux-kernel, linuxppc-dev
In-Reply-To: <20070725165318.5331.23795.stgit@localhost.localdomain>
On Wed, 25 Jul 2007, Vitaly Bordug wrote:
>
> This is now very similar to pata_platform.c, they both use
> same platform data structure and same resources.
>
> To achieve that, byte_lanes_swapping platform data variable
> and platform specified iops removed from that driver. It's fine,
> since those were never used anyway.
>
> pata_platform and ide_platform are carrying same driver names,
> to easily switch between these drivers, without need to touch
> platform code.
Why? There's a drivers/ide/arm/ide_arm.c IDe driver that some platforms
(not in the mainline) hack to access, e.g., CF cards in true-IDE mode.
About a month ago I submitted a patch to arm-linux-kernel switching that
driver to using platform-device. I got a reply, that it's not worth it now
that IDE is slowly becoming obsolete, and the pata_platform serves the
perpose perfectly well. I found this argument reasonable, I had the same
doubt, just wanted to double-check. So, why do we now need a new legacy
(a/drivers/ide/legacy/ide_platform.c) driver when a "modern" driver
exists?
Thanks
Guennadi
---
Guennadi Liakhovetski
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver
From: Sergei Shtylyov @ 2007-07-25 19:35 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <46A7A1D5.7020003@freescale.com>
Scott Wood wrote:
>>> It doesn't buy us anything in here, but it's conceivable that someone
>>> may want to write a driver that uses a shift in the I/O accessor
>>> rather than an array of port offsets,
>> It wouldn't be IDE driver then, and neither it would be libata
>> which also does this another way this (despite pata_platform uses
>> shifts too -- not in the accessors, so no speed loss).
> The device tree is not just for Linux.
Yeah, and I can't wait to see some other its users. ;-)
This doesn't mean that shift is better anyway. If everyone considers it
better, I give up. But be warned that shift (stride) is not the only property
characterizing register accesses -- the regs might be only accessible as
16/32-bit quantities, for example (16-bit is a real world example -- from
Amiga or smth of that sort, IIRC).
>>> equivalent of the cntlzw innstruction, and shift makes it clear that
>>> the stride must be power-of-two). Plus, using shift is consistent
>>> with what we do on ns16550.
>> Why the heck should we care about the UART code taling about IDE?!
> Consistency?
We're not obliged to be consistent with every piece of the kernel code.
> -Scott
MBR, Sergei
^ permalink raw reply
* [PATCH] mpx5200_uart: drop port lock across tty_flip_buffer() call
From: Thomas Gleixner @ 2007-07-25 19:10 UTC (permalink / raw)
To: linuxppc-embedded
The port lock needs to be dropped across the tty_flip_buffer call, as it
would lead to a deadlock with the spin_lock(&port->lock) in uart_start()
Uncovered by lockdep / preempt-rt
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Index: linux-2.6.22/drivers/serial/mpc52xx_uart.c
===================================================================
--- linux-2.6.22.orig/drivers/serial/mpc52xx_uart.c 2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.22/drivers/serial/mpc52xx_uart.c 2007-07-25 21:06:11.000000000 +0200
@@ -501,7 +501,9 @@ mpc52xx_uart_int_rx_chars(struct uart_po
}
}
+ spin_unlock(&port->lock);
tty_flip_buffer_push(tty);
+ spin_lock(&port->lock);
return in_be16(&PSC(port)->mpc52xx_psc_status) & MPC52xx_PSC_SR_RXRDY;
}
^ permalink raw reply
* Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
From: Sergei Shtylyov @ 2007-07-25 19:30 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <46A798D8.7020906@freescale.com>
Scott Wood wrote:
>> Scott Wood wrote:
>>>> Also, what mmio-ide in the compat properly means in the context
>>>> of ide_platform which is able to handle both port and memory mapped
>>>> IDE.
>>> I/O-space is only valid in the context of PCI, ISA, or similar buses,
>>> and
>>> the bus-specific reg format indicates whether it's mmio-space or
>>> io-space.
>> You could save time on lecturing me (and use it to look on the
>> driver ;-).
> Sorry, I misread the question as being a mismatch between the
> capabilities of the device binding and the driver, not about the
> specific compatible name.
That too. :-)
> Something like "generic-ide" would probably be better.
I strongly disagree with "generic" part. The generic IDE could only be
said of 1:1 I/O mapped IDE ports, not about this fancy mapping.
>>> What is board specific about a set of standard IDE registers at a given
>> The regisrer mapping used is highly non-standard.
> The gap between registers is nonstandard, but that's a fairly common
> type of noncompliance in embedded-land, and probably merits being
That is only a common variation of embedded non-compliancy (which doesn't
make it a compliancy. ;-)
There are worse cases in the bi-endian land, even with the standard 8-bit
regs and 1-byte stride. *Hopefully*, this driver could also support those...
> supported in a generic way. I wouldn't call it "highly" nonstandard.
Yeah, there are also 8250 "compatible" UARTs that use 32-bit memory
accesses, and even worse -- with some registers mapped differently than on
8250 (those can't be called compatible by any means), yet 8250.c drives all of
them. I'm not really sure it was such a good idea to merge, say Alchemy UART
support into 8250.c.
> Is there some other non-standardness that I'm missing?
*Hopefully*, none. The original Kumar's driver pretended to handle
byte-lane swapping too (but that was ugly :-).
>> We're already in board specific code, so why the heck not? :-)
>>> various ns16550-compatibles out there as well?
>> I never suggested that -- what I did suggest was make of_serial.c
>> recognize certain chip types and register them with 8250 driver.
> What would be the advantage of maintaining a list of chips whose only
Nobody's talking about the advantages, just about the device tree accepted
practices (which we've already tried to bypass with MTD node -- causing a lot
of bashing until David Woodhouse came to help :-).
> difference is register spacing, rather than just using reg-shift and
> being done with it?
Please read the linuxppc-dev archive's threads following form David's
patches. Or maybe Segher could repeat this for you. ;-)
> -Scott
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver
From: Scott Wood @ 2007-07-25 19:17 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <46A7A17C.8090505@ru.mvista.com>
Sergei Shtylyov wrote:
>> It doesn't buy us anything in here, but it's conceivable that someone
>> may want to write a driver that uses a shift in the I/O accessor
>> rather than an array of port offsets,
>
>
> It wouldn't be IDE driver then, and neither it would be libata which
> also does this another way this (despite pata_platform uses shifts too
> -- not in the accessors, so no speed loss).
The device tree is not just for Linux.
>> equivalent of the cntlzw innstruction, and shift makes it clear that
>> the stride must be power-of-two). Plus, using shift is consistent
>> with what we do on ns16550.
>
>
> Why the heck should we care about the UART code taling about IDE?!
Consistency?
> So, let me consider your argument purely speculative and invalid. ;-)
Consider it whatever you want. :-)
-Scott
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Sergei Shtylyov @ 2007-07-25 19:16 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <46A79F14.9040409@freescale.com>
Hello.
Scott Wood wrote:
>>> + hwif->hw.io_ports[IDE_DATA_OFFSET] = port;
>>> +
>>> + port += (1 << pdata->ioport_shift);
>>> + for (i = IDE_ERROR_OFFSET; i <= IDE_STATUS_OFFSET;
>>> + i++, port += (1 << pdata->ioport_shift))
>>
>>
>>
>> Looks like shift doesn't buy as anything, why not just use stride?
> It doesn't buy us anything in here, but it's conceivable that someone
> may want to write a driver that uses a shift in the I/O accessor rather
> than an array of port offsets,
It wouldn't be IDE driver then, and neither it would be libata which also
does this another way this (despite pata_platform uses shifts too -- not in
the accessors, so no speed loss).
> and it's easier to convert a shift to a stride than the other way around
> (not all architectures have an
> equivalent of the cntlzw innstruction, and shift makes it clear that the
> stride must be power-of-two). Plus, using shift is consistent with what
> we do on ns16550.
Why the heck should we care about the UART code taling about IDE?!
So, let me consider your argument purely speculative and invalid. ;-)
> -Scott
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Scott Wood @ 2007-07-25 19:05 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-kernel, linux-ide
In-Reply-To: <46A79DE0.8060405@ru.mvista.com>
Sergei Shtylyov wrote:
>>+ hwif->hw.io_ports[IDE_DATA_OFFSET] = port;
>>+
>>+ port += (1 << pdata->ioport_shift);
>>+ for (i = IDE_ERROR_OFFSET; i <= IDE_STATUS_OFFSET;
>>+ i++, port += (1 << pdata->ioport_shift))
>
>
> Looks like shift doesn't buy as anything, why not just use stride?
It doesn't buy us anything in here, but it's conceivable that someone
may want to write a driver that uses a shift in the I/O accessor rather
than an array of port offsets, and it's easier to convert a shift to a
stride than the other way around (not all architectures have an
equivalent of the cntlzw innstruction, and shift makes it clear that the
stride must be power-of-two). Plus, using shift is consistent with what
we do on ns16550.
-Scott
^ permalink raw reply
* Re: [PATCH 1/2] [IDE] Platform IDE driver (was: MMIO IDE driver)
From: Sergei Shtylyov @ 2007-07-25 19:00 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linux-ide, linux-kernel, linuxppc-dev
In-Reply-To: <20070725165318.5331.23795.stgit@localhost.localdomain>
Vitaly Bordug wrote:
> This is now very similar to pata_platform.c, they both use
> same platform data structure and same resources.
> To achieve that, byte_lanes_swapping platform data variable
> and platform specified iops removed from that driver. It's fine,
> since those were never used anyway.
Hopefully, PPC will never need them.
> pata_platform and ide_platform are carrying same driver names,
> to easily switch between these drivers, without need to touch
> platform code.
> diff --git a/drivers/ide/legacy/ide_platform.c b/drivers/ide/legacy/ide_platform.c
> new file mode 100644
> index 0000000..0b3f5b5
> --- /dev/null
> +++ b/drivers/ide/legacy/ide_platform.c
> @@ -0,0 +1,181 @@
> +/*
> + * Platform IDE driver
> + *
> + * Copyright (C) 2007 MontaVista Software
> + *
> + * Maintainer: Kumar Gala <galak@kernel.crashing.org>
Poor Kumar, I guess he was surprised be being assigned a maintainer of the
driver he didn't write (well, even if it borrowed some code from his earlier
one ;-)...
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/ide.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/pata_platform.h>
> +#include <linux/io.h>
> +
> +static struct {
> + void __iomem *plat_ide_mapbase;
> + void __iomem *plat_ide_alt_mapbase;
> + ide_hwif_t *hwif;
> + int index;
> +} hwif_prop;
> +
> +static ide_hwif_t *__devinit plat_ide_locate_hwif(void __iomem *base,
> + void __iomem *ctrl, struct pata_platform_info *pdata, int irq,
> + int mmio)
> +{
> + unsigned long port = (unsigned long)base;
> + ide_hwif_t *hwif;
> + int index, i;
> +
> + for (index = 0; index < MAX_HWIFS; ++index) {
> + hwif = ide_hwifs + index;
> + if (hwif->io_ports[IDE_DATA_OFFSET] == port)
> + goto found;
> + }
> +
> + for (index = 0; index < MAX_HWIFS; ++index) {
> + hwif = ide_hwifs + index;
> + if (hwif->io_ports[IDE_DATA_OFFSET] == 0)
> + goto found;
> + }
> +
> + return NULL;
> +
> +found:
> +
> + hwif->hw.io_ports[IDE_DATA_OFFSET] = port;
> +
> + port += (1 << pdata->ioport_shift);
> + for (i = IDE_ERROR_OFFSET; i <= IDE_STATUS_OFFSET;
> + i++, port += (1 << pdata->ioport_shift))
Looks like shift doesn't buy as anything, why not just use stride?
> + hwif->hw.io_ports[i] = port;
> +
> + hwif->hw.io_ports[IDE_CONTROL_OFFSET] = (unsigned long)ctrl;
> +
> + memcpy(hwif->io_ports, hwif->hw.io_ports, sizeof(hwif->hw.io_ports));
> + hwif->hw.irq = hwif->irq = irq;
> +
> + hwif->hw.dma = NO_DMA;
> + hwif->hw.chipset = ide_generic;
> +
> + if (mmio) {
> + hwif->mmio = 1;
> + default_hwif_mmiops(hwif);
> + }
And why default_hwif_iops(hwif) is not called else?
And I remember the previos variant was overriding INSW()/OUTSW() methods
-- turned out to be unnecessary? :-)
> +
> + hwif_prop.hwif = hwif;
> + hwif_prop.index = index;
> +
> + return hwif;
> +}
> +
> +static int __devinit plat_ide_probe(struct platform_device *pdev)
> +{
> + struct resource *res_base, *res_alt, *res_irq;
> + ide_hwif_t *hwif;
> + struct pata_platform_info *pdata;
> + int ret = 0;
> + int mmio = 0;
> +
> + pdata = pdev->dev.platform_data;
> +
> + /* get a pointer to the register memory */
> + res_base = platform_get_resource(pdev, IORESOURCE_IO, 0);
> + res_alt = platform_get_resource(pdev, IORESOURCE_IO, 1);
> +
> + if (!res_base || !res_alt) {
> + res_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + res_alt = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res_base || !res_alt) {
> + ret = -ENOMEM;
ENODEV or EINVAL you mean? :-)
> + goto out;
Bleh... just return please.
> + }
> + mmio = 1;
> + }
> +
> + res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!res_irq) {
> + ret = -EINVAL;
> + goto out;
Bleh... return -EINVAL, please. Or maybe -ENODEV.
> + }
> +
[...]
> + hwif = plat_ide_locate_hwif(hwif_prop.plat_ide_mapbase,
> + hwif_prop.plat_ide_alt_mapbase, pdata, res_irq->start, mmio);
> +
> + if (!hwif) {
> + ret = -ENODEV;
> + goto out;
Bleh... please just
return -ENODEV.
> + }
> + hwif->gendev.parent = &pdev->dev;
> + hwif->noprobe = 0;
> +
> + probe_hwif_init(hwif);
> +
> + platform_set_drvdata(pdev, hwif);
> + ide_proc_register_port(hwif);
> +
> + return 0;
> +
> +out:
No need to abuse the function cleanup style when you have nothing to
cleanup. :-/
> + return ret;
> +}
> +
> +static int __devexit plat_ide_remove(struct platform_device *pdev)
> +{
> + ide_hwif_t *hwif = pdev->dev.driver_data;
> +
> + if (hwif != hwif_prop.hwif) {
> + dev_printk(KERN_DEBUG, &pdev->dev, "%s: hwif value error",
> + pdev->name);
> + } else {
> + ide_unregister(hwif_prop.index);
> + hwif_prop.index = 0;
> + hwif_prop.hwif = NULL;
> + }
> +
I'd have interchanged the success/failure branches...
> + return 0;
> +}
> +
> +static struct platform_driver platform_ide_driver = {
> + .driver {
> + .name = "pata_platform",
> + },
> + .probe = plat_ide_probe,
> + .remove = __devexit_p(plat_ide_remove),
> +};
> +
> +static int __init platform_ide_init(void)
> +{
> + return platform_driver_register(&platform_ide_driver);
> +}
> +
> +static void __exit platform_ide_exit(void)
> +{
> + platform_driver_unregister(&platform_ide_driver);
> +}
> +
> +MODULE_DESCRIPTION("Platform IDE driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(platform_ide_init);
> +module_exit(platform_ide_exit);
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox