Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 11:50:23PM +, 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 +, 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/pci:00/:00:1b.0/card0/adsp |-- audio -> ../../devices/pci:00/:00:1b.0/card0/audio |-- card0 -> ../../devices/pci:00/:00:1b.0/card0 |-- controlC0 -> ../../devices/pci:00/:00:1b.0/card0/controlC0 |-- dsp -> ../../devices/pci:00/:00:1b.0/card0/dsp |-- mixer -> ../../devices/pci:00/:00:1b.0/card0/mixer |-- pcmC0D0c -> ../../devices/pci:00/:00:1b.0/card0/pcmC0D0c |-- pcmC0D0p -> ../../devices/pci:00/:00:1b.0/card0/pcmC0D0p |-- pcmC0D1p -> ../../devices/pci:00/: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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
Hi Greg, On Thu, 2007-02-08 at 13:23 -0800, Greg KH wrote: > On Thu, Feb 08, 2007 at 06:32:02PM +, 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. 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? 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? Richard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 09:54:21PM +, James Simmons wrote: > > > On Thu, Feb 08, 2007 at 06:32:02PM +, James Simmons wrote: > > > On Thu, 8 Feb 2007, Richard Purdie wrote: > > > > > > > On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: > > > > > I have some patches that move the backlight away from using the class > > > > > stuff. The only problem is the patch requires all backlight devices > > > > > to be linked to a real struct device. Right now the acpi backligths > > > > > are > > > > > not. > > > > > > > > Why would you want to do that? > > > > > > > > The whole point of having this is so that backlights appear as a > > > > standard interface under /sys/class/backlight. > > > > > > > > An example of why standardised interfaces are good would be someone > > > > writing an applet for a handheld to control the backlight brightness. > > > > With the class in place, the applet can easily work with any backlight. > > > > Without it, it has to be written for each backlight. > > > > > > > > So this is a very strong NAK but I'm curious why you'd want to do it... > > > > > > 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? > > > > 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. > > diff --git a/drivers/video/backlight/backlight.c > b/drivers/video/backlight/backlight.c Looks good to me. And it makes the code simpler too :) thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
> On Thu, Feb 08, 2007 at 06:32:02PM +, James Simmons wrote:
> > On Thu, 8 Feb 2007, Richard Purdie wrote:
> >
> > > On Thu, 2007-02-08 at 15:28 +, James Simmons wrote:
> > > > I have some patches that move the backlight away from using the class
> > > > stuff. The only problem is the patch requires all backlight devices
> > > > to be linked to a real struct device. Right now the acpi backligths are
> > > > not.
> > >
> > > Why would you want to do that?
> > >
> > > The whole point of having this is so that backlights appear as a
> > > standard interface under /sys/class/backlight.
> > >
> > > An example of why standardised interfaces are good would be someone
> > > writing an applet for a handheld to control the backlight brightness.
> > > With the class in place, the applet can easily work with any backlight.
> > > Without it, it has to be written for each backlight.
> > >
> > > So this is a very strong NAK but I'm curious why you'd want to do it...
> >
> > 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?
>
> 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.
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 9601bfe..b56fc33 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -14,6 +14,8 @@
#include
#include
+struct class *backlight_class;
+static int index;
#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
@@ -67,10 +69,11 @@ static inline void backlight_unregister_fb(struct
backlight_device *bd)
}
#endif /* CONFIG_FB */
-static ssize_t backlight_show_power(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_power(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(>sem);
if (likely(bd->props))
@@ -80,11 +83,12 @@ static ssize_t backlight_show_power(struct class_device
*cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_power(struct class_device *cdev, const char
*buf, size_t count)
+static ssize_t backlight_store_power(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int power = simple_strtoul(buf, , 0);
size_t size = endp - buf;
@@ -106,10 +110,11 @@ static ssize_t backlight_store_power(struct class_device
*cdev, const char *buf,
return rc;
}
-static ssize_t backlight_show_brightness(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(>sem);
if (likely(bd->props))
@@ -119,11 +124,12 @@ static ssize_t backlight_show_brightness(struct
class_device *cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_brightness(struct class_device *cdev, const
char *buf, size_t count)
+static ssize_t backlight_store_brightness(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int brightness = simple_strtoul(buf, , 0);
size_t size = endp - buf;
@@ -150,10 +156,11 @@ static ssize_t backlight_store_brightness(struct
class_device *cdev, const char
return rc;
}
-static ssize_t backlight_show_max_brightness(struct class_device *cdev, char
*buf)
+static ssize_t backlight_show_max_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(>sem);
if (likely(bd->props))
@@ -163,11 +170,11 @@ static ssize_t backlight_show_max_brightness(struct
class_device *cdev, char *bu
return rc;
}
-static ssize_t backlight_show_actual_brightness(struct class_device *cdev,
+static ssize_t
Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 06:32:02PM +, James Simmons wrote: > On Thu, 8 Feb 2007, Richard Purdie wrote: > > > On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: > > > I have some patches that move the backlight away from using the class > > > stuff. The only problem is the patch requires all backlight devices > > > to be linked to a real struct device. Right now the acpi backligths are > > > not. > > > > Why would you want to do that? > > > > The whole point of having this is so that backlights appear as a > > standard interface under /sys/class/backlight. > > > > An example of why standardised interfaces are good would be someone > > writing an applet for a handheld to control the backlight brightness. > > With the class in place, the applet can easily work with any backlight. > > Without it, it has to be written for each backlight. > > > > So this is a very strong NAK but I'm curious why you'd want to do it... > > 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? 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. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 2007-02-08 at 18:32 +, James Simmons wrote: > On Thu, 8 Feb 2007, Richard Purdie wrote: > > > On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: > > > I have some patches that move the backlight away from using the class > > > stuff. The only problem is the patch requires all backlight devices > > > to be linked to a real struct device. Right now the acpi backligths are > > > not. > > > > Why would you want to do that? [...] > > I CC Greg to explain. The backlight class didn't go away. The way it is > handled is different. >From the changelog and a quick skim though the patch it looked like you were just removing the class functionality and using the struct device directly but I see what you're trying to do now. I'm not sure I agree with it though as it results in two devices for each backlight user as almost all users have an existing struct device. The current approach allows you to attach the class to an existing device and also means you can attach multiple classes to a given device which gives more flexibility. So the above question stills stands, why would you want to do this (apart from removing some code)? Richard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 8 Feb 2007, Richard Purdie wrote: > On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: > > I have some patches that move the backlight away from using the class > > stuff. The only problem is the patch requires all backlight devices > > to be linked to a real struct device. Right now the acpi backligths are > > not. > > Why would you want to do that? > > The whole point of having this is so that backlights appear as a > standard interface under /sys/class/backlight. > > An example of why standardised interfaces are good would be someone > writing an applet for a handheld to control the backlight brightness. > With the class in place, the applet can easily work with any backlight. > Without it, it has to be written for each backlight. > > So this is a very strong NAK but I'm curious why you'd want to do it... I CC Greg to explain. The backlight class didn't go away. The way it is handled is different. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: > I have some patches that move the backlight away from using the class > stuff. The only problem is the patch requires all backlight devices > to be linked to a real struct device. Right now the acpi backligths are > not. Why would you want to do that? The whole point of having this is so that backlights appear as a standard interface under /sys/class/backlight. An example of why standardised interfaces are good would be someone writing an applet for a handheld to control the backlight brightness. With the class in place, the applet can easily work with any backlight. Without it, it has to be written for each backlight. So this is a very strong NAK but I'm curious why you'd want to do it... Richard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
I have some patches that move the backlight away from using the class
stuff. The only problem is the patch requires all backlight devices
to be linked to a real struct device. Right now the acpi backligths are
not.
Signed-Off: James Simmons <[EMAIL PROTECTED]>
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 9601bfe..b56fc33 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -14,6 +14,8 @@
#include
#include
+struct class *backlight_class;
+static int index;
#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
@@ -67,10 +69,11 @@ static inline void backlight_unregister_fb(struct
backlight_device *bd)
}
#endif /* CONFIG_FB */
-static ssize_t backlight_show_power(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_power(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(>sem);
if (likely(bd->props))
@@ -80,11 +83,12 @@ static ssize_t backlight_show_power(struct class_device
*cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_power(struct class_device *cdev, const char
*buf, size_t count)
+static ssize_t backlight_store_power(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int power = simple_strtoul(buf, , 0);
size_t size = endp - buf;
@@ -106,10 +110,11 @@ static ssize_t backlight_store_power(struct class_device
*cdev, const char *buf,
return rc;
}
-static ssize_t backlight_show_brightness(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(>sem);
if (likely(bd->props))
@@ -119,11 +124,12 @@ static ssize_t backlight_show_brightness(struct
class_device *cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_brightness(struct class_device *cdev, const
char *buf, size_t count)
+static ssize_t backlight_store_brightness(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int brightness = simple_strtoul(buf, , 0);
size_t size = endp - buf;
@@ -150,10 +156,11 @@ static ssize_t backlight_store_brightness(struct
class_device *cdev, const char
return rc;
}
-static ssize_t backlight_show_max_brightness(struct class_device *cdev, char
*buf)
+static ssize_t backlight_show_max_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(>sem);
if (likely(bd->props))
@@ -163,11 +170,11 @@ static ssize_t backlight_show_max_brightness(struct
class_device *cdev, char *bu
return rc;
}
-static ssize_t backlight_show_actual_brightness(struct class_device *cdev,
+static ssize_t backlight_show_actual_brightness(struct device *dev, struct
device_attribute *attr,
char *buf)
{
+ struct backlight_device *bd = dev_get_drvdata(dev);
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
down(>sem);
if (likely(bd->props && bd->props->get_brightness))
@@ -177,31 +184,12 @@ static ssize_t backlight_show_actual_brightness(struct
class_device *cdev,
return rc;
}
-static void backlight_class_release(struct class_device *dev)
-{
- struct backlight_device *bd = to_backlight_device(dev);
- kfree(bd);
-}
-
-static struct class backlight_class = {
- .name = "backlight",
- .release = backlight_class_release,
-};
-
-#define DECLARE_ATTR(_name,_mode,_show,_store) \
-{ \
- .attr = { .name = __stringify(_name), .mode = _mode, .owner =
THIS_MODULE }, \
- .show = _show,\
- .store = _store,
Re: Git backlight subsystem tree
I have some patches that move the backlight away from using the class
stuff. The only problem is the patch requires all backlight devices
to be linked to a real struct device. Right now the acpi backligths are
not.
Signed-Off: James Simmons [EMAIL PROTECTED]
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 9601bfe..b56fc33 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -14,6 +14,8 @@
#include linux/err.h
#include linux/fb.h
+struct class *backlight_class;
+static int index;
#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) \
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
@@ -67,10 +69,11 @@ static inline void backlight_unregister_fb(struct
backlight_device *bd)
}
#endif /* CONFIG_FB */
-static ssize_t backlight_show_power(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_power(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(bd-sem);
if (likely(bd-props))
@@ -80,11 +83,12 @@ static ssize_t backlight_show_power(struct class_device
*cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_power(struct class_device *cdev, const char
*buf, size_t count)
+static ssize_t backlight_store_power(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int power = simple_strtoul(buf, endp, 0);
size_t size = endp - buf;
@@ -106,10 +110,11 @@ static ssize_t backlight_store_power(struct class_device
*cdev, const char *buf,
return rc;
}
-static ssize_t backlight_show_brightness(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(bd-sem);
if (likely(bd-props))
@@ -119,11 +124,12 @@ static ssize_t backlight_show_brightness(struct
class_device *cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_brightness(struct class_device *cdev, const
char *buf, size_t count)
+static ssize_t backlight_store_brightness(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int brightness = simple_strtoul(buf, endp, 0);
size_t size = endp - buf;
@@ -150,10 +156,11 @@ static ssize_t backlight_store_brightness(struct
class_device *cdev, const char
return rc;
}
-static ssize_t backlight_show_max_brightness(struct class_device *cdev, char
*buf)
+static ssize_t backlight_show_max_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(bd-sem);
if (likely(bd-props))
@@ -163,11 +170,11 @@ static ssize_t backlight_show_max_brightness(struct
class_device *cdev, char *bu
return rc;
}
-static ssize_t backlight_show_actual_brightness(struct class_device *cdev,
+static ssize_t backlight_show_actual_brightness(struct device *dev, struct
device_attribute *attr,
char *buf)
{
+ struct backlight_device *bd = dev_get_drvdata(dev);
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
down(bd-sem);
if (likely(bd-props bd-props-get_brightness))
@@ -177,31 +184,12 @@ static ssize_t backlight_show_actual_brightness(struct
class_device *cdev,
return rc;
}
-static void backlight_class_release(struct class_device *dev)
-{
- struct backlight_device *bd = to_backlight_device(dev);
- kfree(bd);
-}
-
-static struct class backlight_class = {
- .name = backlight,
- .release = backlight_class_release,
-};
-
-#define DECLARE_ATTR(_name,_mode,_show,_store) \
-{ \
- .attr = { .name = __stringify(_name), .mode = _mode, .owner =
THIS_MODULE }, \
- .show = _show,\
- .store
Re: Git backlight subsystem tree
On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: I have some patches that move the backlight away from using the class stuff. The only problem is the patch requires all backlight devices to be linked to a real struct device. Right now the acpi backligths are not. Why would you want to do that? The whole point of having this is so that backlights appear as a standard interface under /sys/class/backlight. An example of why standardised interfaces are good would be someone writing an applet for a handheld to control the backlight brightness. With the class in place, the applet can easily work with any backlight. Without it, it has to be written for each backlight. So this is a very strong NAK but I'm curious why you'd want to do it... Richard - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 8 Feb 2007, Richard Purdie wrote: On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: I have some patches that move the backlight away from using the class stuff. The only problem is the patch requires all backlight devices to be linked to a real struct device. Right now the acpi backligths are not. Why would you want to do that? The whole point of having this is so that backlights appear as a standard interface under /sys/class/backlight. An example of why standardised interfaces are good would be someone writing an applet for a handheld to control the backlight brightness. With the class in place, the applet can easily work with any backlight. Without it, it has to be written for each backlight. So this is a very strong NAK but I'm curious why you'd want to do it... I CC Greg to explain. The backlight class didn't go away. The way it is handled is different. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 2007-02-08 at 18:32 +, James Simmons wrote: On Thu, 8 Feb 2007, Richard Purdie wrote: On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: I have some patches that move the backlight away from using the class stuff. The only problem is the patch requires all backlight devices to be linked to a real struct device. Right now the acpi backligths are not. Why would you want to do that? [...] I CC Greg to explain. The backlight class didn't go away. The way it is handled is different. From the changelog and a quick skim though the patch it looked like you were just removing the class functionality and using the struct device directly but I see what you're trying to do now. I'm not sure I agree with it though as it results in two devices for each backlight user as almost all users have an existing struct device. The current approach allows you to attach the class to an existing device and also means you can attach multiple classes to a given device which gives more flexibility. So the above question stills stands, why would you want to do this (apart from removing some code)? Richard - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 06:32:02PM +, James Simmons wrote: On Thu, 8 Feb 2007, Richard Purdie wrote: On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: I have some patches that move the backlight away from using the class stuff. The only problem is the patch requires all backlight devices to be linked to a real struct device. Right now the acpi backligths are not. Why would you want to do that? The whole point of having this is so that backlights appear as a standard interface under /sys/class/backlight. An example of why standardised interfaces are good would be someone writing an applet for a handheld to control the backlight brightness. With the class in place, the applet can easily work with any backlight. Without it, it has to be written for each backlight. So this is a very strong NAK but I'm curious why you'd want to do it... 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? 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. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 06:32:02PM +, James Simmons wrote:
On Thu, 8 Feb 2007, Richard Purdie wrote:
On Thu, 2007-02-08 at 15:28 +, James Simmons wrote:
I have some patches that move the backlight away from using the class
stuff. The only problem is the patch requires all backlight devices
to be linked to a real struct device. Right now the acpi backligths are
not.
Why would you want to do that?
The whole point of having this is so that backlights appear as a
standard interface under /sys/class/backlight.
An example of why standardised interfaces are good would be someone
writing an applet for a handheld to control the backlight brightness.
With the class in place, the applet can easily work with any backlight.
Without it, it has to be written for each backlight.
So this is a very strong NAK but I'm curious why you'd want to do it...
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?
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.
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 9601bfe..b56fc33 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -14,6 +14,8 @@
#include linux/err.h
#include linux/fb.h
+struct class *backlight_class;
+static int index;
#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) \
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
@@ -67,10 +69,11 @@ static inline void backlight_unregister_fb(struct
backlight_device *bd)
}
#endif /* CONFIG_FB */
-static ssize_t backlight_show_power(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_power(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(bd-sem);
if (likely(bd-props))
@@ -80,11 +83,12 @@ static ssize_t backlight_show_power(struct class_device
*cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_power(struct class_device *cdev, const char
*buf, size_t count)
+static ssize_t backlight_store_power(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int power = simple_strtoul(buf, endp, 0);
size_t size = endp - buf;
@@ -106,10 +110,11 @@ static ssize_t backlight_store_power(struct class_device
*cdev, const char *buf,
return rc;
}
-static ssize_t backlight_show_brightness(struct class_device *cdev, char *buf)
+static ssize_t backlight_show_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(bd-sem);
if (likely(bd-props))
@@ -119,11 +124,12 @@ static ssize_t backlight_show_brightness(struct
class_device *cdev, char *buf)
return rc;
}
-static ssize_t backlight_store_brightness(struct class_device *cdev, const
char *buf, size_t count)
+static ssize_t backlight_store_brightness(struct device *dev, struct
device_attribute *attr,
+ const char *buf, size_t count)
{
int rc = -ENXIO;
char *endp;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
int brightness = simple_strtoul(buf, endp, 0);
size_t size = endp - buf;
@@ -150,10 +156,11 @@ static ssize_t backlight_store_brightness(struct
class_device *cdev, const char
return rc;
}
-static ssize_t backlight_show_max_brightness(struct class_device *cdev, char
*buf)
+static ssize_t backlight_show_max_brightness(struct device *dev, struct
device_attribute *attr,
+ char *buf)
{
int rc = -ENXIO;
- struct backlight_device *bd = to_backlight_device(cdev);
+ struct backlight_device *bd = dev_get_drvdata(dev);
down(bd-sem);
if (likely(bd-props))
@@ -163,11 +170,11 @@ static ssize_t backlight_show_max_brightness(struct
class_device *cdev, char *bu
return rc;
}
-static ssize_t backlight_show_actual_brightness(struct class_device *cdev,
+static ssize_t backlight_show_actual_brightness(struct device *dev, struct
Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 09:54:21PM +, James Simmons wrote: On Thu, Feb 08, 2007 at 06:32:02PM +, James Simmons wrote: On Thu, 8 Feb 2007, Richard Purdie wrote: On Thu, 2007-02-08 at 15:28 +, James Simmons wrote: I have some patches that move the backlight away from using the class stuff. The only problem is the patch requires all backlight devices to be linked to a real struct device. Right now the acpi backligths are not. Why would you want to do that? The whole point of having this is so that backlights appear as a standard interface under /sys/class/backlight. An example of why standardised interfaces are good would be someone writing an applet for a handheld to control the backlight brightness. With the class in place, the applet can easily work with any backlight. Without it, it has to be written for each backlight. So this is a very strong NAK but I'm curious why you'd want to do it... 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? 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. diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c snip Looks good to me. And it makes the code simpler too :) thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
Hi Greg, On Thu, 2007-02-08 at 13:23 -0800, Greg KH wrote: On Thu, Feb 08, 2007 at 06:32:02PM +, 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. 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? 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? Richard - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, Feb 08, 2007 at 11:50:23PM +, 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 +, 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/pci:00/:00:1b.0/card0/adsp |-- audio - ../../devices/pci:00/:00:1b.0/card0/audio |-- card0 - ../../devices/pci:00/:00:1b.0/card0 |-- controlC0 - ../../devices/pci:00/:00:1b.0/card0/controlC0 |-- dsp - ../../devices/pci:00/:00:1b.0/card0/dsp |-- mixer - ../../devices/pci:00/:00:1b.0/card0/mixer |-- pcmC0D0c - ../../devices/pci:00/:00:1b.0/card0/pcmC0D0c |-- pcmC0D0p - ../../devices/pci:00/:00:1b.0/card0/pcmC0D0p |-- pcmC0D1p - ../../devices/pci:00/: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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 08 Feb 2007 02:30:26 + Richard Purdie <[EMAIL PROTECTED]> wrote: > I'm thinking of more officially volunteering to maintain the Linux > backlight class/subsystem and have been trying to sort out the various > pending patches. I've made a backlight tree available as: > > http://git.o-hand.com/?p=linux-rpurdie-backlight;a=shortlog;h=for-mm > > (or git://git.o-hand.com/linux-rpurdie-backlight) > > This has a for-mm branch containing the backlight patches I've sorted > out so far. I still have some others left to deal with which are > probably more invasive and will need discussion and testing which I'll > send some mails about soon. No probs, I added git://git.o-hand.com/linux-rpurdie-backlight#for-mm to -mm as git-backlight.patch. > I'm planning to setup an LED drivers/class tree too, I have a couple of > pending patches for that. ooh, patches! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Git backlight subsystem tree
I'm thinking of more officially volunteering to maintain the Linux backlight class/subsystem and have been trying to sort out the various pending patches. I've made a backlight tree available as: http://git.o-hand.com/?p=linux-rpurdie-backlight;a=shortlog;h=for-mm (or git://git.o-hand.com/linux-rpurdie-backlight) This has a for-mm branch containing the backlight patches I've sorted out so far. I still have some others left to deal with which are probably more invasive and will need discussion and testing which I'll send some mails about soon. I'm planning to setup an LED drivers/class tree too, I have a couple of pending patches for that. Cheers, Richard - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Git backlight subsystem tree
I'm thinking of more officially volunteering to maintain the Linux backlight class/subsystem and have been trying to sort out the various pending patches. I've made a backlight tree available as: http://git.o-hand.com/?p=linux-rpurdie-backlight;a=shortlog;h=for-mm (or git://git.o-hand.com/linux-rpurdie-backlight) This has a for-mm branch containing the backlight patches I've sorted out so far. I still have some others left to deal with which are probably more invasive and will need discussion and testing which I'll send some mails about soon. I'm planning to setup an LED drivers/class tree too, I have a couple of pending patches for that. Cheers, Richard - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Git backlight subsystem tree
On Thu, 08 Feb 2007 02:30:26 + Richard Purdie [EMAIL PROTECTED] wrote: I'm thinking of more officially volunteering to maintain the Linux backlight class/subsystem and have been trying to sort out the various pending patches. I've made a backlight tree available as: http://git.o-hand.com/?p=linux-rpurdie-backlight;a=shortlog;h=for-mm (or git://git.o-hand.com/linux-rpurdie-backlight) This has a for-mm branch containing the backlight patches I've sorted out so far. I still have some others left to deal with which are probably more invasive and will need discussion and testing which I'll send some mails about soon. No probs, I added git://git.o-hand.com/linux-rpurdie-backlight#for-mm to -mm as git-backlight.patch. I'm planning to setup an LED drivers/class tree too, I have a couple of pending patches for that. ooh, patches! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
