public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Linux Kernel list <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@osdl.org>, Len Brown <len.brown@intel.com>,
	Deepak Saxena <dsaxena@plexity.net>
Subject: Re: [PATCH] Add device addition/removal notifier
Date: Thu, 19 Oct 2006 23:16:18 -0700	[thread overview]
Message-ID: <20061020061618.GA9432@kroah.com> (raw)
In-Reply-To: <1161322979.10524.143.camel@localhost.localdomain>

On Fri, Oct 20, 2006 at 03:42:59PM +1000, Benjamin Herrenschmidt wrote:
> 
> > Ok, as long as you all agree that this does change the behavior, it's
> > fine with me :)
> 
> I should probably split the patch in two: One that does that behaviour
> change (I already have an Acked-by: Len Brown for that one even :) and
> one adding that notifier.

That would be a good idea.

> > Ok, then perhaps you just want a bus specific callback for the devices
> > on that bus?  That would be much simpler and keep you from having to do
> > that mess with the different tests of bus type.
> >
> > Actually, that's the only thing that really makes sense here, now that I
> > think about it, the platform_notify doesn't really make any sense...
> 
> Well... people already use it and go check the bus types :)

That's wrong to do.

> Having a notifier queue per bus type is a bit harder though because bus
> types are generally allocated statically and thus we would need to find
> them all in the kernel to add a proper static initialisation for the
> notifier queue... bus_register() is not a good spot to do it because
> platform code might want to register for bus types before those bus
> types have been registered (it's not always easy to find a place to
> "hook" between a bus is registered and things get added to it).

Why would it be any different from what we have today with the
usb_register_notify() function?  You could change that to be:
	bus_register_notify(struct bus_type *, struct notifier_block *);
and then just pass in the proper bus type that you care about.

And yes, you will have to export those bus_type pointers, but that's ok,
some of them already are there.

> In fact, the whole bus type thing is a mess :) We can't easily register
> for bus types that are in modules. 

Like everything except PCI?  :)

> For example, if I want to use the notifier to catch USB devices in order
> to, for example, link them to firmware nodes, I'm lost if the USB
> subsystem is modular ... unless I use a global notifier and strcmp the
> bus type name in there.

Ick, I'd like to say that this is a pretty bad thing to do.  If you need
that, then just statically link the bus into your code, or make your
code a module and it will depend on the usb core.  I don't know...

Remember, we didn't add a type identifier to the struct device for a
reason, comparing the string of the bus type is not a good idea (for
USB, it will get you in trouble, because two different types of devices
can be on that bus, who's to say other busses will not also have that
issue?)

I think you need to re-evaluate exactly what you are needing to do
here...

thanks,

greg k-h

  reply	other threads:[~2006-10-20  6:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-20  1:55 [PATCH] Add device addition/removal notifier Benjamin Herrenschmidt
2006-10-20  3:26 ` Greg KH
2006-10-20  4:29   ` Benjamin Herrenschmidt
2006-10-20  4:44     ` Greg KH
2006-10-20  5:42       ` Benjamin Herrenschmidt
2006-10-20  6:16         ` Greg KH [this message]
2006-10-20  7:19           ` Benjamin Herrenschmidt
2006-10-20  7:35           ` Benjamin Herrenschmidt
2006-10-20 10:08           ` Paul Mackerras
2006-10-23  4:47           ` [PATCH] Call platform_notify_remove later Benjamin Herrenschmidt
  -- strict thread matches above, loose matches on Subject: below --
2006-10-19  7:56 [PATCH] Add device addition/removal notifier Benjamin Herrenschmidt
2006-10-19 15:55 ` Randy Dunlap
2006-10-20  1:53   ` Benjamin Herrenschmidt

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=20061020061618.GA9432@kroah.com \
    --to=greg@kroah.com \
    --cc=akpm@osdl.org \
    --cc=benh@kernel.crashing.org \
    --cc=dsaxena@plexity.net \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    /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