[PATCH 2/2] powerpc/85xx: workaround for chips with MSI hardware errata

[Jia Hongtao-B38951]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Saturday, March 30, 2013 5:55 AM
> To: Jia Hongtao-B38951
> Cc: linuxppc-dev at lists.ozlabs.org; galak at kernel.crashing.org; Wood Scott-
> B07421; Li Yang-R58472; Jia Hongtao-B38951
> Subject: Re: [PATCH 2/2] powerpc/85xx: workaround for chips with MSI
> hardware errata
> 
> On 03/25/2013 10:28:47 PM, 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 at gmail.com>
> > Signed-off-by: Li Yang <leoli at freescale.com>
> > Signed-off-by: Jia Hongtao <hongtao.jia at freescale.com>
> > ---
> >  arch/powerpc/sysdev/fsl_msi.c | 47
> > ++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 44 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/sysdev/fsl_msi.c
> > b/arch/powerpc/sysdev/fsl_msi.c index 178c994..d2f8040 100644
> > --- a/arch/powerpc/sysdev/fsl_msi.c
> > +++ b/arch/powerpc/sysdev/fsl_msi.c
> > @@ -28,6 +28,8 @@
> >  #include "fsl_msi.h"
> >  #include "fsl_pci.h"
> >
> > +#define MSI_HW_ERRATA_ENDIAN 0x00000010

It seems Kumar like put this just in fsl_msi.c.
Here is the comments from Kumar few days ago:

"Is there any reason to put this in fsl_msi.h rather than just in 
fsl_msi.c itself?"

I think the all the #defines should be together.
Ether all in .h or all in .c.

In this case I prefer your idea.

> 
> This should probably be kept in the same place as the other
> msi->features definitions (e.g. FSL_PIC_IP_*).
> 
> > +/* MPIC version 2.0 has erratum PIC1 */ static int
> > +mpic_has_errata(void) {
> > +	if (mpic_primary_get_version() == 0x0200)
> > +		return 1;
> > +
> > +	return 0;
> > +}
> 
> mpic_has_erratum_pic1()

You are right.
Good advice.

> 
> > +	if ((features->fsl_pic_ip & FSL_PIC_IP_MASK) ==
> > FSL_PIC_IP_MPIC) {
> > +		rc = mpic_has_errata();
> > +		if (rc > 0) {
> > +			msi->feature |= MSI_HW_ERRATA_ENDIAN;
> > +		} else if (rc < 0) {
> > +			err = rc;
> > +			goto error_out;
> > +		}
> 
> When would mpic_has_errata() ever return a negative value (maybe
> mpic_primary_get_version could fail, but you don't allow for that in the
> interface)?
> 
> If you're not going to add a way for errors to be returned back, just
> do:
> 
> if (mpic_has_erratum_pic1())
> 	msi->feature |= MSI_HW_ERRATA_ENDIAN;
> 
> -Scott

I will update the mpic_primary_get_version() and it will return 0 if failed.
Based on the new mpic_primary_get_version() I agree with this.

Thanks.
-Hongtao.