Re: Git backlight subsystem tree

Greg KH
2007-02-09T00:24:37Z

On Thu, Feb 08, 2007 at 11:50:23PM +0000, Richard Purdie wrote:
> Hi Greg,
>
> On Thu, 2007-02-08 at 13:23 -0800, Greg KH wrote:
> > On Thu, Feb 08, 2007 at 06:32:02PM +0000, James Simmons wrote:
> > > I CC Greg to explain. The backlight class didn't go away. The way it is
> > > handled is different.
> >
> > Have a pointer to the patch so I can help explain better?
>
> You've been cc'd on the proposed patch a couple of times in this thread
> so it should be in your mailbox now.
>
> > As a short summary, 'struct class_device' is going away. Using a
> > 'struct device' in its place is what the conversion should have just
> > done, no functionality change otherwise.
>
> The backlight class is drivers/video/backlight/backlight.c and if I
> understand things correctly what shows up in sysfs changes.
>
> At the moment (as I understand the code which could be wrong), the
> backlight functionality is tagged onto an existing device. Taking
> drivers/video/backlight/corgi_bl.c as an example, the corgi_bl device
> exists under /sys/devices/platform/corgi_bl with an associated struct
> device. The backlight code then appends some magic to this to link in
> some class attributes that show up under /sys/class/backlight. There is
> only ever one device.
>
> By replacing class_device_register() with device_create(), the proposed
> patch appears to generate a second struct device with the original as
> its parent. I'm not sure where it appears in /sys/devices? Having
> another full blown struct device around makes me uneasy as wherever it
> appears, we only have one device, not two.

No, your blacklight "device" is also a device now, not just a
class_device. You want to show that heirachy properly for a variety of
different reasons (power management being one of the most important.)

> If I had some kind of platform device which had an LED, backlight and
> say battery component and registered with the three appropriate classes
> would that mean four struct devices? Where under /sys/devices do they
> each appear?

Under the main device itself, as they are children of it.

As an example, look at a sound device now on 2.6.20 with
CONFIG_SYSFS_DEPRECATED turned off:

$ tree /sys/class/sound/
/sys/class/sound/
|-- adsp -> ../../devices/pci0000:00/0000:00:1b.0/card0/adsp
|-- audio -> ../../devices/pci0000:00/0000:00:1b.0/card0/audio
|-- card0 -> ../../devices/pci0000:00/0000:00:1b.0/card0
|-- controlC0 -> ../../devices/pci0000:00/0000:00:1b.0/card0/controlC0
|-- dsp -> ../../devices/pci0000:00/0000:00:1b.0/card0/dsp
|-- mixer -> ../../devices/pci0000:00/0000:00:1b.0/card0/mixer
|-- pcmC0D0c -> ../../devices/pci0000:00/0000:00:1b.0/card0/pcmC0D0c
|-- pcmC0D0p -> ../../devices/pci0000:00/0000:00:1b.0/card0/pcmC0D0p
|-- pcmC0D1p -> ../../devices/pci0000:00/0000:00:1b.0/card0/pcmC0D1p
|-- seq -> ../../devices/virtual/sound/seq
|-- sequencer -> ../../devices/virtual/sound/sequencer
|-- sequencer2 -> ../../devices/virtual/sound/sequencer2
`-- timer -> ../../devices/virtual/sound/timer

There are a bunch of mixer and other interfaces, all now real devices
under the proper PCI device (sound added the "card0" intermediate level
also, but you will not have that for your devices.

> Also, it was mentioned that a parent struct device is a requirement and
> this isn't the case for all backlight users. I think this constraint on
> device_create has been removed in more recent code though?

Yes, if NULL is passed in, it will be created in the
/sys/devices/virtual/CLASS_NAME/ directory. See the above "seq" and
"timer" devices as an example of that.

Hope this helps,

greg k-h