Re: [PATCH 1/3] staging: comedi: amplc_dio200_common: prevent extra free_irq()

[Ian Abbott]

On 2014-07-27 19:45, Greg Kroah-Hartman wrote:
> On Fri, Jul 25, 2014 at 06:23:10PM +0000, Hartley Sweeten wrote:
> > On Friday, July 25, 2014 10:07 AM, Ian Abbott wrote:
> > > `dio200_detach()` in "amplc_dio200.c" calls
> > > `amplc_dio200_common_detach()` in "amplc_dio200_common.c", followed by
> > > `comedi_legacy_detach()` in "../drivers.c".  Both of those functions
> > > call `free_irq()` if `dev->irq` is non-zero.  The second call produces a
> > > warning message because the handler has already been freed.  Prevent
> > > that by setting `dev->irq = 0` in `amplc_dio200_common_detach()`.
> > >
> > > Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> > > ---
> > >   drivers/staging/comedi/drivers/amplc_dio200_common.c | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/comedi/drivers/amplc_dio200_common.c b/drivers/staging/comedi/drivers/amplc_dio200_common.c
> > > index 78700e8..3592e58 100644
> > > --- a/drivers/staging/comedi/drivers/amplc_dio200_common.c
> > > +++ b/drivers/staging/comedi/drivers/amplc_dio200_common.c
> > > @@ -1202,8 +1202,10 @@ void amplc_dio200_common_detach(struct comedi_device *dev)
> > >
> > >   	if (!thisboard || !devpriv)
> > >   		return;
> > > -	if (dev->irq)
> > > +	if (dev->irq) {
> > >   		free_irq(dev->irq, dev);
> > > +		dev->irq = 0;
> > > +	}
> > >   }
> > >   EXPORT_SYMBOL_GPL(amplc_dio200_common_detach);
> >
> > Ian,
> >
> > I have already gave a Reviewed-by signoff for this series.
> >
> > After looking over the code I think a cleaner solution would be to:
> >
> > 1) Use comedi_legacy_detach() directly for the (*detach) in the
> >     legacy ISA driver.
> > 2) Move the code from amplc_dio200_common_detach() into the
> >     (*detach) function for the PCI driver.
> > 3) Remove the exported function amplc_dio200_common_detach().
>
> So should I not apply these patches?
>
> confused,
>
> greg k-h

It fixes a bug (harmless, apart from a kernel warning), and does no 
harm, so might as well apply them.  Hartley's suggestion can be done later.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/