Samsung Laptop driver patches - resend

From: Tommaso Massimi <[email protected]>

my samsung laptop would be very happy if you add these lines to the file drivers/platform/x86/samsung-laptop.c

---
 drivers/platform/x86/samsung-laptop.c |    9 +++++++++
 1 file changed, 9 insertions(+)

@@ -695,6 +695,15 @@ static struct dmi_system_id __initdata s
 		},
 		.callback = dmi_check_cb,
 	},
+		{
+		.ident = "X520",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "X520"),
+			DMI_MATCH(DMI_BOARD_NAME, "X520"),
+		},
+		.callback = dmi_check_cb,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(dmi, samsung_dmi_table);

From: Smelov Andrey <[email protected]>

patch works for me, but I need to add "acpi_backlight=vendor" to kernel params

---
 drivers/platform/x86/samsung-laptop.c |    9 +++++++++
 1 file changed, 9 insertions(+)

@@ -704,6 +704,15 @@ static struct dmi_system_id __initdata s
 		},
 		.callback = dmi_check_cb,
 	},
+	{
+		.ident = "R528/R728",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "R528/R728"),
+			DMI_MATCH(DMI_BOARD_NAME, "R528/R728"),
+		},
+		.callback = dmi_check_cb,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(dmi, samsung_dmi_table);

From: Jason Stubbs <[email protected]>

The min_brightness value of the sabi_config is incorrectly used in brightness calculations. For the config where min_brightness = 1 and max_brightness = 8, the user visible range should be 0 to 7 with hardware being set in the range of 1 to 8. What is actually happening is that the user visible range is 0 to 8 with hardware being set in the range of -1 to 7.

This patch fixes the above issue as well as a miscalculation that would occur in the case of min_brightness > 1.

---
 drivers/platform/x86/samsung-laptop.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

@@ -370,15 +370,17 @@ static u8 read_brightness(void)
 				  &sretval);
 	if (!retval) {
 		user_brightness = sretval.retval[0];
-		if (user_brightness != 0)
+		if (user_brightness > sabi_config->min_brightness)
 			user_brightness -= sabi_config->min_brightness;
+		else
+			user_brightness = 0;
 	}
 	return user_brightness;
 }
 
 static void set_brightness(u8 user_brightness)
 {
-	u8 user_level = user_brightness - sabi_config->min_brightness;
+	u8 user_level = user_brightness + sabi_config->min_brightness;
 
 	sabi_set_command(sabi_config->commands.set_brightness, user_level);
 }

@@ -829,7 +831,8 @@ static int __init samsung_init(void)
 	/* create a backlight device to talk to this one */
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_PLATFORM;
-	props.max_brightness = sabi_config->max_brightness;
+	props.max_brightness = sabi_config->max_brightness -
+				sabi_config->min_brightness;
 	backlight_device = backlight_device_register("samsung", &sdev->dev,
 						     NULL, &backlight_ops,
 						     &props);

From: David Herrmann <[email protected]>

samsung_init() should not return success if not all devices are initialized. Otherwise, samsung_exit() will dereference sdev NULL pointers and others.

---
 drivers/platform/x86/samsung-laptop.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

@@ -799,7 +799,7 @@ static int __init samsung_init(void)
 	sabi_iface = ioremap_nocache(ifaceP, 16);
 	if (!sabi_iface) {
 		pr_err("Can't remap %x\n", ifaceP);
-		goto exit;
+		goto error_no_signature;
 	}
 	if (debug) {
 		printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);

@@ -851,7 +851,6 @@ static int __init samsung_init(void)
 	if (retval)
 		goto error_file_create;
 
-exit:
 	return 0;
 
 error_file_create:

From: Jason Stubbs <[email protected]>

On some Samsung laptops the brightness regulation works slightly different. All SABI commands except for set_brightness work as expected. The behaviour of set_brightness is as follows:

- Setting a new brightness will only step one level toward the new brightness level. For example, setting a level of 5 when the current level is 2 will result in a brightness level of 3.
- A spurious KEY_BRIGHTNESS_UP or KEY_BRIGHTNESS_DOWN event is also generated along with the change in brightness.
- Neither of the above two issues occur when changing from/to brightness level 0.

This patch adds detection and a non-intrusive workaround for the above issues.

---
 drivers/platform/x86/samsung-laptop.c |   43 ++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

@@ -226,6 +226,7 @@ static struct backlight_device *backligh
 static struct mutex sabi_mutex;
 static struct platform_device *sdev;
 static struct rfkill *rfk;
+static bool has_stepping_quirk;
 
 static int force;
 module_param(force, bool, 0);

@@ -382,6 +383,17 @@ static void set_brightness(u8 user_brigh
 {
 	u8 user_level = user_brightness + sabi_config->min_brightness;
 
+	if (has_stepping_quirk && user_level != 0) {
+		/*
+		 * short circuit if the specified level is what's already set
+		 * to prevent the screen from flickering needlessly
+		 */
+		if (user_brightness == read_brightness())
+			return;
+
+		sabi_set_command(sabi_config->commands.set_brightness, 0);
+	}
+
 	sabi_set_command(sabi_config->commands.set_brightness, user_level);
 }
 

@@ -390,6 +402,34 @@ static int get_brightness(struct backlig
 	return (int)read_brightness();
 }
 
+static void check_for_stepping_quirk(void)
+{
+	u8 initial_level = read_brightness();
+	u8 check_level;
+
+	/*
+	 * Some laptops exhibit the strange behaviour of stepping toward
+	 * (rather than setting) the brightness except when changing to/from
+	 * brightness level 0. This behaviour is checked for here and worked
+	 * around in set_brightness.
+	 */
+
+	if (initial_level <= 2)
+		check_level = initial_level + 2;
+	else
+		check_level = initial_level - 2;
+
+	has_stepping_quirk = false;
+	set_brightness(check_level);
+
+	if (read_brightness() != check_level) {
+		has_stepping_quirk = true;
+		pr_info("enabled workaround for brightness stepping quirk\n");
+	}
+
+	set_brightness(initial_level);
+}
+
 static int update_status(struct backlight_device *bd)
 {
 	set_brightness(bd->props.brightness);

@@ -823,6 +863,9 @@ static int __init samsung_init(void)
 		}
 	}
 
