mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Grant Likely <grant.likely@secretlab.ca>
To: Maciej Szmigiero <mhej@o2.pl>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	Anton Vorontsov <avorontsov@ru.mvista.com>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Arnd Bergmann <arnd@arndb.de>, Jonathan Cameron <jic23@cam.ac.uk>,
	Ben Nizette <bn@niasdigital.com>
Subject: Re: [GPIO]implement sleeping GPIO chip removal
Date: Wed, 10 Nov 2010 15:14:12 -0700	[thread overview]
Message-ID: <20101110221412.GB7063@angua.secretlab.ca> (raw)
In-Reply-To: <4CDB0834.4080101@o2.pl>

On Wed, Nov 10, 2010 at 10:01:40PM +0100, Maciej Szmigiero wrote:
> W dniu 10.11.2010 21:42, Thomas Gleixner pisze:
> > On Wed, 10 Nov 2010, Maciej Szmigiero wrote:
> >> W dniu 10.11.2010 10:49, Thomas Gleixner pisze:
> >>> Maybe because it open codes a sloppy refcounting with a loop and magic
> >>> sleeps instead of converting the code to kobjects and proper
> >>> refcounting ?
> >>>
> >>
> >> The only way to do GPIO chip removal in the current code is to busy-loop.
> >> "Sloppy" (as you called it) waiting is still more CPU-friendly than looping 
> >> in hope that somebody will finally release the chip.
> >> If you would like to implement it as kobject then go ahead and post the code
> >> so it can be used in drivers.
> > 
> > Wait a moment. You are getting something backwards here.
> > 
> > Fact is that the current code is not designed for easy hotunplugging
> > and therefor requires looping.
> > 
> > So _you_ propose a work-around to replace the busy-loop by a sleeping
> > loop with "hope that ....". Hope is the least thing what counts in
> > programming.
> > 
> > Now a reviewer tells you that your idea of replacing the busy-loop by
> > a sleeping in hope loop is flawed, because it does not solve the
> > underlying design problem of the GPIO code. And you get a suggestion
> > how to solve it correctly.
> > 
> > Now you go and request from that reviewer to implement that? That's
> > not how it works.
> > 
> > You sent a flawed patch in the first place and people try to tell you
> > how to do it right. Then it's on you to either go and do it right or
> > at least ask politely for help and pointers.
> > 
> > Thanks,
> > 
> > 	tglx
> > 
> 
> You misunderstood me.
> By "looping in hope that somebody will finally release the chip" I meant the only
> real way to handle a GPIO chip unplugging in the current kernel.
> Which is way worse that preventing new requests, then waiting for existing one to be released.
> And this is exactly what my patch does.
> 
> I understand that it could be simplified by removing redundant code (as Grant Likely had suggested before), and
> moving it to completion interface instead of manipulating a task structure directly, but this doesn't mean
> that the whole GPIO code has to be rewritten just to add one functionality.

BTW, switching to using a kobject != rewriting the whole GPIO code.
kobjects are not all that scary and the biggest impact is adding
kobject get/put operation on the gpio get/release apis.  I don't
expect it to end up being overly complex.

The Documentation/kobject.txt file should offer some insight.

g.

  parent reply	other threads:[~2010-11-10 22:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-07 18:30 Maciej Szmigiero
2010-11-10  5:09 ` Grant Likely
2010-11-10  9:49   ` Thomas Gleixner
2010-11-10 15:28   ` Maciej Szmigiero
2010-11-10 20:42     ` Thomas Gleixner
2010-11-10 21:01       ` Maciej Szmigiero
2010-11-10 21:07         ` Thomas Gleixner
2010-11-10 21:15           ` Grant Likely
2010-11-10 22:45             ` Greg KH
2010-11-10 22:47               ` Thomas Gleixner
2010-11-10 23:15               ` Paul Mundt
2010-11-10 22:14         ` Grant Likely [this message]
2010-11-12 20:46           ` Maciej Szmigiero

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=20101110221412.GB7063@angua.secretlab.ca \
    --to=grant.likely@secretlab.ca \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=avorontsov@ru.mvista.com \
    --cc=bn@niasdigital.com \
    --cc=gregkh@suse.de \
    --cc=jic23@cam.ac.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhej@o2.pl \
    --cc=tglx@linutronix.de \
    --cc=u.kleine-koenig@pengutronix.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®