On 9 January 2015 at 20:54, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> On Fri, Jan 09, 2015 at 06:49:12PM +0530, Jassi Brar wrote:
>> On 9 January 2015 at 18:21, Russell King - ARM Linux
>> <linux@xxxxxxxxxxxxxxxx> wrote:
>> > On Fri, Jan 09, 2015 at 07:28:09PM +0800, Vincent Yang wrote:
>> >> +static irqreturn_t mhu_rx_interrupt(int irq, void *p)
>> >> +{
>> >> + struct mbox_chan *chan = p;
>> >> + struct mhu_link *mlink = (struct mhu_link *)chan->con_priv;
>> >> + u32 val;
>> >> +
>> >> + pr_debug("%s:%d\n", __func__, __LINE__);
>> >> + val = readl_relaxed(mlink->rx_reg + INTR_STAT_OFS);
>> >> + mbox_chan_received_data(chan, (void *)val);
>> >> +
>> >> + writel_relaxed(val, mlink->rx_reg + INTR_CLR_OFS);
>> >> +
>> >> + return IRQ_HANDLED;
>> >
>> > What if 'val' was zero - is the interrupt still "handled" ?
>> >
>> This irq shouldn't fire unless RX_STAT register has some non-zero value.
>
> You claim this interrupt handler using IRQF_SHARED - what if another user
> of this interrupt gets stuck? Your handler above will prevent the kernel
> recovering as it will think that you are validly processing the stuck
> interrupt each time.
>
> If it isn't shared, then don't use IRQF_SHARED.
>
> Either way, it is good practice to return IRQ_NONE if there's no work to
> be done.
>
OK, will do.
thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
