[PATCH] powerpc/mpic: Cleanup irq vector accounting

[PATCH] powerpc/mpic: Cleanup irq vector accounting

[Bharat Bhushan]

Available vector space accounts ipis and timer interrupts
while spurious vector was not accounted. Also later
mpic_setup_error_int() escape one more vector, seemingly it
assumes one spurious vector.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
---
 arch/powerpc/sysdev/fsl_mpic_err.c | 2 +-
 arch/powerpc/sysdev/mpic.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_mpic_err.c b/arch/powerpc/sysdev/fsl_mpic_err.c
index 488ec45..2a98837 100644
--- a/arch/powerpc/sysdev/fsl_mpic_err.c
+++ b/arch/powerpc/sysdev/fsl_mpic_err.c
@@ -76,7 +76,7 @@ int mpic_setup_error_int(struct mpic *mpic, int intvec)
 	mpic->flags |= MPIC_FSL_HAS_EIMR;
 	/* allocate interrupt vectors for error interrupts */
 	for (i = MPIC_MAX_ERR - 1; i >= 0; i--)
-		mpic->err_int_vecs[i] = --intvec;
+		mpic->err_int_vecs[i] = intvec--;
 
 	return 0;
 }
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 1d4e0ef6..e098d1e 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1380,12 +1380,12 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 		 * global vector number space, as in case of ipis
 		 * and timer interrupts.
 		 *
-		 * Available vector space = intvec_top - 12, where 12
+		 * Available vector space = intvec_top - 13, where 13
 		 * is the number of vectors which have been consumed by
-		 * ipis and timer interrupts.
+		 * ipis, timer interrupts and spurious.
 		 */
 		if (fsl_version >= 0x401) {
-			ret = mpic_setup_error_int(mpic, intvec_top - 12);
+			ret = mpic_setup_error_int(mpic, intvec_top - 13);
 			if (ret)
 				return NULL;
 		}
-- 
1.9.3

Re: [PATCH] powerpc/mpic: Cleanup irq vector accounting

[Michael Ellerman]

Bharat Bhushan <Bharat.Bhushan@nxp.com> writes:

> Available vector space accounts ipis and timer interrupts
> while spurious vector was not accounted.

OK. What is the symptom of that? Nothing? Total system crash?

Looks like this can be tagged:

Fixes: 0a4081641d72 ("powerpc/mpic: FSL MPIC error interrupt support.")

Which added the code that uses "12".

> Also later
> mpic_setup_error_int() escape one more vector, seemingly it
> assumes one spurious vector.

Ah right, I get it now.

So there is no bug. It's just a disagreement about whether the "intvec"
argument to mpic_setup_error_int() indicates the first number that's
free to use or the last number that has been allocated.

Right?

cheers

> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> ---
>  arch/powerpc/sysdev/fsl_mpic_err.c | 2 +-
>  arch/powerpc/sysdev/mpic.c         | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/fsl_mpic_err.c b/arch/powerpc/sysdev/fsl_mpic_err.c
> index 488ec45..2a98837 100644
> --- a/arch/powerpc/sysdev/fsl_mpic_err.c
> +++ b/arch/powerpc/sysdev/fsl_mpic_err.c
> @@ -76,7 +76,7 @@ int mpic_setup_error_int(struct mpic *mpic, int intvec)
>  	mpic->flags |= MPIC_FSL_HAS_EIMR;
>  	/* allocate interrupt vectors for error interrupts */
>  	for (i = MPIC_MAX_ERR - 1; i >= 0; i--)
> -		mpic->err_int_vecs[i] = --intvec;
> +		mpic->err_int_vecs[i] = intvec--;
>  
>  	return 0;
>  }
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 1d4e0ef6..e098d1e 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1380,12 +1380,12 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>  		 * global vector number space, as in case of ipis
>  		 * and timer interrupts.
>  		 *
> -		 * Available vector space = intvec_top - 12, where 12
> +		 * Available vector space = intvec_top - 13, where 13
>  		 * is the number of vectors which have been consumed by
> -		 * ipis and timer interrupts.
> +		 * ipis, timer interrupts and spurious.
>  		 */
>  		if (fsl_version >= 0x401) {
> -			ret = mpic_setup_error_int(mpic, intvec_top - 12);
> +			ret = mpic_setup_error_int(mpic, intvec_top - 13);
>  			if (ret)
>  				return NULL;
>  		}
> -- 
> 1.9.3

