[PATCH v4] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable

[PATCH v4] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable

[Lubomir Rintel]

On an OLPC XO 1.75 machine, the "security processor" handles the GPIO 71
and 72 interrupts. Don't reset the "route to SP" bit (4).

I'm just assuming the bit 4 is the "route to SP" bit -- it fixes the
SP-based keyboard for me and <mach-mmp/regs-icu.h> defines
ICU_INT_ROUTE_SP_IRQ to be 1 << 4. When asked for a data sheet, Marvell
was not helpful.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Acked-by: Pavel Machek <pavel@ucw.cz>

---
I'm not removing the defines from regs-icu.h at the moment.
Of the MPP2 defines there, only MMP2_ICU_INT4_MASK ends up actually
being used, in one place. That said, the header servers mostly as
a documentation, because the documentation for the platform is lacking.

Changes since v3:
- Use #defined instead of integer literals

Changes since v2:
- Correct subsystem maintainers on Cc (irqchip)

Changes since v1:
- Adjusted wording & ack from Pavel
---
 drivers/irqchip/irq-mmp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
index 25f32e1d7764..65fa5a1d8310 100644
--- a/drivers/irqchip/irq-mmp.c
+++ b/drivers/irqchip/irq-mmp.c
@@ -34,6 +34,10 @@
 #define SEL_INT_PENDING		(1 << 6)
 #define SEL_INT_NUM_MASK	0x3f
 
