From: Greg KH <greg@kroah.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>,
linux-kernel@vger.kernel.org, Linus Torvalds <torvalds@osdl.org>,
Andrew Morton <akpm@osdl.org>
Subject: Re: [PATCH 26/47] Driver core: add groups support to struct device
Date: Tue, 26 Sep 2006 07:23:40 -0700 [thread overview]
Message-ID: <20060926142340.GA11999@kroah.com> (raw)
In-Reply-To: <d120d5000609260701q65039221rac64d043a5b55df9@mail.gmail.com>
On Tue, Sep 26, 2006 at 10:01:06AM -0400, Dmitry Torokhov wrote:
> On 9/26/06, Greg KH <greg@kroah.com> wrote:
> >On Tue, Sep 26, 2006 at 09:20:17AM -0400, Dmitry Torokhov wrote:
> >> On 9/26/06, Greg KH <greg@kroah.com> wrote:
> >> >From: Greg Kroah-Hartman <gregkh@suse.de>
> >> >
> >> >This is needed for the network class devices in order to be able to
> >> >convert over to use struct device.
> >> >
> >>
> >> Greg,
> >>
> >> You keep pushing out patches that merge class devices and standard
> >> devices but you still have not shown the usefullness of this process.
> >
> >I have not? This has been discussed before.
> >
>
> Care to send me a pointer?
Ugh, somewhere back in the lkml archives, sorry for not being more
specific...
> >> Why do you feel the need to change internal kernel structures
> >> (ever-expanding struct device to accomodate everything that is in
> >> struct class_device) when it should be possible to simply adjust sysfs
> >> representation of the kernel tree (moving class devices into
> >> /sys/device/.. part of the tree) to udev's liking and leave the rest
> >> of the kernel alone. You have seen the patch, only minor changes in
> >> driver/base/class.c are needed to accomplish the move.
> >
> >Think about suspend. We want a single device tree so that the class
> >gets called when a device is about to be suspended so that it could shut
> >down the network queue in a common way, before the physical device is
> >called.
>
> Why can't the device itself manage it? If you want to stub out the
> common parts just create a function like netdev_suspend and call it at
> appropriate time.
Because you would then need to add that function call to _every_ network
device driver. This way, we do not need to do that as the class gets
called in the proper place before the device driver does.
In short, it makes it much simpler for the device driver writer, as it
is one less thing for them to get wrong (you can implement it once in
the input core, and have it work for all input devices, no need to touch
every input driver too.)
> >It's also needed if we want to have a single device tree in general.
> >class_device was the wrong thing and is really just a duplicate of
> >struct device in the first place (the driver core code implementing it
> >is pretty much just a cut and paste job.)
>
> They complement each other. They are different and need different
> methods to operate.
Not they are not. They really are just the same thing.
> > The fact that we were
> >arbritrary marking it different has caused problems (look at the mess
> >that input causes to the class_device code, that's just not nice).
> >
>
> The only mess is that you refused to deepen the classification (i.e.
> have sub-classes). If input could be a parent class and
> mice/event/js/ts would grow from it it won't be such a mess.
But that is what we are now allowing you to do with devices. The whole
sub-class stuff was tried and failed. But in the end, it would almost
work with input devices, but then why not just make it a real device, so
you can use whatever heirachy you want to. I would think that you would
welcome this change.
> Alternatively we could go with input vs input_intf classes if flat
> classification is a must. Anyway, I don't think we want to break udev
> again.
Flat classification is not a must at all, and with these changes, you
don't need it. As an example, here's what the input tree looks like
when changed over:
$ tree /sys/class/input/
/sys/class/input/
|-- event0 -> ../../devices/platform/pcspkr/input0/event0
|-- event1 -> ../../devices/platform/i8042/serio4/input1/event1
|-- event2 -> ../../devices/platform/i8042/serio3/input2/event2
|-- input0 -> ../../devices/platform/pcspkr/input0
|-- input1 -> ../../devices/platform/i8042/serio4/input1
|-- input2 -> ../../devices/platform/i8042/serio3/input2
|-- mice -> ../../devices/virtual/input/mice
|-- mouse0 -> ../../devices/platform/i8042/serio3/input2/mouse0
`-- ts0 -> ../../devices/platform/i8042/serio3/input2/ts0
If you want, you can move any of these input devices anywhere in the
heirchay that you wish to do so. A symlink will be automatically
created in /sys/class/input so that userspace tools like udev can find
all of the input devices (which is something that is needed), but there
is no more rigid heirachy being imposed on any one. You are free to
move them at will, and no userspace tools will break as they will be
following the symlink instead.
> >Kay also has a long list of the reasons why, I think he's posted it here
> >before. Kay, care to send that list again?
> >
>
> Kay did send it and I agree with all his reasons as to why we need the
> move.
Great, they why are you objecting to these driver core patches?
> However I do not agree with your implementation.
Which implementation? The one I did for the class subsystem? Ok,
that's fine, your patch is still in my queue to look at, I'm not
ignoring it at all (had a bunch of "real life" work to get through this
last week and weekend, sorry, am still catching up.)
Don't worry, I'm not going to be pushing any input subsystem changes
wihout going through you first :)
These driver core patches are merely the needed functions for us to do
those input and other subsystem changes, they should not affect anything
of yours at all.
> >> I really disappointed that there was no discussion/review of the
> >> implementation at all.
> >
> >There has not been any real implementation yet, only a few patches added
> >to the core that add a few extra functionality to struct device to allow
> >class_device to move that way.
>
> If there was no real discussion why you requesting these changes to be
> pulled in the mainline?
I didn't think these patches were controversial at all. They have been
in -mm for a few months now and work just fine.
Is there anything specfic in these patches that you object to? Becides
the virtual thing (I tried it with a flat /sys/devices/virtual/ tree,
and it was a mess, I like the extra directory for classification, but in
the end, it doesn't matter, we can change it with no problem, as no
userspace tool will break if you move devices around the /sys/devices/
tree.)
thanks,
greg k-h
next prev parent reply other threads:[~2006-09-26 14:23 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-09-26 5:37 [GIT PATCH] Driver Core patches for 2.6.18 Greg KH
2006-09-26 5:37 ` [PATCH 1/47] Documentation/ABI: devfs is not obsolete, but removed! Greg KH
2006-09-26 5:37 ` [PATCH 2/47] deprecate PHYSDEV* keys Greg KH
2006-09-26 5:37 ` [PATCH 3/47] class_device_create(): make fmt argument 'const char *' Greg KH
2006-09-26 5:37 ` [PATCH 4/47] device_create(): " Greg KH
2006-09-26 5:37 ` [PATCH 5/47] Driver core: add const to class_create Greg KH
2006-09-26 5:37 ` [PATCH 6/47] sysfs: Make poll behaviour consistent Greg KH
2006-09-26 5:37 ` [PATCH 7/47] Debugfs: kernel-doc fixes for debugfs Greg KH
2006-09-26 5:37 ` [PATCH 8/47] SYSFS: allow sysfs_create_link to create symlinks in the root of sysfs Greg KH
2006-09-26 5:37 ` [PATCH 9/47] Suspend infrastructure cleanup and extension Greg KH
2006-09-26 5:37 ` [PATCH 10/47] Suspend changes for PCI core Greg KH
2006-09-26 5:37 ` [PATCH 11/47] make suspend quieter Greg KH
2006-09-26 5:37 ` [PATCH 12/47] fix broken/dubious driver suspend() methods Greg KH
2006-09-26 5:37 ` [PATCH 13/47] PM: define PM_EVENT_PRETHAW Greg KH
2006-09-26 5:37 ` [PATCH 14/47] PM: PCI and IDE handle PM_EVENT_PRETHAW Greg KH
2006-09-26 5:37 ` [PATCH 15/47] PM: video drivers and PM_EVENT_PRETHAW Greg KH
2006-09-26 5:37 ` [PATCH 16/47] PM: USB HCDs use PM_EVENT_PRETHAW Greg KH
2006-09-26 5:37 ` [PATCH 17/47] PM: issue PM_EVENT_PRETHAW Greg KH
2006-09-26 5:37 ` [PATCH 18/47] updated Documentation/power/devices.txt Greg KH
2006-09-26 5:37 ` [PATCH 19/47] PM: update docs for writing .../power/state Greg KH
2006-09-26 5:37 ` [PATCH 20/47] PM: add kconfig option for deprecated .../power/state files Greg KH
2006-09-26 5:37 ` [PATCH 21/47] PM: schedule /sys/devices/.../power/state for removal Greg KH
2006-09-26 5:37 ` [PATCH 22/47] PM: no suspend_prepare() phase Greg KH
2006-09-26 5:37 ` [PATCH 23/47] PM: add /sys/power documentation to Documentation/ABI Greg KH
2006-09-26 5:37 ` [PATCH 24/47] PM: device_suspend/resume may sleep Greg KH
2006-09-26 5:37 ` [PATCH 25/47] PM: platform_bus and late_suspend/early_resume Greg KH
2006-09-26 5:37 ` [PATCH 26/47] Driver core: add groups support to struct device Greg KH
2006-09-26 5:37 ` [PATCH 27/47] Driver core: allow devices in classes to have no parent Greg KH
2006-09-26 5:37 ` [PATCH 28/47] Driver core: add ability for classes to handle devices properly Greg KH
2006-09-26 5:37 ` [PATCH 29/47] Driver core: add device_rename function Greg KH
2006-09-26 5:37 ` [PATCH 30/47] Driver core: create devices/virtual/ tree Greg KH
2006-09-26 5:37 ` [PATCH 31/47] Class: add support for class interfaces for devices Greg KH
2006-09-26 5:37 ` [PATCH 32/47] Driver core: add ability for devices to create and remove bin files Greg KH
2006-09-26 5:37 ` [PATCH 33/47] kobject: must_check fixes Greg KH
2006-09-26 5:37 ` [PATCH 34/47] sysfs_remove_bin_file: no return value, dump_stack on error Greg KH
2006-09-26 5:37 ` [PATCH 35/47] Driver core: fix comments in drivers/base/power/resume.c Greg KH
2006-09-26 5:37 ` [PATCH 36/47] Driver core: fixed add_bind_files() definition Greg KH
2006-09-26 5:37 ` [PATCH 37/47] add __must_check to device management code Greg KH
2006-09-26 5:37 ` [PATCH 38/47] add CONFIG_ENABLE_MUST_CHECK Greg KH
2006-09-26 5:37 ` [PATCH 39/47] v4l-dev2: handle __must_check Greg KH
2006-09-26 5:38 ` [PATCH 40/47] drivers/base: Platform notify needs to occur before drivers attach to the device Greg KH
2006-09-26 5:38 ` [PATCH 41/47] drivers/base: check errors Greg KH
2006-09-26 5:38 ` [PATCH 42/47] sysfs: add proper sysfs_init() prototype Greg KH
2006-09-26 5:38 ` [PATCH 43/47] Driver Core: add ability for drivers to do a threaded probe Greg KH
2006-09-26 5:38 ` [PATCH 44/47] PCI: enable driver multi-threaded probe Greg KH
2006-09-26 5:38 ` [PATCH 45/47] Driver core: Fix potential deadlock in driver core Greg KH
2006-09-26 5:38 ` [PATCH 46/47] Driver core: Remove unneeded routines from " Greg KH
2006-09-26 5:38 ` [PATCH 47/47] Driver core: Don't call put methods while holding a spinlock Greg KH
2006-09-27 18:51 ` [PATCH 44/47] PCI: enable driver multi-threaded probe Olaf Hering
2006-09-29 23:32 ` Greg KH
2006-09-30 6:07 ` Olaf Hering
2006-09-26 17:23 ` [PATCH 41/47] drivers/base: check errors Dmitry Torokhov
2006-09-27 4:33 ` Greg KH
2006-09-26 13:24 ` [PATCH 30/47] Driver core: create devices/virtual/ tree Dmitry Torokhov
2006-09-26 13:41 ` Greg KH
2006-09-26 13:51 ` Dmitry Torokhov
2006-09-26 14:26 ` Greg KH
2006-09-26 17:15 ` Dmitry Torokhov
2006-09-26 13:20 ` [PATCH 26/47] Driver core: add groups support to struct device Dmitry Torokhov
2006-09-26 13:46 ` Greg KH
2006-09-26 14:01 ` Dmitry Torokhov
2006-09-26 14:23 ` Greg KH [this message]
2006-09-26 17:10 ` Dmitry Torokhov
2006-09-27 14:40 ` Pavel Machek
2006-09-26 15:18 ` Marcel Holtmann
2006-09-26 12:34 ` [GIT PATCH] Driver Core patches for 2.6.18 Mike Galbraith
2006-09-26 20:39 ` Greg KH
2006-09-27 8:47 ` Mike Galbraith
2006-09-27 6:58 ` Rafael J. Wysocki
2006-09-27 10:48 ` Mike Galbraith
2006-09-27 13:03 ` Mike Galbraith
2006-09-27 11:42 ` Rafael J. Wysocki
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=20060926142340.GA11999@kroah.com \
--to=greg@kroah.com \
--cc=akpm@osdl.org \
--cc=dmitry.torokhov@gmail.com \
--cc=kay.sievers@vrfy.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@osdl.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