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

[Lubomir Rintel <lkundrak@v3.sk>]

On Wed, 2018-12-19 at 18:29 +0000, Marc Zyngier wrote:
> On 19/12/2018 17:28, 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>
> > 
> > ---
> > Changes since v2:
> > - Correct subsystem maintainers on Cc (irqchip)
> > 
> > Changes since v1:
> > - Adjusted wording & ack from Pavel
> > 
> >  drivers/irqchip/irq-mmp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> > index 25f32e1d7764..1ed38f9f1d0a 100644
> > --- a/drivers/irqchip/irq-mmp.c
> > +++ b/drivers/irqchip/irq-mmp.c
> > @@ -190,7 +190,7 @@ 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	= 0x60,
> 
> You seem to have identified that ICU_INT_ROUTE_PJ4_IRQ and
> ICU_INT_ROUTE_PJ4_FIQ bits are the only ones to be touched. So why don't
> you use these constants? This number soup is quite unhealthy.

Yeah, but those #defines live in mach-mmp, so some moving would be
necessary. If you indeed prefer that then I can follow up with a patch
that does that.

> It'd be good to Cc some of the folks who initially wrote this code
> (Haojian Zhuang, Eric Miao -- assuming they are still around) and get
> some testing on a non OLPC platform, just to make sure there is no
> regression due to this. I have the nagging feeling that this could be a
> platform specific thing rather than a universal setting.

They've been Cc'd on previous spins of the patch (and tens of other
mmp-related patches that were in circulation lately), but they never
returned a response. It is safe to assume they're AWOL.

> 
> Thanks,
> 
> 	M.

Thanks
Lubo