Re: [PATCH 26/47] Driver core: add groups support to struct device

Dmitry Torokhov
2006-09-26T14:01:21Z

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?

> > 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.

>
> 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.

> 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.
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.

> 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. However I do not agree with your implementation.

> > 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?

> The patches that move the subsystems
> over will be discussed (and some already have, like networking), when
> they are ready. Right now most of that work is being done by Kay and
> myself as a proof of concept to make sure that we can do this properly
> and that userspace can handle it well.
>
> thanks,
>
> greg k-h
>

--
Dmitry