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

[Lubomir Rintel]

Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> On 19/12/2018 18:37, Lubomir Rintel wrote:
>> 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@xxxxx>
>>>> Acked-by: Pavel Machek <pavel@xxxxxx>
>>>>
>>>> ---
>>>> 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.
> 
> Given that nothing in the tree uses these #defines at all, they can be
> swiftly moved in one single go.

Will do.

>>> 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.
> 
> OK, so what else in the known universe uses the same SoC and runs
> mainline? This really needs wider testing if we can't get information
> from the MV folks.

While I can't possibly know for sure, I think it's fairly sure there are no
users beyond OLPC. The only MMP2 board that would run this was "Brownstone"
(that can't be obtained anymore) and the whole mmp2-dt support was
unbootable until recently without anyone noticing.

I think we're safe here. If anyone with a Brownstone board running mainline
exists (unlikely), linux-next + a few RCs are going to give them a good
chance to test this,

> 
> Thanks,
> 
> 	M.

Cheers,
Lubo