LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Tilman Schmidt <tilman@imap.cc>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Karsten Keil <kkeil@suse.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	i4ldeveloper@listserv.isdn4linux.de,
	linux-serial@vger.kernel.org, Hansjoerg Lipp <hjlipp@web.de>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH] drivers/isdn/gigaset: new M101 driver
Date: Mon, 12 Feb 2007 10:47:12 -0800	[thread overview]
Message-ID: <20070212184712.GC32607@kroah.com> (raw)
In-Reply-To: <45C52848.50705@imap.cc>

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

  reply	other threads:[~2007-02-12 18:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200702012112.l11LCOO4016557@lx1.pxnet.com>
2007-02-02  1:13 ` Andrew Morton
2007-02-03 16:09   ` Greg KH
2007-02-04  0:26     ` Tilman Schmidt
2007-02-12 18:47       ` Greg KH [this message]
2007-02-12 23:49         ` Tilman Schmidt
2007-02-04  1:32   ` Tilman Schmidt
2007-02-04  1:56     ` Andrew Morton
2007-02-05  1:42       ` Tilman Schmidt
2007-02-05  4:58         ` Andrew Morton
2007-02-05 12:29           ` Tilman Schmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070212184712.GC32607@kroah.com \
    --to=greg@kroah.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=hjlipp@web.de \
    --cc=i4ldeveloper@listserv.isdn4linux.de \
    --cc=kkeil@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=tilman@imap.cc \
    --subject='Re: [PATCH] drivers/isdn/gigaset: new M101 driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).