Re: [PATCH] drivers/isdn/gigaset: new M101 driver

[Greg KH <greg@kroah.com>]

On Sun, Feb 04, 2007 at 01:26:48AM +0100, Tilman Schmidt wrote:
> Am 03.02.2007 17:09 schrieb Greg KH:
> > On Thu, Feb 01, 2007 at 05:13:45PM -0800, Andrew Morton wrote:
> >>> +/* dummy to shut up framework warning */
> >>> +static void gigaset_device_release(struct device *dev)
> >>> +{
> >>> +	//FIXME anything to do? cf. platform_device_release()
> >>> +}
> >> Ask Greg ;)
> > 
> > Oh come on people.  Don't provide an empty function because the kernel
> > is complaining that you don't provide a release function.  Yes it will
> > shut the kernel up but did you ever think _why_ the kernel was
> > complaining?
> 
> Actually, I did. Just guess how that FIXME came to be.
> Hint: the kernel shuts up just as well without it.

Sure, but only because I can't do a test for a function pointer that
points to a function that does nothing :(

> > You need to free your memory here, don't just provide an empty function,
> > that shows the driver is broken.
> 
> Call me silly and incapable of thinking, but to me it's far from
> clear _what_ memory I am supposed to free there. Certainly neither
> memory I will still be needing after platform_device_unregister()
> nor memory that's being taken care of elsewhere. Between the two,
> in my case there's nothing left, so the release function is empty.
> If that shows my driver is broken, please explain in what way.

The memory of the platform device itself needs to be freed here,
otherwise, to do it earlier would cause race conditions and oopses.
Look at how the other platform drivers do things.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/