Skip to contentSkip to conversation

Platform: Samsung Q10 backlight driver

PATCH Superseded Frederick van der Wyck 3 participants 5 messages Feb 16, 2011 – Feb 17, 2011 x86 platform drivers

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Platform: Samsung Q10 backlight driver

This adds backlight control on the Samsung Q10 laptop, which does not support the SABI interface. Also tested successfully on the Dell Latitude X200.

---
 drivers/platform/x86/Kconfig       |    8 ++
 drivers/platform/x86/Makefile      |    1 +
 drivers/platform/x86/samsung-q10.c |  152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/samsung-q10.c

The Samsung Q10 is an old model, which does not support the SABI interface, so the samsung-laptop driver does not work. Backlight control is not exposed via ACPI either. This driver works in the same way as the SMI handler triggered by pressing the brightness function keys. Please let me know any feedback towards getting this driver included.

drivers/platform/x86/Kconfig +8 -0
index faec777..7432817 100644
@@ -639,4 +639,12 @@ config XO1_RFKILL
 	  Support for enabling/disabling the WLAN interface on the OLPC XO-1
 	  laptop.
 
+config SAMSUNG_Q10
+	tristate "Samsung Q10 Extras"
+	select BACKLIGHT_CLASS_DEVICE
+	default n
+	---help---
+          This driver provides support for backlight control on Samsung Q10
+          and some other laptops, including Dell Latitude X200.
+
 endif # X86_PLATFORM_DEVICES
drivers/platform/x86/Makefile +1 -0
index 9950ccc..2cd3e52 100644
@@ -33,3 +33,4 @@ obj-$(CONFIG_INTEL_IPS)		+= intel_ips.o
 obj-$(CONFIG_GPIO_INTEL_PMIC)	+= intel_pmic_gpio.o
 obj-$(CONFIG_XO1_RFKILL)	+= xo1-rfkill.o
 obj-$(CONFIG_IBM_RTL)		+= ibm_rtl.o
+obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
drivers/platform/x86/samsung-q10.c new file +152 -0

152 lines not on this page · raw message

#1 lore raw
Quoted 80 lines · Frederick van der Wyck

This adds backlight control on the Samsung Q10 laptop, which does not support the SABI interface. Also tested successfully on the Dell Latitude X200.

---
 drivers/platform/x86/Kconfig       |    8 ++
 drivers/platform/x86/Makefile      |    1 +
 drivers/platform/x86/samsung-q10.c |  152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/samsung-q10.c

The Samsung Q10 is an old model, which does not support the SABI interface, so the samsung-laptop driver does not work. Backlight control is not exposed via ACPI either. This driver works in the same way as the SMI handler triggered by pressing the brightness function keys. Please let me know any feedback towards getting this driver included.

drivers/platform/x86/Kconfig +8 -0
index faec777..7432817 100644
@@ -639,4 +639,12 @@ config XO1_RFKILL
 	  Support for enabling/disabling the WLAN interface on the OLPC XO-1
 	  laptop.
 
+config SAMSUNG_Q10
+	tristate "Samsung Q10 Extras"
+	select BACKLIGHT_CLASS_DEVICE
+	default n
+	---help---
+          This driver provides support for backlight control on Samsung Q10
+          and some other laptops, including Dell Latitude X200.
+
 endif # X86_PLATFORM_DEVICES
drivers/platform/x86/Makefile +1 -0
index 9950ccc..2cd3e52 100644
@@ -33,3 +33,4 @@ obj-$(CONFIG_INTEL_IPS)		+= intel_ips.o
 obj-$(CONFIG_GPIO_INTEL_PMIC)	+= intel_pmic_gpio.o
 obj-$(CONFIG_XO1_RFKILL)	+= xo1-rfkill.o
 obj-$(CONFIG_IBM_RTL)		+= ibm_rtl.o
+obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
drivers/platform/x86/samsung-q10.c new file +31 -0
new file mode 100644
index 0000000..ec1e004
@@ -0,0 +1,152 @@
+/*
+ *  Driver for Samsung Q10: controls the backlight
+ *
+ *  Also works on Dell Latitude X200
+ *
+ *  Copyright (c) 2011 Frederick van der Wyck <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/io.h>
+
+#define SAMSUNGQ10_BL_MAX_INTENSITY 255
+#define SAMSUNGQ10_BL_DEFAULT_INTENSITY 185
+
+static int samsungq10_suspended;
+
+static void samsungq10_bl_send_intensity(struct backlight_device *bd)
+{
+	int brightness = bd->props.brightness;
+
+	if (!samsungq10_suspended) {
+		while (inb(0x64)&2)

drivers/platform/x86/samsung-q10.c line 41

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+			;
+		outb_p(0xbe, 0x64);
+		while (inb(0x64)&2)
+			;
+		outb_p(0x89, 0x60);
+		while (inb(0x64)&2)
+			;
+		outb_p(0x91, 0x60);
+		while (inb(0x64)&2)
+			;

Potentially endless loops are not good, please timeout if you don't get the proper value over a time.

Quoted 38 lines
drivers/platform/x86/samsung-q10.c new file +38 -0
+		outb_p((unsigned char)brightness, 0x60);
+	};
+}
+
+#ifdef CONFIG_PM
+static int samsungq10_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	samsungq10_suspended = 1;
+	return 0;
+}
+
+static int samsungq10_resume(struct platform_device *pdev)
+{
+	struct backlight_device *bd = platform_get_drvdata(pdev);
+	samsungq10_suspended = 0;
+	samsungq10_bl_send_intensity(bd);
+	return 0;
+}
+#else
+#define samsungq10_suspend	NULL
+#define samsungq10_resume	NULL
+#endif
+
+static int samsungq10_bl_set_intensity(struct backlight_device *bd)
+{
+	samsungq10_bl_send_intensity(bd);
+	return 0;
+}
+
+static int samsungq10_bl_get_intensity(struct backlight_device *bd)
+{
+	return 0;
+}
+
+static const struct backlight_ops samsungq10_bl_ops = {
+	.get_brightness = samsungq10_bl_get_intensity,
+	.update_status	= samsungq10_bl_set_intensity,
+};

drivers/platform/x86/samsung-q10.c line 89

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+
+static int __devinit samsungq10_probe(struct platform_device *pdev)
+{
+	struct backlight_properties props;
+	struct backlight_device *bd;
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.max_brightness = SAMSUNGQ10_BL_MAX_INTENSITY;
+	bd = backlight_device_register("samsungq10bl", &pdev->dev, NULL,
+				       &samsungq10_bl_ops, &props);

Why the name "samsungq10bl"? Why not just "samsung"?

Quoted 20 lines
drivers/platform/x86/samsung-q10.c new file +20 -0
+	if (IS_ERR(bd))
+		return PTR_ERR(bd);
+
+	platform_set_drvdata(pdev, bd);
+
+	bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
+	samsungq10_bl_send_intensity(bd);
+
+	return 0;
+}
+
+static int samsungq10_remove(struct platform_device *pdev)
+{
+	struct backlight_device *bd = platform_get_drvdata(pdev);
+
+	bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
+	samsungq10_bl_send_intensity(bd);
+
+	backlight_device_unregister(bd);
+

drivers/platform/x86/samsung-q10.c line 119

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+	return 0;
+}
+
+static struct platform_driver samsungq10_driver = {
+	.probe		= samsungq10_probe,
+	.remove		= samsungq10_remove,
+	.suspend	= samsungq10_suspend,
+	.resume		= samsungq10_resume,
+	.driver		= {
+		.name	= "samsungq10",

Same with the driver name here.

Quoted 23 lines
drivers/platform/x86/samsung-q10.c new file +23 -0
+	},
+};
+
+static struct platform_device *samsungq10_device;
+
+static int __init samsungq10_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&samsungq10_driver);
+	if (ret)
+		return ret;
+	samsungq10_device = platform_device_register_simple("samsungq10", -1,
+							    NULL, 0);
+	if (IS_ERR(samsungq10_device)) {
+		platform_driver_unregister(&samsungq10_driver);
+		return PTR_ERR(samsungq10_device);
+	}
+	return 0;
+}
+
+static void __exit samsungq10_exit(void)
+{

drivers/platform/x86/samsung-q10.c line 152

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+	platform_device_unregister(samsungq10_device);
+	platform_driver_unregister(&samsungq10_driver);
+}
+
+module_init(samsungq10_init);
+module_exit(samsungq10_exit);
+
+MODULE_AUTHOR("Frederick van der Wyck <[email protected]>");
+MODULE_DESCRIPTION("Samsung Q10 Driver");
+MODULE_LICENSE("GPL");

No DMI matching to auto-load the driver and also fail to load on any non-supported hardware?

thanks,

greg k-h

#2 lore raw
Quoted 13 lines · Greg KH
Quoted 12 lines · Frederick van der Wyck
drivers/platform/x86/samsung-q10.c new file +12 -0
+
+static void samsungq10_bl_send_intensity(struct backlight_device *bd)
+{
+	int brightness = bd->props.brightness;
+
+	if (!samsungq10_suspended) {
+		while (inb(0x64)&2)
+			;
+		outb_p(0xbe, 0x64);
+		while (inb(0x64)&2)
+			;
+		outb_p(0x89, 0x60);
Quoted 8 lines

drivers/platform/x86/samsung-q10.c line 41

Quoted 5 lines
drivers/platform/x86/samsung-q10.c new file +5 -0
+		while (inb(0x64)&2)
+			;
+		outb_p(0x91, 0x60);
+		while (inb(0x64)&2)
+			;

Potentially endless loops are not good, please timeout if you don't get the proper value over a time.

Double not good since it is i8042 registers. Use i8042_lock_chip()/i8042_command()/i8042_unlock_chip().

Signature · 1 line
Dmitry
#3 lore raw
In reply to Frederick van der Wyck

Hi Frederick,

Quoted 22 lines · Frederick van der Wyck

This adds backlight control on the Samsung Q10 laptop, which does not support the SABI interface. Also tested successfully on the Dell Latitude X200.

---
 drivers/platform/x86/Kconfig       |    8 ++
 drivers/platform/x86/Makefile      |    1 +
 drivers/platform/x86/samsung-q10.c |  152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/samsung-q10.c

The Samsung Q10 is an old model, which does not support the SABI interface, so the samsung-laptop driver does not work. Backlight control is not exposed via ACPI either. This driver works in the same way as the SMI handler triggered by pressing the brightness function keys. Please let me know any feedback towards getting this driver included.

drivers/platform/x86/Kconfig +0 -0
index faec777..7432817 100644

drivers/platform/x86/Kconfig config XO1_RFKILL · line 645

Quoted 7 lines
drivers/platform/x86/Kconfig +4 -0
@@ -639,4 +639,12 @@ config XO1_RFKILL
 	  Support for enabling/disabling the WLAN interface on the OLPC XO-1
 	  laptop.
 
+config SAMSUNG_Q10
+	tristate "Samsung Q10 Extras"
+	select BACKLIGHT_CLASS_DEVICE
+	default n

You do not need to specify "default n"

Quoted 34 lines
drivers/platform/x86/Kconfig +4 -0
+	---help---
+          This driver provides support for backlight control on Samsung Q10
+          and some other laptops, including Dell Latitude X200.
+
 endif # X86_PLATFORM_DEVICES
drivers/platform/x86/Makefile +1 -0
index 9950ccc..2cd3e52 100644
@@ -33,3 +33,4 @@ obj-$(CONFIG_INTEL_IPS)		+= intel_ips.o
 obj-$(CONFIG_GPIO_INTEL_PMIC)	+= intel_pmic_gpio.o
 obj-$(CONFIG_XO1_RFKILL)	+= xo1-rfkill.o
 obj-$(CONFIG_IBM_RTL)		+= ibm_rtl.o
+obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
drivers/platform/x86/samsung-q10.c new file +14 -0
new file mode 100644
index 0000000..ec1e004
@@ -0,0 +1,152 @@
+/*
+ *  Driver for Samsung Q10: controls the backlight
+ *
+ *  Also works on Dell Latitude X200
+ *
+ *  Copyright (c) 2011 Frederick van der Wyck <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>

drivers/platform/x86/samsung-q10.c line 24

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/io.h>
+
+#define SAMSUNGQ10_BL_MAX_INTENSITY 255
+#define SAMSUNGQ10_BL_DEFAULT_INTENSITY 185
+
+static int samsungq10_suspended;

bool?

Quoted 8 lines
drivers/platform/x86/samsung-q10.c new file +8 -0
+
+static void samsungq10_bl_send_intensity(struct backlight_device *bd)
+{
+	int brightness = bd->props.brightness;
+
+	if (!samsungq10_suspended) {
+		while (inb(0x64)&2)
+			;

drivers/platform/x86/samsung-q10.c line 42

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+		outb_p(0xbe, 0x64);
+		while (inb(0x64)&2)
+			;
+		outb_p(0x89, 0x60);
+		while (inb(0x64)&2)
+			;
+		outb_p(0x91, 0x60);
+		while (inb(0x64)&2)
+			;
+		outb_p((unsigned char)brightness, 0x60);

As I mentioned, these registers are owned by i8042, you should not access them directly.

drivers/platform/x86/samsung-q10.c line 43

Quoted 1 line
drivers/platform/x86/samsung-q10.c new file +1 -0
+	};

Stray semicolon.

drivers/platform/x86/samsung-q10.c line 49

Quoted 6 lines
drivers/platform/x86/samsung-q10.c new file +6 -0
+}
+
+#ifdef CONFIG_PM
+static int samsungq10_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	samsungq10_suspended = 1;

= true

drivers/platform/x86/samsung-q10.c line 55

Quoted 6 lines
drivers/platform/x86/samsung-q10.c new file +6 -0
+	return 0;
+}
+
+static int samsungq10_resume(struct platform_device *pdev)
+{
+	struct backlight_device *bd = platform_get_drvdata(pdev);

Blank line before variable definition and code please.

drivers/platform/x86/samsung-q10.c line 56

Quoted 1 line
drivers/platform/x86/samsung-q10.c new file +1 -0
+	samsungq10_suspended = 0;

= false

drivers/platform/x86/samsung-q10.c line 67

Quoted 11 lines
drivers/platform/x86/samsung-q10.c new file +11 -0
+	samsungq10_bl_send_intensity(bd);
+	return 0;
+}
+#else
+#define samsungq10_suspend	NULL
+#define samsungq10_resume	NULL
+#endif
+
+static int samsungq10_bl_set_intensity(struct backlight_device *bd)
+{
+	samsungq10_bl_send_intensity(bd);

Why do you need this? Just have samsungq10_bl_send_intensity() always return 0 (or result of i8042_command()) and use it directly.

drivers/platform/x86/samsung-q10.c line 72

Quoted 5 lines
drivers/platform/x86/samsung-q10.c new file +5 -0
+	return 0;
+}
+
+static int samsungq10_bl_get_intensity(struct backlight_device *bd)
+{

I believe you should either store last brightness and return it or not provide this method.

Quoted 19 lines
drivers/platform/x86/samsung-q10.c new file +19 -0
+	return 0;
+}
+
+static const struct backlight_ops samsungq10_bl_ops = {
+	.get_brightness = samsungq10_bl_get_intensity,
+	.update_status	= samsungq10_bl_set_intensity,
+};
+
+static int __devinit samsungq10_probe(struct platform_device *pdev)
+{
+	struct backlight_properties props;
+	struct backlight_device *bd;
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.max_brightness = SAMSUNGQ10_BL_MAX_INTENSITY;
+	bd = backlight_device_register("samsungq10bl", &pdev->dev, NULL,
+				       &samsungq10_bl_ops, &props);
+	if (IS_ERR(bd))
+		return PTR_ERR(bd);

drivers/platform/x86/samsung-q10.c line 101

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+
+	platform_set_drvdata(pdev, bd);
+
+	bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
+	samsungq10_bl_send_intensity(bd);
+
+	return 0;
+}
+
+static int samsungq10_remove(struct platform_device *pdev)

__devexit

Quoted 4 lines
drivers/platform/x86/samsung-q10.c new file +4 -0
+{
+	struct backlight_device *bd = platform_get_drvdata(pdev);
+
+	bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;

drivers/platform/x86/samsung-q10.c line 115

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+	samsungq10_bl_send_intensity(bd);
+
+	backlight_device_unregister(bd);
+
+	return 0;
+}
+
+static struct platform_driver samsungq10_driver = {
+	.probe		= samsungq10_probe,
+	.remove		= samsungq10_remove,

__devexit_p()

drivers/platform/x86/samsung-q10.c line 117

Quoted 2 lines
drivers/platform/x86/samsung-q10.c new file +2 -0
+	.suspend	= samsungq10_suspend,
+	.resume		= samsungq10_resume,

Should be converted to dev_pm_ops.

drivers/platform/x86/samsung-q10.c line 119

Quoted 2 lines
drivers/platform/x86/samsung-q10.c new file +2 -0
+	.driver		= {
+		.name	= "samsungq10",

.owner = THIS_MODULE,

Quoted 8 lines
drivers/platform/x86/samsung-q10.c new file +8 -0
+	},
+};
+
+static struct platform_device *samsungq10_device;
+
+static int __init samsungq10_init(void)
+{
+	int ret;

drivers/platform/x86/samsung-q10.c line 137

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+
+	ret = platform_driver_register(&samsungq10_driver);
+	if (ret)
+		return ret;
+	samsungq10_device = platform_device_register_simple("samsungq10", -1,
+							    NULL, 0);
+	if (IS_ERR(samsungq10_device)) {
+		platform_driver_unregister(&samsungq10_driver);
+		return PTR_ERR(samsungq10_device);
+	}

Consider using platform_create_bundle().

Quoted 5 lines
drivers/platform/x86/samsung-q10.c new file +5 -0
+	return 0;
+}
+
+static void __exit samsungq10_exit(void)
+{

drivers/platform/x86/samsung-q10.c line 152

Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10 -0
+	platform_device_unregister(samsungq10_device);
+	platform_driver_unregister(&samsungq10_driver);
+}
+
+module_init(samsungq10_init);
+module_exit(samsungq10_exit);
+
+MODULE_AUTHOR("Frederick van der Wyck <[email protected]>");
+MODULE_DESCRIPTION("Samsung Q10 Driver");
+MODULE_LICENSE("GPL");

Thanks.

Signature · 1 line
Dmitry
#4 lore raw

Greg, Dmitry - thank you both for the detailed feedback. I'll make the changes you suggest and submit a new version.