Re: [PATCH 5/7] gpio: brcmstb: enable masking of interrupts when changing type

From: Gregory Fong
Date: Tue Oct 03 2017 - 23:16:31 EST


On Tue, Oct 3, 2017 at 7:22 PM, Doug Berger <opendmb@xxxxxxxxx> wrote:
> On 10/03/2017 07:10 PM, Gregory Fong wrote:
>> Hi Doug,
>>
>> On Fri, Sep 29, 2017 at 8:40 PM, Doug Berger <opendmb@xxxxxxxxx> wrote:
>>> Mask the GPIO interrupt while its type is being changed, just in case
>>> it can prevent a spurious interrupt.
>>
>> "Just in case"? I don't have access to hardware documentation for
>> this anymore, but I'd expect to some stronger claim that the hardware
>> actually requires masking before changing the trigger type. If you
>> can quote documentation for this or explain an actual problem seen,
>> that would be good.
>
> Well spotted ;). This was a protectionist change added at the request
> of a user of the driver. I believe that it is superfluous and I suppose
> that belief leaked through in the language of my comment.
>
> In actuality, the GPIO APIs don't make provision for clearing GPIO
> interrupts before enabling them so this masking is really only a
> deferral of a spurious interrupt if one is triggered by changes in the
> hardware programming.

Indeed. If the hardware is behaving in unexpected ways that would be
a whole different kind of problem.

>
> I can strike this from the upstream submission and save a couple lines
> of source code (after IRQCHIP_MASK_ON_SUSPEND goes away) if you prefer.
>

Yes, please remove.

Thanks,
Gregory