+	/* Check for stepping quirk */
+	check_for_stepping_quirk();
+
 	/* knock up a platform device to hang stuff off of */
 	sdev = platform_device_register_simple("samsung", -1, NULL, 0);
 	if (IS_ERR(sdev))

From: Jason Stubbs <[email protected]>

This patch just adds the DMI info for the samsung laptop driver to work with the NC210/NC110. It needs the brightness quirk patch for proper support.

---
 drivers/platform/x86/samsung-laptop.c |    9 +++++++++
 1 file changed, 9 insertions(+)

@@ -755,6 +755,15 @@ static struct dmi_system_id __initdata s
 		},
 		.callback = dmi_check_cb,
 	},
+	{
+		.ident = "NC210/NC110",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "NC210/NC110"),
+			DMI_MATCH(DMI_BOARD_NAME, "NC210/NC110"),
+		},
+		.callback = dmi_check_cb,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(dmi, samsung_dmi_table);

From: Raul Gutierrez Segales <[email protected]>

This is a follow-up for commit 78a7539b, which didn't cover the Samsung N220 laptop. With this backlight brightness works nicely on the N220 netbook.

---
 drivers/platform/x86/samsung-laptop.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

@@ -663,6 +663,16 @@ static struct dmi_system_id __initdata s
 		.callback = dmi_check_cb,
 	},
 	{
+		.ident = "N220",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR,
+					"SAMSUNG ELECTRONICS CO., LTD."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "N220"),
+			DMI_MATCH(DMI_BOARD_NAME, "N220"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
 		.ident = "N150/N210/N220/N230",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR,

From: Stefan Beller <[email protected]>

My DMI model is this:

[    0.000000] DMI present.
[    0.000000] DMI: SAMSUNG ELECTRONICS CO., LTD. SR700/SR700, BIOS

04SR 02/20/2008

adding dmi information of Samsung R700 laptops This adds the dmi information of Samsungs R700 laptops.

---
 drivers/platform/x86/samsung-laptop.c |    9 +++++++++
 1 file changed, 9 insertions(+)

@@ -641,6 +641,15 @@ static struct dmi_system_id __initdata s
 		.callback = dmi_check_cb,
 	},
 	{
+		.ident = "R700",
+		.matches = {
+		      DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+		      DMI_MATCH(DMI_PRODUCT_NAME, "SR700"),
+		      DMI_MATCH(DMI_BOARD_NAME, "SR700"),
+		},
+		.callback = dmi_check_cb,
+	},
+	{
 		.ident = "R530/R730",
 		.matches = {
 		      DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),

Hi Matthew,

Here is a resend of the samsung-laptop patches, with the correct authorship info in it. Sorry about the last batch, 'quilt mail' strips them off when sending them out, despite the information being in the original patch.

The patches are against Linus's current tree, but should apply just fine to linux-next as well.

If there are any problems with them, please let me know.

Oh, and if anyone wants to take over maintainership of this driver, please let me know. I no longer have the hardware to be able to test any changes for it, which is why this took so long to get pushed out to you.

thanks,

greg k-h

Hi Greg,

Just got a 90X3A from Intel and I'll try to take a look at samsung-laptop since some features are missing (keyboard backlight for example).

Do you have any doc on the vendor "interface" (other than just reading the code :) ) ?

Thanks,

Corentin Chary
http://xf.iksaif.net

Try adding your device to this driver. Or, watch out, as newer Samsung devices support backlight control "properly" through acpi, which is good, and this driver is not needed by them at all.

I do not have any documentation for the interface, sorry. The code should be self-explanatory.

greg k-h

I'm speaking about *keyboard* backlight here, not screen backlight (which is already available using both the acpi driver and the intel graphic driver). There is also a setting to enable USB power while sleeping without having to change it in the BIOS, and it's probably possible to control the sound and wlan led.

Arg... I can understand the code, I can easilly write ACPI and WMI drivers, but the "SABI" thing will be harder to reverse engineer :).

Thanks,

Corentin Chary
http://xf.iksaif.net

It was in fact very fun to find out new commands, and I found interesting stuff.

However, I have a quick question: what does "wireless button" control exactly ? Because my laptop (SwSmi@ one), seems to have a totally different command to control rfkill (both for bluetooth and wlan). My guess is that command do the same thing that happens when you press the Fn+F12 key, but if it's really that I'd prefer use the "true" rfkill interface with differencied control for bluetooth and wlan.

I have found other interesting information including:
- you can pass up to 11 byte of data for each command (but I didn't found any command using more than 6)
- there is a way to find the model name and bios version near the signature
- wlan and bluetooth control
- USB charge control
- Keyboard backlight control
- Battery life extender control

I'll try to clean that up and send some patches in one week or two.

Corentin Chary
http://xf.iksaif.net

I think it controlled the wireless on/off on some machines, others might be different. And it should be using rfkill, right?

greg k-h