On Tue, 2012-03-13 at 09:23 +0100, Julia Lawall wrote:
> On Tue, 13 Mar 2012, Guan Xuetao wrote:
>
> > On Mon, 2012-03-12 at 06:27 +0100, Julia Lawall wrote:
> >> On Mon, 12 Mar 2012, Guan Xuetao wrote:
> >>
> >>> On Sun, 2012-03-11 at 20:36 +0100, Julia Lawall wrote:
> >>>> From: Julia Lawall <Julia.Lawall@xxxxxxx>
> >>>>
> >>>> Convert calls to free_irq so that the second argument is the same as the
> >>>> last argument of the corresponding call to request_irq, rather than the
> >>>> second to last. Without this property, free_irq does nothing.
> >>>>
> >>>> Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxx>
> >>>>
> >>>> ---
> >>>> arch/unicore32/kernel/dma.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/unicore32/kernel/dma.c b/arch/unicore32/kernel/dma.c
> >>>> index ae441bc..c813fec 100644
> >>>> --- a/arch/unicore32/kernel/dma.c
> >>>> +++ b/arch/unicore32/kernel/dma.c
> >>>> @@ -173,7 +173,7 @@ int __init puv3_init_dma(void)
> >>>> ret = request_irq(IRQ_DMAERR, dma_err_handler, 0, "DMAERR", NULL);
> >>>> if (ret) {
> >>>> printk(KERN_CRIT "Can't register IRQ for DMAERR\n");
> >>>> - free_irq(IRQ_DMA, "DMA");
> >>>> + free_irq(IRQ_DMA, NULL);
> >>>> return ret;
> >>>> }
> >>>>
> >>> Yeah, it's an obvious mistake. Thanks.
> >>> Because the dma device is just located inside PKUnity-3 SoC, and
> >>> request_irq() should always return 0, I prefer to remove this free_irq()
> >>> line.
> >>
> >> Remove the whole if test I guess. Is there a nce way to indicate that the
> >> return value is not needed (eg for the benefit of future bug finding
> >> rules)?
> >>
> >> julia
> > In this case, removing the line containing free_irq() is well enough,
> > because IRQ_DMA can work even when IRQ_DMAERR doesn't work. And we need
> > printk and error return value to get potential logical bug information.
>
> I'm not completely sure to understand. The point is that the first
> request_irq can never fail, so we don't need to clean up when the second
> one fails? Because the lack of cleaning up will not cause the first one
> to fail the next time? free_irq removes an action from a list and does a
> module_put. Are these operations both not needed?
>
> thanks,julia
puv3_init_dma() is called ONCE when initializing.
In logical, if request_irq(IRQ_DMAERR, *) failed, free_irq(IRQ_DMA, *)
is unnecessary, and dma device/driver can keep on working.
The patch could be:
ret = request_irq(IRQ_DMAERR, dma_err_handler, 0, "DMAERR", NULL);
if (ret) {
printk(KERN_CRIT "Can't register IRQ for DMAERR\n");
- free_irq(IRQ_DMA, "DMA");
return ret;
}
Thanks and Regards,
Guan Xuetao
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
