| Message ID | 1372656366-769-1-git-send-email-hongtao.jia@freescale.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 07/01/2013 12:26:06 AM, Jia Hongtao wrote: > From: Hongtao Jia <hongtao.jia@freescale.com> > > MPIC version is useful information for both mpic_alloc() and > mpic_init(). > The patch provide an API to get MPIC version for reusing the code. > Also, some other IP block may need MPIC version for their own use. > The API for external use is also provided. > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > Signed-off-by: Li Yang <leoli@freescale.com> > --- > V6: > * Fix compile error on mpc83xx. I already applied V5 before realizing that it was this patch that you updated, rather than the MSI patch that depends on it. There is no compile error until the MSI patch that starts using this (which I haven't yet applied, due to the error). Could you send a followup patch that just adds the stub? > +/* Get the version of primary MPIC */ > +#ifdef CONFIG_MPIC > +extern u32 fsl_mpic_primary_get_version(void); > +#else > +static inline u32 fsl_mpic_primary_get_version(void) > +{ > + return -ENOTSUPP; > +} > +#endif [snip] > +static u32 fsl_mpic_get_version(struct mpic *mpic) > +{ > + u32 brr1; > + > + if (!(mpic->flags & MPIC_FSL)) > + return 0; In one case, calling this without having an FSL MPIC returns -ENOTSUPP, and in another case it returns 0... Shouldn't it be consistent? Also returning a negative number as a u32 is not very nice. -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Tuesday, July 02, 2013 7:59 AM > To: Jia Hongtao-B38951 > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; > galak@kernel.crashing.org; Li Yang-R58472; Jia Hongtao-B38951 > Subject: Re: [PATCH V6] powerpc/MPIC: Add get_version API both for > internal and external use > > On 07/01/2013 12:26:06 AM, Jia Hongtao wrote: > > From: Hongtao Jia <hongtao.jia@freescale.com> > > > > MPIC version is useful information for both mpic_alloc() and > > mpic_init(). > > The patch provide an API to get MPIC version for reusing the code. > > Also, some other IP block may need MPIC version for their own use. > > The API for external use is also provided. > > > > Signed-off-by: Jia Hongtao <hongtao.jia@freescale.com> > > Signed-off-by: Li Yang <leoli@freescale.com> > > --- > > V6: > > * Fix compile error on mpc83xx. > > I already applied V5 before realizing that it was this patch that you > updated, rather than the MSI patch that depends on it. There is no > compile error until the MSI patch that starts using this (which I haven't > yet applied, due to the error). Could you send a followup patch that > just adds the stub? Yep, sure. > > > +/* Get the version of primary MPIC */ #ifdef CONFIG_MPIC extern u32 > > +fsl_mpic_primary_get_version(void); > > +#else > > +static inline u32 fsl_mpic_primary_get_version(void) > > +{ > > + return -ENOTSUPP; > > +} > > +#endif > [snip] > > +static u32 fsl_mpic_get_version(struct mpic *mpic) { > > + u32 brr1; > > + > > + if (!(mpic->flags & MPIC_FSL)) > > + return 0; > > In one case, calling this without having an FSL MPIC returns -ENOTSUPP, > and in another case it returns 0... Shouldn't it be consistent? > > Also returning a negative number as a u32 is not very nice. > > -Scott My mistake. Will be fixed. Thanks. -Hongtao
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h index c0f9ef9..9d55671 100644 --- a/arch/powerpc/include/asm/mpic.h +++ b/arch/powerpc/include/asm/mpic.h @@ -393,6 +393,16 @@ struct mpic #define MPIC_REGSET_STANDARD MPIC_REGSET(0) /* Original MPIC */ #define MPIC_REGSET_TSI108 MPIC_REGSET(1) /* Tsi108/109 PIC */ +/* Get the version of primary MPIC */ +#ifdef CONFIG_MPIC +extern u32 fsl_mpic_primary_get_version(void); +#else +static inline u32 fsl_mpic_primary_get_version(void) +{ + return -ENOTSUPP; +} +#endif + /* Allocate the controller structure and setup the linux irq descs * for the range if interrupts passed in. No HW initialization is * actually performed. diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c index 3cc2f91..1a4e19c 100644 --- a/arch/powerpc/sysdev/mpic.c +++ b/arch/powerpc/sysdev/mpic.c @@ -1173,10 +1173,33 @@ static struct irq_domain_ops mpic_host_ops = { .xlate = mpic_host_xlate, }; +static u32 fsl_mpic_get_version(struct mpic *mpic) +{ + u32 brr1; + + if (!(mpic->flags & MPIC_FSL)) + return 0; + + brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, + MPIC_FSL_BRR1); + + return brr1 & MPIC_FSL_BRR1_VER; +} + /* * Exported functions */ +u32 fsl_mpic_primary_get_version(void) +{ + struct mpic *mpic = mpic_primary; + + if (mpic) + return fsl_mpic_get_version(mpic); + + return 0; +} + struct mpic * __init mpic_alloc(struct device_node *node, phys_addr_t phys_addr, unsigned int flags, @@ -1323,7 +1346,6 @@ struct mpic * __init mpic_alloc(struct device_node *node, mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000); if (mpic->flags & MPIC_FSL) { - u32 brr1; int ret; /* @@ -1334,9 +1356,7 @@ struct mpic * __init mpic_alloc(struct device_node *node, mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs, MPIC_CPU_THISBASE, 0x1000); - brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, - MPIC_FSL_BRR1); - fsl_version = brr1 & MPIC_FSL_BRR1_VER; + fsl_version = fsl_mpic_get_version(mpic); /* Error interrupt mask register (EIMR) is required for * handling individual device error interrupts. EIMR @@ -1526,9 +1546,7 @@ void __init mpic_init(struct mpic *mpic) mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf); if (mpic->flags & MPIC_FSL) { - u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, - MPIC_FSL_BRR1); - u32 version = brr1 & MPIC_FSL_BRR1_VER; + u32 version = fsl_mpic_get_version(mpic); /* * Timer group B is present at the latest in MPIC 3.1 (e.g.