| Message ID | 1363257323-2801-1-git-send-email-hongtao.jia@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kumar Gala |
| Headers | show |
On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote: > The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), It causes > that neither MSI nor MSI-X can work fine. This is a workaround to allow > MSI-X to function properly. > > Signed-off-by: Liu Shuo <soniccat.liu@gmail.com> > Signed-off-by: Li Yang <leoli@freescale.com> > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > --- > Changes for V2: > - Address almost all the comments from Michael Ellerman for V1. > Here is the link: > http://patchwork.ozlabs.org/patch/226833/ > > arch/powerpc/sysdev/fsl_msi.c | 65 +++++++++++++++++++++++++++++++++++++++++-- > arch/powerpc/sysdev/fsl_msi.h | 2 ++ > 2 files changed, 64 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c > index 178c994..54cb83e 100644 > --- a/arch/powerpc/sysdev/fsl_msi.c > +++ b/arch/powerpc/sysdev/fsl_msi.c > @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct fsl_msi *msi_data) > > static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type) > { > - if (type == PCI_CAP_ID_MSIX) > - pr_debug("fslmsi: MSI-X untested, trying anyway.\n"); > + struct fsl_msi *msi; > + > + if (type == PCI_CAP_ID_MSI) { > + /* > + * MPIC version 2.0 has erratum PIC1. For now MSI > + * could not work. So check to prevent MSI from > + * being used on the board with this erratum. > + */ > + list_for_each_entry(msi, &msi_head, list) > + if (msi->feature & MSI_HW_ERRATA_ENDIAN) > + return -EINVAL; > + } > > return 0; > } > @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq, > msg->address_lo = lower_32_bits(address); > msg->address_hi = upper_32_bits(address); > > - msg->data = hwirq; > + /* > + * MPIC version 2.0 has erratum PIC1. It causes > + * that neither MSI nor MSI-X can work fine. > + * This is a workaround to allow MSI-X to function > + * properly. It only works for MSI-X, we prevent > + * MSI on buggy chips in fsl_msi_check_device(). > + */ > + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) > + msg->data = __swab32(hwirq); > + else > + msg->data = hwirq; > > pr_debug("%s: allocated srs: %d, ibs: %d\n", > __func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG); > @@ -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, struct platform_device *dev, > return 0; > } > > +/* MPIC version 2.0 has erratum PIC1 */ > +static int mpic_has_errata(struct platform_device *dev) > +{ > + struct device_node *mpic_node; > + > + mpic_node = of_irq_find_parent(dev->dev.of_node); > + if (mpic_node) { > + u32 *reg_base, brr1 = 0; > + /* Get the PIC reg base */ > + reg_base = of_iomap(mpic_node, 0); > + of_node_put(mpic_node); > + if (!reg_base) { > + dev_err(&dev->dev, "ioremap problem failed.\n"); > + return -EIO; > + } > + > + /* Get the mpic version from block revision register 1 */ > + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); > + iounmap(reg_base); > + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) > + return 1; > + } else { > + dev_err(&dev->dev, "MSI can't find his parent mpic node.\n"); > + return -ENODEV; > + } > + > + return 0; > +} > + > static const struct of_device_id fsl_of_msi_ids[]; > static int fsl_of_msi_probe(struct platform_device *dev) > { > @@ -423,6 +472,16 @@ static int fsl_of_msi_probe(struct platform_device *dev) > > msi->feature = features->fsl_pic_ip; > > + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == FSL_PIC_IP_MPIC) { > + rc = mpic_has_errata(dev); > + if (rc > 0) { > + msi->feature |= MSI_HW_ERRATA_ENDIAN; > + } else if (rc < 0) { > + err = rc; > + goto error_out; > + } > + } > + > /* > * Remember the phandle, so that we can match with any PCI nodes > * that have an "fsl,msi" property. > diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h > index 8225f86..7389e8e 100644 > --- a/arch/powerpc/sysdev/fsl_msi.h > +++ b/arch/powerpc/sysdev/fsl_msi.h > @@ -25,6 +25,8 @@ > #define FSL_PIC_IP_IPIC 0x00000002 > #define FSL_PIC_IP_VMPIC 0x00000003 > > +#define MSI_HW_ERRATA_ENDIAN 0x00000010 > + Is there any reason to put this in fsl_msi.h rather than just in fsl_msi.c itself? - k > struct fsl_msi { > struct irq_domain *irqhost; > > -- > 1.8.0 >
> -----Original Message----- > From: Kumar Gala [mailto:galak@kernel.crashing.org] > Sent: Friday, March 15, 2013 4:05 AM > To: Jia Hongtao-B38951 > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > michael@ellerman.id.au; Li Yang-R58472; Jia Hongtao-B38951 > Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI > hardware errata > > > On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote: > > > The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), It > > causes that neither MSI nor MSI-X can work fine. This is a workaround > > to allow MSI-X to function properly. > > > > Signed-off-by: Liu Shuo <soniccat.liu@gmail.com> > > Signed-off-by: Li Yang <leoli@freescale.com> > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > > --- > > Changes for V2: > > - Address almost all the comments from Michael Ellerman for V1. > > Here is the link: > > http://patchwork.ozlabs.org/patch/226833/ > > > > arch/powerpc/sysdev/fsl_msi.c | 65 > > +++++++++++++++++++++++++++++++++++++++++-- > > arch/powerpc/sysdev/fsl_msi.h | 2 ++ > > 2 files changed, 64 insertions(+), 3 deletions(-) > > > > diff --git a/arch/powerpc/sysdev/fsl_msi.c > > b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644 > > --- a/arch/powerpc/sysdev/fsl_msi.c > > +++ b/arch/powerpc/sysdev/fsl_msi.c > > @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct fsl_msi > > *msi_data) > > > > static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int > > type) { > > - if (type == PCI_CAP_ID_MSIX) > > - pr_debug("fslmsi: MSI-X untested, trying anyway.\n"); > > + struct fsl_msi *msi; > > + > > + if (type == PCI_CAP_ID_MSI) { > > + /* > > + * MPIC version 2.0 has erratum PIC1. For now MSI > > + * could not work. So check to prevent MSI from > > + * being used on the board with this erratum. > > + */ > > + list_for_each_entry(msi, &msi_head, list) > > + if (msi->feature & MSI_HW_ERRATA_ENDIAN) > > + return -EINVAL; > > + } > > > > return 0; > > } > > @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct pci_dev > *pdev, int hwirq, > > msg->address_lo = lower_32_bits(address); > > msg->address_hi = upper_32_bits(address); > > > > - msg->data = hwirq; > > + /* > > + * MPIC version 2.0 has erratum PIC1. It causes > > + * that neither MSI nor MSI-X can work fine. > > + * This is a workaround to allow MSI-X to function > > + * properly. It only works for MSI-X, we prevent > > + * MSI on buggy chips in fsl_msi_check_device(). > > + */ > > + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) > > + msg->data = __swab32(hwirq); > > + else > > + msg->data = hwirq; > > > > pr_debug("%s: allocated srs: %d, ibs: %d\n", > > __func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG); > @@ > > -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, > struct platform_device *dev, > > return 0; > > } > > > > +/* MPIC version 2.0 has erratum PIC1 */ static int > > +mpic_has_errata(struct platform_device *dev) { > > + struct device_node *mpic_node; > > + > > + mpic_node = of_irq_find_parent(dev->dev.of_node); > > + if (mpic_node) { > > + u32 *reg_base, brr1 = 0; > > + /* Get the PIC reg base */ > > + reg_base = of_iomap(mpic_node, 0); > > + of_node_put(mpic_node); > > + if (!reg_base) { > > + dev_err(&dev->dev, "ioremap problem failed.\n"); > > + return -EIO; > > + } > > + > > + /* Get the mpic version from block revision register 1 */ > > + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); > > + iounmap(reg_base); > > + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) > > + return 1; > > + } else { > > + dev_err(&dev->dev, "MSI can't find his parent mpic node.\n"); > > + return -ENODEV; > > + } > > + > > + return 0; > > +} > > + > > static const struct of_device_id fsl_of_msi_ids[]; static int > > fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 +472,16 @@ > > static int fsl_of_msi_probe(struct platform_device *dev) > > > > msi->feature = features->fsl_pic_ip; > > > > + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == FSL_PIC_IP_MPIC) { > > + rc = mpic_has_errata(dev); > > + if (rc > 0) { > > + msi->feature |= MSI_HW_ERRATA_ENDIAN; > > + } else if (rc < 0) { > > + err = rc; > > + goto error_out; > > + } > > + } > > + > > /* > > * Remember the phandle, so that we can match with any PCI nodes > > * that have an "fsl,msi" property. > > diff --git a/arch/powerpc/sysdev/fsl_msi.h > > b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644 > > --- a/arch/powerpc/sysdev/fsl_msi.h > > +++ b/arch/powerpc/sysdev/fsl_msi.h > > @@ -25,6 +25,8 @@ > > #define FSL_PIC_IP_IPIC 0x00000002 > > #define FSL_PIC_IP_VMPIC 0x00000003 > > > > +#define MSI_HW_ERRATA_ENDIAN 0x00000010 > > + > > Is there any reason to put this in fsl_msi.h rather than just in > fsl_msi.c itself? > > - k Actually no. This micro is only used by fsl_msi.c. Will move it to fsl_msi.c. Thanks. -Hongtao. > > > struct fsl_msi { > > struct irq_domain *irqhost; > > > > -- > > 1.8.0 > > >
On Mar 14, 2013, at 9:00 PM, Jia Hongtao-B38951 wrote: > > >> -----Original Message----- >> From: Kumar Gala [mailto:galak@kernel.crashing.org] >> Sent: Friday, March 15, 2013 4:05 AM >> To: Jia Hongtao-B38951 >> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; >> michael@ellerman.id.au; Li Yang-R58472; Jia Hongtao-B38951 >> Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI >> hardware errata >> >> >> On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote: >> >>> The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), It >>> causes that neither MSI nor MSI-X can work fine. This is a workaround >>> to allow MSI-X to function properly. >>> >>> Signed-off-by: Liu Shuo <soniccat.liu@gmail.com> >>> Signed-off-by: Li Yang <leoli@freescale.com> >>> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> >>> --- >>> Changes for V2: >>> - Address almost all the comments from Michael Ellerman for V1. >>> Here is the link: >>> http://patchwork.ozlabs.org/patch/226833/ >>> >>> arch/powerpc/sysdev/fsl_msi.c | 65 >>> +++++++++++++++++++++++++++++++++++++++++-- >>> arch/powerpc/sysdev/fsl_msi.h | 2 ++ >>> 2 files changed, 64 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/sysdev/fsl_msi.c >>> b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644 >>> --- a/arch/powerpc/sysdev/fsl_msi.c >>> +++ b/arch/powerpc/sysdev/fsl_msi.c >>> @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct fsl_msi >>> *msi_data) >>> >>> static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int >>> type) { >>> - if (type == PCI_CAP_ID_MSIX) >>> - pr_debug("fslmsi: MSI-X untested, trying anyway.\n"); >>> + struct fsl_msi *msi; >>> + >>> + if (type == PCI_CAP_ID_MSI) { >>> + /* >>> + * MPIC version 2.0 has erratum PIC1. For now MSI >>> + * could not work. So check to prevent MSI from >>> + * being used on the board with this erratum. >>> + */ >>> + list_for_each_entry(msi, &msi_head, list) >>> + if (msi->feature & MSI_HW_ERRATA_ENDIAN) >>> + return -EINVAL; >>> + } >>> >>> return 0; >>> } >>> @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct pci_dev >> *pdev, int hwirq, >>> msg->address_lo = lower_32_bits(address); >>> msg->address_hi = upper_32_bits(address); >>> >>> - msg->data = hwirq; >>> + /* >>> + * MPIC version 2.0 has erratum PIC1. It causes >>> + * that neither MSI nor MSI-X can work fine. >>> + * This is a workaround to allow MSI-X to function >>> + * properly. It only works for MSI-X, we prevent >>> + * MSI on buggy chips in fsl_msi_check_device(). >>> + */ >>> + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) >>> + msg->data = __swab32(hwirq); >>> + else >>> + msg->data = hwirq; >>> >>> pr_debug("%s: allocated srs: %d, ibs: %d\n", >>> __func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG); >> @@ >>> -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, >> struct platform_device *dev, >>> return 0; >>> } >>> >>> +/* MPIC version 2.0 has erratum PIC1 */ static int >>> +mpic_has_errata(struct platform_device *dev) { >>> + struct device_node *mpic_node; >>> + >>> + mpic_node = of_irq_find_parent(dev->dev.of_node); >>> + if (mpic_node) { >>> + u32 *reg_base, brr1 = 0; >>> + /* Get the PIC reg base */ >>> + reg_base = of_iomap(mpic_node, 0); >>> + of_node_put(mpic_node); >>> + if (!reg_base) { >>> + dev_err(&dev->dev, "ioremap problem failed.\n"); >>> + return -EIO; >>> + } >>> + >>> + /* Get the mpic version from block revision register 1 */ >>> + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); >>> + iounmap(reg_base); >>> + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) >>> + return 1; >>> + } else { >>> + dev_err(&dev->dev, "MSI can't find his parent mpic node.\n"); >>> + return -ENODEV; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static const struct of_device_id fsl_of_msi_ids[]; static int >>> fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 +472,16 @@ >>> static int fsl_of_msi_probe(struct platform_device *dev) >>> >>> msi->feature = features->fsl_pic_ip; >>> >>> + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == FSL_PIC_IP_MPIC) { >>> + rc = mpic_has_errata(dev); >>> + if (rc > 0) { >>> + msi->feature |= MSI_HW_ERRATA_ENDIAN; >>> + } else if (rc < 0) { >>> + err = rc; >>> + goto error_out; >>> + } >>> + } >>> + >>> /* >>> * Remember the phandle, so that we can match with any PCI nodes >>> * that have an "fsl,msi" property. >>> diff --git a/arch/powerpc/sysdev/fsl_msi.h >>> b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644 >>> --- a/arch/powerpc/sysdev/fsl_msi.h >>> +++ b/arch/powerpc/sysdev/fsl_msi.h >>> @@ -25,6 +25,8 @@ >>> #define FSL_PIC_IP_IPIC 0x00000002 >>> #define FSL_PIC_IP_VMPIC 0x00000003 >>> >>> +#define MSI_HW_ERRATA_ENDIAN 0x00000010 >>> + >> >> Is there any reason to put this in fsl_msi.h rather than just in >> fsl_msi.c itself? >> >> - k > > Actually no. This micro is only used by fsl_msi.c. > Will move it to fsl_msi.c. > > Thanks. > -Hongtao. Also, wondering if we can do the mpic version detection in mpic.c and not here. I'm not sure what means we'd have to get back to the mpic struct so we could possible use mpic->flags. I'll look to see if I can suggest anything along those lines, thus reducing the amount of ioremap() and code to find the mpic registers, etc. - k
> -----Original Message----- > From: Kumar Gala [mailto:galak@kernel.crashing.org] > Sent: Friday, March 15, 2013 11:53 PM > To: Jia Hongtao-B38951 > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > michael@ellerman.id.au; Li Yang-R58472 > Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI > hardware errata > > > On Mar 14, 2013, at 9:00 PM, Jia Hongtao-B38951 wrote: > > > > > > >> -----Original Message----- > >> From: Kumar Gala [mailto:galak@kernel.crashing.org] > >> Sent: Friday, March 15, 2013 4:05 AM > >> To: Jia Hongtao-B38951 > >> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > >> michael@ellerman.id.au; Li Yang-R58472; Jia Hongtao-B38951 > >> Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI > >> hardware errata > >> > >> > >> On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote: > >> > >>> The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), It > >>> causes that neither MSI nor MSI-X can work fine. This is a > >>> workaround to allow MSI-X to function properly. > >>> > >>> Signed-off-by: Liu Shuo <soniccat.liu@gmail.com> > >>> Signed-off-by: Li Yang <leoli@freescale.com> > >>> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > >>> --- > >>> Changes for V2: > >>> - Address almost all the comments from Michael Ellerman for V1. > >>> Here is the link: > >>> http://patchwork.ozlabs.org/patch/226833/ > >>> > >>> arch/powerpc/sysdev/fsl_msi.c | 65 > >>> +++++++++++++++++++++++++++++++++++++++++-- > >>> arch/powerpc/sysdev/fsl_msi.h | 2 ++ > >>> 2 files changed, 64 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.c > >>> b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644 > >>> --- a/arch/powerpc/sysdev/fsl_msi.c > >>> +++ b/arch/powerpc/sysdev/fsl_msi.c > >>> @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct fsl_msi > >>> *msi_data) > >>> > >>> static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int > >>> type) { > >>> - if (type == PCI_CAP_ID_MSIX) > >>> - pr_debug("fslmsi: MSI-X untested, trying anyway.\n"); > >>> + struct fsl_msi *msi; > >>> + > >>> + if (type == PCI_CAP_ID_MSI) { > >>> + /* > >>> + * MPIC version 2.0 has erratum PIC1. For now MSI > >>> + * could not work. So check to prevent MSI from > >>> + * being used on the board with this erratum. > >>> + */ > >>> + list_for_each_entry(msi, &msi_head, list) > >>> + if (msi->feature & MSI_HW_ERRATA_ENDIAN) > >>> + return -EINVAL; > >>> + } > >>> > >>> return 0; > >>> } > >>> @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct pci_dev > >> *pdev, int hwirq, > >>> msg->address_lo = lower_32_bits(address); > >>> msg->address_hi = upper_32_bits(address); > >>> > >>> - msg->data = hwirq; > >>> + /* > >>> + * MPIC version 2.0 has erratum PIC1. It causes > >>> + * that neither MSI nor MSI-X can work fine. > >>> + * This is a workaround to allow MSI-X to function > >>> + * properly. It only works for MSI-X, we prevent > >>> + * MSI on buggy chips in fsl_msi_check_device(). > >>> + */ > >>> + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) > >>> + msg->data = __swab32(hwirq); > >>> + else > >>> + msg->data = hwirq; > >>> > >>> pr_debug("%s: allocated srs: %d, ibs: %d\n", > >>> __func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG); > >> @@ > >>> -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi > >>> *msi, > >> struct platform_device *dev, > >>> return 0; > >>> } > >>> > >>> +/* MPIC version 2.0 has erratum PIC1 */ static int > >>> +mpic_has_errata(struct platform_device *dev) { > >>> + struct device_node *mpic_node; > >>> + > >>> + mpic_node = of_irq_find_parent(dev->dev.of_node); > >>> + if (mpic_node) { > >>> + u32 *reg_base, brr1 = 0; > >>> + /* Get the PIC reg base */ > >>> + reg_base = of_iomap(mpic_node, 0); > >>> + of_node_put(mpic_node); > >>> + if (!reg_base) { > >>> + dev_err(&dev->dev, "ioremap problem failed.\n"); > >>> + return -EIO; > >>> + } > >>> + > >>> + /* Get the mpic version from block revision register 1 */ > >>> + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); > >>> + iounmap(reg_base); > >>> + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) > >>> + return 1; > >>> + } else { > >>> + dev_err(&dev->dev, "MSI can't find his parent mpic node.\n"); > >>> + return -ENODEV; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> static const struct of_device_id fsl_of_msi_ids[]; static int > >>> fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 +472,16 @@ > >>> static int fsl_of_msi_probe(struct platform_device *dev) > >>> > >>> msi->feature = features->fsl_pic_ip; > >>> > >>> + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == FSL_PIC_IP_MPIC) { > >>> + rc = mpic_has_errata(dev); > >>> + if (rc > 0) { > >>> + msi->feature |= MSI_HW_ERRATA_ENDIAN; > >>> + } else if (rc < 0) { > >>> + err = rc; > >>> + goto error_out; > >>> + } > >>> + } > >>> + > >>> /* > >>> * Remember the phandle, so that we can match with any PCI nodes > >>> * that have an "fsl,msi" property. > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.h > >>> b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644 > >>> --- a/arch/powerpc/sysdev/fsl_msi.h > >>> +++ b/arch/powerpc/sysdev/fsl_msi.h > >>> @@ -25,6 +25,8 @@ > >>> #define FSL_PIC_IP_IPIC 0x00000002 > >>> #define FSL_PIC_IP_VMPIC 0x00000003 > >>> > >>> +#define MSI_HW_ERRATA_ENDIAN 0x00000010 > >>> + > >> > >> Is there any reason to put this in fsl_msi.h rather than just in > >> fsl_msi.c itself? > >> > >> - k > > > > Actually no. This micro is only used by fsl_msi.c. > > Will move it to fsl_msi.c. > > > > Thanks. > > -Hongtao. > > Also, wondering if we can do the mpic version detection in mpic.c and not > here. I'm not sure what means we'd have to get back to the mpic struct > so we could possible use mpic->flags. > Use the MPIC version result in mpic.c was my plan. But as you point out there seems no obvious way to get the mpic struct. mpic struct is defined as an automatic variable in platform files. Also MSI driver is not so close to mpic driver under current architecture. If you get some elegant way to do this please feel free to tell me. Thanks. -Hongtao. > I'll look to see if I can suggest anything along those lines, thus > reducing the amount of ioremap() and code to find the mpic registers, etc. > > - k
;On 03/19/2013 03:03:13 AM, Jia Hongtao-B38951 wrote: > > > > -----Original Message----- > > From: Kumar Gala [mailto:galak@kernel.crashing.org] > > Sent: Friday, March 15, 2013 11:53 PM > > To: Jia Hongtao-B38951 > > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > > michael@ellerman.id.au; Li Yang-R58472 > > Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI > > hardware errata > > > > > > On Mar 14, 2013, at 9:00 PM, Jia Hongtao-B38951 wrote: > > > > > > > > > > >> -----Original Message----- > > >> From: Kumar Gala [mailto:galak@kernel.crashing.org] > > >> Sent: Friday, March 15, 2013 4:05 AM > > >> To: Jia Hongtao-B38951 > > >> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > > >> michael@ellerman.id.au; Li Yang-R58472; Jia Hongtao-B38951 > > >> Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with > MSI > > >> hardware errata > > >> > > >> > > >> On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote: > > >> > > >>> The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), > It > > >>> causes that neither MSI nor MSI-X can work fine. This is a > > >>> workaround to allow MSI-X to function properly. > > >>> > > >>> Signed-off-by: Liu Shuo <soniccat.liu@gmail.com> > > >>> Signed-off-by: Li Yang <leoli@freescale.com> > > >>> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > > >>> --- > > >>> Changes for V2: > > >>> - Address almost all the comments from Michael Ellerman for V1. > > >>> Here is the link: > > >>> http://patchwork.ozlabs.org/patch/226833/ > > >>> > > >>> arch/powerpc/sysdev/fsl_msi.c | 65 > > >>> +++++++++++++++++++++++++++++++++++++++++-- > > >>> arch/powerpc/sysdev/fsl_msi.h | 2 ++ > > >>> 2 files changed, 64 insertions(+), 3 deletions(-) > > >>> > > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.c > > >>> b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644 > > >>> --- a/arch/powerpc/sysdev/fsl_msi.c > > >>> +++ b/arch/powerpc/sysdev/fsl_msi.c > > >>> @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct > fsl_msi > > >>> *msi_data) > > >>> > > >>> static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, > int > > >>> type) { > > >>> - if (type == PCI_CAP_ID_MSIX) > > >>> - pr_debug("fslmsi: MSI-X untested, trying > anyway.\n"); > > >>> + struct fsl_msi *msi; > > >>> + > > >>> + if (type == PCI_CAP_ID_MSI) { > > >>> + /* > > >>> + * MPIC version 2.0 has erratum PIC1. For now > MSI > > >>> + * could not work. So check to prevent MSI from > > >>> + * being used on the board with this erratum. > > >>> + */ > > >>> + list_for_each_entry(msi, &msi_head, list) > > >>> + if (msi->feature & MSI_HW_ERRATA_ENDIAN) > > >>> + return -EINVAL; > > >>> + } > > >>> > > >>> return 0; > > >>> } > > >>> @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct > pci_dev > > >> *pdev, int hwirq, > > >>> msg->address_lo = lower_32_bits(address); > > >>> msg->address_hi = upper_32_bits(address); > > >>> > > >>> - msg->data = hwirq; > > >>> + /* > > >>> + * MPIC version 2.0 has erratum PIC1. It causes > > >>> + * that neither MSI nor MSI-X can work fine. > > >>> + * This is a workaround to allow MSI-X to function > > >>> + * properly. It only works for MSI-X, we prevent > > >>> + * MSI on buggy chips in fsl_msi_check_device(). > > >>> + */ > > >>> + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) > > >>> + msg->data = __swab32(hwirq); > > >>> + else > > >>> + msg->data = hwirq; > > >>> > > >>> pr_debug("%s: allocated srs: %d, ibs: %d\n", > > >>> __func__, hwirq / IRQS_PER_MSI_REG, hwirq % > IRQS_PER_MSI_REG); > > >> @@ > > >>> -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi > > >>> *msi, > > >> struct platform_device *dev, > > >>> return 0; > > >>> } > > >>> > > >>> +/* MPIC version 2.0 has erratum PIC1 */ static int > > >>> +mpic_has_errata(struct platform_device *dev) { > > >>> + struct device_node *mpic_node; > > >>> + > > >>> + mpic_node = of_irq_find_parent(dev->dev.of_node); > > >>> + if (mpic_node) { > > >>> + u32 *reg_base, brr1 = 0; > > >>> + /* Get the PIC reg base */ > > >>> + reg_base = of_iomap(mpic_node, 0); > > >>> + of_node_put(mpic_node); > > >>> + if (!reg_base) { > > >>> + dev_err(&dev->dev, "ioremap problem > failed.\n"); > > >>> + return -EIO; > > >>> + } > > >>> + > > >>> + /* Get the mpic version from block revision > register 1 */ > > >>> + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); > > >>> + iounmap(reg_base); > > >>> + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) > > >>> + return 1; > > >>> + } else { > > >>> + dev_err(&dev->dev, "MSI can't find his parent > mpic node.\n"); > > >>> + return -ENODEV; > > >>> + } > > >>> + > > >>> + return 0; > > >>> +} > > >>> + > > >>> static const struct of_device_id fsl_of_msi_ids[]; static int > > >>> fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 > +472,16 @@ > > >>> static int fsl_of_msi_probe(struct platform_device *dev) > > >>> > > >>> msi->feature = features->fsl_pic_ip; > > >>> > > >>> + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == > FSL_PIC_IP_MPIC) { > > >>> + rc = mpic_has_errata(dev); > > >>> + if (rc > 0) { > > >>> + msi->feature |= MSI_HW_ERRATA_ENDIAN; > > >>> + } else if (rc < 0) { > > >>> + err = rc; > > >>> + goto error_out; > > >>> + } > > >>> + } > > >>> + > > >>> /* > > >>> * Remember the phandle, so that we can match with any > PCI nodes > > >>> * that have an "fsl,msi" property. > > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.h > > >>> b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644 > > >>> --- a/arch/powerpc/sysdev/fsl_msi.h > > >>> +++ b/arch/powerpc/sysdev/fsl_msi.h > > >>> @@ -25,6 +25,8 @@ > > >>> #define FSL_PIC_IP_IPIC 0x00000002 > > >>> #define FSL_PIC_IP_VMPIC 0x00000003 > > >>> > > >>> +#define MSI_HW_ERRATA_ENDIAN 0x00000010 > > >>> + > > >> > > >> Is there any reason to put this in fsl_msi.h rather than just in > > >> fsl_msi.c itself? > > >> > > >> - k > > > > > > Actually no. This micro is only used by fsl_msi.c. > > > Will move it to fsl_msi.c. > > > > > > Thanks. > > > -Hongtao. > > > > Also, wondering if we can do the mpic version detection in mpic.c > and not > > here. I'm not sure what means we'd have to get back to the mpic > struct > > so we could possible use mpic->flags. > > > > Use the MPIC version result in mpic.c was my plan. > But as you point out there seems no obvious way to get the mpic > struct. > mpic struct is defined as an automatic variable in platform files. > Also MSI driver is not so close to mpic driver under current > architecture. > > If you get some elegant way to do this please feel free to tell me. Declare a non-static function to retrieve the MPIC version. -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, March 20, 2013 12:32 AM > To: Jia Hongtao-B38951 > Cc: Kumar Gala; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > michael@ellerman.id.au; Li Yang-R58472 > Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI > hardware errata > > ;On 03/19/2013 03:03:13 AM, Jia Hongtao-B38951 wrote: > > > > > > > -----Original Message----- > > > From: Kumar Gala [mailto:galak@kernel.crashing.org] > > > Sent: Friday, March 15, 2013 11:53 PM > > > To: Jia Hongtao-B38951 > > > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > > > michael@ellerman.id.au; Li Yang-R58472 > > > Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with MSI > > > hardware errata > > > > > > > > > On Mar 14, 2013, at 9:00 PM, Jia Hongtao-B38951 wrote: > > > > > > > > > > > > > > >> -----Original Message----- > > > >> From: Kumar Gala [mailto:galak@kernel.crashing.org] > > > >> Sent: Friday, March 15, 2013 4:05 AM > > > >> To: Jia Hongtao-B38951 > > > >> Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > > > >> michael@ellerman.id.au; Li Yang-R58472; Jia Hongtao-B38951 > > > >> Subject: Re: [PATCH V2] powerpc/85xx: workaround for chips with > > MSI > > > >> hardware errata > > > >> > > > >> > > > >> On Mar 14, 2013, at 5:35 AM, Jia Hongtao wrote: > > > >> > > > >>> The MPIC version 2.0 has a MSI errata (errata PIC1 of mpc8544), > > It > > > >>> causes that neither MSI nor MSI-X can work fine. This is a > > > >>> workaround to allow MSI-X to function properly. > > > >>> > > > >>> Signed-off-by: Liu Shuo <soniccat.liu@gmail.com> > > > >>> Signed-off-by: Li Yang <leoli@freescale.com> > > > >>> Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > > > >>> --- > > > >>> Changes for V2: > > > >>> - Address almost all the comments from Michael Ellerman for V1. > > > >>> Here is the link: > > > >>> http://patchwork.ozlabs.org/patch/226833/ > > > >>> > > > >>> arch/powerpc/sysdev/fsl_msi.c | 65 > > > >>> +++++++++++++++++++++++++++++++++++++++++-- > > > >>> arch/powerpc/sysdev/fsl_msi.h | 2 ++ > > > >>> 2 files changed, 64 insertions(+), 3 deletions(-) > > > >>> > > > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.c > > > >>> b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644 > > > >>> --- a/arch/powerpc/sysdev/fsl_msi.c > > > >>> +++ b/arch/powerpc/sysdev/fsl_msi.c > > > >>> @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct > > fsl_msi > > > >>> *msi_data) > > > >>> > > > >>> static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, > > int > > > >>> type) { > > > >>> - if (type == PCI_CAP_ID_MSIX) > > > >>> - pr_debug("fslmsi: MSI-X untested, trying > > anyway.\n"); > > > >>> + struct fsl_msi *msi; > > > >>> + > > > >>> + if (type == PCI_CAP_ID_MSI) { > > > >>> + /* > > > >>> + * MPIC version 2.0 has erratum PIC1. For now > > MSI > > > >>> + * could not work. So check to prevent MSI from > > > >>> + * being used on the board with this erratum. > > > >>> + */ > > > >>> + list_for_each_entry(msi, &msi_head, list) > > > >>> + if (msi->feature & MSI_HW_ERRATA_ENDIAN) > > > >>> + return -EINVAL; > > > >>> + } > > > >>> > > > >>> return 0; > > > >>> } > > > >>> @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct > > pci_dev > > > >> *pdev, int hwirq, > > > >>> msg->address_lo = lower_32_bits(address); > > > >>> msg->address_hi = upper_32_bits(address); > > > >>> > > > >>> - msg->data = hwirq; > > > >>> + /* > > > >>> + * MPIC version 2.0 has erratum PIC1. It causes > > > >>> + * that neither MSI nor MSI-X can work fine. > > > >>> + * This is a workaround to allow MSI-X to function > > > >>> + * properly. It only works for MSI-X, we prevent > > > >>> + * MSI on buggy chips in fsl_msi_check_device(). > > > >>> + */ > > > >>> + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) > > > >>> + msg->data = __swab32(hwirq); > > > >>> + else > > > >>> + msg->data = hwirq; > > > >>> > > > >>> pr_debug("%s: allocated srs: %d, ibs: %d\n", > > > >>> __func__, hwirq / IRQS_PER_MSI_REG, hwirq % > > IRQS_PER_MSI_REG); > > > >> @@ > > > >>> -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi > > > >>> *msi, > > > >> struct platform_device *dev, > > > >>> return 0; > > > >>> } > > > >>> > > > >>> +/* MPIC version 2.0 has erratum PIC1 */ static int > > > >>> +mpic_has_errata(struct platform_device *dev) { > > > >>> + struct device_node *mpic_node; > > > >>> + > > > >>> + mpic_node = of_irq_find_parent(dev->dev.of_node); > > > >>> + if (mpic_node) { > > > >>> + u32 *reg_base, brr1 = 0; > > > >>> + /* Get the PIC reg base */ > > > >>> + reg_base = of_iomap(mpic_node, 0); > > > >>> + of_node_put(mpic_node); > > > >>> + if (!reg_base) { > > > >>> + dev_err(&dev->dev, "ioremap problem > > failed.\n"); > > > >>> + return -EIO; > > > >>> + } > > > >>> + > > > >>> + /* Get the mpic version from block revision > > register 1 */ > > > >>> + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); > > > >>> + iounmap(reg_base); > > > >>> + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) > > > >>> + return 1; > > > >>> + } else { > > > >>> + dev_err(&dev->dev, "MSI can't find his parent > > mpic node.\n"); > > > >>> + return -ENODEV; > > > >>> + } > > > >>> + > > > >>> + return 0; > > > >>> +} > > > >>> + > > > >>> static const struct of_device_id fsl_of_msi_ids[]; static int > > > >>> fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 > > +472,16 @@ > > > >>> static int fsl_of_msi_probe(struct platform_device *dev) > > > >>> > > > >>> msi->feature = features->fsl_pic_ip; > > > >>> > > > >>> + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == > > FSL_PIC_IP_MPIC) { > > > >>> + rc = mpic_has_errata(dev); > > > >>> + if (rc > 0) { > > > >>> + msi->feature |= MSI_HW_ERRATA_ENDIAN; > > > >>> + } else if (rc < 0) { > > > >>> + err = rc; > > > >>> + goto error_out; > > > >>> + } > > > >>> + } > > > >>> + > > > >>> /* > > > >>> * Remember the phandle, so that we can match with any > > PCI nodes > > > >>> * that have an "fsl,msi" property. > > > >>> diff --git a/arch/powerpc/sysdev/fsl_msi.h > > > >>> b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644 > > > >>> --- a/arch/powerpc/sysdev/fsl_msi.h > > > >>> +++ b/arch/powerpc/sysdev/fsl_msi.h > > > >>> @@ -25,6 +25,8 @@ > > > >>> #define FSL_PIC_IP_IPIC 0x00000002 > > > >>> #define FSL_PIC_IP_VMPIC 0x00000003 > > > >>> > > > >>> +#define MSI_HW_ERRATA_ENDIAN 0x00000010 > > > >>> + > > > >> > > > >> Is there any reason to put this in fsl_msi.h rather than just in > > > >> fsl_msi.c itself? > > > >> > > > >> - k > > > > > > > > Actually no. This micro is only used by fsl_msi.c. > > > > Will move it to fsl_msi.c. > > > > > > > > Thanks. > > > > -Hongtao. > > > > > > Also, wondering if we can do the mpic version detection in mpic.c > > and not > > > here. I'm not sure what means we'd have to get back to the mpic > > struct > > > so we could possible use mpic->flags. > > > > > > > Use the MPIC version result in mpic.c was my plan. > > But as you point out there seems no obvious way to get the mpic > > struct. > > mpic struct is defined as an automatic variable in platform files. > > Also MSI driver is not so close to mpic driver under current > > architecture. > > > > If you get some elegant way to do this please feel free to tell me. > > Declare a non-static function to retrieve the MPIC version. > > -Scott The struct pointer "mpic" is for internal use only while the "mpic_primary" could be used for external. So I'd like to declare two functions something like: mpic_get_version(struct *mpic); mpic_primary_get_version(void); The first function is for mpic.c internal use by mpic_alloc() and mpic_init(). The second function is for external use by fsl_msi.c. There will be two patches: 1. Declare the two MPIC get-version APIs. 2. Fix the MSI errata. Any comments for this plan? Thanks. -Hongtao.
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c index 178c994..54cb83e 100644 --- a/arch/powerpc/sysdev/fsl_msi.c +++ b/arch/powerpc/sysdev/fsl_msi.c @@ -98,8 +98,18 @@ static int fsl_msi_init_allocator(struct fsl_msi *msi_data) static int fsl_msi_check_device(struct pci_dev *pdev, int nvec, int type) { - if (type == PCI_CAP_ID_MSIX) - pr_debug("fslmsi: MSI-X untested, trying anyway.\n"); + struct fsl_msi *msi; + + if (type == PCI_CAP_ID_MSI) { + /* + * MPIC version 2.0 has erratum PIC1. For now MSI + * could not work. So check to prevent MSI from + * being used on the board with this erratum. + */ + list_for_each_entry(msi, &msi_head, list) + if (msi->feature & MSI_HW_ERRATA_ENDIAN) + return -EINVAL; + } return 0; } @@ -142,7 +152,17 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq, msg->address_lo = lower_32_bits(address); msg->address_hi = upper_32_bits(address); - msg->data = hwirq; + /* + * MPIC version 2.0 has erratum PIC1. It causes + * that neither MSI nor MSI-X can work fine. + * This is a workaround to allow MSI-X to function + * properly. It only works for MSI-X, we prevent + * MSI on buggy chips in fsl_msi_check_device(). + */ + if (msi_data->feature & MSI_HW_ERRATA_ENDIAN) + msg->data = __swab32(hwirq); + else + msg->data = hwirq; pr_debug("%s: allocated srs: %d, ibs: %d\n", __func__, hwirq / IRQS_PER_MSI_REG, hwirq % IRQS_PER_MSI_REG); @@ -361,6 +381,35 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, struct platform_device *dev, return 0; } +/* MPIC version 2.0 has erratum PIC1 */ +static int mpic_has_errata(struct platform_device *dev) +{ + struct device_node *mpic_node; + + mpic_node = of_irq_find_parent(dev->dev.of_node); + if (mpic_node) { + u32 *reg_base, brr1 = 0; + /* Get the PIC reg base */ + reg_base = of_iomap(mpic_node, 0); + of_node_put(mpic_node); + if (!reg_base) { + dev_err(&dev->dev, "ioremap problem failed.\n"); + return -EIO; + } + + /* Get the mpic version from block revision register 1 */ + brr1 = in_be32(reg_base + MPIC_FSL_BRR1); + iounmap(reg_base); + if ((brr1 & MPIC_FSL_BRR1_VER) == 0x0200) + return 1; + } else { + dev_err(&dev->dev, "MSI can't find his parent mpic node.\n"); + return -ENODEV; + } + + return 0; +} + static const struct of_device_id fsl_of_msi_ids[]; static int fsl_of_msi_probe(struct platform_device *dev) { @@ -423,6 +472,16 @@ static int fsl_of_msi_probe(struct platform_device *dev) msi->feature = features->fsl_pic_ip; + if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) == FSL_PIC_IP_MPIC) { + rc = mpic_has_errata(dev); + if (rc > 0) { + msi->feature |= MSI_HW_ERRATA_ENDIAN; + } else if (rc < 0) { + err = rc; + goto error_out; + } + } + /* * Remember the phandle, so that we can match with any PCI nodes * that have an "fsl,msi" property. diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h index 8225f86..7389e8e 100644 --- a/arch/powerpc/sysdev/fsl_msi.h +++ b/arch/powerpc/sysdev/fsl_msi.h @@ -25,6 +25,8 @@ #define FSL_PIC_IP_IPIC 0x00000002 #define FSL_PIC_IP_VMPIC 0x00000003 +#define MSI_HW_ERRATA_ENDIAN 0x00000010 + struct fsl_msi { struct irq_domain *irqhost;