+#define MMP2_ICU_INT_ROUTE_SP_IRQ	(1 << 4)
+#define MMP2_ICU_INT_ROUTE_PJ4_IRQ	(1 << 5)
+#define MMP2_ICU_INT_ROUTE_PJ4_FIQ	(1 << 6)
+
 struct icu_chip_data {
 	int			nr_irqs;
 	unsigned int		virq_base;
@@ -190,7 +194,8 @@ static const struct mmp_intc_conf mmp_conf = {
 static const struct mmp_intc_conf mmp2_conf = {
 	.conf_enable	= 0x20,
 	.conf_disable	= 0x0,
-	.conf_mask	= 0x7f,
+	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
+			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
 };
 
 static void __exception_irq_entry mmp_handle_irq(struct pt_regs *regs)
-- 
2.19.2

Re: [PATCH v4] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable

[Lubomir Rintel]

On Thu, 2018-12-20 at 22:55 +0100, Lubomir Rintel wrote:
> On an OLPC XO 1.75 machine, the "security processor" handles the GPIO 71
> and 72 interrupts. Don't reset the "route to SP" bit (4).
> 
> I'm just assuming the bit 4 is the "route to SP" bit -- it fixes the
> SP-based keyboard for me and <mach-mmp/regs-icu.h> defines
> ICU_INT_ROUTE_SP_IRQ to be 1 << 4. When asked for a data sheet, Marvell
> was not helpful.

Marc,

I'm wondering if there's any chance this be applied and ideally
submitted for 5.0?

This fixes the keyboard on an OLPC XO 1.75 laptop, and, with the MMP
platform fixes that went into rc1 it is the last major obstacle to the
XO 1.75 machine being generally useful.

Thank you
Lubo

> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> I'm not removing the defines from regs-icu.h at the moment.
> Of the MPP2 defines there, only MMP2_ICU_INT4_MASK ends up actually
> being used, in one place. That said, the header servers mostly as
> a documentation, because the documentation for the platform is lacking.
> 
> Changes since v3:
> - Use #defined instead of integer literals
> 
> Changes since v2:
> - Correct subsystem maintainers on Cc (irqchip)
> 
> Changes since v1:
> - Adjusted wording & ack from Pavel
> ---
>  drivers/irqchip/irq-mmp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> index 25f32e1d7764..65fa5a1d8310 100644
> --- a/drivers/irqchip/irq-mmp.c
> +++ b/drivers/irqchip/irq-mmp.c
> @@ -34,6 +34,10 @@
>  #define SEL_INT_PENDING		(1 << 6)
>  #define SEL_INT_NUM_MASK	0x3f
>  
> +#define MMP2_ICU_INT_ROUTE_SP_IRQ	(1 << 4)
> +#define MMP2_ICU_INT_ROUTE_PJ4_IRQ	(1 << 5)
> +#define MMP2_ICU_INT_ROUTE_PJ4_FIQ	(1 << 6)
> +
>  struct icu_chip_data {
>  	int			nr_irqs;
>  	unsigned int		virq_base;
> @@ -190,7 +194,8 @@ static const struct mmp_intc_conf mmp_conf = {
>  static const struct mmp_intc_conf mmp2_conf = {
>  	.conf_enable	= 0x20,
>  	.conf_disable	= 0x0,
> -	.conf_mask	= 0x7f,
> +	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
> +			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
>  };
>  
>  static void __exception_irq_entry mmp_handle_irq(struct pt_regs *regs)

Re: [PATCH v4] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable

[Thomas Gleixner]

On Thu, 20 Dec 2018, Lubomir Rintel wrote:

> On an OLPC XO 1.75 machine, the "security processor" handles the GPIO 71
> and 72 interrupts. Don't reset the "route to SP" bit (4).
> 
> I'm just assuming the bit 4 is the "route to SP" bit -- it fixes the
> SP-based keyboard for me and <mach-mmp/regs-icu.h> defines
> ICU_INT_ROUTE_SP_IRQ to be 1 << 4. When asked for a data sheet, Marvell
> was not helpful.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> I'm not removing the defines from regs-icu.h at the moment.
> Of the MPP2 defines there, only MMP2_ICU_INT4_MASK ends up actually
> being used, in one place. That said, the header servers mostly as
> a documentation, because the documentation for the platform is lacking.
> 
> Changes since v3:
> - Use #defined instead of integer literals
> 
> Changes since v2:
> - Correct subsystem maintainers on Cc (irqchip)
> 
> Changes since v1:
> - Adjusted wording & ack from Pavel
> ---
>  drivers/irqchip/irq-mmp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> index 25f32e1d7764..65fa5a1d8310 100644
> --- a/drivers/irqchip/irq-mmp.c
> +++ b/drivers/irqchip/irq-mmp.c
> @@ -34,6 +34,10 @@
>  #define SEL_INT_PENDING		(1 << 6)
>  #define SEL_INT_NUM_MASK	0x3f
>  
> +#define MMP2_ICU_INT_ROUTE_SP_IRQ	(1 << 4)

Where is this used?

> +#define MMP2_ICU_INT_ROUTE_PJ4_IRQ	(1 << 5)
> +#define MMP2_ICU_INT_ROUTE_PJ4_FIQ	(1 << 6)
> +
>  struct icu_chip_data {
>  	int			nr_irqs;
>  	unsigned int		virq_base;
> @@ -190,7 +194,8 @@ static const struct mmp_intc_conf mmp_conf = {
>  static const struct mmp_intc_conf mmp2_conf = {
>  	.conf_enable	= 0x20,
>  	.conf_disable	= 0x0,
> -	.conf_mask	= 0x7f,
> +	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
> +			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,

So the mask changes from 0x7f to 0x60, but the changelog does not mention
any of this. It only talks about bit 4 ....

Thanks,

	tglx

Re: [PATCH v4] irqchip/mmp: only touch the PJ4 & FIQ bits on enable/disable

[Lubomir Rintel]

On Tue, 2019-01-15 at 11:22 +0100, Thomas Gleixner wrote:
> On Thu, 20 Dec 2018, Lubomir Rintel wrote:
> 
> > On an OLPC XO 1.75 machine, the "security processor" handles the GPIO 71
> > and 72 interrupts. Don't reset the "route to SP" bit (4).
> > 
> > I'm just assuming the bit 4 is the "route to SP" bit -- it fixes the
> > SP-based keyboard for me and <mach-mmp/regs-icu.h> defines
> > ICU_INT_ROUTE_SP_IRQ to be 1 << 4. When asked for a data sheet, Marvell
> > was not helpful.
> > 
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > 
> > ---
> > I'm not removing the defines from regs-icu.h at the moment.
> > Of the MPP2 defines there, only MMP2_ICU_INT4_MASK ends up actually
> > being used, in one place. That said, the header servers mostly as
> > a documentation, because the documentation for the platform is lacking.
> > 
> > Changes since v3:
> > - Use #defined instead of integer literals
> > 
> > Changes since v2:
> > - Correct subsystem maintainers on Cc (irqchip)
> > 
> > Changes since v1:
> > - Adjusted wording & ack from Pavel
> > ---
> >  drivers/irqchip/irq-mmp.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> > index 25f32e1d7764..65fa5a1d8310 100644
> > --- a/drivers/irqchip/irq-mmp.c
> > +++ b/drivers/irqchip/irq-mmp.c
> > @@ -34,6 +34,10 @@
> >  #define SEL_INT_PENDING		(1 << 6)
> >  #define SEL_INT_NUM_MASK	0x3f
> >  
> > +#define MMP2_ICU_INT_ROUTE_SP_IRQ	(1 << 4)
> 
> Where is this used?

Nowhere really. I can remove it I guess.

> > +#define MMP2_ICU_INT_ROUTE_PJ4_IRQ	(1 << 5)
> > +#define MMP2_ICU_INT_ROUTE_PJ4_FIQ	(1 << 6)
> > +
> >  struct icu_chip_data {
> >  	int			nr_irqs;
> >  	unsigned int		virq_base;
> > @@ -190,7 +194,8 @@ static const struct mmp_intc_conf mmp_conf = {
> >  static const struct mmp_intc_conf mmp2_conf = {
> >  	.conf_enable	= 0x20,
> >  	.conf_disable	= 0x0,
> > -	.conf_mask	= 0x7f,
> > +	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
> > +			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
> 
> So the mask changes from 0x7f to 0x60, but the changelog does not mention
> any of this. It only talks about bit 4 ....

The subject line was supposed to say that.

The point was in general not to touch the bit whose meanings are not
interesting or not known. I'll try to figure out better wording (and
I'm thankful for suggestions).

> 
> Thanks,
> 
> 	tglx

Thank you
Lubo