* [PATCH, RFC] Driver for reading HP laptop LCD brightness
@ 2006-02-05 13:55 Matthew Garrett
2006-02-05 14:27 ` Arjan van de Ven
2006-02-05 21:49 ` Pavel Machek
0 siblings, 2 replies; 6+ messages in thread
From: Matthew Garrett @ 2006-02-05 13:55 UTC (permalink / raw)
To: linux-kernel
The attached patch provides a sysfs mechanism for reading the LCD
brightness on HP laptops. Since there's no clean way to determine
whether a machine is a laptop or not from within the kernel (this
information is in the DMI tables, but we don't currently export the
chassis field), it does nothing until userspace has enabled it.
Right now, I'm looking for comments on how to integrate it sensibly -
leaving it under drivers/misc and registering it as a platform device
/works/, but isn't terribly clean.
Patch is against the Ubuntu kernel tree, so the Makefile and Kconfig
hunks may need a tiny bit of massaging.
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 25f6d65..040ffa1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -28,6 +28,13 @@ config IBM_ASM
If unsure, say N.
+config HPLCD
+ tristate "Device driver HP LCD brightness"
+ depends on X86 && EXPERIMENTAL
+ default n
+ ---help---
+ Provides a sysfs interface to read the screen brightness on HP laptops
+
config AVERATEC_5100P
tristate "Device driver for Averatec 5100P SW Switch"
depends on X86 && EXPERIMENTAL
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 3bf328a..7658427 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -3,6 +3,7 @@
#
obj- := misc.o # Dummy rule to force built-in.o to be made
+obj-$(CONFIG_HPLCD) += hplcd.o
obj-$(CONFIG_IBM_ASM) += ibmasm/
obj-$(CONFIG_HDPU_FEATURES) += hdpuftrs/
obj-$(CONFIG_AVERATEC_5100P) += av5100.o
diff --git a/drivers/misc/hplcd.c b/drivers/misc/hplcd.c
new file mode 100644
index 0000000..4174135
--- /dev/null
+++ b/drivers/misc/hplcd.c
@@ -0,0 +1,159 @@
+/*
+ * hplcd.c - driver to provide backlight information for HP laptops
+ *
+ * Copyright (C) 2006 Matthew Garrett <mjg59@srcf.ucam.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License v2 as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include <linux/module.h>
+#include <linux/dmi.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <asm/io.h>
+
+static int enable;
+static struct platform_device *pdev = NULL;
+
+static struct dmi_system_id __initdata hplcd_device_table[] = {
+ {
+ .ident = "HP",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hewlett-Packard"),
+ },
+ },
+ { }
+};
+
+static ssize_t store_enable(struct device *dev,
+ struct device_attribute *attr,
+ const char * buf,
+ size_t count)
+{
+ char *last = NULL;
+ if (simple_strtoul(buf, &last, 0) >0) {
+ enable = 1;
+ } else {
+ enable = 0;
+ }
+
+ return count;
+}
+
+static ssize_t show_enable(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", enable);
+}
+
+static ssize_t show_ac_brt(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int value;
+
+ if (enable==0) {
+ return snprintf(buf, PAGE_SIZE, "NA\n");
+ }
+
+ disable_irq(8);
+
+ outb(0x97, 0x72);
+ value = inb(0x73);
+
+ enable_irq(8);
+
+ value &= 0x1f; // Brightness is in the lower 5 bits
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static ssize_t show_dc_brt(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int value;
+
+ if (enable==0) {
+ return snprintf(buf, PAGE_SIZE, "NA\n");
+ }
+
+ disable_irq(8);
+
+ outb(0x99, 0x72);
+ value = inb(0x73);
+
+ enable_irq(8);
+
+ value &= 0x1f; // Brightness is in the lower 5 bits
+
+ return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static DEVICE_ATTR(enable, 0644, show_enable, store_enable);
+static DEVICE_ATTR(ac_brt, 0644, show_ac_brt, NULL);
+static DEVICE_ATTR(dc_brt, 0644, show_dc_brt, NULL);
+
+static struct attribute *hplcd_attributes[] = {
+ &dev_attr_enable.attr,
+ &dev_attr_ac_brt.attr,
+ &dev_attr_dc_brt.attr,
+ NULL,
+};
+
+static struct attribute_group hplcd_attribute_group = {
+ .attrs = hplcd_attributes,
+};
+
+static struct platform_driver hplcd_driver = {
+ .driver = {
+ .name = "hplcd",
+ .owner = THIS_MODULE,
+ }
+};
+
+static int __init hplcd_init(void)
+{
+ int rc;
+
+ if (!dmi_check_system(hplcd_device_table))
+ return -ENODEV;
+
+ if (rc = platform_driver_register(&hplcd_driver))
+ return rc;
+
+ pdev = platform_device_register_simple("hplcd", -1, NULL, 0);
+
+ if (IS_ERR(pdev)) {
+ rc = PTR_ERR(pdev);
+ return rc;
+ }
+
+ if (rc = sysfs_create_group(&pdev->dev.kobj, &hplcd_attribute_group))
+ return rc;
+
+ return 0;
+}
+
+static void __exit hplcd_exit(void)
+{
+ sysfs_remove_group(&pdev->dev.kobj, &hplcd_attribute_group);
+ platform_device_unregister(pdev);
+ platform_driver_unregister(&hplcd_driver);
+}
+
+module_init(hplcd_init);
+module_exit(hplcd_exit);
+MODULE_LICENSE("GPL");
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH, RFC] Driver for reading HP laptop LCD brightness
2006-02-05 13:55 [PATCH, RFC] Driver for reading HP laptop LCD brightness Matthew Garrett
@ 2006-02-05 14:27 ` Arjan van de Ven
2006-02-05 14:34 ` Matthew Garrett
2006-02-05 21:49 ` Pavel Machek
1 sibling, 1 reply; 6+ messages in thread
From: Arjan van de Ven @ 2006-02-05 14:27 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-kernel
On Sun, 2006-02-05 at 13:55 +0000, Matthew Garrett wrote:
> + if (enable==0) {
> + return snprintf(buf, PAGE_SIZE, "NA\n");
> + }
> +
> + disable_irq(8);
> +
> + outb(0x97, 0x72);
> + value = inb(0x73);
> +
> + enable_irq(8);
> +
disable_irq() and enable_irq() are really really evil. Are you sure you
need these? To me on first sight it looks like a bug (think of shared
interrupts for example), can you explain what you are trying to achieve
with these?
Greetings,
Arjan van de Ven
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH, RFC] Driver for reading HP laptop LCD brightness
2006-02-05 14:27 ` Arjan van de Ven
@ 2006-02-05 14:34 ` Matthew Garrett
2006-02-05 14:41 ` Arjan van de Ven
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2006-02-05 14:34 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: linux-kernel
On Sun, Feb 05, 2006 at 03:27:26PM +0100, Arjan van de Ven wrote:
> disable_irq() and enable_irq() are really really evil. Are you sure you
> need these? To me on first sight it looks like a bug (think of shared
> interrupts for example), can you explain what you are trying to achieve
> with these?
We're talking to the hardware directly. There's a potential race where
the BIOS will try to access the cmos at the same time, with potentially
interesting results (We set the address we want to read with the outb.
The BIOS runs, outbs its own address, and then reads. We then read from
the address the BIOS was looking at, rather than what we were looking
at).
On all the hardware this will run on, IRQ 8 is the RTC. I don't believe
it can share interrupts.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, RFC] Driver for reading HP laptop LCD brightness
2006-02-05 14:34 ` Matthew Garrett
@ 2006-02-05 14:41 ` Arjan van de Ven
2006-02-05 14:45 ` Matthew Garrett
0 siblings, 1 reply; 6+ messages in thread
From: Arjan van de Ven @ 2006-02-05 14:41 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-kernel
On Sun, 2006-02-05 at 14:34 +0000, Matthew Garrett wrote:
> On Sun, Feb 05, 2006 at 03:27:26PM +0100, Arjan van de Ven wrote:
>
> > disable_irq() and enable_irq() are really really evil. Are you sure you
> > need these? To me on first sight it looks like a bug (think of shared
> > interrupts for example), can you explain what you are trying to achieve
> > with these?
>
> We're talking to the hardware directly. There's a potential race where
> the BIOS will try to access the cmos at the same time, with potentially
> interesting results (We set the address we want to read with the outb.
> The BIOS runs, outbs its own address, and then reads. We then read from
> the address the BIOS was looking at, rather than what we were looking
> at).
.. and just disabling interrupts isn't going to work? Ok sure there is
an SMP issue, but a spinlock ought to be able to fix that properly,
instead of something this evil....
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, RFC] Driver for reading HP laptop LCD brightness
2006-02-05 14:41 ` Arjan van de Ven
@ 2006-02-05 14:45 ` Matthew Garrett
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Garrett @ 2006-02-05 14:45 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: linux-kernel
On Sun, Feb 05, 2006 at 03:41:42PM +0100, Arjan van de Ven wrote:
> .. and just disabling interrupts isn't going to work? Ok sure there is
> an SMP issue, but a spinlock ought to be able to fix that properly,
> instead of something this evil....
How do we get the BIOS to respect kernel spinlocks? (On the other hand,
there ought to be a spinlock there to deal with concurrent access in any
case, I guess)
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, RFC] Driver for reading HP laptop LCD brightness
2006-02-05 13:55 [PATCH, RFC] Driver for reading HP laptop LCD brightness Matthew Garrett
2006-02-05 14:27 ` Arjan van de Ven
@ 2006-02-05 21:49 ` Pavel Machek
1 sibling, 0 replies; 6+ messages in thread
From: Pavel Machek @ 2006-02-05 21:49 UTC (permalink / raw)
To: Matthew Garrett; +Cc: linux-kernel
Hi!
> The attached patch provides a sysfs mechanism for reading the LCD
> brightness on HP laptops. Since there's no clean way to determine
> whether a machine is a laptop or not from within the kernel (this
> information is in the DMI tables, but we don't currently export the
> chassis field), it does nothing until userspace has enabled it.
Can you fix the DMI code to export chasis, then?
> Right now, I'm looking for comments on how to integrate it sensibly -
> leaving it under drivers/misc and registering it as a platform device
> /works/, but isn't terribly clean.
Lots of handhelds ave brightness settings, and it is *very* important
there. Perhaps you could use same interface?
Pavel
--
Thanks, Sharp!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-02-05 21:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-05 13:55 [PATCH, RFC] Driver for reading HP laptop LCD brightness Matthew Garrett
2006-02-05 14:27 ` Arjan van de Ven
2006-02-05 14:34 ` Matthew Garrett
2006-02-05 14:41 ` Arjan van de Ven
2006-02-05 14:45 ` Matthew Garrett
2006-02-05 21:49 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox