[PATCH 01/15] samsung-laptop: put all local variables in a single structure

[PATCH 01/15] samsung-laptop: put all local variables in a single structure

[Corentin Chary]

Even if this driver can only be loaded once, it is still a good
idea to create some kind of context structure.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |  291 +++++++++++++++++++--------------
 1 files changed, 170 insertions(+), 121 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 09e26bf..c502252 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -217,16 +217,23 @@ static const struct sabi_config sabi_configs[] = {
 	{ },
 };
 
-static const struct sabi_config *sabi_config;
+struct samsung_laptop {
+	const struct sabi_config *config;
 
-static void __iomem *sabi;
-static void __iomem *sabi_iface;
-static void __iomem *f0000_segment;
-static struct backlight_device *backlight_device;
-static struct mutex sabi_mutex;
-static struct platform_device *sdev;
-static struct rfkill *rfk;
-static bool has_stepping_quirk;
+	void __iomem *sabi;
+	void __iomem *sabi_iface;
+	void __iomem *f0000_segment;
+
+	struct mutex sabi_mutex;
+
+	struct platform_device *pdev;
+	struct backlight_device *backlight_device;
+	struct rfkill *rfk;
+
+	bool has_stepping_quirk;
+};
+
+static struct samsung_laptop *samsung;
 
 static int force;
 module_param(force, bool, 0);
@@ -239,27 +246,28 @@ MODULE_PARM_DESC(debug, "Debug enabled or not");
 
 static int sabi_get_command(u8 command, struct sabi_retval *sretval)
 {
+	const struct sabi_config *config = samsung->config;
 	int retval = 0;
-	u16 port = readw(sabi + sabi_config->header_offsets.port);
+	u16 port = readw(samsung->sabi + config->header_offsets.port);
 	u8 complete, iface_data;
 
-	mutex_lock(&sabi_mutex);
+	mutex_lock(&samsung->sabi_mutex);
 
 	/* enable memory to be able to write to it */
-	outb(readb(sabi + sabi_config->header_offsets.en_mem), port);
+	outb(readb(samsung->sabi + config->header_offsets.en_mem), port);
 
 	/* write out the command */
-	writew(sabi_config->main_function, sabi_iface + SABI_IFACE_MAIN);
-	writew(command, sabi_iface + SABI_IFACE_SUB);
-	writeb(0, sabi_iface + SABI_IFACE_COMPLETE);
-	outb(readb(sabi + sabi_config->header_offsets.iface_func), port);
+	writew(config->main_function, samsung->sabi_iface + SABI_IFACE_MAIN);
+	writew(command, samsung->sabi_iface + SABI_IFACE_SUB);
+	writeb(0, samsung->sabi_iface + SABI_IFACE_COMPLETE);
+	outb(readb(samsung->sabi + config->header_offsets.iface_func), port);
 
 	/* write protect memory to make it safe */
-	outb(readb(sabi + sabi_config->header_offsets.re_mem), port);
+	outb(readb(samsung->sabi + config->header_offsets.re_mem), port);
 
 	/* see if the command actually succeeded */
-	complete = readb(sabi_iface + SABI_IFACE_COMPLETE);
-	iface_data = readb(sabi_iface + SABI_IFACE_DATA);
+	complete = readb(samsung->sabi_iface + SABI_IFACE_COMPLETE);
+	iface_data = readb(samsung->sabi_iface + SABI_IFACE_DATA);
 	if (complete != 0xaa || iface_data == 0xff) {
 		pr_warn("SABI get command 0x%02x failed with completion flag 0x%02x and data 0x%02x\n",
 		        command, complete, iface_data);
@@ -272,107 +280,112 @@ static int sabi_get_command(u8 command, struct sabi_retval *sretval)
 	 * There are commands that need more, but not for the ones we
 	 * currently care about.
 	 */
-	sretval->retval[0] = readb(sabi_iface + SABI_IFACE_DATA);
-	sretval->retval[1] = readb(sabi_iface + SABI_IFACE_DATA + 1);
-	sretval->retval[2] = readb(sabi_iface + SABI_IFACE_DATA + 2);
-	sretval->retval[3] = readb(sabi_iface + SABI_IFACE_DATA + 3);
+	sretval->retval[0] = readb(samsung->sabi_iface + SABI_IFACE_DATA);
+	sretval->retval[1] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 1);
+	sretval->retval[2] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 2);
+	sretval->retval[3] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 3);
 
 exit:
-	mutex_unlock(&sabi_mutex);
+	mutex_unlock(&samsung->sabi_mutex);
 	return retval;
 
 }
 
 static int sabi_set_command(u8 command, u8 data)
 {
+	const struct sabi_config *config = samsung->config;
 	int retval = 0;
-	u16 port = readw(sabi + sabi_config->header_offsets.port);
+	u16 port = readw(samsung->sabi + config->header_offsets.port);
 	u8 complete, iface_data;
 
-	mutex_lock(&sabi_mutex);
+	mutex_lock(&samsung->sabi_mutex);
 
 	/* enable memory to be able to write to it */
-	outb(readb(sabi + sabi_config->header_offsets.en_mem), port);
+	outb(readb(samsung->sabi + config->header_offsets.en_mem), port);
 
 	/* write out the command */
-	writew(sabi_config->main_function, sabi_iface + SABI_IFACE_MAIN);
-	writew(command, sabi_iface + SABI_IFACE_SUB);
-	writeb(0, sabi_iface + SABI_IFACE_COMPLETE);
-	writeb(data, sabi_iface + SABI_IFACE_DATA);
-	outb(readb(sabi + sabi_config->header_offsets.iface_func), port);
+	writew(config->main_function, samsung->sabi_iface + SABI_IFACE_MAIN);
+	writew(command, samsung->sabi_iface + SABI_IFACE_SUB);
+	writeb(0, samsung->sabi_iface + SABI_IFACE_COMPLETE);
+	writeb(data, samsung->sabi_iface + SABI_IFACE_DATA);
+	outb(readb(samsung->sabi + config->header_offsets.iface_func), port);
 
 	/* write protect memory to make it safe */
-	outb(readb(sabi + sabi_config->header_offsets.re_mem), port);
+	outb(readb(samsung->sabi + config->header_offsets.re_mem), port);
 
 	/* see if the command actually succeeded */
-	complete = readb(sabi_iface + SABI_IFACE_COMPLETE);
-	iface_data = readb(sabi_iface + SABI_IFACE_DATA);
+	complete = readb(samsung->sabi_iface + SABI_IFACE_COMPLETE);
+	iface_data = readb(samsung->sabi_iface + SABI_IFACE_DATA);
 	if (complete != 0xaa || iface_data == 0xff) {
 		pr_warn("SABI set command 0x%02x failed with completion flag 0x%02x and data 0x%02x\n",
 		       command, complete, iface_data);
 		retval = -EINVAL;
 	}
 
-	mutex_unlock(&sabi_mutex);
+	mutex_unlock(&samsung->sabi_mutex);
 	return retval;
 }
 
 static void test_backlight(void)
 {
+	const struct sabi_commands *commands = &samsung->config->commands;
 	struct sabi_retval sretval;
 
-	sabi_get_command(sabi_config->commands.get_backlight, &sretval);
+	sabi_get_command(commands->get_backlight, &sretval);
 	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
 
-	sabi_set_command(sabi_config->commands.set_backlight, 0);
+	sabi_set_command(commands->set_backlight, 0);
 	printk(KERN_DEBUG "backlight should be off\n");
 
-	sabi_get_command(sabi_config->commands.get_backlight, &sretval);
+	sabi_get_command(commands->get_backlight, &sretval);
 	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
 
 	msleep(1000);
 
-	sabi_set_command(sabi_config->commands.set_backlight, 1);
+	sabi_set_command(commands->set_backlight, 1);
 	printk(KERN_DEBUG "backlight should be on\n");
 
-	sabi_get_command(sabi_config->commands.get_backlight, &sretval);
+	sabi_get_command(commands->get_backlight, &sretval);
 	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
 }
 
 static void test_wireless(void)
 {
+	const struct sabi_commands *commands = &samsung->config->commands;
 	struct sabi_retval sretval;
 
-	sabi_get_command(sabi_config->commands.get_wireless_button, &sretval);
+	sabi_get_command(commands->get_wireless_button, &sretval);
 	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
 
-	sabi_set_command(sabi_config->commands.set_wireless_button, 0);
+	sabi_set_command(commands->set_wireless_button, 0);
 	printk(KERN_DEBUG "wireless led should be off\n");
 
-	sabi_get_command(sabi_config->commands.get_wireless_button, &sretval);
+	sabi_get_command(commands->get_wireless_button, &sretval);
 	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
 
 	msleep(1000);
 
-	sabi_set_command(sabi_config->commands.set_wireless_button, 1);
+	sabi_set_command(commands->set_wireless_button, 1);
 	printk(KERN_DEBUG "wireless led should be on\n");
 
-	sabi_get_command(sabi_config->commands.get_wireless_button, &sretval);
+	sabi_get_command(commands->get_wireless_button, &sretval);
 	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
 }
 
 static u8 read_brightness(void)
 {
+	const struct sabi_config *config = samsung->config;
+	const struct sabi_commands *commands = &samsung->config->commands;
 	struct sabi_retval sretval;
 	int user_brightness = 0;
 	int retval;
 
-	retval = sabi_get_command(sabi_config->commands.get_brightness,
+	retval = sabi_get_command(commands->get_brightness,
 				  &sretval);
 	if (!retval) {
 		user_brightness = sretval.retval[0];
-		if (user_brightness > sabi_config->min_brightness)
-			user_brightness -= sabi_config->min_brightness;
+		if (user_brightness > config->min_brightness)
+			user_brightness -= config->min_brightness;
 		else
 			user_brightness = 0;
 	}
@@ -381,9 +394,11 @@ static u8 read_brightness(void)
 
 static void set_brightness(u8 user_brightness)
 {
-	u8 user_level = user_brightness + sabi_config->min_brightness;
+	const struct sabi_config *config = samsung->config;
+	const struct sabi_commands *commands = &samsung->config->commands;
+	u8 user_level = user_brightness + config->min_brightness;
 
-	if (has_stepping_quirk && user_level != 0) {
+	if (samsung->has_stepping_quirk && user_level != 0) {
 		/*
 		 * short circuit if the specified level is what's already set
 		 * to prevent the screen from flickering needlessly
@@ -391,10 +406,10 @@ static void set_brightness(u8 user_brightness)
 		if (user_brightness == read_brightness())
 			return;
 
-		sabi_set_command(sabi_config->commands.set_brightness, 0);
+		sabi_set_command(commands->set_brightness, 0);
 	}
 
-	sabi_set_command(sabi_config->commands.set_brightness, user_level);
+	sabi_set_command(commands->set_brightness, user_level);
 }
 
 static int get_brightness(struct backlight_device *bd)
@@ -425,11 +440,11 @@ static void check_for_stepping_quirk(void)
 	else
 		check_level = initial_level - 2;
 
-	has_stepping_quirk = false;
+	samsung->has_stepping_quirk = false;
 	set_brightness(check_level);
 
 	if (read_brightness() != check_level) {
-		has_stepping_quirk = true;
+		samsung->has_stepping_quirk = true;
 		pr_info("enabled workaround for brightness stepping quirk\n");
 	}
 
@@ -438,12 +453,14 @@ static void check_for_stepping_quirk(void)
 
 static int update_status(struct backlight_device *bd)
 {
+	const struct sabi_commands *commands = &samsung->config->commands;
+
 	set_brightness(bd->props.brightness);
 
 	if (bd->props.power == FB_BLANK_UNBLANK)
-		sabi_set_command(sabi_config->commands.set_backlight, 1);
+		sabi_set_command(commands->set_backlight, 1);
 	else
-		sabi_set_command(sabi_config->commands.set_backlight, 0);
+		sabi_set_command(commands->set_backlight, 0);
 	return 0;
 }
 
@@ -454,15 +471,17 @@ static const struct backlight_ops backlight_ops = {
 
 static int rfkill_set(void *data, bool blocked)
 {
+	const struct sabi_commands *commands = &samsung->config->commands;
+
 	/* Do something with blocked...*/
 	/*
 	 * blocked == false is on
 	 * blocked == true is off
 	 */
 	if (blocked)
-		sabi_set_command(sabi_config->commands.set_wireless_button, 0);
+		sabi_set_command(commands->set_wireless_button, 0);
 	else
-		sabi_set_command(sabi_config->commands.set_wireless_button, 1);
+		sabi_set_command(commands->set_wireless_button, 1);
 
 	return 0;
 }
@@ -471,18 +490,18 @@ static struct rfkill_ops rfkill_ops = {
 	.set_block = rfkill_set,
 };
 
-static int init_wireless(struct platform_device *sdev)
+static int init_wireless(struct platform_device *pdev)
 {
 	int retval;
 
-	rfk = rfkill_alloc("samsung-wifi", &sdev->dev, RFKILL_TYPE_WLAN,
-			   &rfkill_ops, NULL);
-	if (!rfk)
+	samsung->rfk = rfkill_alloc("samsung-wifi", &samsung->pdev->dev, RFKILL_TYPE_WLAN,
+				    &rfkill_ops, NULL);
+	if (!samsung->rfk)
 		return -ENOMEM;
 
-	retval = rfkill_register(rfk);
+	retval = rfkill_register(samsung->rfk);
 	if (retval) {
-		rfkill_destroy(rfk);
+		rfkill_destroy(samsung->rfk);
 		return -ENODEV;
 	}
 
@@ -491,27 +510,28 @@ static int init_wireless(struct platform_device *sdev)
 
 static void destroy_wireless(void)
 {
-	rfkill_unregister(rfk);
-	rfkill_destroy(rfk);
+	rfkill_unregister(samsung->rfk);
+	rfkill_destroy(samsung->rfk);
 }
 
 static ssize_t get_performance_level(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
+	const struct sabi_config *config = samsung->config;
 	struct sabi_retval sretval;
 	int retval;
 	int i;
 
 	/* Read the state */
-	retval = sabi_get_command(sabi_config->commands.get_performance_level,
+	retval = sabi_get_command(config->commands.get_performance_level,
 				  &sretval);
 	if (retval)
 		return retval;
 
 	/* The logic is backwards, yeah, lots of fun... */
-	for (i = 0; sabi_config->performance_levels[i].name; ++i) {
-		if (sretval.retval[0] == sabi_config->performance_levels[i].value)
-			return sprintf(buf, "%s\n", sabi_config->performance_levels[i].name);
+	for (i = 0; config->performance_levels[i].name; ++i) {
+		if (sretval.retval[0] == config->performance_levels[i].value)
+			return sprintf(buf, "%s\n", config->performance_levels[i].name);
 	}
 	return sprintf(buf, "%s\n", "unknown");
 }
@@ -520,18 +540,20 @@ static ssize_t set_performance_level(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
+	const struct sabi_config *config = samsung->config;
+
 	if (count >= 1) {
 		int i;
-		for (i = 0; sabi_config->performance_levels[i].name; ++i) {
+		for (i = 0; config->performance_levels[i].name; ++i) {
 			const struct sabi_performance_level *level =
-				&sabi_config->performance_levels[i];
+				&config->performance_levels[i];
 			if (!strncasecmp(level->name, buf, strlen(level->name))) {
-				sabi_set_command(sabi_config->commands.set_performance_level,
+				sabi_set_command(config->commands.set_performance_level,
 						 level->value);
 				break;
 			}
 		}
-		if (!sabi_config->performance_levels[i].name)
+		if (!config->performance_levels[i].name)
 			return -EINVAL;
 	}
 	return count;
@@ -805,6 +827,9 @@ static int find_signature(void __iomem *memcheck, const char *testStr)
 
 static int __init samsung_init(void)
 {
+	const struct sabi_config *config = NULL;
+	const struct sabi_commands *commands;
+	struct backlight_device *bd;
 	struct backlight_properties props;
 	struct sabi_retval sretval;
 	unsigned int ifaceP;
@@ -812,21 +837,26 @@ static int __init samsung_init(void)
 	int loca;
 	int retval;
 
-	mutex_init(&sabi_mutex);
-
 	if (!force && !dmi_check_system(samsung_dmi_table))
 		return -ENODEV;
 
-	f0000_segment = ioremap_nocache(0xf0000, 0xffff);
-	if (!f0000_segment) {
+	samsung = kzalloc(sizeof(*samsung), GFP_KERNEL);
+	if (!samsung)
+		return -ENOMEM;
+
+	mutex_init(&samsung->sabi_mutex);
+
+	samsung->f0000_segment = ioremap_nocache(0xf0000, 0xffff);
+	if (!samsung->f0000_segment) {
 		pr_err("Can't map the segment at 0xf0000\n");
-		return -EINVAL;
+		goto error_cant_map;
 	}
 
 	/* Try to find one of the signatures in memory to find the header */
 	for (i = 0; sabi_configs[i].test_string != 0; ++i) {
-		sabi_config = &sabi_configs[i];
-		loca = find_signature(f0000_segment, sabi_config->test_string);
+		samsung->config = &sabi_configs[i];
+		loca = find_signature(samsung->f0000_segment,
+				      samsung->config->test_string);
 		if (loca != 0xffff)
 			break;
 	}
@@ -836,51 +866,60 @@ static int __init samsung_init(void)
 		goto error_no_signature;
 	}
 
+	config = samsung->config;
+	commands = &config->commands;
+
 	/* point to the SMI port Number */
 	loca += 1;
-	sabi = (f0000_segment + loca);
+	samsung->sabi = (samsung->f0000_segment + loca);
 
 	if (debug) {
 		printk(KERN_DEBUG "This computer supports SABI==%x\n",
 			loca + 0xf0000 - 6);
 		printk(KERN_DEBUG "SABI header:\n");
 		printk(KERN_DEBUG " SMI Port Number = 0x%04x\n",
-			readw(sabi + sabi_config->header_offsets.port));
+		       readw(samsung->sabi +
+			     config->header_offsets.port));
 		printk(KERN_DEBUG " SMI Interface Function = 0x%02x\n",
-			readb(sabi + sabi_config->header_offsets.iface_func));
+		       readb(samsung->sabi +
+			     config->header_offsets.iface_func));
 		printk(KERN_DEBUG " SMI enable memory buffer = 0x%02x\n",
-			readb(sabi + sabi_config->header_offsets.en_mem));
+		       readb(samsung->sabi +
+			     config->header_offsets.en_mem));
 		printk(KERN_DEBUG " SMI restore memory buffer = 0x%02x\n",
-			readb(sabi + sabi_config->header_offsets.re_mem));
+		       readb(samsung->sabi +
+			     config->header_offsets.re_mem));
 		printk(KERN_DEBUG " SABI data offset = 0x%04x\n",
-			readw(sabi + sabi_config->header_offsets.data_offset));
+		       readw(samsung->sabi +
+			     config->header_offsets.data_offset));
 		printk(KERN_DEBUG " SABI data segment = 0x%04x\n",
-			readw(sabi + sabi_config->header_offsets.data_segment));
+		       readw(samsung->sabi +
+			     config->header_offsets.data_segment));
 	}
 
 	/* Get a pointer to the SABI Interface */
-	ifaceP = (readw(sabi + sabi_config->header_offsets.data_segment) & 0x0ffff) << 4;
-	ifaceP += readw(sabi + sabi_config->header_offsets.data_offset) & 0x0ffff;
-	sabi_iface = ioremap_nocache(ifaceP, 16);
-	if (!sabi_iface) {
+	ifaceP = (readw(samsung->sabi + config->header_offsets.data_segment) & 0x0ffff) << 4;
+	ifaceP += readw(samsung->sabi + config->header_offsets.data_offset) & 0x0ffff;
+	samsung->sabi_iface = ioremap_nocache(ifaceP, 16);
+	if (!samsung->sabi_iface) {
 		pr_err("Can't remap %x\n", ifaceP);
 		goto error_no_signature;
 	}
 	if (debug) {
 		printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
-		printk(KERN_DEBUG "sabi_iface = %p\n", sabi_iface);
+		printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
 
 		test_backlight();
 		test_wireless();
 
-		retval = sabi_get_command(sabi_config->commands.get_brightness,
+		retval = sabi_get_command(commands->get_brightness,
 					  &sretval);
 		printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.retval[0]);
 	}
 
 	/* Turn on "Linux" mode in the BIOS */
-	if (sabi_config->commands.set_linux != 0xff) {
-		retval = sabi_set_command(sabi_config->commands.set_linux,
+	if (commands->set_linux != 0xff) {
+		retval = sabi_set_command(commands->set_linux,
 					  0x81);
 		if (retval) {
 			pr_warn("Linux mode was not set!\n");
@@ -892,30 +931,32 @@ static int __init samsung_init(void)
 	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))
+	samsung->pdev = platform_device_register_simple("samsung", -1, NULL, 0);
+	if (IS_ERR(samsung->pdev))
 		goto error_no_platform;
 
 	/* 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 -
-				sabi_config->min_brightness;
-	backlight_device = backlight_device_register("samsung", &sdev->dev,
-						     NULL, &backlight_ops,
-						     &props);
-	if (IS_ERR(backlight_device))
+	props.max_brightness = config->max_brightness -
+				config->min_brightness;
+	bd = backlight_device_register("samsung", &samsung->pdev->dev,
+				       NULL, &backlight_ops,
+				       &props);
+	if (IS_ERR(bd))
 		goto error_no_backlight;
 
-	backlight_device->props.brightness = read_brightness();
-	backlight_device->props.power = FB_BLANK_UNBLANK;
-	backlight_update_status(backlight_device);
+	samsung->backlight_device = bd;
+	samsung->backlight_device->props.brightness = read_brightness();
+	samsung->backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(samsung->backlight_device);
 
-	retval = init_wireless(sdev);
+	retval = init_wireless(samsung->pdev);
 	if (retval)
 		goto error_no_rfk;
 
-	retval = device_create_file(&sdev->dev, &dev_attr_performance_level);
+	retval = device_create_file(&samsung->pdev->dev,
+				    &dev_attr_performance_level);
 	if (retval)
 		goto error_file_create;
 
@@ -925,31 +966,39 @@ error_file_create:
 	destroy_wireless();
 
 error_no_rfk:
-	backlight_device_unregister(backlight_device);
+	backlight_device_unregister(samsung->backlight_device);
 
 error_no_backlight:
-	platform_device_unregister(sdev);
+	platform_device_unregister(samsung->pdev);
 
 error_no_platform:
-	iounmap(sabi_iface);
+	iounmap(samsung->sabi_iface);
 
 error_no_signature:
-	iounmap(f0000_segment);
+	iounmap(samsung->f0000_segment);
+
+error_cant_map:
+	kfree(samsung);
+	samsung = NULL;
 	return -EINVAL;
 }
 
 static void __exit samsung_exit(void)
 {
+	const struct sabi_commands *commands = &samsung->config->commands;
+
 	/* Turn off "Linux" mode in the BIOS */
-	if (sabi_config->commands.set_linux != 0xff)
-		sabi_set_command(sabi_config->commands.set_linux, 0x80);
+	if (commands->set_linux != 0xff)
+		sabi_set_command(commands->set_linux, 0x80);
 
-	device_remove_file(&sdev->dev, &dev_attr_performance_level);
-	backlight_device_unregister(backlight_device);
+	device_remove_file(&samsung->pdev->dev, &dev_attr_performance_level);
+	backlight_device_unregister(samsung->backlight_device);
 	destroy_wireless();
-	iounmap(sabi_iface);
-	iounmap(f0000_segment);
-	platform_device_unregister(sdev);
+	iounmap(samsung->sabi_iface);
+	iounmap(samsung->f0000_segment);
+	platform_device_unregister(samsung->pdev);
+	kfree(samsung);
+	samsung = NULL;
 }
 
 module_init(samsung_init);
-- 
1.7.5.4

Re: [PATCH 01/15] samsung-laptop: put all local variables in a single structure

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:32PM +0100, Corentin Chary wrote:
> Even if this driver can only be loaded once, it is still a good
> idea to create some kind of context structure.
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>


> ---
>  drivers/platform/x86/samsung-laptop.c |  291 +++++++++++++++++++--------------
>  1 files changed, 170 insertions(+), 121 deletions(-)
> 
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index 09e26bf..c502252 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -217,16 +217,23 @@ static const struct sabi_config sabi_configs[] = {
>  	{ },
>  };
>  
> -static const struct sabi_config *sabi_config;
> +struct samsung_laptop {
> +	const struct sabi_config *config;
>  
> -static void __iomem *sabi;
> -static void __iomem *sabi_iface;
> -static void __iomem *f0000_segment;
> -static struct backlight_device *backlight_device;
> -static struct mutex sabi_mutex;
> -static struct platform_device *sdev;
> -static struct rfkill *rfk;
> -static bool has_stepping_quirk;
> +	void __iomem *sabi;
> +	void __iomem *sabi_iface;
> +	void __iomem *f0000_segment;
> +
> +	struct mutex sabi_mutex;
> +
> +	struct platform_device *pdev;
> +	struct backlight_device *backlight_device;
> +	struct rfkill *rfk;
> +
> +	bool has_stepping_quirk;
> +};
> +
> +static struct samsung_laptop *samsung;
>  
>  static int force;
>  module_param(force, bool, 0);
> @@ -239,27 +246,28 @@ MODULE_PARM_DESC(debug, "Debug enabled or not");
>  
>  static int sabi_get_command(u8 command, struct sabi_retval *sretval)
>  {
> +	const struct sabi_config *config = samsung->config;
>  	int retval = 0;
> -	u16 port = readw(sabi + sabi_config->header_offsets.port);
> +	u16 port = readw(samsung->sabi + config->header_offsets.port);
>  	u8 complete, iface_data;
>  
> -	mutex_lock(&sabi_mutex);
> +	mutex_lock(&samsung->sabi_mutex);
>  
>  	/* enable memory to be able to write to it */
> -	outb(readb(sabi + sabi_config->header_offsets.en_mem), port);
> +	outb(readb(samsung->sabi + config->header_offsets.en_mem), port);
>  
>  	/* write out the command */
> -	writew(sabi_config->main_function, sabi_iface + SABI_IFACE_MAIN);
> -	writew(command, sabi_iface + SABI_IFACE_SUB);
> -	writeb(0, sabi_iface + SABI_IFACE_COMPLETE);
> -	outb(readb(sabi + sabi_config->header_offsets.iface_func), port);
> +	writew(config->main_function, samsung->sabi_iface + SABI_IFACE_MAIN);
> +	writew(command, samsung->sabi_iface + SABI_IFACE_SUB);
> +	writeb(0, samsung->sabi_iface + SABI_IFACE_COMPLETE);
> +	outb(readb(samsung->sabi + config->header_offsets.iface_func), port);
>  
>  	/* write protect memory to make it safe */
> -	outb(readb(sabi + sabi_config->header_offsets.re_mem), port);
> +	outb(readb(samsung->sabi + config->header_offsets.re_mem), port);
>  
>  	/* see if the command actually succeeded */
> -	complete = readb(sabi_iface + SABI_IFACE_COMPLETE);
> -	iface_data = readb(sabi_iface + SABI_IFACE_DATA);
> +	complete = readb(samsung->sabi_iface + SABI_IFACE_COMPLETE);
> +	iface_data = readb(samsung->sabi_iface + SABI_IFACE_DATA);
>  	if (complete != 0xaa || iface_data == 0xff) {
>  		pr_warn("SABI get command 0x%02x failed with completion flag 0x%02x and data 0x%02x\n",
>  		        command, complete, iface_data);
> @@ -272,107 +280,112 @@ static int sabi_get_command(u8 command, struct sabi_retval *sretval)
>  	 * There are commands that need more, but not for the ones we
>  	 * currently care about.
>  	 */
> -	sretval->retval[0] = readb(sabi_iface + SABI_IFACE_DATA);
> -	sretval->retval[1] = readb(sabi_iface + SABI_IFACE_DATA + 1);
> -	sretval->retval[2] = readb(sabi_iface + SABI_IFACE_DATA + 2);
> -	sretval->retval[3] = readb(sabi_iface + SABI_IFACE_DATA + 3);
> +	sretval->retval[0] = readb(samsung->sabi_iface + SABI_IFACE_DATA);
> +	sretval->retval[1] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 1);
> +	sretval->retval[2] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 2);
> +	sretval->retval[3] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 3);
>  
>  exit:
> -	mutex_unlock(&sabi_mutex);
> +	mutex_unlock(&samsung->sabi_mutex);
>  	return retval;
>  
>  }
>  
>  static int sabi_set_command(u8 command, u8 data)
>  {
> +	const struct sabi_config *config = samsung->config;
>  	int retval = 0;
> -	u16 port = readw(sabi + sabi_config->header_offsets.port);
> +	u16 port = readw(samsung->sabi + config->header_offsets.port);
>  	u8 complete, iface_data;
>  
> -	mutex_lock(&sabi_mutex);
> +	mutex_lock(&samsung->sabi_mutex);
>  
>  	/* enable memory to be able to write to it */
> -	outb(readb(sabi + sabi_config->header_offsets.en_mem), port);
> +	outb(readb(samsung->sabi + config->header_offsets.en_mem), port);
>  
>  	/* write out the command */
> -	writew(sabi_config->main_function, sabi_iface + SABI_IFACE_MAIN);
> -	writew(command, sabi_iface + SABI_IFACE_SUB);
> -	writeb(0, sabi_iface + SABI_IFACE_COMPLETE);
> -	writeb(data, sabi_iface + SABI_IFACE_DATA);
> -	outb(readb(sabi + sabi_config->header_offsets.iface_func), port);
> +	writew(config->main_function, samsung->sabi_iface + SABI_IFACE_MAIN);
> +	writew(command, samsung->sabi_iface + SABI_IFACE_SUB);
> +	writeb(0, samsung->sabi_iface + SABI_IFACE_COMPLETE);
> +	writeb(data, samsung->sabi_iface + SABI_IFACE_DATA);
> +	outb(readb(samsung->sabi + config->header_offsets.iface_func), port);
>  
>  	/* write protect memory to make it safe */
> -	outb(readb(sabi + sabi_config->header_offsets.re_mem), port);
> +	outb(readb(samsung->sabi + config->header_offsets.re_mem), port);
>  
>  	/* see if the command actually succeeded */
> -	complete = readb(sabi_iface + SABI_IFACE_COMPLETE);
> -	iface_data = readb(sabi_iface + SABI_IFACE_DATA);
> +	complete = readb(samsung->sabi_iface + SABI_IFACE_COMPLETE);
> +	iface_data = readb(samsung->sabi_iface + SABI_IFACE_DATA);
>  	if (complete != 0xaa || iface_data == 0xff) {
>  		pr_warn("SABI set command 0x%02x failed with completion flag 0x%02x and data 0x%02x\n",
>  		       command, complete, iface_data);
>  		retval = -EINVAL;
>  	}
>  
> -	mutex_unlock(&sabi_mutex);
> +	mutex_unlock(&samsung->sabi_mutex);
>  	return retval;
>  }
>  
>  static void test_backlight(void)
>  {
> +	const struct sabi_commands *commands = &samsung->config->commands;
>  	struct sabi_retval sretval;
>  
> -	sabi_get_command(sabi_config->commands.get_backlight, &sretval);
> +	sabi_get_command(commands->get_backlight, &sretval);
>  	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
>  
> -	sabi_set_command(sabi_config->commands.set_backlight, 0);
> +	sabi_set_command(commands->set_backlight, 0);
>  	printk(KERN_DEBUG "backlight should be off\n");
>  
> -	sabi_get_command(sabi_config->commands.get_backlight, &sretval);
> +	sabi_get_command(commands->get_backlight, &sretval);
>  	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
>  
>  	msleep(1000);
>  
> -	sabi_set_command(sabi_config->commands.set_backlight, 1);
> +	sabi_set_command(commands->set_backlight, 1);
>  	printk(KERN_DEBUG "backlight should be on\n");
>  
> -	sabi_get_command(sabi_config->commands.get_backlight, &sretval);
> +	sabi_get_command(commands->get_backlight, &sretval);
>  	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
>  }
>  
>  static void test_wireless(void)
>  {
> +	const struct sabi_commands *commands = &samsung->config->commands;
>  	struct sabi_retval sretval;
>  
> -	sabi_get_command(sabi_config->commands.get_wireless_button, &sretval);
> +	sabi_get_command(commands->get_wireless_button, &sretval);
>  	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
>  
> -	sabi_set_command(sabi_config->commands.set_wireless_button, 0);
> +	sabi_set_command(commands->set_wireless_button, 0);
>  	printk(KERN_DEBUG "wireless led should be off\n");
>  
> -	sabi_get_command(sabi_config->commands.get_wireless_button, &sretval);
> +	sabi_get_command(commands->get_wireless_button, &sretval);
>  	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
>  
>  	msleep(1000);
>  
> -	sabi_set_command(sabi_config->commands.set_wireless_button, 1);
> +	sabi_set_command(commands->set_wireless_button, 1);
>  	printk(KERN_DEBUG "wireless led should be on\n");
>  
> -	sabi_get_command(sabi_config->commands.get_wireless_button, &sretval);
> +	sabi_get_command(commands->get_wireless_button, &sretval);
>  	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
>  }
>  
>  static u8 read_brightness(void)
>  {
> +	const struct sabi_config *config = samsung->config;
> +	const struct sabi_commands *commands = &samsung->config->commands;
>  	struct sabi_retval sretval;
>  	int user_brightness = 0;
>  	int retval;
>  
> -	retval = sabi_get_command(sabi_config->commands.get_brightness,
> +	retval = sabi_get_command(commands->get_brightness,
>  				  &sretval);
>  	if (!retval) {
>  		user_brightness = sretval.retval[0];
> -		if (user_brightness > sabi_config->min_brightness)
> -			user_brightness -= sabi_config->min_brightness;
> +		if (user_brightness > config->min_brightness)
> +			user_brightness -= config->min_brightness;
>  		else
>  			user_brightness = 0;
>  	}
> @@ -381,9 +394,11 @@ static u8 read_brightness(void)
>  
>  static void set_brightness(u8 user_brightness)
>  {
> -	u8 user_level = user_brightness + sabi_config->min_brightness;
> +	const struct sabi_config *config = samsung->config;
> +	const struct sabi_commands *commands = &samsung->config->commands;
> +	u8 user_level = user_brightness + config->min_brightness;
>  
> -	if (has_stepping_quirk && user_level != 0) {
> +	if (samsung->has_stepping_quirk && user_level != 0) {
>  		/*
>  		 * short circuit if the specified level is what's already set
>  		 * to prevent the screen from flickering needlessly
> @@ -391,10 +406,10 @@ static void set_brightness(u8 user_brightness)
>  		if (user_brightness == read_brightness())
>  			return;
>  
> -		sabi_set_command(sabi_config->commands.set_brightness, 0);
> +		sabi_set_command(commands->set_brightness, 0);
>  	}
>  
> -	sabi_set_command(sabi_config->commands.set_brightness, user_level);
> +	sabi_set_command(commands->set_brightness, user_level);
>  }
>  
>  static int get_brightness(struct backlight_device *bd)
> @@ -425,11 +440,11 @@ static void check_for_stepping_quirk(void)
>  	else
>  		check_level = initial_level - 2;
>  
> -	has_stepping_quirk = false;
> +	samsung->has_stepping_quirk = false;
>  	set_brightness(check_level);
>  
>  	if (read_brightness() != check_level) {
> -		has_stepping_quirk = true;
> +		samsung->has_stepping_quirk = true;
>  		pr_info("enabled workaround for brightness stepping quirk\n");
>  	}
>  
> @@ -438,12 +453,14 @@ static void check_for_stepping_quirk(void)
>  
>  static int update_status(struct backlight_device *bd)
>  {
> +	const struct sabi_commands *commands = &samsung->config->commands;
> +
>  	set_brightness(bd->props.brightness);
>  
>  	if (bd->props.power == FB_BLANK_UNBLANK)
> -		sabi_set_command(sabi_config->commands.set_backlight, 1);
> +		sabi_set_command(commands->set_backlight, 1);
>  	else
> -		sabi_set_command(sabi_config->commands.set_backlight, 0);
> +		sabi_set_command(commands->set_backlight, 0);
>  	return 0;
>  }
>  
> @@ -454,15 +471,17 @@ static const struct backlight_ops backlight_ops = {
>  
>  static int rfkill_set(void *data, bool blocked)
>  {
> +	const struct sabi_commands *commands = &samsung->config->commands;
> +
>  	/* Do something with blocked...*/
>  	/*
>  	 * blocked == false is on
>  	 * blocked == true is off
>  	 */
>  	if (blocked)
> -		sabi_set_command(sabi_config->commands.set_wireless_button, 0);
> +		sabi_set_command(commands->set_wireless_button, 0);
>  	else
> -		sabi_set_command(sabi_config->commands.set_wireless_button, 1);
> +		sabi_set_command(commands->set_wireless_button, 1);
>  
>  	return 0;
>  }
> @@ -471,18 +490,18 @@ static struct rfkill_ops rfkill_ops = {
>  	.set_block = rfkill_set,
>  };
>  
> -static int init_wireless(struct platform_device *sdev)
> +static int init_wireless(struct platform_device *pdev)
>  {
>  	int retval;
>  
> -	rfk = rfkill_alloc("samsung-wifi", &sdev->dev, RFKILL_TYPE_WLAN,
> -			   &rfkill_ops, NULL);
> -	if (!rfk)
> +	samsung->rfk = rfkill_alloc("samsung-wifi", &samsung->pdev->dev, RFKILL_TYPE_WLAN,
> +				    &rfkill_ops, NULL);
> +	if (!samsung->rfk)
>  		return -ENOMEM;
>  
> -	retval = rfkill_register(rfk);
> +	retval = rfkill_register(samsung->rfk);
>  	if (retval) {
> -		rfkill_destroy(rfk);
> +		rfkill_destroy(samsung->rfk);
>  		return -ENODEV;
>  	}
>  
> @@ -491,27 +510,28 @@ static int init_wireless(struct platform_device *sdev)
>  
>  static void destroy_wireless(void)
>  {
> -	rfkill_unregister(rfk);
> -	rfkill_destroy(rfk);
> +	rfkill_unregister(samsung->rfk);
> +	rfkill_destroy(samsung->rfk);
>  }
>  
>  static ssize_t get_performance_level(struct device *dev,
>  				     struct device_attribute *attr, char *buf)
>  {
> +	const struct sabi_config *config = samsung->config;
>  	struct sabi_retval sretval;
>  	int retval;
>  	int i;
>  
>  	/* Read the state */
> -	retval = sabi_get_command(sabi_config->commands.get_performance_level,
> +	retval = sabi_get_command(config->commands.get_performance_level,
>  				  &sretval);
>  	if (retval)
>  		return retval;
>  
>  	/* The logic is backwards, yeah, lots of fun... */
> -	for (i = 0; sabi_config->performance_levels[i].name; ++i) {
> -		if (sretval.retval[0] == sabi_config->performance_levels[i].value)
> -			return sprintf(buf, "%s\n", sabi_config->performance_levels[i].name);
> +	for (i = 0; config->performance_levels[i].name; ++i) {
> +		if (sretval.retval[0] == config->performance_levels[i].value)
> +			return sprintf(buf, "%s\n", config->performance_levels[i].name);
>  	}
>  	return sprintf(buf, "%s\n", "unknown");
>  }
> @@ -520,18 +540,20 @@ static ssize_t set_performance_level(struct device *dev,
>  				struct device_attribute *attr, const char *buf,
>  				size_t count)
>  {
> +	const struct sabi_config *config = samsung->config;
> +
>  	if (count >= 1) {
>  		int i;
> -		for (i = 0; sabi_config->performance_levels[i].name; ++i) {
> +		for (i = 0; config->performance_levels[i].name; ++i) {
>  			const struct sabi_performance_level *level =
> -				&sabi_config->performance_levels[i];
> +				&config->performance_levels[i];
>  			if (!strncasecmp(level->name, buf, strlen(level->name))) {
> -				sabi_set_command(sabi_config->commands.set_performance_level,
> +				sabi_set_command(config->commands.set_performance_level,
>  						 level->value);
>  				break;
>  			}
>  		}
> -		if (!sabi_config->performance_levels[i].name)
> +		if (!config->performance_levels[i].name)
>  			return -EINVAL;
>  	}
>  	return count;
> @@ -805,6 +827,9 @@ static int find_signature(void __iomem *memcheck, const char *testStr)
>  
>  static int __init samsung_init(void)
>  {
> +	const struct sabi_config *config = NULL;
> +	const struct sabi_commands *commands;
> +	struct backlight_device *bd;
>  	struct backlight_properties props;
>  	struct sabi_retval sretval;
>  	unsigned int ifaceP;
> @@ -812,21 +837,26 @@ static int __init samsung_init(void)
>  	int loca;
>  	int retval;
>  
> -	mutex_init(&sabi_mutex);
> -
>  	if (!force && !dmi_check_system(samsung_dmi_table))
>  		return -ENODEV;
>  
> -	f0000_segment = ioremap_nocache(0xf0000, 0xffff);
> -	if (!f0000_segment) {
> +	samsung = kzalloc(sizeof(*samsung), GFP_KERNEL);
> +	if (!samsung)
> +		return -ENOMEM;
> +
> +	mutex_init(&samsung->sabi_mutex);
> +
> +	samsung->f0000_segment = ioremap_nocache(0xf0000, 0xffff);
> +	if (!samsung->f0000_segment) {
>  		pr_err("Can't map the segment at 0xf0000\n");
> -		return -EINVAL;
> +		goto error_cant_map;
>  	}
>  
>  	/* Try to find one of the signatures in memory to find the header */
>  	for (i = 0; sabi_configs[i].test_string != 0; ++i) {
> -		sabi_config = &sabi_configs[i];
> -		loca = find_signature(f0000_segment, sabi_config->test_string);
> +		samsung->config = &sabi_configs[i];
> +		loca = find_signature(samsung->f0000_segment,
> +				      samsung->config->test_string);
>  		if (loca != 0xffff)
>  			break;
>  	}
> @@ -836,51 +866,60 @@ static int __init samsung_init(void)
>  		goto error_no_signature;
>  	}
>  
> +	config = samsung->config;
> +	commands = &config->commands;
> +
>  	/* point to the SMI port Number */
>  	loca += 1;
> -	sabi = (f0000_segment + loca);
> +	samsung->sabi = (samsung->f0000_segment + loca);
>  
>  	if (debug) {
>  		printk(KERN_DEBUG "This computer supports SABI==%x\n",
>  			loca + 0xf0000 - 6);
>  		printk(KERN_DEBUG "SABI header:\n");
>  		printk(KERN_DEBUG " SMI Port Number = 0x%04x\n",
> -			readw(sabi + sabi_config->header_offsets.port));
> +		       readw(samsung->sabi +
> +			     config->header_offsets.port));
>  		printk(KERN_DEBUG " SMI Interface Function = 0x%02x\n",
> -			readb(sabi + sabi_config->header_offsets.iface_func));
> +		       readb(samsung->sabi +
> +			     config->header_offsets.iface_func));
>  		printk(KERN_DEBUG " SMI enable memory buffer = 0x%02x\n",
> -			readb(sabi + sabi_config->header_offsets.en_mem));
> +		       readb(samsung->sabi +
> +			     config->header_offsets.en_mem));
>  		printk(KERN_DEBUG " SMI restore memory buffer = 0x%02x\n",
> -			readb(sabi + sabi_config->header_offsets.re_mem));
> +		       readb(samsung->sabi +
> +			     config->header_offsets.re_mem));
>  		printk(KERN_DEBUG " SABI data offset = 0x%04x\n",
> -			readw(sabi + sabi_config->header_offsets.data_offset));
> +		       readw(samsung->sabi +
> +			     config->header_offsets.data_offset));
>  		printk(KERN_DEBUG " SABI data segment = 0x%04x\n",
> -			readw(sabi + sabi_config->header_offsets.data_segment));
> +		       readw(samsung->sabi +
> +			     config->header_offsets.data_segment));
>  	}
>  
>  	/* Get a pointer to the SABI Interface */
> -	ifaceP = (readw(sabi + sabi_config->header_offsets.data_segment) & 0x0ffff) << 4;
> -	ifaceP += readw(sabi + sabi_config->header_offsets.data_offset) & 0x0ffff;
> -	sabi_iface = ioremap_nocache(ifaceP, 16);
> -	if (!sabi_iface) {
> +	ifaceP = (readw(samsung->sabi + config->header_offsets.data_segment) & 0x0ffff) << 4;
> +	ifaceP += readw(samsung->sabi + config->header_offsets.data_offset) & 0x0ffff;
> +	samsung->sabi_iface = ioremap_nocache(ifaceP, 16);
> +	if (!samsung->sabi_iface) {
>  		pr_err("Can't remap %x\n", ifaceP);
>  		goto error_no_signature;
>  	}
>  	if (debug) {
>  		printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
> -		printk(KERN_DEBUG "sabi_iface = %p\n", sabi_iface);
> +		printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
>  
>  		test_backlight();
>  		test_wireless();
>  
> -		retval = sabi_get_command(sabi_config->commands.get_brightness,
> +		retval = sabi_get_command(commands->get_brightness,
>  					  &sretval);
>  		printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.retval[0]);
>  	}
>  
>  	/* Turn on "Linux" mode in the BIOS */
> -	if (sabi_config->commands.set_linux != 0xff) {
> -		retval = sabi_set_command(sabi_config->commands.set_linux,
> +	if (commands->set_linux != 0xff) {
> +		retval = sabi_set_command(commands->set_linux,
>  					  0x81);
>  		if (retval) {
>  			pr_warn("Linux mode was not set!\n");
> @@ -892,30 +931,32 @@ static int __init samsung_init(void)
>  	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))
> +	samsung->pdev = platform_device_register_simple("samsung", -1, NULL, 0);
> +	if (IS_ERR(samsung->pdev))
>  		goto error_no_platform;
>  
>  	/* 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 -
> -				sabi_config->min_brightness;
> -	backlight_device = backlight_device_register("samsung", &sdev->dev,
> -						     NULL, &backlight_ops,
> -						     &props);
> -	if (IS_ERR(backlight_device))
> +	props.max_brightness = config->max_brightness -
> +				config->min_brightness;
> +	bd = backlight_device_register("samsung", &samsung->pdev->dev,
> +				       NULL, &backlight_ops,
> +				       &props);
> +	if (IS_ERR(bd))
>  		goto error_no_backlight;
>  
> -	backlight_device->props.brightness = read_brightness();
> -	backlight_device->props.power = FB_BLANK_UNBLANK;
> -	backlight_update_status(backlight_device);
> +	samsung->backlight_device = bd;
> +	samsung->backlight_device->props.brightness = read_brightness();
> +	samsung->backlight_device->props.power = FB_BLANK_UNBLANK;
> +	backlight_update_status(samsung->backlight_device);
>  
> -	retval = init_wireless(sdev);
> +	retval = init_wireless(samsung->pdev);
>  	if (retval)
>  		goto error_no_rfk;
>  
> -	retval = device_create_file(&sdev->dev, &dev_attr_performance_level);
> +	retval = device_create_file(&samsung->pdev->dev,
> +				    &dev_attr_performance_level);
>  	if (retval)
>  		goto error_file_create;
>  
> @@ -925,31 +966,39 @@ error_file_create:
>  	destroy_wireless();
>  
>  error_no_rfk:
> -	backlight_device_unregister(backlight_device);
> +	backlight_device_unregister(samsung->backlight_device);
>  
>  error_no_backlight:
> -	platform_device_unregister(sdev);
> +	platform_device_unregister(samsung->pdev);
>  
>  error_no_platform:
> -	iounmap(sabi_iface);
> +	iounmap(samsung->sabi_iface);
>  
>  error_no_signature:
> -	iounmap(f0000_segment);
> +	iounmap(samsung->f0000_segment);
> +
> +error_cant_map:
> +	kfree(samsung);
> +	samsung = NULL;
>  	return -EINVAL;
>  }
>  
>  static void __exit samsung_exit(void)
>  {
> +	const struct sabi_commands *commands = &samsung->config->commands;
> +
>  	/* Turn off "Linux" mode in the BIOS */
> -	if (sabi_config->commands.set_linux != 0xff)
> -		sabi_set_command(sabi_config->commands.set_linux, 0x80);
> +	if (commands->set_linux != 0xff)
> +		sabi_set_command(commands->set_linux, 0x80);
>  
> -	device_remove_file(&sdev->dev, &dev_attr_performance_level);
> -	backlight_device_unregister(backlight_device);
> +	device_remove_file(&samsung->pdev->dev, &dev_attr_performance_level);
> +	backlight_device_unregister(samsung->backlight_device);
>  	destroy_wireless();
> -	iounmap(sabi_iface);
> -	iounmap(f0000_segment);
> -	platform_device_unregister(sdev);
> +	iounmap(samsung->sabi_iface);
> +	iounmap(samsung->f0000_segment);
> +	platform_device_unregister(samsung->pdev);
> +	kfree(samsung);
> +	samsung = NULL;
>  }
>  
>  module_init(samsung_init);
> -- 
> 1.7.5.4

[PATCH 02/15] samsung-laptop: move code into init/exit functions

[Corentin Chary]

Create _init()/_exit() function for each subsystem, remove
the local struct samsung_laptop * and only keep a
struct platform_device * that can only be used in samsung_init()
and samsung_exit().

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |  609 +++++++++++++++++++--------------
 1 files changed, 360 insertions(+), 249 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index c502252..be96910 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -226,14 +226,14 @@ struct samsung_laptop {
 
 	struct mutex sabi_mutex;
 
-	struct platform_device *pdev;
+	struct platform_device *platform_device;
 	struct backlight_device *backlight_device;
 	struct rfkill *rfk;
 
 	bool has_stepping_quirk;
 };
 
-static struct samsung_laptop *samsung;
+
 
 static int force;
 module_param(force, bool, 0);
@@ -244,7 +244,8 @@ static int debug;
 module_param(debug, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(debug, "Debug enabled or not");
 
-static int sabi_get_command(u8 command, struct sabi_retval *sretval)
+static int sabi_get_command(struct samsung_laptop *samsung,
+			    u8 command, struct sabi_retval *sretval)
 {
 	const struct sabi_config *config = samsung->config;
 	int retval = 0;
@@ -291,7 +292,8 @@ exit:
 
 }
 
-static int sabi_set_command(u8 command, u8 data)
+static int sabi_set_command(struct samsung_laptop *samsung,
+			    u8 command, u8 data)
 {
 	const struct sabi_config *config = samsung->config;
 	int retval = 0;
@@ -326,53 +328,53 @@ static int sabi_set_command(u8 command, u8 data)
 	return retval;
 }
 
-static void test_backlight(void)
+static void test_backlight(struct samsung_laptop *samsung)
 {
 	const struct sabi_commands *commands = &samsung->config->commands;
 	struct sabi_retval sretval;
 
-	sabi_get_command(commands->get_backlight, &sretval);
+	sabi_get_command(samsung, commands->get_backlight, &sretval);
 	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
 
-	sabi_set_command(commands->set_backlight, 0);
+	sabi_set_command(samsung, commands->set_backlight, 0);
 	printk(KERN_DEBUG "backlight should be off\n");
 
-	sabi_get_command(commands->get_backlight, &sretval);
+	sabi_get_command(samsung, commands->get_backlight, &sretval);
 	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
 
 	msleep(1000);
 
-	sabi_set_command(commands->set_backlight, 1);
+	sabi_set_command(samsung, commands->set_backlight, 1);
 	printk(KERN_DEBUG "backlight should be on\n");
 
-	sabi_get_command(commands->get_backlight, &sretval);
+	sabi_get_command(samsung, commands->get_backlight, &sretval);
 	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
 }
 
-static void test_wireless(void)
+static void test_wireless(struct samsung_laptop *samsung)
 {
 	const struct sabi_commands *commands = &samsung->config->commands;
 	struct sabi_retval sretval;
 
-	sabi_get_command(commands->get_wireless_button, &sretval);
+	sabi_get_command(samsung, commands->get_wireless_button, &sretval);
 	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
 
-	sabi_set_command(commands->set_wireless_button, 0);
+	sabi_set_command(samsung, commands->set_wireless_button, 0);
 	printk(KERN_DEBUG "wireless led should be off\n");
 
-	sabi_get_command(commands->get_wireless_button, &sretval);
+	sabi_get_command(samsung, commands->get_wireless_button, &sretval);
 	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
 
 	msleep(1000);
 
-	sabi_set_command(commands->set_wireless_button, 1);
+	sabi_set_command(samsung, commands->set_wireless_button, 1);
 	printk(KERN_DEBUG "wireless led should be on\n");
 
-	sabi_get_command(commands->get_wireless_button, &sretval);
+	sabi_get_command(samsung, commands->get_wireless_button, &sretval);
 	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
 }
 
-static u8 read_brightness(void)
+static int read_brightness(struct samsung_laptop *samsung)
 {
 	const struct sabi_config *config = samsung->config;
 	const struct sabi_commands *commands = &samsung->config->commands;
@@ -380,7 +382,7 @@ static u8 read_brightness(void)
 	int user_brightness = 0;
 	int retval;
 
-	retval = sabi_get_command(commands->get_brightness,
+	retval = sabi_get_command(samsung, commands->get_brightness,
 				  &sretval);
 	if (!retval) {
 		user_brightness = sretval.retval[0];
@@ -392,7 +394,7 @@ static u8 read_brightness(void)
 	return user_brightness;
 }
 
-static void set_brightness(u8 user_brightness)
+static void set_brightness(struct samsung_laptop *samsung, u8 user_brightness)
 {
 	const struct sabi_config *config = samsung->config;
 	const struct sabi_commands *commands = &samsung->config->commands;
@@ -403,25 +405,27 @@ static void set_brightness(u8 user_brightness)
 		 * short circuit if the specified level is what's already set
 		 * to prevent the screen from flickering needlessly
 		 */
-		if (user_brightness == read_brightness())
+		if (user_brightness == read_brightness(samsung))
 			return;
 
-		sabi_set_command(commands->set_brightness, 0);
+		sabi_set_command(samsung, commands->set_brightness, 0);
 	}
 
-	sabi_set_command(commands->set_brightness, user_level);
+	sabi_set_command(samsung, commands->set_brightness, user_level);
 }
 
 static int get_brightness(struct backlight_device *bd)
 {
-	return (int)read_brightness();
+	struct samsung_laptop *samsung = bl_get_data(bd);
+
+	return read_brightness(samsung);
 }
 
-static void check_for_stepping_quirk(void)
+static void check_for_stepping_quirk(struct samsung_laptop *samsung)
 {
-	u8 initial_level;
-	u8 check_level;
-	u8 orig_level = read_brightness();
+	int initial_level;
+	int check_level;
+	int orig_level = read_brightness(samsung);
 
 	/*
 	 * Some laptops exhibit the strange behaviour of stepping toward
@@ -431,9 +435,9 @@ static void check_for_stepping_quirk(void)
 	 */
 
 	if (orig_level == 0)
-		set_brightness(1);
+		set_brightness(samsung, 1);
 
-	initial_level = read_brightness();
+	initial_level = read_brightness(samsung);
 
 	if (initial_level <= 2)
 		check_level = initial_level + 2;
@@ -441,26 +445,28 @@ static void check_for_stepping_quirk(void)
 		check_level = initial_level - 2;
 
 	samsung->has_stepping_quirk = false;
-	set_brightness(check_level);
+	set_brightness(samsung, check_level);
 
-	if (read_brightness() != check_level) {
+	if (read_brightness(samsung) != check_level) {
 		samsung->has_stepping_quirk = true;
 		pr_info("enabled workaround for brightness stepping quirk\n");
 	}
 
-	set_brightness(orig_level);
+	set_brightness(samsung, orig_level);
 }
 
 static int update_status(struct backlight_device *bd)
 {
+	struct samsung_laptop *samsung = bl_get_data(bd);
 	const struct sabi_commands *commands = &samsung->config->commands;
 
-	set_brightness(bd->props.brightness);
+	set_brightness(samsung, bd->props.brightness);
 
 	if (bd->props.power == FB_BLANK_UNBLANK)
-		sabi_set_command(commands->set_backlight, 1);
+		sabi_set_command(samsung, commands->set_backlight, 1);
 	else
-		sabi_set_command(commands->set_backlight, 0);
+		sabi_set_command(samsung, commands->set_backlight, 0);
+
 	return 0;
 }
 
@@ -471,6 +477,7 @@ static const struct backlight_ops backlight_ops = {
 
 static int rfkill_set(void *data, bool blocked)
 {
+	struct samsung_laptop *samsung = data;
 	const struct sabi_commands *commands = &samsung->config->commands;
 
 	/* Do something with blocked...*/
@@ -479,9 +486,9 @@ static int rfkill_set(void *data, bool blocked)
 	 * blocked == true is off
 	 */
 	if (blocked)
-		sabi_set_command(commands->set_wireless_button, 0);
+		sabi_set_command(samsung, commands->set_wireless_button, 0);
 	else
-		sabi_set_command(commands->set_wireless_button, 1);
+		sabi_set_command(samsung, commands->set_wireless_button, 1);
 
 	return 0;
 }
@@ -490,40 +497,18 @@ static struct rfkill_ops rfkill_ops = {
 	.set_block = rfkill_set,
 };
 
-static int init_wireless(struct platform_device *pdev)
-{
-	int retval;
-
-	samsung->rfk = rfkill_alloc("samsung-wifi", &samsung->pdev->dev, RFKILL_TYPE_WLAN,
-				    &rfkill_ops, NULL);
-	if (!samsung->rfk)
-		return -ENOMEM;
-
-	retval = rfkill_register(samsung->rfk);
-	if (retval) {
-		rfkill_destroy(samsung->rfk);
-		return -ENODEV;
-	}
-
-	return 0;
-}
-
-static void destroy_wireless(void)
-{
-	rfkill_unregister(samsung->rfk);
-	rfkill_destroy(samsung->rfk);
-}
-
 static ssize_t get_performance_level(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
+	struct samsung_laptop *samsung = dev_get_drvdata(dev);
 	const struct sabi_config *config = samsung->config;
+	const struct sabi_commands *commands = &config->commands;
 	struct sabi_retval sretval;
 	int retval;
 	int i;
 
 	/* Read the state */
-	retval = sabi_get_command(config->commands.get_performance_level,
+	retval = sabi_get_command(samsung, commands->get_performance_level,
 				  &sretval);
 	if (retval)
 		return retval;
@@ -540,32 +525,285 @@ static ssize_t set_performance_level(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
+	struct samsung_laptop *samsung = dev_get_drvdata(dev);
 	const struct sabi_config *config = samsung->config;
+	const struct sabi_commands *commands = &config->commands;
+	int i;
 
-	if (count >= 1) {
-		int i;
-		for (i = 0; config->performance_levels[i].name; ++i) {
-			const struct sabi_performance_level *level =
-				&config->performance_levels[i];
-			if (!strncasecmp(level->name, buf, strlen(level->name))) {
-				sabi_set_command(config->commands.set_performance_level,
-						 level->value);
-				break;
-			}
+	if (count < 1)
+		return count;
+
+	for (i = 0; config->performance_levels[i].name; ++i) {
+		const struct sabi_performance_level *level =
+			&config->performance_levels[i];
+		if (!strncasecmp(level->name, buf, strlen(level->name))) {
+			sabi_set_command(samsung,
+					 commands->set_performance_level,
+					 level->value);
+			break;
 		}
-		if (!config->performance_levels[i].name)
-			return -EINVAL;
 	}
+
+	if (!config->performance_levels[i].name)
+		return -EINVAL;
+
 	return count;
 }
+
 static DEVICE_ATTR(performance_level, S_IWUSR | S_IRUGO,
 		   get_performance_level, set_performance_level);
 
 
+static int find_signature(void __iomem *memcheck, const char *testStr)
+{
+	int i = 0;
+	int loca;
+
+	for (loca = 0; loca < 0xffff; loca++) {
+		char temp = readb(memcheck + loca);
+
+		if (temp == testStr[i]) {
+			if (i == strlen(testStr)-1)
+				break;
+			++i;
+		} else {
+			i = 0;
+		}
+	}
+	return loca;
+}
+
+static void samsung_rfkill_exit(struct samsung_laptop *samsung)
+{
+	if (samsung->rfk) {
+		rfkill_unregister(samsung->rfk);
+		rfkill_destroy(samsung->rfk);
+		samsung->rfk = NULL;
+	}
+}
+
+static int __init samsung_rfkill_init(struct samsung_laptop *samsung)
+{
+	int retval;
+
+	samsung->rfk = rfkill_alloc("samsung-wifi",
+				    &samsung->platform_device->dev,
+				    RFKILL_TYPE_WLAN,
+				    &rfkill_ops, samsung);
+	if (!samsung->rfk)
+		return -ENOMEM;
+
+	retval = rfkill_register(samsung->rfk);
+	if (retval) {
+		rfkill_destroy(samsung->rfk);
+		samsung->rfk = NULL;
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void samsung_backlight_exit(struct samsung_laptop *samsung)
+{
+	if (samsung->backlight_device) {
+		backlight_device_unregister(samsung->backlight_device);
+		samsung->backlight_device = NULL;
+	}
+}
+
+static int __init samsung_backlight_init(struct samsung_laptop *samsung)
+{
+	struct backlight_device *bd;
+	struct backlight_properties props;
+
+	memset(&props, 0, sizeof(struct backlight_properties));
+	props.type = BACKLIGHT_PLATFORM;
+	props.max_brightness = samsung->config->max_brightness -
+		samsung->config->min_brightness;
+
+	bd = backlight_device_register("samsung",
+				       &samsung->platform_device->dev,
+				       samsung, &backlight_ops,
+				       &props);
+	if (IS_ERR(bd))
+		return PTR_ERR(bd);
+
+	samsung->backlight_device = bd;
+	samsung->backlight_device->props.brightness = read_brightness(samsung);
+	samsung->backlight_device->props.power = FB_BLANK_UNBLANK;
+	backlight_update_status(samsung->backlight_device);
+
+	return 0;
+}
+
+static void samsung_sysfs_exit(struct samsung_laptop *samsung)
+{
+	device_remove_file(&samsung->platform_device->dev,
+			   &dev_attr_performance_level);
+}
+
+static int __init samsung_sysfs_init(struct samsung_laptop *samsung)
+{
+	return device_create_file(&samsung->platform_device->dev,
+				  &dev_attr_performance_level);
+}
+
+static void samsung_sabi_exit(struct samsung_laptop *samsung)
+{
+	const struct sabi_config *config = samsung->config;
+
+	/* Turn off "Linux" mode in the BIOS */
+	if (config && config->commands.set_linux != 0xff)
+		sabi_set_command(samsung, config->commands.set_linux, 0x80);
+
+	if (samsung->sabi_iface) {
+		iounmap(samsung->sabi_iface);
+		samsung->sabi_iface = NULL;
+	}
+	if (samsung->f0000_segment) {
+		iounmap(samsung->f0000_segment);
+		samsung->f0000_segment = NULL;
+	}
+
+	samsung->config = NULL;
+}
+
+static __init void samsung_sabi_infos(struct samsung_laptop *samsung, int loca)
+{
+	const struct sabi_config *config = samsung->config;
+
+	printk(KERN_DEBUG "This computer supports SABI==%x\n",
+	       loca + 0xf0000 - 6);
+	printk(KERN_DEBUG "SABI header:\n");
+	printk(KERN_DEBUG " SMI Port Number = 0x%04x\n",
+	       readw(samsung->sabi + config->header_offsets.port));
+	printk(KERN_DEBUG " SMI Interface Function = 0x%02x\n",
+	       readb(samsung->sabi + config->header_offsets.iface_func));
+	printk(KERN_DEBUG " SMI enable memory buffer = 0x%02x\n",
+	       readb(samsung->sabi + config->header_offsets.en_mem));
+	printk(KERN_DEBUG " SMI restore memory buffer = 0x%02x\n",
+	       readb(samsung->sabi + config->header_offsets.re_mem));
+	printk(KERN_DEBUG " SABI data offset = 0x%04x\n",
+	       readw(samsung->sabi + config->header_offsets.data_offset));
+	printk(KERN_DEBUG " SABI data segment = 0x%04x\n",
+	       readw(samsung->sabi + config->header_offsets.data_segment));
+}
+
+static void __init samsung_sabi_selftest(struct samsung_laptop *samsung,
+					unsigned int ifaceP)
+{
+	const struct sabi_config *config = samsung->config;
+	struct sabi_retval sretval;
+
+	printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
+	printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
+
+	test_backlight(samsung);
+	test_wireless(samsung);
+
+	sabi_get_command(samsung, config->commands.get_brightness, &sretval);
+	printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.retval[0]);
+}
+
+static int __init samsung_sabi_init(struct samsung_laptop *samsung)
+{
+	const struct sabi_config *config = NULL;
+	const struct sabi_commands *commands;
+	unsigned int ifaceP;
+	int ret = 0;
+	int i;
+	int loca;
+
+	samsung->f0000_segment = ioremap_nocache(0xf0000, 0xffff);
+	if (!samsung->f0000_segment) {
+		pr_err("Can't map the segment at 0xf0000\n");
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	/* Try to find one of the signatures in memory to find the header */
+	for (i = 0; sabi_configs[i].test_string != 0; ++i) {
+		samsung->config = &sabi_configs[i];
+		loca = find_signature(samsung->f0000_segment,
+				      samsung->config->test_string);
+		if (loca != 0xffff)
+			break;
+	}
+
+	if (loca == 0xffff) {
+		pr_err("This computer does not support SABI\n");
+		ret = -ENODEV;
+		goto exit;
+	}
+
+	config = samsung->config;
+	commands = &config->commands;
+
+	/* point to the SMI port Number */
+	loca += 1;
+	samsung->sabi = (samsung->f0000_segment + loca);
+
+	if (debug)
+		samsung_sabi_infos(samsung, loca);
+
+	/* Get a pointer to the SABI Interface */
+	ifaceP = (readw(samsung->sabi + config->header_offsets.data_segment) & 0x0ffff) << 4;
+	ifaceP += readw(samsung->sabi + config->header_offsets.data_offset) & 0x0ffff;
+	samsung->sabi_iface = ioremap_nocache(ifaceP, 16);
+	if (!samsung->sabi_iface) {
+		pr_err("Can't remap %x\n", ifaceP);
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	if (debug)
+		samsung_sabi_selftest(samsung, ifaceP);
+
+	/* Turn on "Linux" mode in the BIOS */
+	if (commands->set_linux != 0xff) {
+		int retval = sabi_set_command(samsung,
+					      commands->set_linux, 0x81);
+		if (retval) {
+			pr_warn("Linux mode was not set!\n");
+			ret = -ENODEV;
+			goto exit;
+		}
+	}
+
+	/* Check for stepping quirk */
+	check_for_stepping_quirk(samsung);
+
+exit:
+	if (ret)
+		samsung_sabi_exit(samsung);
+
+	return ret;
+}
+
+static void samsung_platform_exit(struct samsung_laptop *samsung)
+{
+	if (samsung->platform_device) {
+		platform_device_unregister(samsung->platform_device);
+		samsung->platform_device = NULL;
+	}
+}
+
+static int __init samsung_platform_init(struct samsung_laptop *samsung)
+{
+	struct platform_device *pdev;
+
+	pdev = platform_device_register_simple("samsung", -1, NULL, 0);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	samsung->platform_device = pdev;
+	platform_set_drvdata(samsung->platform_device, samsung);
+	return 0;
+}
+
 static int __init dmi_check_cb(const struct dmi_system_id *id)
 {
-	pr_info("found laptop model '%s'\n",
-		id->ident);
+	pr_info("found laptop model '%s'\n", id->ident);
 	return 1;
 }
 
@@ -806,36 +1044,12 @@ static struct dmi_system_id __initdata samsung_dmi_table[] = {
 };
 MODULE_DEVICE_TABLE(dmi, samsung_dmi_table);
 
-static int find_signature(void __iomem *memcheck, const char *testStr)
-{
-	int i = 0;
-	int loca;
-
-	for (loca = 0; loca < 0xffff; loca++) {
-		char temp = readb(memcheck + loca);
-
-		if (temp == testStr[i]) {
-			if (i == strlen(testStr)-1)
-				break;
-			++i;
-		} else {
-			i = 0;
-		}
-	}
-	return loca;
-}
+static struct platform_device *samsung_platform_device;
 
 static int __init samsung_init(void)
 {
-	const struct sabi_config *config = NULL;
-	const struct sabi_commands *commands;
-	struct backlight_device *bd;
-	struct backlight_properties props;
-	struct sabi_retval sretval;
-	unsigned int ifaceP;
-	int i;
-	int loca;
-	int retval;
+	struct samsung_laptop *samsung;
+	int ret;
 
 	if (!force && !dmi_check_system(samsung_dmi_table))
 		return -ENODEV;
@@ -846,159 +1060,56 @@ static int __init samsung_init(void)
 
 	mutex_init(&samsung->sabi_mutex);
 
-	samsung->f0000_segment = ioremap_nocache(0xf0000, 0xffff);
-	if (!samsung->f0000_segment) {
-		pr_err("Can't map the segment at 0xf0000\n");
-		goto error_cant_map;
-	}
-
-	/* Try to find one of the signatures in memory to find the header */
-	for (i = 0; sabi_configs[i].test_string != 0; ++i) {
-		samsung->config = &sabi_configs[i];
-		loca = find_signature(samsung->f0000_segment,
-				      samsung->config->test_string);
-		if (loca != 0xffff)
-			break;
-	}
-
-	if (loca == 0xffff) {
-		pr_err("This computer does not support SABI\n");
-		goto error_no_signature;
-	}
-
-	config = samsung->config;
-	commands = &config->commands;
-
-	/* point to the SMI port Number */
-	loca += 1;
-	samsung->sabi = (samsung->f0000_segment + loca);
-
-	if (debug) {
-		printk(KERN_DEBUG "This computer supports SABI==%x\n",
-			loca + 0xf0000 - 6);
-		printk(KERN_DEBUG "SABI header:\n");
-		printk(KERN_DEBUG " SMI Port Number = 0x%04x\n",
-		       readw(samsung->sabi +
-			     config->header_offsets.port));
-		printk(KERN_DEBUG " SMI Interface Function = 0x%02x\n",
-		       readb(samsung->sabi +
-			     config->header_offsets.iface_func));
-		printk(KERN_DEBUG " SMI enable memory buffer = 0x%02x\n",
-		       readb(samsung->sabi +
-			     config->header_offsets.en_mem));
-		printk(KERN_DEBUG " SMI restore memory buffer = 0x%02x\n",
-		       readb(samsung->sabi +
-			     config->header_offsets.re_mem));
-		printk(KERN_DEBUG " SABI data offset = 0x%04x\n",
-		       readw(samsung->sabi +
-			     config->header_offsets.data_offset));
-		printk(KERN_DEBUG " SABI data segment = 0x%04x\n",
-		       readw(samsung->sabi +
-			     config->header_offsets.data_segment));
-	}
-
-	/* Get a pointer to the SABI Interface */
-	ifaceP = (readw(samsung->sabi + config->header_offsets.data_segment) & 0x0ffff) << 4;
-	ifaceP += readw(samsung->sabi + config->header_offsets.data_offset) & 0x0ffff;
-	samsung->sabi_iface = ioremap_nocache(ifaceP, 16);
-	if (!samsung->sabi_iface) {
-		pr_err("Can't remap %x\n", ifaceP);
-		goto error_no_signature;
-	}
-	if (debug) {
-		printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
-		printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
-
-		test_backlight();
-		test_wireless();
-
-		retval = sabi_get_command(commands->get_brightness,
-					  &sretval);
-		printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.retval[0]);
-	}
-
-	/* Turn on "Linux" mode in the BIOS */
-	if (commands->set_linux != 0xff) {
-		retval = sabi_set_command(commands->set_linux,
-					  0x81);
-		if (retval) {
-			pr_warn("Linux mode was not set!\n");
-			goto error_no_platform;
-		}
-	}
-
-	/* Check for stepping quirk */
-	check_for_stepping_quirk();
-
-	/* knock up a platform device to hang stuff off of */
-	samsung->pdev = platform_device_register_simple("samsung", -1, NULL, 0);
-	if (IS_ERR(samsung->pdev))
-		goto error_no_platform;
-
-	/* create a backlight device to talk to this one */
-	memset(&props, 0, sizeof(struct backlight_properties));
-	props.type = BACKLIGHT_PLATFORM;
-	props.max_brightness = config->max_brightness -
-				config->min_brightness;
-	bd = backlight_device_register("samsung", &samsung->pdev->dev,
-				       NULL, &backlight_ops,
-				       &props);
-	if (IS_ERR(bd))
-		goto error_no_backlight;
-
-	samsung->backlight_device = bd;
-	samsung->backlight_device->props.brightness = read_brightness();
-	samsung->backlight_device->props.power = FB_BLANK_UNBLANK;
-	backlight_update_status(samsung->backlight_device);
-
-	retval = init_wireless(samsung->pdev);
-	if (retval)
-		goto error_no_rfk;
-
-	retval = device_create_file(&samsung->pdev->dev,
-				    &dev_attr_performance_level);
-	if (retval)
-		goto error_file_create;
-
-	return 0;
-
-error_file_create:
-	destroy_wireless();
-
-error_no_rfk:
-	backlight_device_unregister(samsung->backlight_device);
-
-error_no_backlight:
-	platform_device_unregister(samsung->pdev);
-
-error_no_platform:
-	iounmap(samsung->sabi_iface);
-
-error_no_signature:
-	iounmap(samsung->f0000_segment);
-
-error_cant_map:
+	ret = samsung_platform_init(samsung);
+	if (ret)
+		goto error_platform;
+
+	ret = samsung_sabi_init(samsung);
+	if (ret)
+		goto error_sabi;
+
+	ret = samsung_sysfs_init(samsung);
+	if (ret)
+		goto error_sysfs;
+
+	ret = samsung_backlight_init(samsung);
+	if (ret)
+		goto error_backlight;
+
+	ret = samsung_rfkill_init(samsung);
+	if (ret)
+		goto error_rfkill;
+
+	samsung_platform_device = samsung->platform_device;
+	return ret;
+
+error_rfkill:
+	samsung_backlight_exit(samsung);
+error_backlight:
+	samsung_sysfs_exit(samsung);
+error_sysfs:
+	samsung_sabi_exit(samsung);
+error_sabi:
+	samsung_platform_exit(samsung);
+error_platform:
 	kfree(samsung);
-	samsung = NULL;
-	return -EINVAL;
+	return ret;
 }
 
 static void __exit samsung_exit(void)
 {
-	const struct sabi_commands *commands = &samsung->config->commands;
+	struct samsung_laptop *samsung;
+
+	samsung = platform_get_drvdata(samsung_platform_device);
+
+	samsung_rfkill_exit(samsung);
+	samsung_backlight_exit(samsung);
+	samsung_sysfs_exit(samsung);
+	samsung_sabi_exit(samsung);
+	samsung_platform_exit(samsung);
 
-	/* Turn off "Linux" mode in the BIOS */
-	if (commands->set_linux != 0xff)
-		sabi_set_command(commands->set_linux, 0x80);
-
-	device_remove_file(&samsung->pdev->dev, &dev_attr_performance_level);
-	backlight_device_unregister(samsung->backlight_device);
-	destroy_wireless();
-	iounmap(samsung->sabi_iface);
-	iounmap(samsung->f0000_segment);
-	platform_device_unregister(samsung->pdev);
 	kfree(samsung);
-	samsung = NULL;
+	samsung_platform_device = NULL;
 }
 
 module_init(samsung_init);
-- 
1.7.5.4

Re: [PATCH 02/15] samsung-laptop: move code into init/exit functions

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:33PM +0100, Corentin Chary wrote:
> Create _init()/_exit() function for each subsystem, remove
> the local struct samsung_laptop * and only keep a
> struct platform_device * that can only be used in samsung_init()
> and samsung_exit().
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

It seems a little gratitous, but hey, if you want to maintain this
driver, I have no objection to you rewriting the thing :)

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH 02/15] samsung-laptop: move code into init/exit functions

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:19 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:33PM +0100, Corentin Chary wrote:
>> Create _init()/_exit() function for each subsystem, remove
>> the local struct samsung_laptop * and only keep a
>> struct platform_device * that can only be used in samsung_init()
>> and samsung_exit().
>>
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> It seems a little gratitous, but hey, if you want to maintain this
> driver, I have no objection to you rewriting the thing :)
>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Well, doing that seems to help with error paths, and make it easy
separate functionalities, in my opinion.

-- 
Corentin Chary
http://xf.iksaif.net

[PATCH 03/15] samsung-laptop: don't handle backlight if handled by acpi/video

[Corentin Chary]

samsung-laptop is not at all related to ACPI, but since this interface
is not documented at all, and the driver has to use it at load to
understand how it works on the laptop, I think it's a good idea to
disable it if a better solution is available.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index be96910..49933d2 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -21,6 +21,7 @@
 #include <linux/dmi.h>
 #include <linux/platform_device.h>
 #include <linux/rfkill.h>
+#include <linux/acpi.h>
 
 /*
  * This driver is needed because a number of Samsung laptops do not hook
@@ -230,6 +231,7 @@ struct samsung_laptop {
 	struct backlight_device *backlight_device;
 	struct rfkill *rfk;
 
+	bool handle_backlight;
 	bool has_stepping_quirk;
 };
 
@@ -616,6 +618,9 @@ static int __init samsung_backlight_init(struct samsung_laptop *samsung)
 	struct backlight_device *bd;
 	struct backlight_properties props;
 
+	if (!samsung->handle_backlight)
+		return 0;
+
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_PLATFORM;
 	props.max_brightness = samsung->config->max_brightness -
@@ -698,7 +703,8 @@ static void __init samsung_sabi_selftest(struct samsung_laptop *samsung,
 	printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
 	printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
 
-	test_backlight(samsung);
+	if (samsung->handle_backlight)
+		test_backlight(samsung);
 	test_wireless(samsung);
 
 	sabi_get_command(samsung, config->commands.get_brightness, &sretval);
@@ -771,7 +777,8 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 	}
 
 	/* Check for stepping quirk */
-	check_for_stepping_quirk(samsung);
+	if (samsung->handle_backlight)
+		check_for_stepping_quirk(samsung);
 
 exit:
 	if (ret)
@@ -1059,6 +1066,15 @@ static int __init samsung_init(void)
 		return -ENOMEM;
 
 	mutex_init(&samsung->sabi_mutex);
+	samsung->handle_backlight = true;
+
+#ifdef CONFIG_ACPI
+	/* Don't handle backlight here if the acpi video already handle it */
+	if (acpi_video_backlight_support()) {
+		pr_info("Backlight controlled by ACPI video driver\n");
+		samsung->handle_backlight = false;
+	}
+#endif
 
 	ret = samsung_platform_init(samsung);
 	if (ret)
-- 
1.7.5.4

Re: [PATCH 03/15] samsung-laptop: don't handle backlight if handled by acpi/video

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:34PM +0100, Corentin Chary wrote:
> samsung-laptop is not at all related to ACPI, but since this interface
> is not documented at all, and the driver has to use it at load to
> understand how it works on the laptop, I think it's a good idea to
> disable it if a better solution is available.

I wish this would work, but on the machine that I had, there was a valid
ACPI table for the video device, yet I was told to "ignore it, it
doesn't work and use the sabi interface instead", so this patch would
break that machine :(

We might want to trigger off of the machine type for this kind of
thing, that's the only way I can see this working properly.

So, sorry, I can't approve this patch.

greg k-h

Re: [PATCH 03/15] samsung-laptop: don't handle backlight if handled by acpi/video

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:16 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:34PM +0100, Corentin Chary wrote:
>> samsung-laptop is not at all related to ACPI, but since this interface
>> is not documented at all, and the driver has to use it at load to
>> understand how it works on the laptop, I think it's a good idea to
>> disable it if a better solution is available.
>
> I wish this would work, but on the machine that I had, there was a valid
> ACPI table for the video device, yet I was told to "ignore it, it
> doesn't work and use the sabi interface instead", so this patch would
> break that machine :(

Well, it's why acpi_backlight=vendor is here: to enable and use the
vendor backlight instead of using the standard ACPI interface.
And nowadays, even gpu drivers provide a backlight class (at least for
intel, nvidia/nouveau, and maybe radeon.

> We might want to trigger off of the machine type for this kind of
> thing, that's the only way I can see this working properly.

But, yes, this is a "regression". Maybe we should do that:
- enable the backlight by default for all SECLINUX models
- disable it by default for swsmi, but let the user be able to enable
it with acpi_backlight=vendor (this is what is done with this patch).




-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 03/15] samsung-laptop: don't handle backlight if handled by acpi/video

[Greg KH]

On Wed, Nov 23, 2011 at 07:58:20AM +0100, Corentin Chary wrote:
> On Tue, Nov 22, 2011 at 11:16 PM, Greg KH <gregkh@suse.de> wrote:
> > On Tue, Nov 22, 2011 at 11:02:34PM +0100, Corentin Chary wrote:
> >> samsung-laptop is not at all related to ACPI, but since this interface
> >> is not documented at all, and the driver has to use it at load to
> >> understand how it works on the laptop, I think it's a good idea to
> >> disable it if a better solution is available.
> >
> > I wish this would work, but on the machine that I had, there was a valid
> > ACPI table for the video device, yet I was told to "ignore it, it
> > doesn't work and use the sabi interface instead", so this patch would
> > break that machine :(
> 
> Well, it's why acpi_backlight=vendor is here: to enable and use the
> vendor backlight instead of using the standard ACPI interface.
> And nowadays, even gpu drivers provide a backlight class (at least for
> intel, nvidia/nouveau, and maybe radeon.
> 
> > We might want to trigger off of the machine type for this kind of
> > thing, that's the only way I can see this working properly.
> 
> But, yes, this is a "regression". Maybe we should do that:
> - enable the backlight by default for all SECLINUX models
> - disable it by default for swsmi, but let the user be able to enable
> it with acpi_backlight=vendor (this is what is done with this patch).

That sounds reasonable.

thanks,

greg k-h

Re: [PATCH 03/15] samsung-laptop: don't handle backlight if handled by acpi/video

[David Herrmann]

Hi Corentin

On Tue, Nov 22, 2011 at 11:02 PM, Corentin Chary <corentincj@iksaif.net> wrote:
> samsung-laptop is not at all related to ACPI, but since this interface
> is not documented at all, and the driver has to use it at load to
> understand how it works on the laptop, I think it's a good idea to
> disable it if a better solution is available.
>
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  drivers/platform/x86/samsung-laptop.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index be96910..49933d2 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -21,6 +21,7 @@
>  #include <linux/dmi.h>
>  #include <linux/platform_device.h>
>  #include <linux/rfkill.h>
> +#include <linux/acpi.h>
>
>  /*
>  * This driver is needed because a number of Samsung laptops do not hook
> @@ -230,6 +231,7 @@ struct samsung_laptop {
>        struct backlight_device *backlight_device;
>        struct rfkill *rfk;
>
> +       bool handle_backlight;
>        bool has_stepping_quirk;
>  };
>
> @@ -616,6 +618,9 @@ static int __init samsung_backlight_init(struct samsung_laptop *samsung)
>        struct backlight_device *bd;
>        struct backlight_properties props;
>
> +       if (!samsung->handle_backlight)
> +               return 0;
> +
>        memset(&props, 0, sizeof(struct backlight_properties));
>        props.type = BACKLIGHT_PLATFORM;
>        props.max_brightness = samsung->config->max_brightness -
> @@ -698,7 +703,8 @@ static void __init samsung_sabi_selftest(struct samsung_laptop *samsung,
>        printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
>        printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
>
> -       test_backlight(samsung);
> +       if (samsung->handle_backlight)
> +               test_backlight(samsung);
>        test_wireless(samsung);
>
>        sabi_get_command(samsung, config->commands.get_brightness, &sretval);
> @@ -771,7 +777,8 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
>        }
>
>        /* Check for stepping quirk */
> -       check_for_stepping_quirk(samsung);
> +       if (samsung->handle_backlight)
> +               check_for_stepping_quirk(samsung);
>
>  exit:
>        if (ret)
> @@ -1059,6 +1066,15 @@ static int __init samsung_init(void)
>                return -ENOMEM;
>
>        mutex_init(&samsung->sabi_mutex);
> +       samsung->handle_backlight = true;
> +
> +#ifdef CONFIG_ACPI
> +       /* Don't handle backlight here if the acpi video already handle it */
> +       if (acpi_video_backlight_support()) {
> +               pr_info("Backlight controlled by ACPI video driver\n");
> +               samsung->handle_backlight = false;
> +       }
> +#endif
>
>        ret = samsung_platform_init(samsung);
>        if (ret)
> --
> 1.7.5.4

My Samsung model has an intel card which previously did not support
the acpi video backlight so I needed the samsung driver.
However, with this patch I couldn't find the samsung backlight and so
I tried the intel back light again and now it works.
So:
Tested-by: David Herrmann <dh.herrmann@googlemail.com>

My model is Samsung N210 btw.

Regards
David

[PATCH 04/15] samsung-laptop: use a sysfs group

[Corentin Chary]

Will be usefull later when we will have more platform sysfs files.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |   34 +++++++++++++++++++++++++++++---
 1 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 49933d2..a21e0c3 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -555,6 +555,10 @@ static ssize_t set_performance_level(struct device *dev,
 static DEVICE_ATTR(performance_level, S_IWUSR | S_IRUGO,
 		   get_performance_level, set_performance_level);
 
+static struct attribute *platform_attributes[] = {
+	&dev_attr_performance_level.attr,
+	NULL
+};
 
 static int find_signature(void __iomem *memcheck, const char *testStr)
 {
@@ -641,16 +645,38 @@ static int __init samsung_backlight_init(struct samsung_laptop *samsung)
 	return 0;
 }
 
+static mode_t samsung_sysfs_is_visible(struct kobject *kobj,
+				       struct attribute *attr, int idx)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct samsung_laptop *samsung = platform_get_drvdata(pdev);
+	bool ok = true;
+
+	if (attr == &dev_attr_performance_level.attr)
+		ok = !!samsung->config->performance_levels[0].name;
+
+	return ok ? attr->mode : 0;
+}
+
+static struct attribute_group platform_attribute_group = {
+	.is_visible = samsung_sysfs_is_visible,
+	.attrs = platform_attributes
+};
+
 static void samsung_sysfs_exit(struct samsung_laptop *samsung)
 {
-	device_remove_file(&samsung->platform_device->dev,
-			   &dev_attr_performance_level);
+	struct platform_device *device = samsung->platform_device;
+
+	sysfs_remove_group(&device->dev.kobj, &platform_attribute_group);
 }
 
 static int __init samsung_sysfs_init(struct samsung_laptop *samsung)
 {
-	return device_create_file(&samsung->platform_device->dev,
-				  &dev_attr_performance_level);
+	struct platform_device *device = samsung->platform_device;
+
+	return sysfs_create_group(&device->dev.kobj, &platform_attribute_group);
+
 }
 
 static void samsung_sabi_exit(struct samsung_laptop *samsung)
-- 
1.7.5.4

Re: [PATCH 04/15] samsung-laptop: use a sysfs group

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:35PM +0100, Corentin Chary wrote:
> Will be usefull later when we will have more platform sysfs files.

We will?  For what?

> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

[PATCH 05/15] samsung-laptop: ehance SABI support

[Corentin Chary]

* SABI command are on 16 bits, not 8
* SABI can read/write up to 11 byte of data
* There is not real difference between "get" and "set"
  commands, so refactorise the code of both functions

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |  224 ++++++++++++++++----------------
 1 files changed, 112 insertions(+), 112 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index a21e0c3..9dcb582 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -42,9 +42,17 @@
 #define SABI_IFACE_COMPLETE		0x04
 #define SABI_IFACE_DATA			0x05
 
-/* Structure to get data back to the calling function */
-struct sabi_retval {
-	u8 retval[20];
+/* Structure get/set data using sabi */
+struct sabi_data {
+	union {
+		struct {
+			u32 d0;
+			u32 d1;
+			u16 d2;
+			u8  d3;
+		};
+		u8 data[11];
+	};
 };
 
 struct sabi_header_offsets {
@@ -61,8 +69,8 @@ struct sabi_commands {
 	 * Brightness is 0 - 8, as described above.
 	 * Value 0 is for the BIOS to use
 	 */
-	u8 get_brightness;
-	u8 set_brightness;
+	u16 get_brightness;
+	u16 set_brightness;
 
 	/*
 	 * first byte:
@@ -73,37 +81,37 @@ struct sabi_commands {
 	 * 0x03 - 3G is on
 	 * TODO, verify 3G is correct, that doesn't seem right...
 	 */
-	u8 get_wireless_button;
-	u8 set_wireless_button;
+	u16 get_wireless_button;
+	u16 set_wireless_button;
 
 	/* 0 is off, 1 is on */
-	u8 get_backlight;
-	u8 set_backlight;
+	u16 get_backlight;
+	u16 set_backlight;
 
 	/*
 	 * 0x80 or 0x00 - no action
 	 * 0x81 - recovery key pressed
 	 */
-	u8 get_recovery_mode;
-	u8 set_recovery_mode;
+	u16 get_recovery_mode;
+	u16 set_recovery_mode;
 
 	/*
 	 * on seclinux: 0 is low, 1 is high,
 	 * on swsmi: 0 is normal, 1 is silent, 2 is turbo
 	 */
-	u8 get_performance_level;
-	u8 set_performance_level;
+	u16 get_performance_level;
+	u16 set_performance_level;
 
 	/*
 	 * Tell the BIOS that Linux is running on this machine.
 	 * 81 is on, 80 is off
 	 */
-	u8 set_linux;
+	u16 set_linux;
 };
 
 struct sabi_performance_level {
 	const char *name;
-	u8 value;
+	u16 value;
 };
 
 struct sabi_config {
@@ -246,16 +254,25 @@ static int debug;
 module_param(debug, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(debug, "Debug enabled or not");
 
-static int sabi_get_command(struct samsung_laptop *samsung,
-			    u8 command, struct sabi_retval *sretval)
+static int sabi_command(struct samsung_laptop *samsung, u16 command,
+			struct sabi_data *in,
+			struct sabi_data *out)
 {
 	const struct sabi_config *config = samsung->config;
-	int retval = 0;
+	int ret = 0;
 	u16 port = readw(samsung->sabi + config->header_offsets.port);
 	u8 complete, iface_data;
 
 	mutex_lock(&samsung->sabi_mutex);
 
+	if (debug) {
+		if (in)
+			pr_info("SABI 0x%04x {0x%08x, 0x%08x, 0x%04x, 0x%02x}",
+				command, in->d0, in->d1, in->d2, in->d3);
+		else
+			pr_info("SABI 0x%04x", command);
+	}
+
 	/* enable memory to be able to write to it */
 	outb(readb(samsung->sabi + config->header_offsets.en_mem), port);
 
@@ -263,6 +280,12 @@ static int sabi_get_command(struct samsung_laptop *samsung,
 	writew(config->main_function, samsung->sabi_iface + SABI_IFACE_MAIN);
 	writew(command, samsung->sabi_iface + SABI_IFACE_SUB);
 	writeb(0, samsung->sabi_iface + SABI_IFACE_COMPLETE);
+	if (in) {
+		writel(in->d0, samsung->sabi_iface + SABI_IFACE_DATA);
+		writel(in->d1, samsung->sabi_iface + SABI_IFACE_DATA + 4);
+		writew(in->d2, samsung->sabi_iface + SABI_IFACE_DATA + 8);
+		writeb(in->d3, samsung->sabi_iface + SABI_IFACE_DATA + 10);
+	}
 	outb(readb(samsung->sabi + config->header_offsets.iface_func), port);
 
 	/* write protect memory to make it safe */
@@ -272,127 +295,104 @@ static int sabi_get_command(struct samsung_laptop *samsung,
 	complete = readb(samsung->sabi_iface + SABI_IFACE_COMPLETE);
 	iface_data = readb(samsung->sabi_iface + SABI_IFACE_DATA);
 	if (complete != 0xaa || iface_data == 0xff) {
-		pr_warn("SABI get command 0x%02x failed with completion flag 0x%02x and data 0x%02x\n",
-		        command, complete, iface_data);
-		retval = -EINVAL;
+		pr_warn("SABI command 0x%04x failed with"
+			" completion flag 0x%02x", command, complete);
+		ret = -EINVAL;
 		goto exit;
 	}
-	/*
-	 * Save off the data into a structure so the caller use it.
-	 * Right now we only want the first 4 bytes,
-	 * There are commands that need more, but not for the ones we
-	 * currently care about.
-	 */
-	sretval->retval[0] = readb(samsung->sabi_iface + SABI_IFACE_DATA);
-	sretval->retval[1] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 1);
-	sretval->retval[2] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 2);
-	sretval->retval[3] = readb(samsung->sabi_iface + SABI_IFACE_DATA + 3);
+
+	if (out) {
+		out->d0 = readl(samsung->sabi_iface + SABI_IFACE_DATA);
+		out->d1 = readl(samsung->sabi_iface + SABI_IFACE_DATA + 4);
+		out->d2 = readw(samsung->sabi_iface + SABI_IFACE_DATA + 2);
+		out->d3 = readb(samsung->sabi_iface + SABI_IFACE_DATA + 1);
+	}
+
+	if (debug && out) {
+		pr_info("SABI {0x%08x, 0x%08x, 0x%04x, 0x%02x}",
+			out->d0, out->d1, out->d2, out->d3);
+	}
 
 exit:
 	mutex_unlock(&samsung->sabi_mutex);
-	return retval;
-
+	return ret;
 }
 
-static int sabi_set_command(struct samsung_laptop *samsung,
-			    u8 command, u8 data)
+/* simple wrappers usable with most commands */
+static int sabi_set_commandb(struct samsung_laptop *samsung,
+			     u16 command, u8 data)
 {
-	const struct sabi_config *config = samsung->config;
-	int retval = 0;
-	u16 port = readw(samsung->sabi + config->header_offsets.port);
-	u8 complete, iface_data;
+	struct sabi_data in = { .d0 = 0, .d1 = 0, .d2 = 0, .d3 = 0 };
 
-	mutex_lock(&samsung->sabi_mutex);
-
-	/* enable memory to be able to write to it */
-	outb(readb(samsung->sabi + config->header_offsets.en_mem), port);
-
-	/* write out the command */
-	writew(config->main_function, samsung->sabi_iface + SABI_IFACE_MAIN);
-	writew(command, samsung->sabi_iface + SABI_IFACE_SUB);
-	writeb(0, samsung->sabi_iface + SABI_IFACE_COMPLETE);
-	writeb(data, samsung->sabi_iface + SABI_IFACE_DATA);
-	outb(readb(samsung->sabi + config->header_offsets.iface_func), port);
-
-	/* write protect memory to make it safe */
-	outb(readb(samsung->sabi + config->header_offsets.re_mem), port);
-
-	/* see if the command actually succeeded */
-	complete = readb(samsung->sabi_iface + SABI_IFACE_COMPLETE);
-	iface_data = readb(samsung->sabi_iface + SABI_IFACE_DATA);
-	if (complete != 0xaa || iface_data == 0xff) {
-		pr_warn("SABI set command 0x%02x failed with completion flag 0x%02x and data 0x%02x\n",
-		       command, complete, iface_data);
-		retval = -EINVAL;
-	}
-
-	mutex_unlock(&samsung->sabi_mutex);
-	return retval;
+	in.data[0] = data;
+	return sabi_command(samsung, command, &in, NULL);
 }
 
 static void test_backlight(struct samsung_laptop *samsung)
 {
 	const struct sabi_commands *commands = &samsung->config->commands;
-	struct sabi_retval sretval;
+	struct sabi_data sretval;
 
-	sabi_get_command(samsung, commands->get_backlight, &sretval);
-	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, commands->get_backlight, NULL, &sretval);
+	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.data[0]);
 
-	sabi_set_command(samsung, commands->set_backlight, 0);
+	sabi_set_commandb(samsung, commands->set_backlight, 0);
 	printk(KERN_DEBUG "backlight should be off\n");
 
-	sabi_get_command(samsung, commands->get_backlight, &sretval);
-	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, commands->get_backlight, NULL, &sretval);
+	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.data[0]);
 
 	msleep(1000);
 
-	sabi_set_command(samsung, commands->set_backlight, 1);
+	sabi_set_commandb(samsung, commands->set_backlight, 1);
 	printk(KERN_DEBUG "backlight should be on\n");
 
-	sabi_get_command(samsung, commands->get_backlight, &sretval);
-	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, commands->get_backlight, NULL, &sretval);
+	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.data[0]);
 }
 
 static void test_wireless(struct samsung_laptop *samsung)
 {
 	const struct sabi_commands *commands = &samsung->config->commands;
-	struct sabi_retval sretval;
+	struct sabi_data sretval;
 
-	sabi_get_command(samsung, commands->get_wireless_button, &sretval);
-	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, commands->get_wireless_button, NULL, &sretval);
+	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.data[0]);
 
-	sabi_set_command(samsung, commands->set_wireless_button, 0);
+	sabi_set_commandb(samsung, commands->set_wireless_button, 0);
 	printk(KERN_DEBUG "wireless led should be off\n");
 
-	sabi_get_command(samsung, commands->get_wireless_button, &sretval);
-	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, commands->get_wireless_button, NULL, &sretval);
+	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.data[0]);
 
 	msleep(1000);
 
-	sabi_set_command(samsung, commands->set_wireless_button, 1);
+	sabi_set_commandb(samsung, commands->set_wireless_button, 1);
 	printk(KERN_DEBUG "wireless led should be on\n");
 
-	sabi_get_command(samsung, commands->get_wireless_button, &sretval);
-	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, commands->get_wireless_button, NULL, &sretval);
+	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.data[0]);
 }
 
 static int read_brightness(struct samsung_laptop *samsung)
 {
 	const struct sabi_config *config = samsung->config;
 	const struct sabi_commands *commands = &samsung->config->commands;
-	struct sabi_retval sretval;
+	struct sabi_data sretval;
 	int user_brightness = 0;
 	int retval;
 
-	retval = sabi_get_command(samsung, commands->get_brightness,
-				  &sretval);
-	if (!retval) {
-		user_brightness = sretval.retval[0];
-		if (user_brightness > config->min_brightness)
-			user_brightness -= config->min_brightness;
-		else
-			user_brightness = 0;
-	}
+	retval = sabi_command(samsung, commands->get_brightness,
+			      NULL, &sretval);
+	if (retval)
+		return retval;
+
+	user_brightness = sretval.data[0];
+	if (user_brightness > config->min_brightness)
+		user_brightness -= config->min_brightness;
+	else
+		user_brightness = 0;
+
 	return user_brightness;
 }
 
@@ -410,10 +410,10 @@ static void set_brightness(struct samsung_laptop *samsung, u8 user_brightness)
 		if (user_brightness == read_brightness(samsung))
 			return;
 
-		sabi_set_command(samsung, commands->set_brightness, 0);
+		sabi_set_commandb(samsung, commands->set_brightness, 0);
 	}
 
-	sabi_set_command(samsung, commands->set_brightness, user_level);
+	sabi_set_commandb(samsung, commands->set_brightness, user_level);
 }
 
 static int get_brightness(struct backlight_device *bd)
@@ -465,9 +465,9 @@ static int update_status(struct backlight_device *bd)
 	set_brightness(samsung, bd->props.brightness);
 
 	if (bd->props.power == FB_BLANK_UNBLANK)
-		sabi_set_command(samsung, commands->set_backlight, 1);
+		sabi_set_commandb(samsung, commands->set_backlight, 1);
 	else
-		sabi_set_command(samsung, commands->set_backlight, 0);
+		sabi_set_commandb(samsung, commands->set_backlight, 0);
 
 	return 0;
 }
@@ -488,9 +488,9 @@ static int rfkill_set(void *data, bool blocked)
 	 * blocked == true is off
 	 */
 	if (blocked)
-		sabi_set_command(samsung, commands->set_wireless_button, 0);
+		sabi_set_commandb(samsung, commands->set_wireless_button, 0);
 	else
-		sabi_set_command(samsung, commands->set_wireless_button, 1);
+		sabi_set_commandb(samsung, commands->set_wireless_button, 1);
 
 	return 0;
 }
@@ -505,19 +505,19 @@ static ssize_t get_performance_level(struct device *dev,
 	struct samsung_laptop *samsung = dev_get_drvdata(dev);
 	const struct sabi_config *config = samsung->config;
 	const struct sabi_commands *commands = &config->commands;
-	struct sabi_retval sretval;
+	struct sabi_data sretval;
 	int retval;
 	int i;
 
 	/* Read the state */
-	retval = sabi_get_command(samsung, commands->get_performance_level,
-				  &sretval);
+	retval = sabi_command(samsung, commands->get_performance_level,
+			      NULL, &sretval);
 	if (retval)
 		return retval;
 
 	/* The logic is backwards, yeah, lots of fun... */
 	for (i = 0; config->performance_levels[i].name; ++i) {
-		if (sretval.retval[0] == config->performance_levels[i].value)
+		if (sretval.data[0] == config->performance_levels[i].value)
 			return sprintf(buf, "%s\n", config->performance_levels[i].name);
 	}
 	return sprintf(buf, "%s\n", "unknown");
@@ -539,9 +539,9 @@ static ssize_t set_performance_level(struct device *dev,
 		const struct sabi_performance_level *level =
 			&config->performance_levels[i];
 		if (!strncasecmp(level->name, buf, strlen(level->name))) {
-			sabi_set_command(samsung,
-					 commands->set_performance_level,
-					 level->value);
+			sabi_set_commandb(samsung,
+					  commands->set_performance_level,
+					  level->value);
 			break;
 		}
 	}
@@ -685,7 +685,7 @@ static void samsung_sabi_exit(struct samsung_laptop *samsung)
 
 	/* Turn off "Linux" mode in the BIOS */
 	if (config && config->commands.set_linux != 0xff)
-		sabi_set_command(samsung, config->commands.set_linux, 0x80);
+		sabi_set_commandb(samsung, config->commands.set_linux, 0x80);
 
 	if (samsung->sabi_iface) {
 		iounmap(samsung->sabi_iface);
@@ -724,7 +724,7 @@ static void __init samsung_sabi_selftest(struct samsung_laptop *samsung,
 					unsigned int ifaceP)
 {
 	const struct sabi_config *config = samsung->config;
-	struct sabi_retval sretval;
+	struct sabi_data sretval;
 
 	printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
 	printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
@@ -733,8 +733,8 @@ static void __init samsung_sabi_selftest(struct samsung_laptop *samsung,
 		test_backlight(samsung);
 	test_wireless(samsung);
 
-	sabi_get_command(samsung, config->commands.get_brightness, &sretval);
-	printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.retval[0]);
+	sabi_command(samsung, config->commands.get_brightness, NULL, &sretval);
+	printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.data[0]);
 }
 
 static int __init samsung_sabi_init(struct samsung_laptop *samsung)
@@ -793,8 +793,8 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 
 	/* Turn on "Linux" mode in the BIOS */
 	if (commands->set_linux != 0xff) {
-		int retval = sabi_set_command(samsung,
-					      commands->set_linux, 0x81);
+		int retval = sabi_set_commandb(samsung,
+					       commands->set_linux, 0x81);
 		if (retval) {
 			pr_warn("Linux mode was not set!\n");
 			ret = -ENODEV;
-- 
1.7.5.4

Re: [PATCH 05/15] samsung-laptop: ehance SABI support

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:36PM +0100, Corentin Chary wrote:
> * SABI command are on 16 bits, not 8

Are you sure about that?  I guess this would work, and we rely on the
bits being in the right order so everything works properly?

> * SABI can read/write up to 11 byte of data

It can?

> * There is not real difference between "get" and "set"
>   commands, so refactorise the code of both functions

Ah, nice, I always wanted to do that.

> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH 05/15] samsung-laptop: ehance SABI support

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:18 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:36PM +0100, Corentin Chary wrote:
>> * SABI command are on 16 bits, not 8
>
> Are you sure about that?  I guess this would work, and we rely on the
> bits being in the right order so everything works properly?

Yep, I'm pretty sure this is the case, but I've not yet found a
command using more than 8 bits. Anyway the 16 bit code works
flawlessly here since we were already using "writew(command, ...);"
since the beginning.

Regarding the bits order, the driver depends on X86, so I don't see
this as a real issue.

>> * SABI can read/write up to 11 byte of data
>
> It can?

That's how it seems to work, but I didn't see any command using more
that 24 bits yet.

>> * There is not real difference between "get" and "set"
>>   commands, so refactorise the code of both functions
>
> Ah, nice, I always wanted to do that.
>
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
>
>



-- 
Corentin Chary
http://xf.iksaif.net

[PATCH 06/15] samsung-laptop: add small debugfs interface

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |  136 +++++++++++++++++++++++++++++++++
 1 files changed, 136 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 9dcb582..8dc71a3 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -22,6 +22,8 @@
 #include <linux/platform_device.h>
 #include <linux/rfkill.h>
 #include <linux/acpi.h>
+#include <linux/seq_file.h>
+#include <linux/debugfs.h>
 
 /*
  * This driver is needed because a number of Samsung laptops do not hook
@@ -226,6 +228,24 @@ static const struct sabi_config sabi_configs[] = {
 	{ },
 };
 
+/*
+ * samsung-laptop/    - debugfs root directory
+ *   f0000_segment    - dump f0000 segment
+ *   command          - current command
+ *   data             - current data
+ *   d0, d1, d2, d3   - data fields
+ *   call             - call SABI using command and data
+ */
+
+struct samsung_laptop_debug {
+	struct dentry *root;
+	struct sabi_data data;
+	u16 command;
+
+	struct debugfs_blob_wrapper f0000_wrapper;
+	struct debugfs_blob_wrapper data_wrapper;
+};
+
 struct samsung_laptop {
 	const struct sabi_config *config;
 
@@ -239,6 +259,8 @@ struct samsung_laptop {
 	struct backlight_device *backlight_device;
 	struct rfkill *rfk;
 
+	struct samsung_laptop_debug debug;
+
 	bool handle_backlight;
 	bool has_stepping_quirk;
 };
@@ -679,6 +701,113 @@ static int __init samsung_sysfs_init(struct samsung_laptop *samsung)
 
 }
 
+static int show_call(struct seq_file *m, void *data)
+{
+	struct samsung_laptop *samsung = m->private;
+	struct sabi_data *sdata = &samsung->debug.data;
+	int ret;
+
+	seq_printf(m, "SABI 0x%04x {0x%08x, 0x%08x, 0x%04x, 0x%02x}\n",
+		   samsung->debug.command,
+		   sdata->d0, sdata->d1, sdata->d2, sdata->d3);
+
+	ret = sabi_command(samsung, samsung->debug.command, sdata, sdata);
+
+	if (ret) {
+		seq_printf(m, "SABI command 0x%04x failed\n",
+			   samsung->debug.command);
+		return ret;
+	}
+
+	seq_printf(m, "SABI {0x%08x, 0x%08x, 0x%04x, 0x%02x}\n",
+		   sdata->d0, sdata->d1, sdata->d2, sdata->d3);
+	return 0;
+}
+
+static int samsung_debugfs_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_call, inode->i_private);
+}
+
+static const struct file_operations samsung_laptop_call_io_ops = {
+	.owner = THIS_MODULE,
+	.open = samsung_debugfs_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static void samsung_debugfs_exit(struct samsung_laptop *samsung)
+{
+	debugfs_remove_recursive(samsung->debug.root);
+}
+
+static int samsung_debugfs_init(struct samsung_laptop *samsung)
+{
+	struct dentry *dent;
+
+	samsung->debug.root = debugfs_create_dir("samsung-laptop", NULL);
+	if (!samsung->debug.root) {
+		pr_err("failed to create debugfs directory");
+		goto error_debugfs;
+	}
+
+	samsung->debug.f0000_wrapper.data = samsung->f0000_segment;
+	samsung->debug.f0000_wrapper.size = 0xffff;
+
+	samsung->debug.data_wrapper.data = &samsung->debug.data;
+	samsung->debug.data_wrapper.size = sizeof(samsung->debug.data);
+
+	dent = debugfs_create_u16("command", S_IRUGO | S_IWUSR,
+				  samsung->debug.root, &samsung->debug.command);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_u32("d0", S_IRUGO | S_IWUSR, samsung->debug.root,
+				  &samsung->debug.data.d0);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_u32("d1", S_IRUGO | S_IWUSR, samsung->debug.root,
+				  &samsung->debug.data.d1);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_u16("d2", S_IRUGO | S_IWUSR, samsung->debug.root,
+				  &samsung->debug.data.d2);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_u8("d3", S_IRUGO | S_IWUSR, samsung->debug.root,
+				 &samsung->debug.data.d3);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_blob("data", S_IRUGO | S_IWUSR,
+				   samsung->debug.root,
+				   &samsung->debug.data_wrapper);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_blob("f0000_segment", S_IRUSR | S_IWUSR,
+				   samsung->debug.root,
+				   &samsung->debug.f0000_wrapper);
+	if (!dent)
+		goto error_debugfs;
+
+	dent = debugfs_create_file("call", S_IFREG | S_IRUGO,
+				   samsung->debug.root, samsung,
+				   &samsung_laptop_call_io_ops);
+	if (!dent)
+		goto error_debugfs;
+
+	return 0;
+
+error_debugfs:
+	samsung_debugfs_exit(samsung);
+	return -ENOMEM;
+}
+
 static void samsung_sabi_exit(struct samsung_laptop *samsung)
 {
 	const struct sabi_config *config = samsung->config;
@@ -1122,9 +1251,15 @@ static int __init samsung_init(void)
 	if (ret)
 		goto error_rfkill;
 
+	ret = samsung_debugfs_init(samsung);
+	if (ret)
+		goto error_debugfs;
+
 	samsung_platform_device = samsung->platform_device;
 	return ret;
 
+error_debugfs:
+	samsung_rfkill_exit(samsung);
 error_rfkill:
 	samsung_backlight_exit(samsung);
 error_backlight:
@@ -1144,6 +1279,7 @@ static void __exit samsung_exit(void)
 
 	samsung = platform_get_drvdata(samsung_platform_device);
 
+	samsung_debugfs_exit(samsung);
 	samsung_rfkill_exit(samsung);
 	samsung_backlight_exit(samsung);
 	samsung_sysfs_exit(samsung);
-- 
1.7.5.4

Re: [PATCH 06/15] samsung-laptop: add small debugfs interface

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:37PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

You should describe what the debugfs interface does please.

I have an idea, and I like it a lot, but still, it would be nice to let
the others know :)

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH 06/15] samsung-laptop: add small debugfs interface

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:20 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:37PM +0100, Corentin Chary wrote:
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> You should describe what the debugfs interface does please.
>
> I have an idea, and I like it a lot, but still, it would be nice to let
> the others know :)
>
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
>

It's in the code:

+/*+ * samsung-laptop/    - debugfs root directory+ *   f0000_segment
  - dump f0000 segment+ *   command          - current command+ *
data             - current data+ *   d0, d1, d2, d3   - data fields+ *
  call             - call SABI using command and data+ */
But maybe I should add it to the changelog and add an example on how to use it.

But yeah, it was very useful for experimentation.

-- 
Corentin Chary
http://xf.iksaif.net

[PATCH 07/15] samsung-laptop: remove selftest

[Corentin Chary]

We can now do the self test using debugfs, so remove the code
and keep the debug flag to enable more traces.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |   77 +++-----------------------------
 1 files changed, 8 insertions(+), 69 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 8dc71a3..6493542 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -350,52 +350,6 @@ static int sabi_set_commandb(struct samsung_laptop *samsung,
 	return sabi_command(samsung, command, &in, NULL);
 }
 
-static void test_backlight(struct samsung_laptop *samsung)
-{
-	const struct sabi_commands *commands = &samsung->config->commands;
-	struct sabi_data sretval;
-
-	sabi_command(samsung, commands->get_backlight, NULL, &sretval);
-	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.data[0]);
-
-	sabi_set_commandb(samsung, commands->set_backlight, 0);
-	printk(KERN_DEBUG "backlight should be off\n");
-
-	sabi_command(samsung, commands->get_backlight, NULL, &sretval);
-	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.data[0]);
-
-	msleep(1000);
-
-	sabi_set_commandb(samsung, commands->set_backlight, 1);
-	printk(KERN_DEBUG "backlight should be on\n");
-
-	sabi_command(samsung, commands->get_backlight, NULL, &sretval);
-	printk(KERN_DEBUG "backlight = 0x%02x\n", sretval.data[0]);
-}
-
-static void test_wireless(struct samsung_laptop *samsung)
-{
-	const struct sabi_commands *commands = &samsung->config->commands;
-	struct sabi_data sretval;
-
-	sabi_command(samsung, commands->get_wireless_button, NULL, &sretval);
-	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.data[0]);
-
-	sabi_set_commandb(samsung, commands->set_wireless_button, 0);
-	printk(KERN_DEBUG "wireless led should be off\n");
-
-	sabi_command(samsung, commands->get_wireless_button, NULL, &sretval);
-	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.data[0]);
-
-	msleep(1000);
-
-	sabi_set_commandb(samsung, commands->set_wireless_button, 1);
-	printk(KERN_DEBUG "wireless led should be on\n");
-
-	sabi_command(samsung, commands->get_wireless_button, NULL, &sretval);
-	printk(KERN_DEBUG "wireless led = 0x%02x\n", sretval.data[0]);
-}
-
 static int read_brightness(struct samsung_laptop *samsung)
 {
 	const struct sabi_config *config = samsung->config;
@@ -828,12 +782,14 @@ static void samsung_sabi_exit(struct samsung_laptop *samsung)
 	samsung->config = NULL;
 }
 
-static __init void samsung_sabi_infos(struct samsung_laptop *samsung, int loca)
+static __init void samsung_sabi_infos(struct samsung_laptop *samsung, int loca,
+				      unsigned int ifaceP)
 {
 	const struct sabi_config *config = samsung->config;
 
 	printk(KERN_DEBUG "This computer supports SABI==%x\n",
 	       loca + 0xf0000 - 6);
+
 	printk(KERN_DEBUG "SABI header:\n");
 	printk(KERN_DEBUG " SMI Port Number = 0x%04x\n",
 	       readw(samsung->sabi + config->header_offsets.port));
@@ -847,23 +803,8 @@ static __init void samsung_sabi_infos(struct samsung_laptop *samsung, int loca)
 	       readw(samsung->sabi + config->header_offsets.data_offset));
 	printk(KERN_DEBUG " SABI data segment = 0x%04x\n",
 	       readw(samsung->sabi + config->header_offsets.data_segment));
-}
-
-static void __init samsung_sabi_selftest(struct samsung_laptop *samsung,
-					unsigned int ifaceP)
-{
-	const struct sabi_config *config = samsung->config;
-	struct sabi_data sretval;
-
-	printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
-	printk(KERN_DEBUG "sabi_iface = %p\n", samsung->sabi_iface);
 
-	if (samsung->handle_backlight)
-		test_backlight(samsung);
-	test_wireless(samsung);
-
-	sabi_command(samsung, config->commands.get_brightness, NULL, &sretval);
-	printk(KERN_DEBUG "brightness = 0x%02x\n", sretval.data[0]);
+	printk(KERN_DEBUG " SABI pointer = 0x%08x\n", ifaceP);
 }
 
 static int __init samsung_sabi_init(struct samsung_laptop *samsung)
@@ -904,12 +845,13 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 	loca += 1;
 	samsung->sabi = (samsung->f0000_segment + loca);
 
-	if (debug)
-		samsung_sabi_infos(samsung, loca);
-
 	/* Get a pointer to the SABI Interface */
 	ifaceP = (readw(samsung->sabi + config->header_offsets.data_segment) & 0x0ffff) << 4;
 	ifaceP += readw(samsung->sabi + config->header_offsets.data_offset) & 0x0ffff;
+
+	if (debug)
+		samsung_sabi_infos(samsung, loca, ifaceP);
+
 	samsung->sabi_iface = ioremap_nocache(ifaceP, 16);
 	if (!samsung->sabi_iface) {
 		pr_err("Can't remap %x\n", ifaceP);
@@ -917,9 +859,6 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 		goto exit;
 	}
 
-	if (debug)
-		samsung_sabi_selftest(samsung, ifaceP);
-
 	/* Turn on "Linux" mode in the BIOS */
 	if (commands->set_linux != 0xff) {
 		int retval = sabi_set_commandb(samsung,
-- 
1.7.5.4

Re: [PATCH 07/15] samsung-laptop: remove selftest

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:38PM +0100, Corentin Chary wrote:
> We can now do the self test using debugfs, so remove the code
> and keep the debug flag to enable more traces.
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

[PATCH 08/15] samsung-laptop: add battery life extender support

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 .../ABI/testing/sysfs-driver-samsung-laptop        |   10 +++
 drivers/platform/x86/samsung-laptop.c              |   82 ++++++++++++++++++++
 2 files changed, 92 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-samsung-laptop b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
index 0a81023..a6a56e1 100644
--- a/Documentation/ABI/testing/sysfs-driver-samsung-laptop
+++ b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
@@ -17,3 +17,13 @@ Description:	Some Samsung laptops have different "performance levels"
 		Specifically, not all support the "overclock" option,
 		and it's still unknown if this value even changes
 		anything, other than making the user feel a bit better.
+
+What:		/sys/devices/platform/samsung/battery_life_extender
+Date:		December 1, 2011
+KernelVersion:	3.3
+Contact:	Corentin Chary <corentin.chary@gmail.com>
+Description:	Max battery charge level can be modified, battery cycle
+		life can be extended by reducing the max battery charge
+		level.
+		0 means normal battery mode (100% charge)
+		1 means battery life extender mode (80% charge)
diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 6493542..16f45da 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -104,6 +104,10 @@ struct sabi_commands {
 	u16 get_performance_level;
 	u16 set_performance_level;
 
+	/* 0x80 is off, 0x81 is on */
+	u16 get_battery_life_extender;
+	u16 set_battery_life_extender;
+
 	/*
 	 * Tell the BIOS that Linux is running on this machine.
 	 * 81 is on, 80 is off
@@ -157,6 +161,9 @@ static const struct sabi_config sabi_configs[] = {
 			.get_performance_level = 0x08,
 			.set_performance_level = 0x09,
 
+			.get_battery_life_extender = 0xFFFF,
+			.set_battery_life_extender = 0xFFFF,
+
 			.set_linux = 0x0a,
 		},
 
@@ -204,6 +211,9 @@ static const struct sabi_config sabi_configs[] = {
 			.get_performance_level = 0x31,
 			.set_performance_level = 0x32,
 
+			.get_battery_life_extender = 0x65,
+			.set_battery_life_extender = 0x66,
+
 			.set_linux = 0xff,
 		},
 
@@ -531,8 +541,78 @@ static ssize_t set_performance_level(struct device *dev,
 static DEVICE_ATTR(performance_level, S_IWUSR | S_IRUGO,
 		   get_performance_level, set_performance_level);
 
+static int read_battery_life_extender(struct samsung_laptop *samsung)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+	int retval;
+
+	if (commands->get_battery_life_extender == 0xFFFF)
+		return -ENODEV;
+
+	memset(&data, 0, sizeof(data));
+	data.data[0] = 0x80;
+	retval = sabi_command(samsung, commands->get_battery_life_extender,
+			      &data, &data);
+
+	if (retval)
+		return retval;
+
+	if (data.data[0] != 0 && data.data[0] != 1)
+		return -ENODEV;
+
+	return data.data[0];
+}
+
+static int write_battery_life_extender(struct samsung_laptop *samsung,
+				       int enabled)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+
+	memset(&data, 0, sizeof(data));
+	data.data[0] = 0x80 | enabled;
+	return sabi_command(samsung, commands->set_battery_life_extender,
+			    &data, NULL);
+}
+
+static ssize_t get_battery_life_extender(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct samsung_laptop *samsung = dev_get_drvdata(dev);
+	int ret;
+
+	ret = read_battery_life_extender(samsung);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t set_battery_life_extender(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct samsung_laptop *samsung = dev_get_drvdata(dev);
+	int ret, value;
+
+	if (!count || sscanf(buf, "%i", &value) != 1)
+		return -EINVAL;
+
+	ret = write_battery_life_extender(samsung, !!value);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR(battery_life_extender, S_IWUSR | S_IRUGO,
+		   get_battery_life_extender, set_battery_life_extender);
+
 static struct attribute *platform_attributes[] = {
 	&dev_attr_performance_level.attr,
+	&dev_attr_battery_life_extender.attr,
 	NULL
 };
 
@@ -631,6 +711,8 @@ static mode_t samsung_sysfs_is_visible(struct kobject *kobj,
 
 	if (attr == &dev_attr_performance_level.attr)
 		ok = !!samsung->config->performance_levels[0].name;
+	if (attr == &dev_attr_battery_life_extender.attr)
+		ok = !!(read_battery_life_extender(samsung) >= 0);
 
 	return ok ? attr->mode : 0;
 }
-- 
1.7.5.4

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:39PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  .../ABI/testing/sysfs-driver-samsung-laptop        |   10 +++
>  drivers/platform/x86/samsung-laptop.c              |   82 ++++++++++++++++++++
>  2 files changed, 92 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-samsung-laptop b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> index 0a81023..a6a56e1 100644
> --- a/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> +++ b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> @@ -17,3 +17,13 @@ Description:	Some Samsung laptops have different "performance levels"
>  		Specifically, not all support the "overclock" option,
>  		and it's still unknown if this value even changes
>  		anything, other than making the user feel a bit better.
> +
> +What:		/sys/devices/platform/samsung/battery_life_extender
> +Date:		December 1, 2011
> +KernelVersion:	3.3
> +Contact:	Corentin Chary <corentin.chary@gmail.com>
> +Description:	Max battery charge level can be modified, battery cycle
> +		life can be extended by reducing the max battery charge
> +		level.
> +		0 means normal battery mode (100% charge)
> +		1 means battery life extender mode (80% charge)

Really?  Huh, learn something new every day, that's rally cool, nice
job.

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:21 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:39PM +0100, Corentin Chary wrote:
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>> ---
>>  .../ABI/testing/sysfs-driver-samsung-laptop        |   10 +++
>>  drivers/platform/x86/samsung-laptop.c              |   82 ++++++++++++++++++++
>>  2 files changed, 92 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-samsung-laptop b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
>> index 0a81023..a6a56e1 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-samsung-laptop
>> +++ b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
>> @@ -17,3 +17,13 @@ Description:       Some Samsung laptops have different "performance levels"
>>               Specifically, not all support the "overclock" option,
>>               and it's still unknown if this value even changes
>>               anything, other than making the user feel a bit better.
>> +
>> +What:                /sys/devices/platform/samsung/battery_life_extender
>> +Date:                December 1, 2011
>> +KernelVersion:       3.3
>> +Contact:     Corentin Chary <corentin.chary@gmail.com>
>> +Description: Max battery charge level can be modified, battery cycle
>> +             life can be extended by reducing the max battery charge
>> +             level.
>> +             0 means normal battery mode (100% charge)
>> +             1 means battery life extender mode (80% charge)
>
> Really?  Huh, learn something new every day, that's rally cool, nice
> job.

Maybe it's only marketing bullshit... but the option is present both
in the BIOS and on windows, so it looks like a real feature.


-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[David Herrmann]

Hi Corentin

On Tue, Nov 22, 2011 at 11:02 PM, Corentin Chary <corentincj@iksaif.net> wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  .../ABI/testing/sysfs-driver-samsung-laptop        |   10 +++
>  drivers/platform/x86/samsung-laptop.c              |   82 ++++++++++++++++++++
>  2 files changed, 92 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-samsung-laptop b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> index 0a81023..a6a56e1 100644
> --- a/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> +++ b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> @@ -17,3 +17,13 @@ Description: Some Samsung laptops have different "performance levels"
>                Specifically, not all support the "overclock" option,
>                and it's still unknown if this value even changes
>                anything, other than making the user feel a bit better.
> +
> +What:          /sys/devices/platform/samsung/battery_life_extender
> +Date:          December 1, 2011
> +KernelVersion: 3.3
> +Contact:       Corentin Chary <corentin.chary@gmail.com>
> +Description:   Max battery charge level can be modified, battery cycle
> +               life can be extended by reducing the max battery charge
> +               level.
> +               0 means normal battery mode (100% charge)
> +               1 means battery life extender mode (80% charge)
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index 6493542..16f45da 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -104,6 +104,10 @@ struct sabi_commands {
>        u16 get_performance_level;
>        u16 set_performance_level;
>
> +       /* 0x80 is off, 0x81 is on */
> +       u16 get_battery_life_extender;
> +       u16 set_battery_life_extender;
> +
>        /*
>         * Tell the BIOS that Linux is running on this machine.
>         * 81 is on, 80 is off
> @@ -157,6 +161,9 @@ static const struct sabi_config sabi_configs[] = {
>                        .get_performance_level = 0x08,
>                        .set_performance_level = 0x09,
>
> +                       .get_battery_life_extender = 0xFFFF,
> +                       .set_battery_life_extender = 0xFFFF,
> +
>                        .set_linux = 0x0a,
>                },
>
> @@ -204,6 +211,9 @@ static const struct sabi_config sabi_configs[] = {
>                        .get_performance_level = 0x31,
>                        .set_performance_level = 0x32,
>
> +                       .get_battery_life_extender = 0x65,
> +                       .set_battery_life_extender = 0x66,
> +
>                        .set_linux = 0xff,
>                },
>
> @@ -531,8 +541,78 @@ static ssize_t set_performance_level(struct device *dev,
>  static DEVICE_ATTR(performance_level, S_IWUSR | S_IRUGO,
>                   get_performance_level, set_performance_level);
>
> +static int read_battery_life_extender(struct samsung_laptop *samsung)
> +{
> +       const struct sabi_commands *commands = &samsung->config->commands;
> +       struct sabi_data data;
> +       int retval;
> +
> +       if (commands->get_battery_life_extender == 0xFFFF)
> +               return -ENODEV;
> +
> +       memset(&data, 0, sizeof(data));
> +       data.data[0] = 0x80;
> +       retval = sabi_command(samsung, commands->get_battery_life_extender,
> +                             &data, &data);
> +
> +       if (retval)
> +               return retval;
> +
> +       if (data.data[0] != 0 && data.data[0] != 1)
> +               return -ENODEV;
> +
> +       return data.data[0];
> +}
> +
> +static int write_battery_life_extender(struct samsung_laptop *samsung,
> +                                      int enabled)
> +{
> +       const struct sabi_commands *commands = &samsung->config->commands;
> +       struct sabi_data data;
> +
> +       memset(&data, 0, sizeof(data));
> +       data.data[0] = 0x80 | enabled;
> +       return sabi_command(samsung, commands->set_battery_life_extender,
> +                           &data, NULL);
> +}
> +
> +static ssize_t get_battery_life_extender(struct device *dev,
> +                                        struct device_attribute *attr,
> +                                        char *buf)
> +{
> +       struct samsung_laptop *samsung = dev_get_drvdata(dev);
> +       int ret;
> +
> +       ret = read_battery_life_extender(samsung);
> +       if (ret < 0)
> +               return ret;
> +
> +       return sprintf(buf, "%d\n", ret);
> +}
> +
> +static ssize_t set_battery_life_extender(struct device *dev,
> +                                       struct device_attribute *attr,
> +                                       const char *buf, size_t count)
> +{
> +       struct samsung_laptop *samsung = dev_get_drvdata(dev);
> +       int ret, value;
> +
> +       if (!count || sscanf(buf, "%i", &value) != 1)
> +               return -EINVAL;
> +
> +       ret = write_battery_life_extender(samsung, !!value);
> +       if (ret < 0)
> +               return ret;
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR(battery_life_extender, S_IWUSR | S_IRUGO,
> +                  get_battery_life_extender, set_battery_life_extender);
> +
>  static struct attribute *platform_attributes[] = {
>        &dev_attr_performance_level.attr,
> +       &dev_attr_battery_life_extender.attr,
>        NULL
>  };
>
> @@ -631,6 +711,8 @@ static mode_t samsung_sysfs_is_visible(struct kobject *kobj,
>
>        if (attr == &dev_attr_performance_level.attr)
>                ok = !!samsung->config->performance_levels[0].name;
> +       if (attr == &dev_attr_battery_life_extender.attr)
> +               ok = !!(read_battery_life_extender(samsung) >= 0);
>
>        return ok ? attr->mode : 0;
>  }
> --
> 1.7.5.4

My BIOS provides this option, too. However, this driver does not
detect it on my machine. During initialization I get:
samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa

Maybe you can help debugging this. If there is something I can test
via debugfs, just tell me.

Regards
David

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[Corentin Chary]

> My BIOS provides this option, too. However, this driver does not
> detect it on my machine. During initialization I get:
> samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa
>
> Maybe you can help debugging this. If there is something I can test
> via debugfs, just tell me.

Could you try to load the module with debug=1, type these commands

$ cd /sys/kernel/debug/
$ echo 0x65 > command
$ for i in d0 d1 d2 d3; echo 0 > $i; done
$ echo 0x80 > d0
$ cat call

and then show me the output of dmesg ?

Thanks,
-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[David Herrmann]

On Wed, Nov 23, 2011 at 5:54 PM, Corentin Chary
<corentin.chary@gmail.com> wrote:
>> My BIOS provides this option, too. However, this driver does not
>> detect it on my machine. During initialization I get:
>> samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa
>>
>> Maybe you can help debugging this. If there is something I can test
>> via debugfs, just tell me.
>
> Could you try to load the module with debug=1, type these commands
>
> $ cd /sys/kernel/debug/
> $ echo 0x65 > command
> $ for i in d0 d1 d2 d3; echo 0 > $i; done
> $ echo 0x80 > d0
> $ cat call

Would be great if you could add a short comment, what it does, next time ;)

SABI 0x0065 {0x00000080, 0x00000000, 0x0000, 0x00}
SABI {0x00000000, 0x00000000, 0x0000, 0x00}

dmesg:
[ 4988.985360] This computer supports SABI==f518a
[ 4988.985374] SABI header:
[ 4988.985384]  SMI Port Number = 0x00b2
[ 4988.985393]  SMI Interface Function = 0xc0
[ 4988.985402]  SMI enable memory buffer = 0xc1
[ 4988.985412]  SMI restore memory buffer = 0xc2
[ 4988.985421]  SABI data offset = 0x0f00
[ 4988.985429]  SABI data segment = 0xdf01
[ 4988.985438]  SABI pointer = 0x000dff10
[ 4988.985448] samsung_laptop: Backlight controlled by ACPI video driver
[ 4988.985478] samsung_laptop: SABI 0x0065 {0x00000080, 0x00000000,
0x0000, 0x00}
[ 4988.991847] samsung_laptop: SABI {0x00000000, 0x00000000, 0x0000, 0x00}
[ 4988.991886] samsung_laptop: SABI 0x0067 {0x00000080, 0x00000000,
0x0000, 0x00}
[ 4988.993298] samsung_laptop: SABI {0x00000000, 0x00000000, 0x0000, 0x00}
[ 4988.993332] samsung_laptop: SABI 0x0069
[ 4988.994856] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4988.998294] samsung_laptop: SABI 0x0069
[ 4988.999780] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4988.999801] samsung_laptop: SABI 0x0069
[ 4989.001247] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4989.001269] samsung_laptop: SABI 0x006a {0x00010001, 0x00000000,
0x0001, 0x02}
[ 4989.002847] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4989.007453] samsung_laptop: SABI 0x0069
[ 4989.008914] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4989.008936] samsung_laptop: SABI 0x0069
[ 4989.010409] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4989.010430] samsung_laptop: SABI 0x006a {0x00010001, 0x00000000,
0x0001, 0x02}
[ 4989.011998] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
[ 4989.012589] samsung_laptop: SABI 0x0078 {0x0000aabb, 0x00000000,
0x0000, 0x00}
[ 4989.013996] samsung_laptop: SABI command 0x0078 failed with
completion flag 0xaa


> and then show me the output of dmesg ?
>
> Thanks,
> --
> Corentin Chary
> http://xf.iksaif.net
>

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[Corentin Chary]

On Wed, Nov 23, 2011 at 6:10 PM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> On Wed, Nov 23, 2011 at 5:54 PM, Corentin Chary
> <corentin.chary@gmail.com> wrote:
>>> My BIOS provides this option, too. However, this driver does not
>>> detect it on my machine. During initialization I get:
>>> samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa
>>>
>>> Maybe you can help debugging this. If there is something I can test
>>> via debugfs, just tell me.
>>
>> Could you try to load the module with debug=1, type these commands
>>
>> $ cd /sys/kernel/debug/
>> $ echo 0x65 > command
>> $ for i in d0 d1 d2 d3; echo 0 > $i; done
>> $ echo 0x80 > d0
>> $ cat call
>
> Would be great if you could add a short comment, what it does, next time ;)

Hum sorry, basically it calls the SABI command 0x65 with 0x80 as the
first argument byte.

> SABI 0x0065 {0x00000080, 0x00000000, 0x0000, 0x00}
> SABI {0x00000000, 0x00000000, 0x0000, 0x00}
>
> dmesg:
> [ 4988.985360] This computer supports SABI==f518a
> [ 4988.985374] SABI header:
> [ 4988.985384]  SMI Port Number = 0x00b2
> [ 4988.985393]  SMI Interface Function = 0xc0
> [ 4988.985402]  SMI enable memory buffer = 0xc1
> [ 4988.985412]  SMI restore memory buffer = 0xc2
> [ 4988.985421]  SABI data offset = 0x0f00
> [ 4988.985429]  SABI data segment = 0xdf01
> [ 4988.985438]  SABI pointer = 0x000dff10
> [ 4988.985448] samsung_laptop: Backlight controlled by ACPI video driver
> [ 4988.985478] samsung_laptop: SABI 0x0065 {0x00000080, 0x00000000,
> 0x0000, 0x00}
> [ 4988.991847] samsung_laptop: SABI {0x00000000, 0x00000000, 0x0000, 0x00}
> [ 4988.991886] samsung_laptop: SABI 0x0067 {0x00000080, 0x00000000,
> 0x0000, 0x00}
> [ 4988.993298] samsung_laptop: SABI {0x00000000, 0x00000000, 0x0000, 0x00}
> [ 4988.993332] samsung_laptop: SABI 0x0069
> [ 4988.994856] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4988.998294] samsung_laptop: SABI 0x0069
> [ 4988.999780] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4988.999801] samsung_laptop: SABI 0x0069
> [ 4989.001247] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4989.001269] samsung_laptop: SABI 0x006a {0x00010001, 0x00000000,
> 0x0001, 0x02}
> [ 4989.002847] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4989.007453] samsung_laptop: SABI 0x0069
> [ 4989.008914] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4989.008936] samsung_laptop: SABI 0x0069
> [ 4989.010409] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4989.010430] samsung_laptop: SABI 0x006a {0x00010001, 0x00000000,
> 0x0001, 0x02}
> [ 4989.011998] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
> [ 4989.012589] samsung_laptop: SABI 0x0078 {0x0000aabb, 0x00000000,
> 0x0000, 0x00}
> [ 4989.013996] samsung_laptop: SABI command 0x0078 failed with
> completion flag 0xaa

Something is very strange here, the 0x65 didn't fail so you should
really have /sys/devices/platform/samsung/battery_life_extender .
Are you really sure it's not there ?



-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 08/15] samsung-laptop: add battery life extender support

[David Herrmann]

On Wed, Nov 23, 2011 at 7:11 PM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> On Wed, Nov 23, 2011 at 6:10 PM, David Herrmann
> <dh.herrmann@googlemail.com> wrote:
>> On Wed, Nov 23, 2011 at 5:54 PM, Corentin Chary
>> <corentin.chary@gmail.com> wrote:
>>>> My BIOS provides this option, too. However, this driver does not
>>>> detect it on my machine. During initialization I get:
>>>> samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa
>>>>
>>>> Maybe you can help debugging this. If there is something I can test
>>>> via debugfs, just tell me.
>>>
>>> Could you try to load the module with debug=1, type these commands
>>>
>>> $ cd /sys/kernel/debug/
>>> $ echo 0x65 > command
>>> $ for i in d0 d1 d2 d3; echo 0 > $i; done
>>> $ echo 0x80 > d0
>>> $ cat call
>>
>> Would be great if you could add a short comment, what it does, next time ;)
>
> Hum sorry, basically it calls the SABI command 0x65 with 0x80 as the
> first argument byte.
>
>> SABI 0x0065 {0x00000080, 0x00000000, 0x0000, 0x00}
>> SABI {0x00000000, 0x00000000, 0x0000, 0x00}
>>
>> dmesg:
>> [ 4988.985360] This computer supports SABI==f518a
>> [ 4988.985374] SABI header:
>> [ 4988.985384]  SMI Port Number = 0x00b2
>> [ 4988.985393]  SMI Interface Function = 0xc0
>> [ 4988.985402]  SMI enable memory buffer = 0xc1
>> [ 4988.985412]  SMI restore memory buffer = 0xc2
>> [ 4988.985421]  SABI data offset = 0x0f00
>> [ 4988.985429]  SABI data segment = 0xdf01
>> [ 4988.985438]  SABI pointer = 0x000dff10
>> [ 4988.985448] samsung_laptop: Backlight controlled by ACPI video driver
>> [ 4988.985478] samsung_laptop: SABI 0x0065 {0x00000080, 0x00000000,
>> 0x0000, 0x00}
>> [ 4988.991847] samsung_laptop: SABI {0x00000000, 0x00000000, 0x0000, 0x00}
>> [ 4988.991886] samsung_laptop: SABI 0x0067 {0x00000080, 0x00000000,
>> 0x0000, 0x00}
>> [ 4988.993298] samsung_laptop: SABI {0x00000000, 0x00000000, 0x0000, 0x00}
>> [ 4988.993332] samsung_laptop: SABI 0x0069
>> [ 4988.994856] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4988.998294] samsung_laptop: SABI 0x0069
>> [ 4988.999780] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4988.999801] samsung_laptop: SABI 0x0069
>> [ 4989.001247] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4989.001269] samsung_laptop: SABI 0x006a {0x00010001, 0x00000000,
>> 0x0001, 0x02}
>> [ 4989.002847] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4989.007453] samsung_laptop: SABI 0x0069
>> [ 4989.008914] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4989.008936] samsung_laptop: SABI 0x0069
>> [ 4989.010409] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4989.010430] samsung_laptop: SABI 0x006a {0x00010001, 0x00000000,
>> 0x0001, 0x02}
>> [ 4989.011998] samsung_laptop: SABI {0x00010201, 0x00000000, 0x0001, 0x02}
>> [ 4989.012589] samsung_laptop: SABI 0x0078 {0x0000aabb, 0x00000000,
>> 0x0000, 0x00}
>> [ 4989.013996] samsung_laptop: SABI command 0x0078 failed with
>> completion flag 0xaa
>
> Something is very strange here, the 0x65 didn't fail so you should
> really have /sys/devices/platform/samsung/battery_life_extender .
> Are you really sure it's not there ?

Argh. I did see the usb_charge but somehow missed the
battery_life_extender. I am sorry. Works both on my machine and sets
the BIOS flags correctly.

Thanks
David

[PATCH 09/15] samsung-laptop: add usb charge support

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 .../ABI/testing/sysfs-driver-samsung-laptop        |    8 ++
 drivers/platform/x86/samsung-laptop.c              |   82 ++++++++++++++++++++
 2 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-samsung-laptop b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
index a6a56e1..f05b897 100644
--- a/Documentation/ABI/testing/sysfs-driver-samsung-laptop
+++ b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
@@ -27,3 +27,11 @@ Description:	Max battery charge level can be modified, battery cycle
 		level.
 		0 means normal battery mode (100% charge)
 		1 means battery life extender mode (80% charge)
+
+What:		/sys/devices/platform/samsung/usb_charge
+Date:		December 1, 2011
+KernelVersion:	3.3
+Contact:	Corentin Chary <corentin.chary@gmail.com>
+Description:	Use your USB ports to charge devices, even
+		when your laptop is powered off.
+		1 means enabled, 0 means disabled.
diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 16f45da..88acec9 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -108,6 +108,10 @@ struct sabi_commands {
 	u16 get_battery_life_extender;
 	u16 set_battery_life_extender;
 
+	/* 0x80 is off, 0x81 is on */
+	u16 get_usb_charge;
+	u16 set_usb_charge;
+
 	/*
 	 * Tell the BIOS that Linux is running on this machine.
 	 * 81 is on, 80 is off
@@ -164,6 +168,9 @@ static const struct sabi_config sabi_configs[] = {
 			.get_battery_life_extender = 0xFFFF,
 			.set_battery_life_extender = 0xFFFF,
 
+			.get_usb_charge = 0xFFFF,
+			.set_usb_charge = 0xFFFF,
+
 			.set_linux = 0x0a,
 		},
 
@@ -214,6 +221,9 @@ static const struct sabi_config sabi_configs[] = {
 			.get_battery_life_extender = 0x65,
 			.set_battery_life_extender = 0x66,
 
+			.get_usb_charge = 0x67,
+			.set_usb_charge = 0x68,
+
 			.set_linux = 0xff,
 		},
 
@@ -610,9 +620,79 @@ static ssize_t set_battery_life_extender(struct device *dev,
 static DEVICE_ATTR(battery_life_extender, S_IWUSR | S_IRUGO,
 		   get_battery_life_extender, set_battery_life_extender);
 
+static int read_usb_charge(struct samsung_laptop *samsung)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+	int retval;
+
+	if (commands->get_usb_charge == 0xFFFF)
+		return -ENODEV;
+
+	memset(&data, 0, sizeof(data));
+	data.data[0] = 0x80;
+	retval = sabi_command(samsung, commands->get_usb_charge,
+			      &data, &data);
+
+	if (retval)
+		return retval;
+
+	if (data.data[0] != 0 && data.data[0] != 1)
+		return -ENODEV;
+
+	return data.data[0];
+}
+
+static int write_usb_charge(struct samsung_laptop *samsung,
+			    int enabled)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+
+	memset(&data, 0, sizeof(data));
+	data.data[0] = 0x80 | enabled;
+	return sabi_command(samsung, commands->set_usb_charge,
+			    &data, NULL);
+}
+
+static ssize_t get_usb_charge(struct device *dev,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	struct samsung_laptop *samsung = dev_get_drvdata(dev);
+	int ret;
+
+	ret = read_usb_charge(samsung);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", ret);
+}
+
+static ssize_t set_usb_charge(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct samsung_laptop *samsung = dev_get_drvdata(dev);
+	int ret, value;
+
+	if (!count || sscanf(buf, "%i", &value) != 1)
+		return -EINVAL;
+
+	ret = write_usb_charge(samsung, !!value);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR(usb_charge, S_IWUSR | S_IRUGO,
+		   get_usb_charge, set_usb_charge);
+
 static struct attribute *platform_attributes[] = {
 	&dev_attr_performance_level.attr,
 	&dev_attr_battery_life_extender.attr,
+	&dev_attr_usb_charge.attr,
 	NULL
 };
 
@@ -713,6 +793,8 @@ static mode_t samsung_sysfs_is_visible(struct kobject *kobj,
 		ok = !!samsung->config->performance_levels[0].name;
 	if (attr == &dev_attr_battery_life_extender.attr)
 		ok = !!(read_battery_life_extender(samsung) >= 0);
+	if (attr == &dev_attr_usb_charge.attr)
+		ok = !!(read_usb_charge(samsung) >= 0);
 
 	return ok ? attr->mode : 0;
 }
-- 
1.7.5.4

Re: [PATCH 09/15] samsung-laptop: add usb charge support

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:40PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  .../ABI/testing/sysfs-driver-samsung-laptop        |    8 ++
>  drivers/platform/x86/samsung-laptop.c              |   82 ++++++++++++++++++++
>  2 files changed, 90 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-samsung-laptop b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> index a6a56e1..f05b897 100644
> --- a/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> +++ b/Documentation/ABI/testing/sysfs-driver-samsung-laptop
> @@ -27,3 +27,11 @@ Description:	Max battery charge level can be modified, battery cycle
>  		level.
>  		0 means normal battery mode (100% charge)
>  		1 means battery life extender mode (80% charge)
> +
> +What:		/sys/devices/platform/samsung/usb_charge
> +Date:		December 1, 2011
> +KernelVersion:	3.3
> +Contact:	Corentin Chary <corentin.chary@gmail.com>
> +Description:	Use your USB ports to charge devices, even
> +		when your laptop is powered off.
> +		1 means enabled, 0 means disabled.

Very cool.

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

[PATCH 10/15] samsung-laptop: cleanup KConfig

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/Kconfig |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7f43cf8..2f7a9a7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -744,13 +744,16 @@ config XO15_EBOOK
 
 config SAMSUNG_LAPTOP
 	tristate "Samsung Laptop driver"
-	depends on RFKILL && BACKLIGHT_CLASS_DEVICE && X86
+	depends on X86
+	depends on RFKILL || RFKILL = n
+	depends on BACKLIGHT_CLASS_DEVICE
 	---help---
 	  This module implements a driver for a wide range of different
 	  Samsung laptops.  It offers control over the different
-	  function keys, wireless LED, LCD backlight level, and
-	  sometimes provides a "performance_control" sysfs file to allow
-	  the performance level of the laptop to be changed.
+	  function keys, wireless LED, LCD backlight level.
+
+	  It may also provides some sysfs files described in
+	  <file:Documentation/ABI/testing/sysfs-platform-samsung-laptop>
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called samsung-laptop.
-- 
1.7.5.4

Re: [PATCH 10/15] samsung-laptop: cleanup KConfig

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:41PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH 10/15] samsung-laptop: cleanup KConfig

[David Herrmann]

On Tue, Nov 22, 2011 at 11:02 PM, Corentin Chary <corentincj@iksaif.net> wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  drivers/platform/x86/Kconfig |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 7f43cf8..2f7a9a7 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -744,13 +744,16 @@ config XO15_EBOOK
>
>  config SAMSUNG_LAPTOP
>        tristate "Samsung Laptop driver"
> -       depends on RFKILL && BACKLIGHT_CLASS_DEVICE && X86
> +       depends on X86
> +       depends on RFKILL || RFKILL = n
> +       depends on BACKLIGHT_CLASS_DEVICE
>        ---help---
>          This module implements a driver for a wide range of different
>          Samsung laptops.  It offers control over the different
> -         function keys, wireless LED, LCD backlight level, and
> -         sometimes provides a "performance_control" sysfs file to allow
> -         the performance level of the laptop to be changed.
> +         function keys, wireless LED, LCD backlight level.
> +
> +         It may also provides some sysfs files described in

If you are resending this anyway:
"provide"

> +         <file:Documentation/ABI/testing/sysfs-platform-samsung-laptop>
>
>          To compile this driver as a module, choose M here: the module
>          will be called samsung-laptop.
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Cheers
David

[PATCH 11/15] samsung-laptop: add keyboard backlight support

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/Kconfig          |    2 +
 drivers/platform/x86/samsung-laptop.c |  136 +++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 2f7a9a7..7e594e5 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -747,6 +747,8 @@ config SAMSUNG_LAPTOP
 	depends on X86
 	depends on RFKILL || RFKILL = n
 	depends on BACKLIGHT_CLASS_DEVICE
+	select LEDS_CLASS
+	select NEW_LEDS
 	---help---
 	  This module implements a driver for a wide range of different
 	  Samsung laptops.  It offers control over the different
diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 88acec9..17afb13 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -17,6 +17,7 @@
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/backlight.h>
+#include <linux/leds.h>
 #include <linux/fb.h>
 #include <linux/dmi.h>
 #include <linux/platform_device.h>
@@ -112,6 +113,9 @@ struct sabi_commands {
 	u16 get_usb_charge;
 	u16 set_usb_charge;
 
+	/* 0x81 to read, (0x82 | level << 8) to set, 0xaabb to enable */
+	u16 kbd_backlight;
+
 	/*
 	 * Tell the BIOS that Linux is running on this machine.
 	 * 81 is on, 80 is off
@@ -171,6 +175,8 @@ static const struct sabi_config sabi_configs[] = {
 			.get_usb_charge = 0xFFFF,
 			.set_usb_charge = 0xFFFF,
 
+			.kbd_backlight = 0xFFFF,
+
 			.set_linux = 0x0a,
 		},
 
@@ -224,6 +230,8 @@ static const struct sabi_config sabi_configs[] = {
 			.get_usb_charge = 0x67,
 			.set_usb_charge = 0x68,
 
+			.kbd_backlight = 0x78,
+
 			.set_linux = 0xff,
 		},
 
@@ -279,6 +287,11 @@ struct samsung_laptop {
 	struct backlight_device *backlight_device;
 	struct rfkill *rfk;
 
+	struct led_classdev kbd_led;
+	int kbd_led_wk;
+	struct workqueue_struct *led_workqueue;
+	struct work_struct kbd_led_work;
+
 	struct samsung_laptop_debug debug;
 
 	bool handle_backlight;
@@ -745,6 +758,122 @@ static int __init samsung_rfkill_init(struct samsung_laptop *samsung)
 	return 0;
 }
 
+static int kbd_backlight_enable(struct samsung_laptop *samsung)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+	int retval;
+
+	if (commands->kbd_backlight == 0xFFFF)
+		return -ENODEV;
+
+	memset(&data, 0, sizeof(data));
+	data.d0 = 0xaabb;
+	retval = sabi_command(samsung, commands->kbd_backlight,
+			      &data, &data);
+
+	if (retval)
+		return retval;
+
+	if (data.d0 != 0xccdd)
+		return -ENODEV;
+	return 0;
+}
+
+static int kbd_backlight_read(struct samsung_laptop *samsung)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+	int retval;
+
+	memset(&data, 0, sizeof(data));
+	data.data[0] = 0x81;
+	retval = sabi_command(samsung, commands->kbd_backlight,
+			      &data, &data);
+
+	if (retval)
+		return retval;
+
+	return data.data[0];
+}
+
+static int kbd_backlight_write(struct samsung_laptop *samsung, int brightness)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+
+	memset(&data, 0, sizeof(data));
+	data.d0 = 0x82 | ((brightness & 0xFF) << 8);
+	return sabi_command(samsung, commands->kbd_backlight,
+			    &data, NULL);
+}
+
+static void kbd_led_update(struct work_struct *work)
+{
+	struct samsung_laptop *samsung;
+
+	samsung = container_of(work, struct samsung_laptop, kbd_led_work);
+	kbd_backlight_write(samsung, samsung->kbd_led_wk);
+}
+
+static void kbd_led_set(struct led_classdev *led_cdev,
+			enum led_brightness value)
+{
+	struct samsung_laptop *samsung;
+
+	samsung = container_of(led_cdev, struct samsung_laptop, kbd_led);
+
+	if (value > samsung->kbd_led.max_brightness)
+		value = samsung->kbd_led.max_brightness;
+	else if (value < 0)
+		value = 0;
+
+	samsung->kbd_led_wk = value;
+	queue_work(samsung->led_workqueue, &samsung->kbd_led_work);
+}
+
+static enum led_brightness kbd_led_get(struct led_classdev *led_cdev)
+{
+	struct samsung_laptop *samsung;
+
+	samsung = container_of(led_cdev, struct samsung_laptop, kbd_led);
+	return kbd_backlight_read(samsung);
+}
+
+static void samsung_leds_exit(struct samsung_laptop *samsung)
+{
+	if (!IS_ERR_OR_NULL(samsung->kbd_led.dev))
+		led_classdev_unregister(&samsung->kbd_led);
+	if (samsung->led_workqueue)
+		destroy_workqueue(samsung->led_workqueue);
+}
+
+static int __init samsung_leds_init(struct samsung_laptop *samsung)
+{
+	int ret = 0;
+
+	samsung->led_workqueue = create_singlethread_workqueue("led_workqueue");
+	if (!samsung->led_workqueue)
+		return -ENOMEM;
+
+	if (kbd_backlight_enable(samsung) >= 0) {
+		INIT_WORK(&samsung->kbd_led_work, kbd_led_update);
+
+		samsung->kbd_led.name = "samsung::kbd_backlight";
+		samsung->kbd_led.brightness_set = kbd_led_set;
+		samsung->kbd_led.brightness_get = kbd_led_get;
+		samsung->kbd_led.max_brightness = 8;
+
+		ret = led_classdev_register(&samsung->platform_device->dev,
+					   &samsung->kbd_led);
+	}
+
+	if (ret)
+		samsung_leds_exit(samsung);
+
+	return ret;
+}
+
 static void samsung_backlight_exit(struct samsung_laptop *samsung)
 {
 	if (samsung->backlight_device) {
@@ -1354,6 +1483,10 @@ static int __init samsung_init(void)
 	if (ret)
 		goto error_rfkill;
 
+	ret = samsung_leds_init(samsung);
+	if (ret)
+		goto error_leds;
+
 	ret = samsung_debugfs_init(samsung);
 	if (ret)
 		goto error_debugfs;
@@ -1362,6 +1495,8 @@ static int __init samsung_init(void)
 	return ret;
 
 error_debugfs:
+	samsung_leds_exit(samsung);
+error_leds:
 	samsung_rfkill_exit(samsung);
 error_rfkill:
 	samsung_backlight_exit(samsung);
@@ -1383,6 +1518,7 @@ static void __exit samsung_exit(void)
 	samsung = platform_get_drvdata(samsung_platform_device);
 
 	samsung_debugfs_exit(samsung);
+	samsung_leds_exit(samsung);
 	samsung_rfkill_exit(samsung);
 	samsung_backlight_exit(samsung);
 	samsung_sysfs_exit(samsung);
-- 
1.7.5.4

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:42PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

What happens on boxes that don't have this type of hardware?  Will the
LED device still show up, yet it will not work?

You should be able to trigger this off of the model type somehow, much
like the video backlight should work.

thanks,

greg k-h

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:23 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:42PM +0100, Corentin Chary wrote:
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> What happens on boxes that don't have this type of hardware?  Will the
> LED device still show up, yet it will not work?
>
> You should be able to trigger this off of the model type somehow, much
> like the video backlight should work.

kbd_backlight_enable(samsung) will be called, it will fire a {0x78,
0xaabb} command on machines using swsmi and this command will fail if
the feature is not available. I did some test, and unknown commands
seems to fail gracefully, and that makes sabi_command() (and callers)
return -EINVAL).

It think {0x78, 0x80} is also here to tell if the backlight is
available or not, but I can't test it, since it always returns 1 here.

So yeah, if someone could test that on a swsmi laptop without keyboard
backlight, that would be great.
-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[David Herrmann]

On Wed, Nov 23, 2011 at 7:35 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> On Tue, Nov 22, 2011 at 11:23 PM, Greg KH <gregkh@suse.de> wrote:
>> On Tue, Nov 22, 2011 at 11:02:42PM +0100, Corentin Chary wrote:
>>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>>
>> What happens on boxes that don't have this type of hardware?  Will the
>> LED device still show up, yet it will not work?
>>
>> You should be able to trigger this off of the model type somehow, much
>> like the video backlight should work.
>
> kbd_backlight_enable(samsung) will be called, it will fire a {0x78,
> 0xaabb} command on machines using swsmi and this command will fail if
> the feature is not available. I did some test, and unknown commands
> seems to fail gracefully, and that makes sabi_command() (and callers)
> return -EINVAL).
>
> It think {0x78, 0x80} is also here to tell if the backlight is
> available or not, but I can't test it, since it always returns 1 here.

I do not have a keyboard backlight and hence always get an error
message on boot up. It would be great to suppress this error message
as this is not really an error (for me):
samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa

> So yeah, if someone could test that on a swsmi laptop without keyboard
> backlight, that would be great.
> --
> Corentin Chary
> http://xf.iksaif.net
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[Corentin Chary]

On Wed, Nov 23, 2011 at 5:41 PM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> On Wed, Nov 23, 2011 at 7:35 AM, Corentin Chary
> <corentin.chary@gmail.com> wrote:
>> On Tue, Nov 22, 2011 at 11:23 PM, Greg KH <gregkh@suse.de> wrote:
>>> On Tue, Nov 22, 2011 at 11:02:42PM +0100, Corentin Chary wrote:
>>>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>>>
>>> What happens on boxes that don't have this type of hardware?  Will the
>>> LED device still show up, yet it will not work?
>>>
>>> You should be able to trigger this off of the model type somehow, much
>>> like the video backlight should work.
>>
>> kbd_backlight_enable(samsung) will be called, it will fire a {0x78,
>> 0xaabb} command on machines using swsmi and this command will fail if
>> the feature is not available. I did some test, and unknown commands
>> seems to fail gracefully, and that makes sabi_command() (and callers)
>> return -EINVAL).
>>
>> It think {0x78, 0x80} is also here to tell if the backlight is
>> available or not, but I can't test it, since it always returns 1 here.
>
> I do not have a keyboard backlight and hence always get an error
> message on boot up. It would be great to suppress this error message
> as this is not really an error (for me):
> samsung_laptop: SABI command 0x0078 failed with completion flag 0xaa
>
>> So yeah, if someone could test that on a swsmi laptop without keyboard
>> backlight, that would be great.

Yep I've seen that, working on it.



-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[Corentin Chary]

Hi David,I refreshed the patches on the github repo, could you re-try
them ?Thanks
-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[David Herrmann]

On Thu, Nov 24, 2011 at 8:07 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> Hi David,I refreshed the patches on the github repo, could you re-try
> them ?Thanks

Works great. The error message doesn't appear anymore. The devices
still work as expected.
Tested-by: David Herrmann <dh.herrmann@googlemail.com>

Thanks
David

> --
> Corentin Chary
> http://xf.iksaif.net
>

Re: [PATCH 11/15] samsung-laptop: add keyboard backlight support

[Corentin Chary]

On Fri, Nov 25, 2011 at 8:18 PM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> On Thu, Nov 24, 2011 at 8:07 AM, Corentin Chary
> <corentin.chary@gmail.com> wrote:
>> Hi David,I refreshed the patches on the github repo, could you re-try
>> them ?Thanks
>
> Works great. The error message doesn't appear anymore. The devices
> still work as expected.
> Tested-by: David Herrmann <dh.herrmann@googlemail.com>
>

Great, re-sending the updated series  tomorow.



-- 
Corentin Chary
http://xf.iksaif.net

[PATCH 12/15] samsung-laptop: add true rfkill support for swsmi

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |  201 ++++++++++++++++++++++++++++-----
 1 files changed, 170 insertions(+), 31 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 17afb13..c9f574c 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -45,6 +45,9 @@
 #define SABI_IFACE_COMPLETE		0x04
 #define SABI_IFACE_DATA			0x05
 
+#define WL_STATUS_WLAN			0x0
+#define WL_STATUS_BT			0x2
+
 /* Structure get/set data using sabi */
 struct sabi_data {
 	union {
@@ -113,6 +116,10 @@ struct sabi_commands {
 	u16 get_usb_charge;
 	u16 set_usb_charge;
 
+	/* the first byte is for bluetooth and the third one is for wlan */
+	u16 get_wireless_status;
+	u16 set_wireless_status;
+
 	/* 0x81 to read, (0x82 | level << 8) to set, 0xaabb to enable */
 	u16 kbd_backlight;
 
@@ -129,6 +136,7 @@ struct sabi_performance_level {
 };
 
 struct sabi_config {
+	int sabi_version;
 	const char *test_string;
 	u16 main_function;
 	const struct sabi_header_offsets header_offsets;
@@ -140,6 +148,10 @@ struct sabi_config {
 
 static const struct sabi_config sabi_configs[] = {
 	{
+		/* I don't know if it is really 2, but it it is
+		 * less than 3 anyway */
+		.sabi_version = 2,
+
 		.test_string = "SECLINUX",
 
 		.main_function = 0x4c49,
@@ -175,6 +187,9 @@ static const struct sabi_config sabi_configs[] = {
 			.get_usb_charge = 0xFFFF,
 			.set_usb_charge = 0xFFFF,
 
+			.get_wireless_status = 0xFFFF,
+			.set_wireless_status = 0xFFFF,
+
 			.kbd_backlight = 0xFFFF,
 
 			.set_linux = 0x0a,
@@ -195,6 +210,8 @@ static const struct sabi_config sabi_configs[] = {
 		.max_brightness = 8,
 	},
 	{
+		.sabi_version = 3,
+
 		.test_string = "SwSmi@",
 
 		.main_function = 0x5843,
@@ -230,6 +247,9 @@ static const struct sabi_config sabi_configs[] = {
 			.get_usb_charge = 0x67,
 			.set_usb_charge = 0x68,
 
+			.get_wireless_status = 0x69,
+			.set_wireless_status = 0x6a,
+
 			.kbd_backlight = 0x78,
 
 			.set_linux = 0xff,
@@ -274,6 +294,14 @@ struct samsung_laptop_debug {
 	struct debugfs_blob_wrapper data_wrapper;
 };
 
+struct samsung_laptop;
+
+struct samsung_rfkill {
+	struct samsung_laptop *samsung;
+	struct rfkill *rfkill;
+	enum rfkill_type type;
+};
+
 struct samsung_laptop {
 	const struct sabi_config *config;
 
@@ -285,7 +313,9 @@ struct samsung_laptop {
 
 	struct platform_device *platform_device;
 	struct backlight_device *backlight_device;
-	struct rfkill *rfk;
+
+	struct samsung_rfkill wlan;
+	struct samsung_rfkill bluetooth;
 
 	struct led_classdev kbd_led;
 	int kbd_led_wk;
@@ -486,26 +516,74 @@ static const struct backlight_ops backlight_ops = {
 	.update_status	= update_status,
 };
 
-static int rfkill_set(void *data, bool blocked)
+static int seclinux_rfkill_set(void *data, bool blocked)
 {
 	struct samsung_laptop *samsung = data;
 	const struct sabi_commands *commands = &samsung->config->commands;
 
-	/* Do something with blocked...*/
-	/*
-	 * blocked == false is on
-	 * blocked == true is off
-	 */
-	if (blocked)
-		sabi_set_commandb(samsung, commands->set_wireless_button, 0);
+	return sabi_set_commandb(samsung, commands->set_wireless_button,
+				 !blocked);
+}
+
+static struct rfkill_ops seclinux_rfkill_ops = {
+	.set_block = seclinux_rfkill_set,
+};
+
+static int swsmi_wireless_status(struct samsung_laptop *samsung,
+				 struct sabi_data *data)
+{
+	const struct sabi_commands *commands = &samsung->config->commands;
+
+	return sabi_command(samsung, commands->get_wireless_status,
+			    NULL, data);
+}
+
+static int swsmi_rfkill_set(void *priv, bool blocked)
+{
+	struct samsung_rfkill *srfkill = priv;
+	struct samsung_laptop *samsung = srfkill->samsung;
+	const struct sabi_commands *commands = &samsung->config->commands;
+	struct sabi_data data;
+	int ret;
+
+	ret = swsmi_wireless_status(samsung, &data);
+	if (ret)
+		return ret;
+
+	data.data[1] = 0;
+	if (srfkill->type == RFKILL_TYPE_WLAN)
+		data.data[WL_STATUS_WLAN] = !blocked;
+	else if (srfkill->type == RFKILL_TYPE_BLUETOOTH)
+		data.data[WL_STATUS_BT] = !blocked;
+
+	return sabi_command(samsung, commands->set_wireless_status,
+			    &data, &data);
+}
+
+static void swsmi_rfkill_query(struct rfkill *rfkill, void *priv)
+{
+	struct samsung_rfkill *srfkill = priv;
+	struct samsung_laptop *samsung = srfkill->samsung;
+	struct sabi_data data;
+	int ret;
+
+	ret = swsmi_wireless_status(samsung, &data);
+	if (ret)
+		return ;
+
+	if (srfkill->type == RFKILL_TYPE_WLAN)
+		ret = data.data[WL_STATUS_WLAN];
+	else if (srfkill->type == RFKILL_TYPE_BLUETOOTH)
+		ret = data.data[WL_STATUS_BT];
 	else
-		sabi_set_commandb(samsung, commands->set_wireless_button, 1);
+		return ;
 
-	return 0;
+	rfkill_set_sw_state(rfkill, !ret);
 }
 
-static struct rfkill_ops rfkill_ops = {
-	.set_block = rfkill_set,
+static struct rfkill_ops swsmi_rfkill_ops = {
+	.set_block = swsmi_rfkill_set,
+	.query = swsmi_rfkill_query,
 };
 
 static ssize_t get_performance_level(struct device *dev,
@@ -730,31 +808,92 @@ static int find_signature(void __iomem *memcheck, const char *testStr)
 
 static void samsung_rfkill_exit(struct samsung_laptop *samsung)
 {
-	if (samsung->rfk) {
-		rfkill_unregister(samsung->rfk);
-		rfkill_destroy(samsung->rfk);
-		samsung->rfk = NULL;
+	if (samsung->wlan.rfkill) {
+		rfkill_unregister(samsung->wlan.rfkill);
+		rfkill_destroy(samsung->wlan.rfkill);
+		samsung->wlan.rfkill = NULL;
+	}
+	if (samsung->bluetooth.rfkill) {
+		rfkill_unregister(samsung->bluetooth.rfkill);
+		rfkill_destroy(samsung->bluetooth.rfkill);
+		samsung->bluetooth.rfkill = NULL;
 	}
 }
 
-static int __init samsung_rfkill_init(struct samsung_laptop *samsung)
+static int samsung_new_rfkill(struct samsung_laptop *samsung,
+			      struct samsung_rfkill *arfkill,
+			      const char *name, enum rfkill_type type,
+			      const struct rfkill_ops *ops,
+			      int blocked)
 {
-	int retval;
+	struct rfkill **rfkill = &arfkill->rfkill;
+	int ret;
 
-	samsung->rfk = rfkill_alloc("samsung-wifi",
-				    &samsung->platform_device->dev,
-				    RFKILL_TYPE_WLAN,
-				    &rfkill_ops, samsung);
-	if (!samsung->rfk)
-		return -ENOMEM;
+	arfkill->type = type;
+	arfkill->samsung = samsung;
 
-	retval = rfkill_register(samsung->rfk);
-	if (retval) {
-		rfkill_destroy(samsung->rfk);
-		samsung->rfk = NULL;
-		return -ENODEV;
+	*rfkill = rfkill_alloc(name, &samsung->platform_device->dev,
+			       type, ops, arfkill);
+
+	if (!*rfkill)
+		return -EINVAL;
+
+	if (blocked != -1)
+		rfkill_init_sw_state(*rfkill, blocked);
+
+	ret = rfkill_register(*rfkill);
+	if (ret) {
+		rfkill_destroy(*rfkill);
+		*rfkill = NULL;
+		return ret;
 	}
+	return 0;
+}
 
+static int __init samsung_rfkill_init_seclinux(struct samsung_laptop *samsung)
+{
+	return samsung_new_rfkill(samsung, &samsung->wlan, "samsung-wlan",
+				  RFKILL_TYPE_WLAN, &seclinux_rfkill_ops, -1);
+}
+
+static int __init samsung_rfkill_init_swsmi(struct samsung_laptop *samsung)
+{
+	struct sabi_data data;
+	int ret;
+
+	ret = swsmi_wireless_status(samsung, &data);
+	if (ret)
+		return ret;
+
+	ret = samsung_new_rfkill(samsung, &samsung->wlan,
+				 "samsung-wlan",
+				 RFKILL_TYPE_WLAN,
+				 &swsmi_rfkill_ops,
+				 !data.data[WL_STATUS_WLAN]);
+	if (ret)
+		goto exit;
+
+	ret = samsung_new_rfkill(samsung, &samsung->bluetooth,
+				 "samsung-bluetooth",
+				 RFKILL_TYPE_BLUETOOTH,
+				 &swsmi_rfkill_ops,
+				 !data.data[WL_STATUS_BT]);
+	if (ret)
+		goto exit;
+
+exit:
+	if (ret)
+		samsung_rfkill_exit(samsung);
+
+	return ret;
+}
+
+static int __init samsung_rfkill_init(struct samsung_laptop *samsung)
+{
+	if (samsung->config->sabi_version == 2)
+		return samsung_rfkill_init_seclinux(samsung);
+	if (samsung->config->sabi_version == 3)
+		return samsung_rfkill_init_swsmi(samsung);
 	return 0;
 }
 
-- 
1.7.5.4

Re: [PATCH 12/15] samsung-laptop: add true rfkill support for swsmi

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:43PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

What happens on machines that don't support this?

thanks,

greg k-h

Re: [PATCH 12/15] samsung-laptop: add true rfkill support for swsmi

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:24 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:43PM +0100, Corentin Chary wrote:
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> What happens on machines that don't support this?

As I understand things, SwSMI == SABI3, and it looks a very clear and
stable interface. My guess is that all SABI3 machines support this
command. If it is not the case, swsmi_wireless_status() will fail, and
rfkill won't be created.

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 12/15] samsung-laptop: add true rfkill support for swsmi

[Greg KH]

On Wed, Nov 23, 2011 at 07:37:17AM +0100, Corentin Chary wrote:
> On Tue, Nov 22, 2011 at 11:24 PM, Greg KH <gregkh@suse.de> wrote:
> > On Tue, Nov 22, 2011 at 11:02:43PM +0100, Corentin Chary wrote:
> >> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> >
> > What happens on machines that don't support this?
> 
> As I understand things, SwSMI == SABI3, and it looks a very clear and
> stable interface. My guess is that all SABI3 machines support this
> command. If it is not the case, swsmi_wireless_status() will fail, and
> rfkill won't be created.

Ok, that sounds good.

greg k-h

[PATCH 13/15] samsung-laptop: make the dmi check less strict

[Corentin Chary]

This enable the driver for everything that look like
a laptop and is from vendor "SAMSUNG ELECTRONICS CO., LTD.".
Note that laptop supported by samsung-q10 seem to have a different
vendor strict.

Also remove every log output until we know that we have a SABI interface
(except if the driver is forced to load, or debug is enabled).

Keeping a whitelist of laptop with a model granularity is something that can't
work without close vendor cooperation (and we don't have that).

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |  235 ++------------------------------
 1 files changed, 15 insertions(+), 220 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index c9f574c..9940525 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -1250,7 +1250,8 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 
 	samsung->f0000_segment = ioremap_nocache(0xf0000, 0xffff);
 	if (!samsung->f0000_segment) {
-		pr_err("Can't map the segment at 0xf0000\n");
+		if (debug || force)
+			pr_err("Can't map the segment at 0xf0000\n");
 		ret = -EINVAL;
 		goto exit;
 	}
@@ -1265,7 +1266,8 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 	}
 
 	if (loca == 0xffff) {
-		pr_err("This computer does not support SABI\n");
+		if (debug || force)
+			pr_err("This computer does not support SABI\n");
 		ret = -ENODEV;
 		goto exit;
 	}
@@ -1334,244 +1336,34 @@ static int __init samsung_platform_init(struct samsung_laptop *samsung)
 	return 0;
 }
 
-static int __init dmi_check_cb(const struct dmi_system_id *id)
-{
-	pr_info("found laptop model '%s'\n", id->ident);
-	return 1;
-}
-
 static struct dmi_system_id __initdata samsung_dmi_table[] = {
 	{
-		.ident = "N128",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N128"),
-			DMI_MATCH(DMI_BOARD_NAME, "N128"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "N130",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N130"),
-			DMI_MATCH(DMI_BOARD_NAME, "N130"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "N510",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N510"),
-			DMI_MATCH(DMI_BOARD_NAME, "N510"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "X125",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "X125"),
-			DMI_MATCH(DMI_BOARD_NAME, "X125"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "X120/X170",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "X120/X170"),
-			DMI_MATCH(DMI_BOARD_NAME, "X120/X170"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "NC10",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "NC10"),
-			DMI_MATCH(DMI_BOARD_NAME, "NC10"),
-		},
-		.callback = dmi_check_cb,
-	},
-		{
-		.ident = "NP-Q45",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "SQ45S70S"),
-			DMI_MATCH(DMI_BOARD_NAME, "SQ45S70S"),
-		},
-		.callback = dmi_check_cb,
-		},
-	{
-		.ident = "X360",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "X360"),
-			DMI_MATCH(DMI_BOARD_NAME, "X360"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "R410 Plus",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "R410P"),
-			DMI_MATCH(DMI_BOARD_NAME, "R460"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "R518",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR,
 					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "R518"),
-			DMI_MATCH(DMI_BOARD_NAME, "R518"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "8"), /* Portable */
 		},
-		.callback = dmi_check_cb,
 	},
 	{
-		.ident = "R519/R719",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR,
 					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "R519/R719"),
-			DMI_MATCH(DMI_BOARD_NAME, "R519/R719"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
 		},
-		.callback = dmi_check_cb,
 	},
 	{
-		.ident = "N150/N210/N220",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR,
 					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N150/N210/N220"),
-			DMI_MATCH(DMI_BOARD_NAME, "N150/N210/N220"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
 		},
-		.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"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
 		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "N150/N210/N220/N230",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N150/N210/N220/N230"),
-			DMI_MATCH(DMI_BOARD_NAME, "N150/N210/N220/N230"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "N150P/N210P/N220P",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N150P/N210P/N220P"),
-			DMI_MATCH(DMI_BOARD_NAME, "N150P/N210P/N220P"),
-		},
-		.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."),
-		      DMI_MATCH(DMI_PRODUCT_NAME, "R530/R730"),
-		      DMI_MATCH(DMI_BOARD_NAME, "R530/R730"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "NF110/NF210/NF310",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "NF110/NF210/NF310"),
-			DMI_MATCH(DMI_BOARD_NAME, "NF110/NF210/NF310"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "N145P/N250P/N260P",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "N145P/N250P/N260P"),
-			DMI_MATCH(DMI_BOARD_NAME, "N145P/N250P/N260P"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "R70/R71",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR,
-					"SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "R70/R71"),
-			DMI_MATCH(DMI_BOARD_NAME, "R70/R71"),
-		},
-		.callback = dmi_check_cb,
-	},
-	{
-		.ident = "P460",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "P460"),
-			DMI_MATCH(DMI_BOARD_NAME, "P460"),
-		},
-		.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,
-	},
-	{
-		.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,
-	},
-		{
-		.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,
 	},
 	{ },
 };
@@ -1596,12 +1388,9 @@ static int __init samsung_init(void)
 
 #ifdef CONFIG_ACPI
 	/* Don't handle backlight here if the acpi video already handle it */
-	if (acpi_video_backlight_support()) {
-		pr_info("Backlight controlled by ACPI video driver\n");
+	if (acpi_video_backlight_support())
 		samsung->handle_backlight = false;
-	}
 #endif
-
 	ret = samsung_platform_init(samsung);
 	if (ret)
 		goto error_platform;
@@ -1610,6 +1399,12 @@ static int __init samsung_init(void)
 	if (ret)
 		goto error_sabi;
 
+#ifdef CONFIG_ACPI
+	/* Only log that if we are really on a sabi platform */
+	if (acpi_video_backlight_support())
+		pr_info("Backlight controlled by ACPI video driver\n");
+#endif
+
 	ret = samsung_sysfs_init(samsung);
 	if (ret)
 		goto error_sysfs;
-- 
1.7.5.4

Re: [PATCH 13/15] samsung-laptop: make the dmi check less strict

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:44PM +0100, Corentin Chary wrote:
> This enable the driver for everything that look like
> a laptop and is from vendor "SAMSUNG ELECTRONICS CO., LTD.".
> Note that laptop supported by samsung-q10 seem to have a different
> vendor strict.
> 
> Also remove every log output until we know that we have a SABI interface
> (except if the driver is forced to load, or debug is enabled).
> 
> Keeping a whitelist of laptop with a model granularity is something that can't
> work without close vendor cooperation (and we don't have that).
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Oh, this is bold.

I would like to see this work, but I would need to see it tested on a
bunch of machines before I trust it.

I guess you now have the acpi video backlight check, which is good, but
again, doesn't work for all devices.

And yeah, a whitelist is tough, especially as the vendor isn't working
with us at all, but I don't want to break machines that don't need this
driver (and there are some, although I thought the newest ones would not
need it, but that might be true.)

Hm, if you can figure out how to get the acpi-video thing working for
some machines properly, I'll support this kind of change.

thanks,

greg k-h

Re: [PATCH 13/15] samsung-laptop: make the dmi check less strict

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:27 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:44PM +0100, Corentin Chary wrote:
>> This enable the driver for everything that look like
>> a laptop and is from vendor "SAMSUNG ELECTRONICS CO., LTD.".
>> Note that laptop supported by samsung-q10 seem to have a different
>> vendor strict.
>>
>> Also remove every log output until we know that we have a SABI interface
>> (except if the driver is forced to load, or debug is enabled).
>>
>> Keeping a whitelist of laptop with a model granularity is something that can't
>> work without close vendor cooperation (and we don't have that).
>>
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> Oh, this is bold.
>
> I would like to see this work, but I would need to see it tested on a
> bunch of machines before I trust it.
>
> I guess you now have the acpi video backlight check, which is good, but
> again, doesn't work for all devices.
>
> And yeah, a whitelist is tough, especially as the vendor isn't working
> with us at all, but I don't want to break machines that don't need this
> driver (and there are some, although I thought the newest ones would not
> need it, but that might be true.)

Here is what the driver does with the patch:
- automatically load the driver and each machine with vendor =
"SAMSUNG ELECTRONICS CO., LTD." and type looks like a laptop or
netbook
- map the f0000 segment (or exit gracefully)
- find SwSMI or SECLINUX in this segment (or exit gracefully)

At this point, if the driver is still loaded, but the laptop doesn't
support SABI, then it really looks like a model that should be
blacklisted.
I'm ok to use dmi for quirks, or blacklist, but honestly if a *samsung
laptop* advertise SABI support, the driver should be loaded.
We can't prevent samsung to built a laptop that will burn if we use
SABI, but if samsung does that, they can also add some fake DMI
strings too ! And has I said in the changelog, all models supported by
samsung-q10 are not affected because they don't use the same vendor
string, and even if they were, I doubt we would found a valid
signature.

But yes, it needs testing, so if you know a samsung laptop user,
please ask him to test this patch.

I set up a git repo that can help to build the driver more easilly:
https://github.com/iksaif/samsung-laptop-dkms (yeah, it's named dkms,
but there is no dkms.conf yet, that will come later).

> Hm, if you can figure out how to get the acpi-video thing working for
> some machines properly, I'll support this kind of change.

For those, boot with acpi_backlight=vendor  :)

Thanks,

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 13/15] samsung-laptop: make the dmi check less strict

[Greg KH]

On Wed, Nov 23, 2011 at 08:17:08AM +0100, Corentin Chary wrote:
> On Tue, Nov 22, 2011 at 11:27 PM, Greg KH <gregkh@suse.de> wrote:
> > On Tue, Nov 22, 2011 at 11:02:44PM +0100, Corentin Chary wrote:
> >> This enable the driver for everything that look like
> >> a laptop and is from vendor "SAMSUNG ELECTRONICS CO., LTD.".
> >> Note that laptop supported by samsung-q10 seem to have a different
> >> vendor strict.
> >>
> >> Also remove every log output until we know that we have a SABI interface
> >> (except if the driver is forced to load, or debug is enabled).
> >>
> >> Keeping a whitelist of laptop with a model granularity is something that can't
> >> work without close vendor cooperation (and we don't have that).
> >>
> >> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> >
> > Oh, this is bold.
> >
> > I would like to see this work, but I would need to see it tested on a
> > bunch of machines before I trust it.
> >
> > I guess you now have the acpi video backlight check, which is good, but
> > again, doesn't work for all devices.
> >
> > And yeah, a whitelist is tough, especially as the vendor isn't working
> > with us at all, but I don't want to break machines that don't need this
> > driver (and there are some, although I thought the newest ones would not
> > need it, but that might be true.)
> 
> Here is what the driver does with the patch:
> - automatically load the driver and each machine with vendor =
> "SAMSUNG ELECTRONICS CO., LTD." and type looks like a laptop or
> netbook
> - map the f0000 segment (or exit gracefully)
> - find SwSMI or SECLINUX in this segment (or exit gracefully)
> 
> At this point, if the driver is still loaded, but the laptop doesn't
> support SABI, then it really looks like a model that should be
> blacklisted.
> I'm ok to use dmi for quirks, or blacklist, but honestly if a *samsung
> laptop* advertise SABI support, the driver should be loaded.
> We can't prevent samsung to built a laptop that will burn if we use
> SABI, but if samsung does that, they can also add some fake DMI
> strings too ! And has I said in the changelog, all models supported by
> samsung-q10 are not affected because they don't use the same vendor
> string, and even if they were, I doubt we would found a valid
> signature.

Ok, as long as non-sabi modules will not have problems, I'm ok with
this.

thanks,

greg k-h

[PATCH 14/15] samsung-laptop: dump model and version informations

[Corentin Chary]

We still need to figure out exactly what each of different fields
represent, but they contain at least model and version informatiosn.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/platform/x86/samsung-laptop.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index 9940525..c03a466 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -326,6 +326,8 @@ struct samsung_laptop {
 
 	bool handle_backlight;
 	bool has_stepping_quirk;
+
+	char sdiag[64];
 };
 
 
@@ -1239,6 +1241,34 @@ static __init void samsung_sabi_infos(struct samsung_laptop *samsung, int loca,
 	printk(KERN_DEBUG " SABI pointer = 0x%08x\n", ifaceP);
 }
 
+static void __init samsung_sabi_diag(struct samsung_laptop *samsung)
+{
+	int loca = find_signature(samsung->f0000_segment, "SDiaG@");
+	int i;
+
+	if (loca == 0xffff)
+		return ;
+
+	/* Example:
+	 * Ident: @SDiaG@686XX-N90X3A/966-SEC-07HL-S90X3A
+	 *
+	 * Product name: 90X3A
+	 * BIOS Version: 07HL
+	 */
+	loca += 1;
+	for (i = 0; loca < 0xffff && i < sizeof(samsung->sdiag) - 1; loca++) {
+		char temp = readb(samsung->f0000_segment + loca);
+
+		if (isalnum(temp) || temp == '/' || temp == '-')
+			samsung->sdiag[i++] = temp;
+		else
+			break ;
+	}
+
+	if (samsung->sdiag[0])
+		pr_info("sdiag: %s", samsung->sdiag);
+}
+
 static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 {
 	const struct sabi_config *config = NULL;
@@ -1256,6 +1286,8 @@ static int __init samsung_sabi_init(struct samsung_laptop *samsung)
 		goto exit;
 	}
 
+	samsung_sabi_diag(samsung);
+
 	/* Try to find one of the signatures in memory to find the header */
 	for (i = 0; sabi_configs[i].test_string != 0; ++i) {
 		samsung->config = &sabi_configs[i];
-- 
1.7.5.4

Re: [PATCH 14/15] samsung-laptop: dump model and version informations

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:45PM +0100, Corentin Chary wrote:
> We still need to figure out exactly what each of different fields
> represent, but they contain at least model and version informatiosn.
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

For every time the driver loads?  That's not nice, how about putting
this into debugfs instead?

thanks,

greg k-h

Re: [PATCH 14/15] samsung-laptop: dump model and version informations

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:28 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:45PM +0100, Corentin Chary wrote:
>> We still need to figure out exactly what each of different fields
>> represent, but they contain at least model and version informatiosn.
>>
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> For every time the driver loads?  That's not nice, how about putting
> this into debugfs instead?

Ok, why not.

-- 
Corentin Chary
http://xf.iksaif.net

[PATCH 15/15] samsung-laptop: promote myself as maintainer of samsung-laptop

[Corentin Chary]

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 MAINTAINERS |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3523ab0..bdde26f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5666,6 +5666,12 @@ F:	drivers/media/common/saa7146*
 F:	drivers/media/video/*7146*
 F:	include/media/*7146*
 
+SAMSUNG LAPTOP DRIVER
+M:	Corentin Chary <corentincj@iksaif.net>
+L:	platform-driver-x86@vger.kernel.org
+S:	Maintained
+F:	drivers/platform/x86/samsung-laptop.c
+
 SAMSUNG AUDIO (ASoC) DRIVERS
 M:	Jassi Brar <jassisinghbrar@gmail.com>
 M:	Sangbeom Kim <sbkim73@samsung.com>
-- 
1.7.5.4

Re: [PATCH 15/15] samsung-laptop: promote myself as maintainer of samsung-laptop

[Greg KH]

On Tue, Nov 22, 2011 at 11:02:46PM +0100, Corentin Chary wrote:
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

YEAH!!!!!

Note, I am willing to dig up my old "special Linux bios" machine for
testing if you want me to verify nothing gets broken after you respin
this patch series.

thanks,

greg k-h

Re: [PATCH 15/15] samsung-laptop: promote myself as maintainer of samsung-laptop

[Corentin Chary]

On Tue, Nov 22, 2011 at 11:29 PM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Nov 22, 2011 at 11:02:46PM +0100, Corentin Chary wrote:
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
> YEAH!!!!!
>
> Note, I am willing to dig up my old "special Linux bios" machine for
> testing if you want me to verify nothing gets broken after you respin
> this patch series.

Great, I'll ping you.

> thanks,
>
> greg k-h
>



-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH 15/15] samsung-laptop: promote myself as maintainer of samsung-laptop

[David Herrmann]

Hi Corentin

On Wed, Nov 23, 2011 at 7:38 AM, Corentin Chary
<corentin.chary@gmail.com> wrote:
> On Tue, Nov 22, 2011 at 11:29 PM, Greg KH <gregkh@suse.de> wrote:
>> On Tue, Nov 22, 2011 at 11:02:46PM +0100, Corentin Chary wrote:
>>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>>
>> YEAH!!!!!
>>
>> Note, I am willing to dig up my old "special Linux bios" machine for
>> testing if you want me to verify nothing gets broken after you respin
>> this patch series.
>
> Great, I'll ping you.
>

I have tested the whole patchset on my Samsung N210. It works for me.
I've commented on a few patches about some small stuff. But it didn't
break any existing stuff / introduce regressions.

Feel free to carry this on the patches:
Tested-by: David Herrmann <dh.herrmann@googlemail.com
I haven't had the time to review the code more deeply, though. I might
do that later.

Thanks for the patches
David