Re: [PATCH 1/3] pinctrl: msm: Really mask level interrupts to prevent latching

[Doug Anderson]

Hi,

On Mon, Jun 18, 2018 at 4:28 PM, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> Quoting Doug Anderson (2018-06-18 15:43:06)
>>
>> On Mon, Jun 18, 2018 at 1:52 PM, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>>
>> > +        */
>> > +       if (irqd_get_trigger_type(d) & IRQ_TYPE_LEVEL_MASK) {
>> > +               val &= ~BIT(g->intr_raw_status_bit);
>> > +               writel(val, pctrl->regs + g->intr_cfg_reg);
>>
>> Do you know if it's important to do a 2nd write here, or could this be
>> combined with the next writel()?
>
> I haven't tried combining the writes. It felt safer to keep them split
> up so that both bits don't toggle at the same time, but I don't know if
> it actually matters.

Maybe I'm a glutton for punishment, but I'd say go for it, unless
someone from Qualcomm says "no way".

In the very least in the "unmask" case it seems pretty safe.  IMHO if
re-enabling the "raw" status caused a glitch we'd already be hitting
problems.  Specifically the glitch would end up getting latched
(whee!) and then we'd unmask and see the glitch anyway.

...and actually for the "mask" case it seems like you've written it
the less-safe way anyway.  We know that masking can't cause some sort
of glitch (since that's the old code), but I guess we don't know
whether disabling the "raw" status could cause a glitch.  To be the
absolutely safest you'd do the new disable of the "raw" status _after_
the old masking.  ...but as per above I'd just go whole hog and
combine them.  :-P

As with everything I write, feel free to tell me I'm being stupid and
I'll try to shut up.  ;-)


-Doug