RE: [PATCH] powerpc/mpic: Cleanup irq vector accounting

[Bharat Bhushan]

> -----Original Message-----
> From: Michael Ellerman [mailto:mpe@ellerman.id.au]
> Sent: Wednesday, July 4, 2018 6:57 PM
> To: Bharat Bhushan <bharat.bhushan@nxp.com>;
> benh@kernel.crashing.org; paulus@samba.org; robh@kernel.org;
> geoff@infradead.org; tyreld@linux.vnet.ibm.com; linuxppc-
> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Cc: Bharat Bhushan <bharat.bhushan@nxp.com>
> Subject: Re: [PATCH] powerpc/mpic: Cleanup irq vector accounting
> 
> Bharat Bhushan <Bharat.Bhushan@nxp.com> writes:
> 
> > Available vector space accounts ipis and timer interrupts while
> > spurious vector was not accounted.
> 
> OK. What is the symptom of that? Nothing? Total system crash?
> 
> Looks like this can be tagged:
> 
> Fixes: 0a4081641d72 ("powerpc/mpic: FSL MPIC error interrupt support.")
> 
> Which added the code that uses "12".
> 
> > Also later
> > mpic_setup_error_int() escape one more vector, seemingly it assumes
> > one spurious vector.
> 
> Ah right, I get it now.
> 
> So there is no bug. It's just a disagreement about whether the "intvec"
> argument to mpic_setup_error_int() indicates the first number that's free to
> use or the last number that has been allocated.
> 
> Right?

Yes, it is not any bug fix. This is minor cleanup where passing rather than passing "last intvec used" to "intvec to be used" in mpic_setup_error_int().

Thanks
-Bharat

> 
> cheers
> 
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> > ---
> >  arch/powerpc/sysdev/fsl_mpic_err.c | 2 +-
> >  arch/powerpc/sysdev/mpic.c         | 6 +++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/sysdev/fsl_mpic_err.c
> > b/arch/powerpc/sysdev/fsl_mpic_err.c
> > index 488ec45..2a98837 100644
> > --- a/arch/powerpc/sysdev/fsl_mpic_err.c
> > +++ b/arch/powerpc/sysdev/fsl_mpic_err.c
> > @@ -76,7 +76,7 @@ int mpic_setup_error_int(struct mpic *mpic, int
> intvec)
> >  	mpic->flags |= MPIC_FSL_HAS_EIMR;
> >  	/* allocate interrupt vectors for error interrupts */
> >  	for (i = MPIC_MAX_ERR - 1; i >= 0; i--)
> > -		mpic->err_int_vecs[i] = --intvec;
> > +		mpic->err_int_vecs[i] = intvec--;
> >
> >  	return 0;
> >  }
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 1d4e0ef6..e098d1e 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -1380,12 +1380,12 @@ struct mpic * __init mpic_alloc(struct
> device_node *node,
> >  		 * global vector number space, as in case of ipis
> >  		 * and timer interrupts.
> >  		 *
> > -		 * Available vector space = intvec_top - 12, where 12
> > +		 * Available vector space = intvec_top - 13, where 13
> >  		 * is the number of vectors which have been consumed by
> > -		 * ipis and timer interrupts.
> > +		 * ipis, timer interrupts and spurious.
> >  		 */
> >  		if (fsl_version >= 0x401) {
> > -			ret = mpic_setup_error_int(mpic, intvec_top - 12);
> > +			ret = mpic_setup_error_int(mpic, intvec_top - 13);
> >  			if (ret)
> >  				return NULL;
> >  		}
> > --
> > 1.9.3

Re: powerpc/mpic: Cleanup irq vector accounting

[Michael Ellerman]

On Fri, 2018-06-29 at 10:24:32 UTC, Bharat Bhushan wrote:
> Available vector space accounts ipis and timer interrupts
> while spurious vector was not accounted. Also later
> mpic_setup_error_int() escape one more vector, seemingly it
> assumes one spurious vector.
> 
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/fca7bf946e31688edce446fdaa571c

cheers