All of lore.kernel.org
 help / color / mirror / Atom feed
* experimental patch for toshiba_acpi
@ 2009-02-25 14:54 Charles
  2009-02-25 15:24 ` Matthew Garrett
  0 siblings, 1 reply; 34+ messages in thread
From: Charles @ 2009-02-25 14:54 UTC (permalink / raw)
  To: linux-acpi; +Cc: charles, jonathan, john

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hello--

  Please apply the following patch to toshiba_acpi.c. This patch has
existed for about five years and is necessary for proper operation of
some tools on toshiba laptops with toshiba BIOSes (such as toshset
http://www.schwieters.org/toshset/). 

This patch comes from http://memebeam.org/free-software/toshiba_acpi/.
With this patch, the module conflicts with the toshiba module (which
works only on non-acpi-enabled kernels). Optimally, some sort of check
should be made to allow only one (toshiba or toshiba_acpi) to load. In
practice, this conflict has not proven to be a problem.

thank you--
Charles

- --- toshiba_acpi.c.orig	2008-08-30 22:12:50.000000000 -0700
+++ toshiba_acpi.c	2008-08-31 12:03:07.000000000 -0700
@@ -28,12 +28,26 @@
  *	Yasushi Nagato - changes for linux kernel 2.4 -> 2.5
  *	Rob Miller - TV out and hotkeys help
  *
+ *  PLEASE NOTE
+ *
+ *  This is an experimental version of toshiba_acpi which includes emulation
+ *  of the original toshiba driver's /proc/toshiba and /dev/toshiba,
+ *  allowing Toshiba userspace utilities to work.  The relevant code was
+ *  based on toshiba.c (copyright 1996-2001 Jonathan A. Buzzard) and
+ *  incorporated into this driver with help from Gintautas Miliauskas,
+ *  Charles Schwieters, and Christoph Burger-Scheidlin.
+ *
+ *  Caveats:
+ *      * hotkey status in /proc/toshiba is not implemented
+ *      * to make accesses to /dev/toshiba load this driver instead of
+ *          the original driver, you will have to modify your module
+ *          auto-loading configuration
  *
  *  TODO
  *
  */
 
- -#define TOSHIBA_ACPI_VERSION	"0.18"
+#define TOSHIBA_ACPI_VERSION	"experimental-dev-toshiba-test-5"
 #define PROC_INTERFACE_VERSION	1
 
 #include <linux/kernel.h>
@@ -41,6 +55,10 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <linux/miscdevice.h>
+#include <linux/toshiba.h>
+#include <asm/io.h>
 #include <linux/backlight.h>
 
 #include <asm/uaccess.h>
@@ -497,6 +515,191 @@
 	return p;
 }
 
+/* /dev/toshiba and /proc/toshiba handlers {{{
+ *
+ * ISSUE: lots of magic numbers and mysterious code
+ */
+
+#define TOSH_MINOR_DEV		181
+#define OLD_PROC_TOSHIBA	"toshiba"
+
+static int
+tosh_acpi_bridge(SMMRegisters* regs)
+{
+	acpi_status status;
+
+	/* assert(sizeof(SMMRegisters) == sizeof(u32)*HCI_WORDS); */
+	status = hci_raw((u32*)regs, (u32*)regs);
+	if (status == AE_OK && (regs->eax & 0xff00) == HCI_SUCCESS)
+		return 0;
+
+	return -EINVAL;
+}
+
+static int
+tosh_ioctl(struct inode* ip, struct file* fp, unsigned int cmd,
+	unsigned long arg)
+{
+	SMMRegisters regs;
+	unsigned short ax,bx;
+	int err;
+
+	if ((!arg) || (cmd != TOSH_SMM))
+		return -EINVAL;
+
+	if (copy_from_user(&regs, (SMMRegisters*)arg, sizeof(SMMRegisters)))
+		return -EFAULT;
+
+	ax = regs.eax & 0xff00;
+	bx = regs.ebx & 0xffff;
+
+	/* block HCI calls to read/write memory & PCI devices */
+	if (((ax==HCI_SET) || (ax==HCI_GET)) && (bx>0x0069))
+		return -EINVAL;
+
+	err = tosh_acpi_bridge(&regs);
+
+	if (copy_to_user((SMMRegisters*)arg, &regs, sizeof(SMMRegisters)))
+		return -EFAULT;
+
+	return err;
+}
+
+static int
+tosh_get_machine_id(void __iomem *bios)
+{
+	int id;
+	unsigned short bx,cx;
+	unsigned long address;
+
+	id = (0x100*(int) readb(bios+0xfffe))+((int) readb(bios+0xfffa));
+
+	/* do we have a SCTTable machine identication number on our hands */
+	if (id==0xfc2f) {
+		bx = 0xe6f5; /* cheat */
+		/* now twiddle with our pointer a bit */
+		address = 0x00000000 + bx;
+		cx = readw(bios + address);
+		address = 0x00000009 + bx + cx;
+		cx = readw(bios + address);
+		address = 0x0000000a + cx;
+		cx = readw(bios + address);
+		/* now construct our machine identification number */
+		id = ((cx & 0xff)<<8)+((cx & 0xff00)>>8);
+	}
+
+	return id;
+}
+
+static int tosh_id;
+static int tosh_bios;
+static int tosh_date;
+static int tosh_sci;
+
+static struct file_operations tosh_fops = {
+	.owner = THIS_MODULE,
+	.ioctl = tosh_ioctl
+};
+
+static struct miscdevice tosh_device = {
+	TOSH_MINOR_DEV,
+	"toshiba",
+	&tosh_fops
+};
+
+static void
+setup_tosh_info(void __iomem *bios)
+{
+	int major, minor;
+	int day, month, year;
+
+	tosh_id = tosh_get_machine_id(bios);
+
+	/* get the BIOS version */
+	major = readb(bios + 0xe009)-'0';
+	minor = ((readb(bios + 0xe00b)-'0')*10)+(readb(bios + 0xe00c)-'0');
+	tosh_bios = (major*0x100)+minor;
+
+	/* get the BIOS date */
+	day = ((readb(bios + 0xfff5)-'0')*10)+(readb(bios + 0xfff6)-'0');
+	month = ((readb(bios + 0xfff8)-'0')*10)+(readb(bios + 0xfff9)-'0');
+	year = ((readb(bios + 0xfffb)-'0')*10)+(readb(bios + 0xfffc)-'0');
+	tosh_date = (((year-90) & 0x1f)<<10) | ((month & 0xf)<<6)
+		| ((day & 0x1f)<<1);
+}
+
+/* /proc/toshiba read handler */
+static int
+tosh_proc_show(struct seq_file *m, void *v)
+{
+	/* TODO: tosh_fn_status() */
+	int key = 0;
+
+	/* Format:
+	 *    0) Linux driver version (this will change if format changes)
+	 *    1) Machine ID
+	 *    2) SCI version
+	 *    3) BIOS version (major, minor)
+	 *    4) BIOS date (in SCI date format)
+	 *    5) Fn Key status
+	 */
+
+	seq_printf(m, "1.1 0x%04x %d.%d %d.%d 0x%04x 0x%02x\n",
+		tosh_id,
+		(tosh_sci & 0xff00)>>8,
+		tosh_sci & 0xff,
+		(tosh_bios & 0xff00)>>8,
+		tosh_bios & 0xff,
+		tosh_date,
+		key);
+
+	return 0;
+}
+
+static int tosh_proc_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, tosh_proc_show, NULL);
+}
+
+static const struct file_operations tosh_proc_fops = {
+	.owner		= THIS_MODULE,
+	.open		= tosh_proc_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static int __init
+old_driver_emulation_init(void)
+{
+	int status;
+	void __iomem *bios = ioremap(0xf0000, 0x10000);
+	if (!bios)
+		return -ENOMEM;
+
+	if ((status = misc_register(&tosh_device))) {
+		printk(MY_ERR "failed to register misc device %d (\"%s\")\n",
+			tosh_device.minor, tosh_device.name);
+		return status;
+	}
+
+	setup_tosh_info(bios);
+	proc_create(OLD_PROC_TOSHIBA, 0, NULL, &tosh_proc_fops);
+
+	iounmap(bios);
+
+	return 0;
+}
+
+static void __exit
+old_driver_emulation_exit(void)
+{
+	remove_proc_entry(OLD_PROC_TOSHIBA, NULL);
+	misc_deregister(&tosh_device);
+}
+
+/* }}} end of /dev/toshiba and /proc/toshiba handlers */
+
 /* proc and module init
  */
 
@@ -555,6 +758,8 @@
 	if (toshiba_proc_dir)
 		remove_proc_entry(PROC_TOSHIBA, acpi_root_dir);
 
+	old_driver_emulation_exit();
+
 	return;
 }
 
@@ -562,6 +767,7 @@
 {
 	acpi_status status = AE_OK;
 	u32 hci_result;
+	int status2;
 
 	if (acpi_disabled)
 		return -ENODEV;
@@ -578,6 +784,9 @@
 	       TOSHIBA_ACPI_VERSION);
 	printk(MY_INFO "    HCI method: %s\n", method_hci);
 
+	if ((status2 = old_driver_emulation_init()))
+		return status2;
+
 	force_fan = 0;
 	key_event_valid = 0;
 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8+ <http://mailcrypt.sourceforge.net/>

iD8DBQFJpVuzPK2zrJwS/lYRAsY9AJ0Uo1MSALzL2uvkFl8wdj3mh5xf2wCfZ7nA
3IJnQ9OKwIM1WFtlyP0iSX4=
=aW4m
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 14:54 experimental patch for toshiba_acpi Charles
@ 2009-02-25 15:24 ` Matthew Garrett
  2009-02-25 16:18   ` Jonathan Buzzard
  0 siblings, 1 reply; 34+ messages in thread
From: Matthew Garrett @ 2009-02-25 15:24 UTC (permalink / raw)
  To: Charles; +Cc: linux-acpi, jonathan, john

On Wed, Feb 25, 2009 at 09:54:43AM -0500, Charles@schwieters.org wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> 
> Hello--
> 
>   Please apply the following patch to toshiba_acpi.c. This patch has
> existed for about five years and is necessary for proper operation of
> some tools on toshiba laptops with toshiba BIOSes (such as toshset
> http://www.schwieters.org/toshset/). 

Why are the toshset features not being added to the toshiba_acpi driver?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 15:24 ` Matthew Garrett
@ 2009-02-25 16:18   ` Jonathan Buzzard
  2009-02-25 16:51     ` Matthew Garrett
  0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-25 16:18 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Charles, linux-acpi, john


On Wed, 2009-02-25 at 15:24 +0000, Matthew Garrett wrote:
> On Wed, Feb 25, 2009 at 09:54:43AM -0500, Charles@schwieters.org wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > 
> > Hello--
> > 
> >   Please apply the following patch to toshiba_acpi.c. This patch has
> > existed for about five years and is necessary for proper operation of
> > some tools on toshiba laptops with toshiba BIOSes (such as toshset
> > http://www.schwieters.org/toshset/). 
> 
> Why are the toshset features not being added to the toshiba_acpi driver?
> 

Because it makes far more sense to have a user mode program to drive the
features, and for the kernel to provide the necessary thin layer to
access the features.

JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 16:18   ` Jonathan Buzzard
@ 2009-02-25 16:51     ` Matthew Garrett
  2009-02-25 17:12       ` Jonathan Buzzard
  2009-02-26 13:12       ` John Belmonte
  0 siblings, 2 replies; 34+ messages in thread
From: Matthew Garrett @ 2009-02-25 16:51 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Charles, linux-acpi, john

On Wed, Feb 25, 2009 at 04:18:25PM +0000, Jonathan Buzzard wrote:

> Because it makes far more sense to have a user mode program to drive the
> features, and for the kernel to provide the necessary thin layer to
> access the features.

It makes sense for the kernel to provide a consistent abstraction of 
hardware functionality where possible. Almost every kernel driver could 
be rewritten in userspace - that doesn't make it a good idea.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 16:51     ` Matthew Garrett
@ 2009-02-25 17:12       ` Jonathan Buzzard
  2009-02-25 17:28         ` Matthew Garrett
  2009-02-25 17:33         ` Azael Avalos
  2009-02-26 13:12       ` John Belmonte
  1 sibling, 2 replies; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-25 17:12 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Charles, linux-acpi, john


On Wed, 2009-02-25 at 16:51 +0000, Matthew Garrett wrote:
> On Wed, Feb 25, 2009 at 04:18:25PM +0000, Jonathan Buzzard wrote:
> 
> > Because it makes far more sense to have a user mode program to drive the
> > features, and for the kernel to provide the necessary thin layer to
> > access the features.
> 
> It makes sense for the kernel to provide a consistent abstraction of 
> hardware functionality where possible. Almost every kernel driver could 
> be rewritten in userspace - that doesn't make it a good idea.
>

The driver *does* provide a consistent abstraction of the Toshiba
Hardware Configuration Interface (HCI) on an ACPI enabled "pucker"
Toshiba laptop, that is a laptop designed and manufactured by Toshiba
and not OEM'ed.

That patch causes the toshiba_acpi driver to provide the same
abstraction of the HCI that the toshiba driver provides for "pucker"
Toshiba laptops with APM and has been doing so for over a decade now.

The HCI interface has existed on "pucker" Toshiba laptops for a very
long time, nearly 20 years now. The calls that are valid in the HCI are
specific to the model in question.

Surely a consistent abstraction of the HCI interface with a user mode
program to tickle that interface as necessary is better than adding
hundreds of lines of code to the kernel, and having to constantly update
the driver when new models come out?

On top of that this is the way it has been done now for over a decade.
The patch enables modern ACPI laptops to use the existing tools for
Toshiba laptops, without having to compile your own out of band kernel
module. It is unlikely that someone is going to write and debug the
kernel code to move the likes of toshset into kernel space in the first
place and keep it up to date in the second.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 17:12       ` Jonathan Buzzard
@ 2009-02-25 17:28         ` Matthew Garrett
  2009-02-25 17:53           ` Jonathan Buzzard
  2009-02-25 17:33         ` Azael Avalos
  1 sibling, 1 reply; 34+ messages in thread
From: Matthew Garrett @ 2009-02-25 17:28 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Charles, linux-acpi, john

On Wed, Feb 25, 2009 at 05:12:58PM +0000, Jonathan Buzzard wrote:

> Surely a consistent abstraction of the HCI interface with a user mode
> program to tickle that interface as necessary is better than adding
> hundreds of lines of code to the kernel, and having to constantly update
> the driver when new models come out?

The same argument encourages us to put rfkill and brightness control 
support in a userland tool, despite the existing kernel interfaces for 
controlling them. We could replace almost every driver in platform/x86 
with a generic driver that allowed arbitrary ACPI methods to be called 
and gave access to EC bits. The reason we haven't done this is because 
that's what the kernel is there for.

> On top of that this is the way it has been done now for over a decade.
> The patch enables modern ACPI laptops to use the existing tools for
> Toshiba laptops, without having to compile your own out of band kernel
> module. It is unlikely that someone is going to write and debug the
> kernel code to move the likes of toshset into kernel space in the first
> place and keep it up to date in the second.

The effort required to maintain toshset is identical to the effort 
required to maintain the kernel-level driver. Porting this functionality 
isn't difficult.

Please don't add an interface that exists purely in order to avoid 
writing a proper kernel driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 17:12       ` Jonathan Buzzard
  2009-02-25 17:28         ` Matthew Garrett
@ 2009-02-25 17:33         ` Azael Avalos
  1 sibling, 0 replies; 34+ messages in thread
From: Azael Avalos @ 2009-02-25 17:33 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Matthew Garrett, Charles, linux-acpi, john

Hi there,

Sorry if I go off-topic...

Toshiba released last year (September) some specs about Fn+Fx key
handling for Linux that is "supposed" to be standard across all their models
(including Phoenix BIOS), maybe some of the devs want to take a look at it.

If indeed that new specification is "universal" this could mean the beggining
of a new "unified driver"

here's the link

http://linux.toshiba-dme.co.jp/linux/eng/develop.htm

Saludos
Azael Avalos




-- 
-- El mundo apesta y vosotros apestais tambien --

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 17:28         ` Matthew Garrett
@ 2009-02-25 17:53           ` Jonathan Buzzard
  2009-02-25 17:59             ` Matthew Garrett
  0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-25 17:53 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Charles, linux-acpi, john


On Wed, 2009-02-25 at 17:28 +0000, Matthew Garrett wrote:
> On Wed, Feb 25, 2009 at 05:12:58PM +0000, Jonathan Buzzard wrote:
> 
> > Surely a consistent abstraction of the HCI interface with a user mode
> > program to tickle that interface as necessary is better than adding
> > hundreds of lines of code to the kernel, and having to constantly update
> > the driver when new models come out?
> 
> The same argument encourages us to put rfkill and brightness control 
> support in a userland tool, despite the existing kernel interfaces for 
> controlling them. We could replace almost every driver in platform/x86 
> with a generic driver that allowed arbitrary ACPI methods to be called 
> and gave access to EC bits. The reason we haven't done this is because 
> that's what the kernel is there for.

Quite correct they should be removed. The first step of which is to
provide a generic interface to the HCI.

> > On top of that this is the way it has been done now for over a decade.
> > The patch enables modern ACPI laptops to use the existing tools for
> > Toshiba laptops, without having to compile your own out of band kernel
> > module. It is unlikely that someone is going to write and debug the
> > kernel code to move the likes of toshset into kernel space in the first
> > place and keep it up to date in the second.
> 
> The effort required to maintain toshset is identical to the effort 
> required to maintain the kernel-level driver. Porting this functionality 
> isn't difficult.
> 

You do it, test it then maintain it then. To claim that maintaining this
in kernel space is as easy as users space is patently ludicrous.

It also makes it harder for end users, because it means that you need a
new kernel for new functionality. My new laptop is not supported by
distro X, rather than grab a user mode program I have to fiddle with
patching a kernel and keeping it up to date.

Lot of drivers are user space, for example almost all USB scanner
drivers are user space.

> Please don't add an interface that exists purely in order to avoid 
> writing a proper kernel driver.
> 

A "proper" kernel driver as you put it is is completely inappropriate.
You want to unnecessarily pollute the kernel with hundreds of lines of
code for no actual gain in functionality.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 17:53           ` Jonathan Buzzard
@ 2009-02-25 17:59             ` Matthew Garrett
  2009-02-25 20:18               ` Charles
  2009-02-26  0:22               ` Jonathan Buzzard
  0 siblings, 2 replies; 34+ messages in thread
From: Matthew Garrett @ 2009-02-25 17:59 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Charles, linux-acpi, john

On Wed, Feb 25, 2009 at 05:53:39PM +0000, Jonathan Buzzard wrote:
> 
> On Wed, 2009-02-25 at 17:28 +0000, Matthew Garrett wrote:
> > The same argument encourages us to put rfkill and brightness control 
> > support in a userland tool, despite the existing kernel interfaces for 
> > controlling them. We could replace almost every driver in platform/x86 
> > with a generic driver that allowed arbitrary ACPI methods to be called 
> > and gave access to EC bits. The reason we haven't done this is because 
> > that's what the kernel is there for.
> 
> Quite correct they should be removed. The first step of which is to
> provide a generic interface to the HCI.

Yeah. No.

> You do it, test it then maintain it then. To claim that maintaining this
> in kernel space is as easy as users space is patently ludicrous.

How so? C is C. Whether you do it in userspace or kernel space, all you 
have to do is make a function call with the appropriate arguments.

> A "proper" kernel driver as you put it is is completely inappropriate.
> You want to unnecessarily pollute the kernel with hundreds of lines of
> code for no actual gain in functionality.

Yes. I want a proper kernel driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 17:59             ` Matthew Garrett
@ 2009-02-25 20:18               ` Charles
  2009-02-26  0:22               ` Jonathan Buzzard
  1 sibling, 0 replies; 34+ messages in thread
From: Charles @ 2009-02-25 20:18 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Jonathan Buzzard, Charles, linux-acpi, john

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> 
> > A "proper" kernel driver as you put it is is completely inappropriate.
> > You want to unnecessarily pollute the kernel with hundreds of lines of
> > code for no actual gain in functionality.
> 
> Yes. I want a proper kernel driver.
> 

I can definitely see reasons for putting it in the kernel, and I welcome
anyone who implements this solution. However, I don't think it will
happen. I no longer have time for this sort of work, and no one else has
stepped up in the past five years. I am willing to live with the
half-ass out-of-tree solution, so it sounds like that will be the way
things stay.

bummer--

Charles
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8+ <http://mailcrypt.sourceforge.net/>

iD8DBQFJpaeWPK2zrJwS/lYRAnhDAJ9yb8ecbgm8LcvQF6oEBPR4fEZ1jgCfYb3U
0QOkmdwb/NRoM5niqNOBJlA=
=wd7q
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 17:59             ` Matthew Garrett
  2009-02-25 20:18               ` Charles
@ 2009-02-26  0:22               ` Jonathan Buzzard
  2009-02-26  8:39                 ` Richard Hughes
  1 sibling, 1 reply; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-26  0:22 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Charles, linux-acpi, john

Matthew Garrett wrote:
> On Wed, Feb 25, 2009 at 05:53:39PM +0000, Jonathan Buzzard wrote:
>> On Wed, 2009-02-25 at 17:28 +0000, Matthew Garrett wrote:
>>> The same argument encourages us to put rfkill and brightness control 
>>> support in a userland tool, despite the existing kernel interfaces for 
>>> controlling them. We could replace almost every driver in platform/x86 
>>> with a generic driver that allowed arbitrary ACPI methods to be called 
>>> and gave access to EC bits. The reason we haven't done this is because 
>>> that's what the kernel is there for.
>> Quite correct they should be removed. The first step of which is to
>> provide a generic interface to the HCI.
> 
> Yeah. No.

Yeah, yes

> 
>> You do it, test it then maintain it then. To claim that maintaining this
>> in kernel space is as easy as users space is patently ludicrous.
> 
> How so? C is C. Whether you do it in userspace or kernel space, all you 
> have to do is make a function call with the appropriate arguments.
>

No it is not. C that is running in kernel space is not the same as C 
that is runing in user space. The potential for a bug to have security 
implications is *far* higher. If you start pushing hundreds of lines of 
string parsing into the kernel that just got a whole lot more likely.

Then one has to go through the whole rigmarole of submitting patches to 
various kernel developers and hoping that it gets in the next kernel. As 
opposed to releasing your own user land code, that might not even be in 
C, it could be C++, Perl, Python whatever takes your fancy when ever it 
takes your fancy.

Finally I speak from actual experience on this matter. The very early 
versions of the toshiba drive did everything via a proc interface. It 
sucked, was buggy and hundreds of lines long. I then stripped it down 
wrote a wrapper to the HCI, reduced the amount of kernel code by an 
order of magnitude.

>> A "proper" kernel driver as you put it is is completely inappropriate.
>> You want to unnecessarily pollute the kernel with hundreds of lines of
>> code for no actual gain in functionality.
> 
> Yes. I want a proper kernel driver.
> 

Well write it yourself, because I certainly am not. In the meantime 
there is perfectly good method that allows lots of existing code to just 
work. The only thing I am likely to do is update the toshiba driver so 
it detects whether ACPI is enabled and uses ACPI methods if that is the 
case.

I would be interested in what on earth makes you thing putting hundreds 
of lines of code into a "proper" kernel driver as you put it is better 
as it is simply not the Unix way.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26  0:22               ` Jonathan Buzzard
@ 2009-02-26  8:39                 ` Richard Hughes
  2009-02-26 10:34                   ` Jonathan Buzzard
  0 siblings, 1 reply; 34+ messages in thread
From: Richard Hughes @ 2009-02-26  8:39 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Matthew Garrett, Charles, linux-acpi, john

On Thu, 2009-02-26 at 00:22 +0000, Jonathan Buzzard wrote:
> Matthew Garrett wrote:
> > On Wed, Feb 25, 2009 at 05:53:39PM +0000, Jonathan Buzzard wrote:
> >> On Wed, 2009-02-25 at 17:28 +0000, Matthew Garrett wrote:
> >>> The same argument encourages us to put rfkill and brightness control 
> >>> support in a userland tool, despite the existing kernel interfaces for 
> >>> controlling them. We could replace almost every driver in platform/x86 
> >>> with a generic driver that allowed arbitrary ACPI methods to be called 
> >>> and gave access to EC bits. The reason we haven't done this is because 
> >>> that's what the kernel is there for.
> >> Quite correct they should be removed. The first step of which is to
> >> provide a generic interface to the HCI.
> > 
> > Yeah. No.
> 
> Yeah, yes

No.

> > 
> >> You do it, test it then maintain it then. To claim that maintaining this
> >> in kernel space is as easy as users space is patently ludicrous.
> > 
> > How so? C is C. Whether you do it in userspace or kernel space, all you 
> > have to do is make a function call with the appropriate arguments.
> >
> 
> No it is not. C that is running in kernel space is not the same as C 
> that is runing in user space. The potential for a bug to have security 
> implications is *far* higher. If you start pushing hundreds of lines of 
> string parsing into the kernel that just got a whole lot more likely.

Then you don't do string parsing, or if you do it, you do it very
carefully. You have to be just as careful with code running as root in
userspace.

> Then one has to go through the whole rigmarole of submitting patches to 
> various kernel developers and hoping that it gets in the next kernel.

Welcome to kernel development. It's really not that hard, even I can
manage to get patches upstream, and I consider myself a userspace
hacker.

> As 
> opposed to releasing your own user land code, that might not even be in 
> C, it could be C++, Perl, Python whatever takes your fancy when ever it 
> takes your fancy.

And that is the problem. You're doing low level hardware stuff in
userspace in odd languages. If you stuck this in the kernel then we can
use standard classes like rfkill, backlight and power_supply to control
things. Then userspace "just works" without odd access libraries and
yet-another-daemon.

> I would be interested in what on earth makes you thing putting hundreds 
> of lines of code into a "proper" kernel driver as you put it is better 
> as it is simply not the Unix way.

If the UNIX way is to expose raw devices /proc/toshiba and /dev/toshiba
and then use userspace tools to poke random values in them, then get me
back to Linux real quick. The way to do this in the Linux kernel is to
use the existing kernel abstractions like backlight and power_supply
using a proper kernel driver. This means that Linux userspace can do
simply echo values into known locations and change the state no matter
what the hardware.

We've spent a lot of time removing insane kernel bodges like exposing
hotkeys through a polled semi-interface, and instead using the input
layer properly, and I don't want to regress on that work now.

Thanks,

Richard.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26  8:39                 ` Richard Hughes
@ 2009-02-26 10:34                   ` Jonathan Buzzard
  2009-02-26 12:52                     ` Richard Hughes
  0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-26 10:34 UTC (permalink / raw)
  To: Richard Hughes; +Cc: Matthew Garrett, Charles, linux-acpi, john


On Thu, 2009-02-26 at 08:39 +0000, Richard Hughes wrote:
> On Thu, 2009-02-26 at 00:22 +0000, Jonathan Buzzard wrote:
> > Matthew Garrett wrote:
> > > On Wed, Feb 25, 2009 at 05:53:39PM +0000, Jonathan Buzzard wrote:
> > >> On Wed, 2009-02-25 at 17:28 +0000, Matthew Garrett wrote:
> > >>> The same argument encourages us to put rfkill and brightness control 
> > >>> support in a userland tool, despite the existing kernel interfaces for 
> > >>> controlling them. We could replace almost every driver in platform/x86 
> > >>> with a generic driver that allowed arbitrary ACPI methods to be called 
> > >>> and gave access to EC bits. The reason we haven't done this is because 
> > >>> that's what the kernel is there for.
> > >> Quite correct they should be removed. The first step of which is to
> > >> provide a generic interface to the HCI.
> > > 
> > > Yeah. No.
> > 
> > Yeah, yes
> 
> No.

Yes. You really don't understand the Toshiba Hardware Configuration
Interface. It is like making a old style BIOS INT 13 call. There are
potentially 2^16 possible calls, and there is no way to determine what
calls are valid on what laptop models other than a large lookup table. I
know of several dozen HCI calls, and there are hundreds of models, with
new models coming out all the time.

You proposal requires embedding this logic in kernel space, and it is
simply not appropriate.

> 
> > > 
> > >> You do it, test it then maintain it then. To claim that maintaining this
> > >> in kernel space is as easy as users space is patently ludicrous.
> > > 
> > > How so? C is C. Whether you do it in userspace or kernel space, all you 
> > > have to do is make a function call with the appropriate arguments.
> > >
> > 
> > No it is not. C that is running in kernel space is not the same as C 
> > that is runing in user space. The potential for a bug to have security 
> > implications is *far* higher. If you start pushing hundreds of lines of 
> > string parsing into the kernel that just got a whole lot more likely.
> 
> Then you don't do string parsing, or if you do it, you do it very
> carefully. You have to be just as careful with code running as root in
> userspace.

Bad news to implement *ALL* HCI methods you will, and worse besides.
That is the reason I ditched this approach ten years ago.

> 
> > Then one has to go through the whole rigmarole of submitting patches to 
> > various kernel developers and hoping that it gets in the next kernel.
> 
> Welcome to kernel development. It's really not that hard, even I can
> manage to get patches upstream, and I consider myself a userspace
> hacker.

You explain to an end user then that they cannot use their new Toshiba
laptop with their favourite Linux distribution without doing their own
kernel module, as the lookup table for what HCI calls are valid on their
laptop is not implemented in the kernel their distribution comes with. 

Is that the road you are proposing to go down? Because it is completely
divorced from reality and totally impractical.

> > As 
> > opposed to releasing your own user land code, that might not even be in 
> > C, it could be C++, Perl, Python whatever takes your fancy when ever it 
> > takes your fancy.
> 
> And that is the problem. You're doing low level hardware stuff in
> userspace in odd languages. If you stuck this in the kernel then we can
> use standard classes like rfkill, backlight and power_supply to control
> things. Then userspace "just works" without odd access libraries and
> yet-another-daemon.

Why would I put code into the kernel code for changing the ownerstring
on my laptop. Something I might do a couple of times in the lifetime of
the laptop?

What other models of computers have Linux kernel drivers to change the
BIOS boot order, or any other random model specific BIOS setting?

How do you propose to deal with the dozens of HCI calls and the hundreds
of models of laptops, with not all HCI calls being valid on all models,
and with the potential for a HCI call on the wrong laptop to brick it
and yes this *DOES* happen?

How do you propose echoing a password string for say the supervisor
password is going to translate that to the BIOS scan codes of the
relevant keys on that laptop, dealing with all the possible keyboard
layouts that might be in play? Why is this appropriate in a kernel
driver anyway for the half dozen times I might change it on a laptop?

Why if I install a distribution of Linux on my new Toshiba laptop should
I have to install a new kernel module and keep it updated to make some
change because the table specifying which HCI calls can be made is not
up to date in my distro's kernel?

> 
> > I would be interested in what on earth makes you thing putting hundreds 
> > of lines of code into a "proper" kernel driver as you put it is better 
> > as it is simply not the Unix way.
> 
> If the UNIX way is to expose raw devices /proc/toshiba and /dev/toshiba
> and then use userspace tools to poke random values in them, then get me
> back to Linux real quick.

It is not the Unix or Linux way to put huge quantities of code like this
into the kernel. It is not necessary or desirable, you *will* create
local privilege escalation bugs if you attempt it.

> The way to do this in the Linux kernel is to
> use the existing kernel abstractions like backlight and power_supply
> using a proper kernel driver.

The problem is your failing to understand the shear depth of possible
calls to the HCI interface. I am quite sure that a 10,000+ line kernel
module to implement all the HCI methods would be rejected upstream.
Somewhere I have an email from Alan Cox from a decade ago telling me
just that. That is why the current method was developed.

Finally actual code wins. If you want all this in kernel space then stop
winging and implement it. The fact that nobody has in the last five
years clearly indicates that there is little will to do so.

Exposing all the HCI calls through a /proc interface is just plain
stupid.

JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 10:34                   ` Jonathan Buzzard
@ 2009-02-26 12:52                     ` Richard Hughes
  2009-02-26 13:27                       ` Jonathan Buzzard
  0 siblings, 1 reply; 34+ messages in thread
From: Richard Hughes @ 2009-02-26 12:52 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Matthew Garrett, Charles, linux-acpi, john

On Thu, 2009-02-26 at 10:34 +0000, Jonathan Buzzard wrote:
> Yes. You really don't understand the Toshiba Hardware Configuration
> Interface. It is like making a old style BIOS INT 13 call. There are
> potentially 2^16 possible calls, and there is no way to determine what
> calls are valid on what laptop models other than a large lookup table.

You mean there's no way of using dmi matching to do subsets of models?
Is the A20 very much different from the A10?

> You explain to an end user then that they cannot use their new Toshiba
> laptop with their favourite Linux distribution without doing their own
> kernel module, as the lookup table for what HCI calls are valid on their
> laptop is not implemented in the kernel their distribution comes with. 
> 
> Is that the road you are proposing to go down? Because it is completely
> divorced from reality and totally impractical.

So your view of reality is to tell the user to install a userspace
library, and perhaps another daemon, and then patch all the userspace
tools to use your library as well as the kernel interfaces? I don't
think so.

> Why would I put code into the kernel code for changing the ownerstring
> on my laptop. Something I might do a couple of times in the lifetime of
> the laptop?

Why would you want to do that? That's focusing on the couple of people
on the planet using a laptop that know what an ownerstring is.

> What other models of computers have Linux kernel drivers to change the
> BIOS boot order, or any other random model specific BIOS setting?

Leave that to the BIOS. Just because it can be done, doesn't mean it
should be done.

> How do you propose to deal with the dozens of HCI calls and the hundreds
> of models of laptops, with not all HCI calls being valid on all models,
> and with the potential for a HCI call on the wrong laptop to brick it
> and yes this *DOES* happen?

How many different HCI calls are there to increase the backlight
brightness up by one unit?

> Why if I install a distribution of Linux on my new Toshiba laptop should
> I have to install a new kernel module and keep it updated to make some
> change because the table specifying which HCI calls can be made is not
> up to date in my distro's kernel?

Dude, that happens all the time with other kernel modules. You see a
patch on LKML saying "add product ID for foobuzz" and then it gets
picked up by downstream as a patch until a new version is released.

> > > I would be interested in what on earth makes you thing putting hundreds 
> > > of lines of code into a "proper" kernel driver as you put it is better 
> > > as it is simply not the Unix way.
> > 
> > If the UNIX way is to expose raw devices /proc/toshiba and /dev/toshiba
> > and then use userspace tools to poke random values in them, then get me
> > back to Linux real quick.
> 
> It is not the Unix or Linux way to put huge quantities of code like this
> into the kernel. It is not necessary or desirable, you *will* create
> local privilege escalation bugs if you attempt it.

Will I?

> > The way to do this in the Linux kernel is to
> > use the existing kernel abstractions like backlight and power_supply
> > using a proper kernel driver.
> 
> The problem is your failing to understand the shear depth of possible
> calls to the HCI interface. I am quite sure that a 10,000+ line kernel
> module to implement all the HCI methods would be rejected upstream.
> Somewhere I have an email from Alan Cox from a decade ago telling me
> just that. That is why the current method was developed.

You don't need all of them. You need backlight control, fan control,
battery and maybe a few others. You don't need to interact with the EC
in all possible ways.

> Finally actual code wins. If you want all this in kernel space then stop
> winging and implement it. The fact that nobody has in the last five
> years clearly indicates that there is little will to do so.

Well, I think the onus is on you to provide a proper kernel patch,
rather than just exposing userspace to /dev/toshiba, afterall, that was
the thing that's prompted my mail

Richard.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-25 16:51     ` Matthew Garrett
  2009-02-25 17:12       ` Jonathan Buzzard
@ 2009-02-26 13:12       ` John Belmonte
  2009-02-26 14:03         ` Richard Hughes
  1 sibling, 1 reply; 34+ messages in thread
From: John Belmonte @ 2009-02-26 13:12 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Jonathan Buzzard, Charles, linux-acpi, Richard Hughes

On Wed, Feb 25, 2009 at 11:51 AM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Wed, Feb 25, 2009 at 04:18:25PM +0000, Jonathan Buzzard wrote:
>
>> Because it makes far more sense to have a user mode program to drive the
>> features, and for the kernel to provide the necessary thin layer to
>> access the features.
>
> It makes sense for the kernel to provide a consistent abstraction of
> hardware functionality where possible. Almost every kernel driver could
> be rewritten in userspace - that doesn't make it a good idea.

That argument would be most applicable if we were talking about adding
some new /dev interface-- but we're not.  Toshiba's HCI has been
around for a couple of decades; /dev/toshiba, which exposes it, has
been around for over 10 years; toshset, a useful and popular tool
which depends on /dev/toshiba, nearly as long as /dev/toshiba.

The proposed patch makes /dev/toshiba, and hence toshset, work on more
laptops-- it's a no-brainer to anyone with a practical mindset.
You've got Jonathan and Charles-- the two most-knowledgeable people on
Toshiba's firmware and /dev/toshiba in the free software community--
saying that it's not practical to jam all that functionality into the
kernel.  Please listen to them.

Regards,
--John

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 12:52                     ` Richard Hughes
@ 2009-02-26 13:27                       ` Jonathan Buzzard
  2009-02-26 13:59                         ` Richard Hughes
  0 siblings, 1 reply; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-26 13:27 UTC (permalink / raw)
  To: Richard Hughes; +Cc: Matthew Garrett, Charles, linux-acpi, john


On Thu, 2009-02-26 at 12:52 +0000, Richard Hughes wrote:
> On Thu, 2009-02-26 at 10:34 +0000, Jonathan Buzzard wrote:
> > Yes. You really don't understand the Toshiba Hardware Configuration
> > Interface. It is like making a old style BIOS INT 13 call. There are
> > potentially 2^16 possible calls, and there is no way to determine what
> > calls are valid on what laptop models other than a large lookup table.
> 
> You mean there's no way of using dmi matching to do subsets of models?
> Is the A20 very much different from the A10?

Potentially. Sometimes yes sometimes no. Let's face it the same model
can have optional Bluetooth, and HCI calls have been known to brick a
laptop.

> > You explain to an end user then that they cannot use their new Toshiba
> > laptop with their favourite Linux distribution without doing their own
> > kernel module, as the lookup table for what HCI calls are valid on their
> > laptop is not implemented in the kernel their distribution comes with. 
> > 
> > Is that the road you are proposing to go down? Because it is completely
> > divorced from reality and totally impractical.
> 
> So your view of reality is to tell the user to install a userspace
> library, and perhaps another daemon, and then patch all the userspace
> tools to use your library as well as the kernel interfaces? I don't
> think so.

No because the patch provides a generic interface to the Toshiba HCI.
They may need to install a *single* user space program that works with
the generic kernel mode interface.

> > Why would I put code into the kernel code for changing the ownerstring
> > on my laptop. Something I might do a couple of times in the lifetime of
> > the laptop?
> 
> Why would you want to do that? That's focusing on the couple of people
> on the planet using a laptop that know what an ownerstring is.

Well I want to change my ownerstring on my laptop from Linux, and
without the patch in question I have to put a whole bunch of code into
the  kernel.

> 
> > What other models of computers have Linux kernel drivers to change the
> > BIOS boot order, or any other random model specific BIOS setting?
> 
> Leave that to the BIOS. Just because it can be done, doesn't mean it
> should be done.

Now you are dodging the issue. Besides a whole bunch of the settings
take effect after a suspend, why should I have to reboot?

> 
> > How do you propose to deal with the dozens of HCI calls and the hundreds
> > of models of laptops, with not all HCI calls being valid on all models,
> > and with the potential for a HCI call on the wrong laptop to brick it
> > and yes this *DOES* happen?
> 
> How many different HCI calls are there to increase the backlight
> brightness up by one unit?

Several, depends on the model in question. But we are not talking about
the backlight, we are talking about all the other methods that you
clearly know nothing about.

> > Why if I install a distribution of Linux on my new Toshiba laptop should
> > I have to install a new kernel module and keep it updated to make some
> > change because the table specifying which HCI calls can be made is not
> > up to date in my distro's kernel?
> 
> Dude, that happens all the time with other kernel modules. You see a
> patch on LKML saying "add product ID for foobuzz" and then it gets
> picked up by downstream as a patch until a new version is released.

Yes and it is sub optimal.

You also failed to explain how the supervisor, and user password setting
was going to work from a kernel mode proc interface.


> > > > I would be interested in what on earth makes you thing putting hundreds 
> > > > of lines of code into a "proper" kernel driver as you put it is better 
> > > > as it is simply not the Unix way.
> > > 
> > > If the UNIX way is to expose raw devices /proc/toshiba and /dev/toshiba
> > > and then use userspace tools to poke random values in them, then get me
> > > back to Linux real quick.
> > 
> > It is not the Unix or Linux way to put huge quantities of code like this
> > into the kernel. It is not necessary or desirable, you *will* create
> > local privilege escalation bugs if you attempt it.
> 
> Will I?

You write perfect bug free C code first time every time? You don't so
you will introduce bugs.

> > > The way to do this in the Linux kernel is to
> > > use the existing kernel abstractions like backlight and power_supply
> > > using a proper kernel driver.
> > 
> > The problem is your failing to understand the shear depth of possible
> > calls to the HCI interface. I am quite sure that a 10,000+ line kernel
> > module to implement all the HCI methods would be rejected upstream.
> > Somewhere I have an email from Alan Cox from a decade ago telling me
> > just that. That is why the current method was developed.
> 
> You don't need all of them. You need backlight control, fan control,
> battery and maybe a few others. You don't need to interact with the EC
> in all possible ways.

You miss entirely the point of Toshiba's HCI. We are not talking about
backlight control here. We are talking about a bunch of other stuff.

Besides since when did you get to decide what *I* want to do? You don't.
So because I have a perfectly legitimate reason to change the
ownerstring on my laptop, rather than burdening the kernel with a whole
bunch of string handling code for the rare occasions I want to do that
we expose the generic interface that lets me change this and bunch of
other stuff to user space.


> > Finally actual code wins. If you want all this in kernel space then stop
> > winging and implement it. The fact that nobody has in the last five
> > years clearly indicates that there is little will to do so.
> 
> Well, I think the onus is on you to provide a proper kernel patch,
> rather than just exposing userspace to /dev/toshiba, afterall, that was
> the thing that's prompted my mail

We have a proper kernel patch, the use of /dev/toshiba was excepted
upstream a decade ago. There is a range of software that supports this
interface. The patch extends this to modern Toshiba hardware. It is a no
brainer to anyone with any practical sense.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 13:27                       ` Jonathan Buzzard
@ 2009-02-26 13:59                         ` Richard Hughes
  2009-02-26 15:49                           ` Jonathan Buzzard
  0 siblings, 1 reply; 34+ messages in thread
From: Richard Hughes @ 2009-02-26 13:59 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Matthew Garrett, Charles, linux-acpi, john

On Thu, 2009-02-26 at 13:27 +0000, Jonathan Buzzard wrote:
> On Thu, 2009-02-26 at 12:52 +0000, Richard Hughes wrote:
> > On Thu, 2009-02-26 at 10:34 +0000, Jonathan Buzzard wrote:
> > > Yes. You really don't understand the Toshiba Hardware Configuration
> > > Interface. It is like making a old style BIOS INT 13 call. There are
> > > potentially 2^16 possible calls, and there is no way to determine what
> > > calls are valid on what laptop models other than a large lookup table.
> > 
> > You mean there's no way of using dmi matching to do subsets of models?
> > Is the A20 very much different from the A10?
> 
> Potentially. Sometimes yes sometimes no. Let's face it the same model
> can have optional Bluetooth, and HCI calls have been known to brick a
> laptop.

So if I do an HCI that enables bluetooth on a model without bluetooth,
that will brick it? Sounds implausible to me.

> > Leave that to the BIOS. Just because it can be done, doesn't mean it
> > should be done.
> 
> Now you are dodging the issue. Besides a whole bunch of the settings
> take effect after a suspend, why should I have to reboot?

Re-read my reply again.

> > > How do you propose to deal with the dozens of HCI calls and the hundreds
> > > of models of laptops, with not all HCI calls being valid on all models,
> > > and with the potential for a HCI call on the wrong laptop to brick it
> > > and yes this *DOES* happen?
> > 
> > How many different HCI calls are there to increase the backlight
> > brightness up by one unit?
> 
> Several, depends on the model in question. But we are not talking about
> the backlight, we are talking about all the other methods that you
> clearly know nothing about.

No, we're talking about sensible abstractions, rather than poking bits
of memory in a device file that happen to do stuff on specific models.

> > > Why if I install a distribution of Linux on my new Toshiba laptop should
> > > I have to install a new kernel module and keep it updated to make some
> > > change because the table specifying which HCI calls can be made is not
> > > up to date in my distro's kernel?
> > 
> > Dude, that happens all the time with other kernel modules. You see a
> > patch on LKML saying "add product ID for foobuzz" and then it gets
> > picked up by downstream as a patch until a new version is released.
> 
> Yes and it is sub optimal.

If there's new Toshiba hardware created, I have to update your client
program. I don't see how it's any different to updating the kernel
module.

> You also failed to explain how the supervisor, and user password setting
> was going to work from a kernel mode proc interface.

Can't you do this from the BIOS?

> > Will I?
> 
> You write perfect bug free C code first time every time? You don't so
> you will introduce bugs.

No, you said "you *will* create local privilege escalation bugs" which
is very different to "introducing bugs".

> You miss entirely the point of Toshiba's HCI. We are not talking about
> backlight control here. We are talking about a bunch of other stuff.

"Bunch of other stuff". Could we not decide on a proper framework for
this functionality?

> > Well, I think the onus is on you to provide a proper kernel patch,
> > rather than just exposing userspace to /dev/toshiba, afterall, that was
> > the thing that's prompted my mail
> 
> We have a proper kernel patch, the use of /dev/toshiba was excepted
> upstream a decade ago.

As was /proc/apm, /dev/pmu and all the other _obsolete_ interfaces. They
were bad then, and they would be bad now. Userspace and the kernel have
moved on from a decade ago.

>  There is a range of software that supports this
> interface. The patch extends this to modern Toshiba hardware. It is a no
> brainer to anyone with any practical sense.

Maybe I have no brain.

Richard.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 13:12       ` John Belmonte
@ 2009-02-26 14:03         ` Richard Hughes
  2009-02-26 15:51           ` Jonathan Buzzard
  0 siblings, 1 reply; 34+ messages in thread
From: Richard Hughes @ 2009-02-26 14:03 UTC (permalink / raw)
  To: John Belmonte; +Cc: Matthew Garrett, Jonathan Buzzard, Charles, linux-acpi

On Thu, 2009-02-26 at 08:12 -0500, John Belmonte wrote:
> The proposed patch makes /dev/toshiba, and hence toshset, work on more
> laptops-- it's a no-brainer to anyone with a practical mindset.

Making toshset work is not a practical goal. Making hardware "just work"
without any vendor specific commands is a better goal. Exposing low
level, device specific, hardware details up through a device node isn't
a good plan at all.

Richard.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 13:59                         ` Richard Hughes
@ 2009-02-26 15:49                           ` Jonathan Buzzard
  0 siblings, 0 replies; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-26 15:49 UTC (permalink / raw)
  To: Richard Hughes; +Cc: John Belmonte, Matthew Garrett, Charles, linux-acpi


On Thu, 2009-02-26 at 13:59 +0000, Richard Hughes wrote:
> On Thu, 2009-02-26 at 13:27 +0000, Jonathan Buzzard wrote:
> > On Thu, 2009-02-26 at 12:52 +0000, Richard Hughes wrote:
> > > On Thu, 2009-02-26 at 10:34 +0000, Jonathan Buzzard wrote:
> > > > Yes. You really don't understand the Toshiba Hardware Configuration
> > > > Interface. It is like making a old style BIOS INT 13 call. There are
> > > > potentially 2^16 possible calls, and there is no way to determine what
> > > > calls are valid on what laptop models other than a large lookup table.
> > > 
> > > You mean there's no way of using dmi matching to do subsets of models?
> > > Is the A20 very much different from the A10?
> > 
> > Potentially. Sometimes yes sometimes no. Let's face it the same model
> > can have optional Bluetooth, and HCI calls have been known to brick a
> > laptop.
> 
> So if I do an HCI that enables bluetooth on a model without bluetooth,
> that will brick it? Sounds implausible to me.
> 

Possibly. The Toshiba provided Windows user space programs go to some
length to establish what models they are running on before attempting
any funny business. Over the years a number of users have tried randomly
poking the HCI without knowing what they are doing and bricked their
laptops. If you understood how the HCI really works under the hood, you
would understand. The error checking in it last time I looked is
virtually none existent.


> > > Leave that to the BIOS. Just because it can be done, doesn't mean it
> > > should be done.
> > 
> > Now you are dodging the issue. Besides a whole bunch of the settings
> > take effect after a suspend, why should I have to reboot?
> 
> Re-read my reply again.

Who the hell are you to decide that? If for example I want to change my
pointer from internal to external and go through a suspend cycle rather
than a reboot to get it, that is my business not yours.

> 
> > > > How do you propose to deal with the dozens of HCI calls and the hundreds
> > > > of models of laptops, with not all HCI calls being valid on all models,
> > > > and with the potential for a HCI call on the wrong laptop to brick it
> > > > and yes this *DOES* happen?
> > > 
> > > How many different HCI calls are there to increase the backlight
> > > brightness up by one unit?
> > 
> > Several, depends on the model in question. But we are not talking about
> > the backlight, we are talking about all the other methods that you
> > clearly know nothing about.
> 
> No, we're talking about sensible abstractions, rather than poking bits
> of memory in a device file that happen to do stuff on specific models.
> 

There are no sensible abstractions. If you had any knowledge of the HCI
you would understand that. However you don't and are arguing about
things you do not know about.


> > > > Why if I install a distribution of Linux on my new Toshiba laptop should
> > > > I have to install a new kernel module and keep it updated to make some
> > > > change because the table specifying which HCI calls can be made is not
> > > > up to date in my distro's kernel?
> > > 
> > > Dude, that happens all the time with other kernel modules. You see a
> > > patch on LKML saying "add product ID for foobuzz" and then it gets
> > > picked up by downstream as a patch until a new version is released.
> > 
> > Yes and it is sub optimal.
> 
> If there's new Toshiba hardware created, I have to update your client
> program. I don't see how it's any different to updating the kernel
> module.

Then you don't live in the real world. Let's say I run Debian (but it
could be any Linux distribution), lets say that Debian is running a
version of the Linux kernel that has the relevant patch for the Toshiba
HCI in the toshiba_acpi module.

I buy my brand new Toshiba laptop which has some fancy new feature that
is controlled by the HCI (say like the transflective screen on the
Portage R500 and R600). I spend an bit of time in Windows working out
what the HCI calls are, and write a small user space program to do the
same under Linux. That's it, I am now home free no need to do any work
going forward.

Your method I write and debug a kernel module. Quite possibly hard
locking laptop in the process. I run a genuine risk of bricking the
laptop as well in the meantime. Then Debian release a security update,
so I need to compile the kernel module again. Maybe patches to the
kernel module don't make it in time for the next release of the
distribution so I am on a kernel module compile treadmill for years.

It also means I have to run the latest version of the kernel anyway to
develop the module. Maybe I run RHEL/CentOS, I can hardly use a
RHEL/CentOS kernel to develop a kernel module can I. So I have to break
my distribution to do this rather than get on with more important
things.

Too need this explaining beggars belief. In addition it is just feasible
to get an end user to download a program to /usr/local/bin and run it.
Getting them involved in a constant cycle of kernel module compiling is
just not going to happen.

> > You also failed to explain how the supervisor, and user password setting
> > was going to work from a kernel mode proc interface.
> 
> Can't you do this from the BIOS?
> 

Do you even own a "pucker" Toshiba laptop??? The short answer is no,
there are a whole bunch of things that you can set with the HCI that can
be set *NO* other way.

> > You miss entirely the point of Toshiba's HCI. We are not talking
> about
> > backlight control here. We are talking about a bunch of other stuff.
> 
> "Bunch of other stuff". Could we not decide on a proper framework for
> this functionality?
> 

The proper framework is the HCI interface. What you can do with the HCI
is extremely wide ranging. While much of the pre ACPI, HCI commands (and
the complete Software Configuration Interface SCI) have been absorbed in
ACPI much of it has not. If the authors of the ACPI specification, and
manufactures of the laptop cannot come up with a consistent way of doing
this what makes you think you can?

> > > Well, I think the onus is on you to provide a proper kernel patch,
> > > rather than just exposing userspace to /dev/toshiba, afterall, that was
> > > the thing that's prompted my mail
> > 
> > We have a proper kernel patch, the use of /dev/toshiba was excepted
> > upstream a decade ago.
> 
> As was /proc/apm, /dev/pmu and all the other _obsolete_ interfaces. They
> were bad then, and they would be bad now. Userspace and the kernel have
> moved on from a decade ago.

These describe specified interfaces. Something that the Toshiba HCI is
not. It can do literally *anything* on your laptop. If you look in the
toshiba driver you will see some of the calls are deliberately blocked
for this reason.

> >  There is a range of software that supports this
> > interface. The patch extends this to modern Toshiba hardware. It is a no
> > brainer to anyone with any practical sense.
> 
> Maybe I have no brain.
> 

Clearly not.

In the meantime actual code trumps.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 14:03         ` Richard Hughes
@ 2009-02-26 15:51           ` Jonathan Buzzard
  2009-02-26 16:01             ` Matthew Garrett
  2009-02-27 16:49             ` Richard Hughes
  0 siblings, 2 replies; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-26 15:51 UTC (permalink / raw)
  To: Richard Hughes; +Cc: John Belmonte, Matthew Garrett, Charles, linux-acpi


On Thu, 2009-02-26 at 14:03 +0000, Richard Hughes wrote:
> On Thu, 2009-02-26 at 08:12 -0500, John Belmonte wrote:
> > The proposed patch makes /dev/toshiba, and hence toshset, work on more
> > laptops-- it's a no-brainer to anyone with a practical mindset.
> 
> Making toshset work is not a practical goal.

I suggest you get a dictionary out and look up the work practical.
Because making it works is eminently practical.

>  Making hardware "just work"
> without any vendor specific commands is a better goal. Exposing low
> level, device specific, hardware details up through a device node isn't
> a good plan at all.
> 

Well Toshiba who in part wrote the ACPI specification take *EXACTLY* the
same route on Windows. There is a device driver that creates a TVALD
device (the name has changed slightly on newwer models and I cannot
remember it off hand). To change an HCI setting you open up the TVALD
and do a ioctl in *exactly* the same manner as you do on /dev/toshiba.
The similarity is so striking I could almost say Toshiba copied me.

Now if Toshiba who in part wrote the ACPI specification think this is
the way to go, and the people who know the most about the HCI in the
free software world also think this is the way to go, what are your
qualifications to argue with them, given you have virtually no knowledge
of the Toshiba HCI and would not appear to even own a Toshiba designed
and manufactured laptop anyway.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 15:51           ` Jonathan Buzzard
@ 2009-02-26 16:01             ` Matthew Garrett
  2009-02-27 16:49             ` Richard Hughes
  1 sibling, 0 replies; 34+ messages in thread
From: Matthew Garrett @ 2009-02-26 16:01 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: Richard Hughes, John Belmonte, Charles, linux-acpi

On Thu, Feb 26, 2009 at 03:51:58PM +0000, Jonathan Buzzard wrote:

> Well Toshiba who in part wrote the ACPI specification take *EXACTLY* the
> same route on Windows. There is a device driver that creates a TVALD
> device (the name has changed slightly on newwer models and I cannot
> remember it off hand). To change an HCI setting you open up the TVALD
> and do a ioctl in *exactly* the same manner as you do on /dev/toshiba.
> The similarity is so striking I could almost say Toshiba copied me.

And Dell provided us with a set of userspace drivers to use their smbios 
functionality and still encouraged me to move large chunks of them to a 
kernel driver.

> Now if Toshiba who in part wrote the ACPI specification think this is
> the way to go, and the people who know the most about the HCI in the
> free software world also think this is the way to go, what are your
> qualifications to argue with them, given you have virtually no knowledge
> of the Toshiba HCI and would not appear to even own a Toshiba designed
> and manufactured laptop anyway.

You're really not selling yourself here.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-26 15:51           ` Jonathan Buzzard
  2009-02-26 16:01             ` Matthew Garrett
@ 2009-02-27 16:49             ` Richard Hughes
  2009-02-27 17:18               ` Jonathan Buzzard
  2009-02-28 15:19               ` Henrique de Moraes Holschuh
  1 sibling, 2 replies; 34+ messages in thread
From: Richard Hughes @ 2009-02-27 16:49 UTC (permalink / raw)
  To: Jonathan Buzzard; +Cc: John Belmonte, Matthew Garrett, Charles, linux-acpi

On Thu, 2009-02-26 at 15:51 +0000, Jonathan Buzzard wrote:
> Now if Toshiba who in part wrote the ACPI specification think this is
> the way to go, and the people who know the most about the HCI in the
> free software world also think this is the way to go, what are your
> qualifications to argue with them, given you have virtually no
> knowledge of the Toshiba HCI and would not appear to even own a
> Toshiba designed and manufactured laptop anyway.

I still own a Satellite Pro A10, model number PSA15E-03U7V-EN. I no
longer use it day-to-day, as it's so very slow compared to my T61.

I know exactly what HCI is, how it works, and have a good idea of what
it can do. I've written patches for the toshiba_acpi driver before for
the key mapping functionality.

In the meantime, you've insulted me enough for one email thread.

Len, I would agree with Matthew that the patch should not be merged, and
that any missing functionality should be added to the existing ACPI
driver.

Richard.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-27 16:49             ` Richard Hughes
@ 2009-02-27 17:18               ` Jonathan Buzzard
  2009-02-28 15:19               ` Henrique de Moraes Holschuh
  1 sibling, 0 replies; 34+ messages in thread
From: Jonathan Buzzard @ 2009-02-27 17:18 UTC (permalink / raw)
  To: Richard Hughes; +Cc: John Belmonte, Matthew Garrett, Charles, linux-acpi


On Fri, 2009-02-27 at 16:49 +0000, Richard Hughes wrote:
> On Thu, 2009-02-26 at 15:51 +0000, Jonathan Buzzard wrote:
> > Now if Toshiba who in part wrote the ACPI specification think this is
> > the way to go, and the people who know the most about the HCI in the
> > free software world also think this is the way to go, what are your
> > qualifications to argue with them, given you have virtually no
> > knowledge of the Toshiba HCI and would not appear to even own a
> > Toshiba designed and manufactured laptop anyway.
> 
> I still own a Satellite Pro A10, model number PSA15E-03U7V-EN. I no
> longer use it day-to-day, as it's so very slow compared to my T61.
> 

I am pretty sure that this is not a Toshiba designed and manufactured
laptop, but is one of their Compal OEM jobs with a Phoenix BIOS. The
Toshiba HCI that I am talking about is not relevant on such a laptop.

> I know exactly what HCI is, how it works, and have a good idea of what
> it can do. I've written patches for the toshiba_acpi driver before for
> the key mapping functionality.

Except you don't. By your own admission you thought that you could set
things like the supervisor, and user passwords via the BIOS. I notice
that nobody has suggested a safe way to do this via echoing stuff in a
proc interface...

You also questioned whether it is possible to brick a Toshiba laptop
using the HCI. If you genuinely understood what it did and was capable
of you would not question that. You can do *ANYTHING* purely using the
HCI.

> In the meantime, you've insulted me enough for one email thread.

You have told me what I should be able to do under Linux. You told me
that if I want to change BIOS settings I have to reboot because you
don't think exposing the HCI interface is a good idea. You have told me
that if I want to change the ownerstring I should boot Windows. You have
told me that I want to change the supervisor and user passwords I should
again boot Windows. If you find my response to your dictation of what I
should be able to do insulting look in the mirror.

> Len, I would agree with Matthew that the patch should not be merged, and
> that any missing functionality should be added to the existing ACPI
> driver.

Nobody has at this time suggested a workable alternative. It is also not
possible to add all the functionality to the existing ACPI drive in the
manner in which you suggest. I accept you could add some, but all is not
possible. 

Give that it is not possible to add do what you suggest we need a plan
B. If you cannot come up with a plan B and nobody has then the patch
should go in.


JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan (at) buzzard.me.uk
Fife, United Kingdom.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-27 16:49             ` Richard Hughes
  2009-02-27 17:18               ` Jonathan Buzzard
@ 2009-02-28 15:19               ` Henrique de Moraes Holschuh
  2009-03-13  1:17                 ` Len Brown
  1 sibling, 1 reply; 34+ messages in thread
From: Henrique de Moraes Holschuh @ 2009-02-28 15:19 UTC (permalink / raw)
  To: Richard Hughes
  Cc: Jonathan Buzzard, John Belmonte, Matthew Garrett, Charles, linux-acpi

On Fri, 27 Feb 2009, Richard Hughes wrote:
> In the meantime, you've insulted me enough for one email thread.
> 
> Len, I would agree with Matthew that the patch should not be merged, and
> that any missing functionality should be added to the existing ACPI
> driver.

FWIW, I also think the patch, as it stands, is NOT a good idea.

I will ignore the human interaction protocol screwups that happened in
the thread even if they left me with a bad taste in the mouth just
from watching from the outside.

There are strong technical reasons to not accept the patch IMHO:

1. If something can brick a box, access to it must not be provided
   unfiltered, not even kernel-side (through exported functions,
   active virtual memory regions that a bug could overwrite, etc),
   let alone to userland.  At least not as the normal operational mode
   of a driver (might-brick-something functionality hidden behind a
   developer mode accessed through a Kconfig option would be OK).

   If there isn't a safe group of functions for all models the driver
   will load at, filtering must be done by whitelisting.  It really
   doesn't matter if the table will be big, it can live in __initdata,
   in fact, if it were not for the can-brick stuff, it could be done
   through the firmware interface (easy upgrading for new models).

   If filtering is implemented, giving userspace access through
   /dev/toshiba for selected HCI functions would not be a problem.
   However, see (2) below.

2. Standard kernel functionality is to be made accessible through
   standard kernel interfaces.  This means brightness control and
   rfkill control, power supplies, thermal and fan control, hotkeys
   and generic event reporting, among others.

   Userspace could use /dev/toshiba for anything else there isn't a
   generic interface for, though.

   Access to such functionality also through /dev/toshiba should be
   provided only for backwards compatibility, with a set date for
   removal (say, one year).

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-28 15:19               ` Henrique de Moraes Holschuh
@ 2009-03-13  1:17                 ` Len Brown
  2009-03-14  0:37                   ` Charles
  2009-03-14  7:02                   ` Andrey Borzenkov
  0 siblings, 2 replies; 34+ messages in thread
From: Len Brown @ 2009-03-13  1:17 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Richard Hughes, Jonathan Buzzard, John Belmonte, Matthew Garrett,
	Charles, linux-acpi

I acknowledge that the SMM control method doesn't fit well
into our current generic software abstractions for
controlling hardware.

I also acknowledge that this control has a right to exist
if people find it useful.  Further, I don't by the fact
that it root can use it to brick a system as sufficient
reason not to provide it.  Root can already do worse...

However, I'm not excited about extending toshiba_acpi to
include /dev/toshiba SMM interface that is already
provided by the toshiba driver.

I'd rather see the 'toshiba' driver (perhaps renamed to be toshiba_smm)
extended so that it is the only driver to provide this interface.

BTW. I notice that both toshiba_acpi and toshiba ship in Fedora
and can load on my satellite pro.  However, fedora doesn't ship
toshset -- is toshiba and /dev/toshiba useful without it?

Are new machines still using this interface, or it is going away over 
time?

thanks,
-Len



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-03-13  1:17                 ` Len Brown
@ 2009-03-14  0:37                   ` Charles
  2009-03-14  7:02                   ` Andrey Borzenkov
  1 sibling, 0 replies; 34+ messages in thread
From: Charles @ 2009-03-14  0:37 UTC (permalink / raw)
  To: Len Brown
  Cc: Henrique de Moraes Holschuh, Richard Hughes, Jonathan Buzzard,
	John Belmonte, Matthew Garrett, Charles, linux-acpi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Len Brown <lenb@kernel.org> wrote:

> However, I'm not excited about extending toshiba_acpi to
> include /dev/toshiba SMM interface that is already
> provided by the toshiba driver.
> 
> I'd rather see the 'toshiba' driver (perhaps renamed to be toshiba_smm)
> extended so that it is the only driver to provide this interface.
> 

no objection here. I'd just like to have a working solution.

> BTW. I notice that both toshiba_acpi and toshiba ship in Fedora
> and can load on my satellite pro.  However, fedora doesn't ship
> toshset -- is toshiba and /dev/toshiba useful without it?

toshiba_acpi does provide support for a few of the bios settings through
entries in /proc/acpi/toshiba.

> 
> Are new machines still using this interface, or it is going away over 
> time?
> 

It seems that most laptops in the Tecra and Portege lines have this
interface. Many (most?) of the recent Satellites have a Phoenix
Bios which do not support this interface. Most recently, it seems that
some toshiba machines have Insyde BIOS.

Charles
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8+ <http://mailcrypt.sourceforge.net/>

iD8DBQFJuvxlPK2zrJwS/lYRAtEGAJsEMSCMZvVe6FiFUF7QW2Zc2Y3I1ACfV3c0
bjBVX77ycCHz0qC6SF4wwhU=
=cx50
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-03-13  1:17                 ` Len Brown
  2009-03-14  0:37                   ` Charles
@ 2009-03-14  7:02                   ` Andrey Borzenkov
  2009-03-14 12:05                     ` Matthew Garrett
  1 sibling, 1 reply; 34+ messages in thread
From: Andrey Borzenkov @ 2009-03-14  7:02 UTC (permalink / raw)
  To: Len Brown, linux-acpi

Len Brown wrote:

> 
> BTW. I notice that both toshiba_acpi and toshiba ship in Fedora
> and can load on my satellite pro.  However, fedora doesn't ship
> toshset -- is toshiba and /dev/toshiba useful without it?
> 

Yes,  there are other utils using this interface; e.g. Mandriva ships 
toshutils with

/usr/bin/alarm
/usr/bin/dispswitch
/usr/bin/fan
/usr/bin/hotkey
/usr/bin/ownerstring
/usr/bin/svpw
/usr/bin/tbacklight
/usr/bin/tdocked
/usr/bin/thotswap
/usr/bin/tpasswd



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-03-14  7:02                   ` Andrey Borzenkov
@ 2009-03-14 12:05                     ` Matthew Garrett
  0 siblings, 0 replies; 34+ messages in thread
From: Matthew Garrett @ 2009-03-14 12:05 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: Len Brown, linux-acpi

On Sat, Mar 14, 2009 at 10:02:09AM +0300, Andrey Borzenkov wrote:

> /usr/bin/alarm

Should be hooked into the rtc interface, but I'd be surprised if it 
doesn't just work with the existing alarm infrastructure.

> /usr/bin/dispswitch

Should be hooked into the video output class.

> /usr/bin/fan

Should be an hwmon or thermal device.

> /usr/bin/hotkey

Unneeded now.

> /usr/bin/tbacklight

There's already a backlight class device.

> /usr/bin/tdocked

Should be a platform device emitting dock events.

> /usr/bin/thotswap

Should already be unnecessary on any ACPI system - impossible to make 
work properly without being in the kernel.

> /usr/bin/ownerstring
> /usr/bin/svpw
> /usr/bin/tpasswd

These are the ones of interest.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-03-01  7:00       ` Andrey Borzenkov
@ 2009-03-01 10:31         ` Charles
  0 siblings, 0 replies; 34+ messages in thread
From: Charles @ 2009-03-01 10:31 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: Charles, linux-acpi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Andrey Borzenkov <arvidjaar@mail.ru> wrote:

> On 28 ??????? 2009 21:00:26 Charles@schwieters.org wrote:
> >
> > > But then I do not understand why you need this in toshiba_acpi in
> > > the first place. We already have in-kernel module (toshiba) that
> > > provides raw user space HCI access to those who need it. Why is it
> > > necessary to duplicate this functionality in toshiba_acpi in the
> > > first place?
> >
> > I did have reports of some machines like yours. This is the result on
> > a portege r100 (with unpatched toshiba_acpi:
> >
> > % lsmod | grep tosh
> > toshiba_acpi            5336  0
> > toshiba                 4152  0
> > % fan
> > fan: laptop does not have cooling fan or kernel module not installed.
> >
> > Behavior on most machines (and all three of mine) seems to be the
> > same.
> >
> 
> Sorry for dumb question but are you doing it as root?

actually, I get the same result whether running root or not.

Charles
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8+ <http://mailcrypt.sourceforge.net/>

iD8DBQFJqmPrPK2zrJwS/lYRAu38AJ9+NmkjhTr/AZH7HMZ6wthlPrnVSACcD9TU
zVBe/SjEYAZLlRpsoXDrbNA=
=HrY/
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-28 18:00     ` Charles
@ 2009-03-01  7:00       ` Andrey Borzenkov
  2009-03-01 10:31         ` Charles
  0 siblings, 1 reply; 34+ messages in thread
From: Andrey Borzenkov @ 2009-03-01  7:00 UTC (permalink / raw)
  To: Charles; +Cc: linux-acpi

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On 28 февраля 2009 21:00:26 Charles@schwieters.org wrote:
>
> > But then I do not understand why you need this in toshiba_acpi in
> > the first place. We already have in-kernel module (toshiba) that
> > provides raw user space HCI access to those who need it. Why is it
> > necessary to duplicate this functionality in toshiba_acpi in the
> > first place?
>
> I did have reports of some machines like yours. This is the result on
> a portege r100 (with unpatched toshiba_acpi:
>
> % lsmod | grep tosh
> toshiba_acpi            5336  0
> toshiba                 4152  0
> % fan
> fan: laptop does not have cooling fan or kernel module not installed.
>
> Behavior on most machines (and all three of mine) seems to be the
> same.
>

Sorry for dumb question but are you doing it as root?

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-28  6:13   ` Andrey Borzenkov
@ 2009-02-28 18:00     ` Charles
  2009-03-01  7:00       ` Andrey Borzenkov
  0 siblings, 1 reply; 34+ messages in thread
From: Charles @ 2009-02-28 18:00 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: Charles, linux-acpi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Andrey Borzenkov <arvidjaar@mail.ru> wrote:

> On 28 ??????? 2009 00:31:59 Charles@schwieters.org wrote:
> > Hello Andrey--
> >
> > > >This patch comes from
> > > > http://memebeam.org/free-software/toshiba_acpi/. With this patch,
> > > > the module conflicts with the toshiba module (which works only on
> > > > non-acpi-enabled kernels).
> > >
> > > Could you please elaborate? Because (Portege 4000):
> > >
> > > {pts/2}% lsmod | grep tosh
> > > toshiba                 4012  0
> > > toshiba_acpi            7540  0
> > > rfkill                 10480  1 toshiba_acpi
> > > backlight               4560  2 toshiba_acpi,video
> > > input_polldev           4012  1 toshiba_acpi
> > > {pts/2}% sudo fan
> > > Fan is on.
> >
> > My understanding is that with the experimental patch, both toshiba
> > and toshiba_acpi use /dev/toshiba. Do you have the experimental patch
> > applied? Does fan work if you remove the toshiba module?
> >
> 
> May be I misunderstood you then; do you mean that *if* this patch is 
> applied module toshiba works on non-ACPI kernels only? That would be the 
> case then.

that's right.

> 
> But then I do not understand why you need this in toshiba_acpi in the 
> first place. We already have in-kernel module (toshiba) that provides 
> raw user space HCI access to those who need it. Why is it necessary to 
> duplicate this functionality in toshiba_acpi in the first place?

I did have reports of some machines like yours. This is the result on a
portege r100 (with unpatched toshiba_acpi:

% lsmod | grep tosh
toshiba_acpi            5336  0 
toshiba                 4152  0 
% fan
fan: laptop does not have cooling fan or kernel module not installed.

Behavior on most machines (and all three of mine) seems to be the same.

Charles
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8+ <http://mailcrypt.sourceforge.net/>

iD8DBQFJqXu6PK2zrJwS/lYRArFkAJ4mUzyCekXST4fIKn3Uhw2srNdcdACZAVSo
Pis83ewju56RXMqPUdv+A0M=
=KU7l
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-27 21:31 ` Charles
@ 2009-02-28  6:13   ` Andrey Borzenkov
  2009-02-28 18:00     ` Charles
  0 siblings, 1 reply; 34+ messages in thread
From: Andrey Borzenkov @ 2009-02-28  6:13 UTC (permalink / raw)
  To: Charles; +Cc: linux-acpi

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

On 28 февраля 2009 00:31:59 Charles@schwieters.org wrote:
> Hello Andrey--
>
> > >This patch comes from
> > > http://memebeam.org/free-software/toshiba_acpi/. With this patch,
> > > the module conflicts with the toshiba module (which works only on
> > > non-acpi-enabled kernels).
> >
> > Could you please elaborate? Because (Portege 4000):
> >
> > {pts/2}% lsmod | grep tosh
> > toshiba                 4012  0
> > toshiba_acpi            7540  0
> > rfkill                 10480  1 toshiba_acpi
> > backlight               4560  2 toshiba_acpi,video
> > input_polldev           4012  1 toshiba_acpi
> > {pts/2}% sudo fan
> > Fan is on.
>
> My understanding is that with the experimental patch, both toshiba
> and toshiba_acpi use /dev/toshiba. Do you have the experimental patch
> applied? Does fan work if you remove the toshiba module?
>

May be I misunderstood you then; do you mean that *if* this patch is 
applied module toshiba works on non-ACPI kernels only? That would be the 
case then.

But then I do not understand why you need this in toshiba_acpi in the 
first place. We already have in-kernel module (toshiba) that provides 
raw user space HCI access to those who need it. Why is it necessary to 
duplicate this functionality in toshiba_acpi in the first place?

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
  2009-02-27 21:15 Andrey Borzenkov
@ 2009-02-27 21:31 ` Charles
  2009-02-28  6:13   ` Andrey Borzenkov
  0 siblings, 1 reply; 34+ messages in thread
From: Charles @ 2009-02-27 21:31 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: Charles, linux-acpi

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hello Andrey--

> >This patch comes from http://memebeam.org/free-software/toshiba_acpi/.
> >With this patch, the module conflicts with the toshiba module (which
> >works only on non-acpi-enabled kernels).
> 
> Could you please elaborate? Because (Portege 4000):
> 
> {pts/2}% lsmod | grep tosh
> toshiba                 4012  0
> toshiba_acpi            7540  0
> rfkill                 10480  1 toshiba_acpi
> backlight               4560  2 toshiba_acpi,video
> input_polldev           4012  1 toshiba_acpi
> {pts/2}% sudo fan
> Fan is on.
> 

My understanding is that with the experimental patch, both toshiba and
toshiba_acpi use /dev/toshiba. Do you have the experimental patch
applied? Does fan work if you remove the toshiba module?

Charles
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Processed by Mailcrypt 3.5.8+ <http://mailcrypt.sourceforge.net/>

iD8DBQFJqFvPPK2zrJwS/lYRApaPAJwK7jbkxT070MBKm+TyWI9/gEO89wCfS4sy
iukYMH2E2g9UiQ2EB+YIaAE=
=AFBQ
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: experimental patch for toshiba_acpi
@ 2009-02-27 21:15 Andrey Borzenkov
  2009-02-27 21:31 ` Charles
  0 siblings, 1 reply; 34+ messages in thread
From: Andrey Borzenkov @ 2009-02-27 21:15 UTC (permalink / raw)
  To: Charles, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

>This patch comes from http://memebeam.org/free-software/toshiba_acpi/.
>With this patch, the module conflicts with the toshiba module (which
>works only on non-acpi-enabled kernels).

Could you please elaborate? Because (Portege 4000):

{pts/2}% lsmod | grep tosh
toshiba                 4012  0
toshiba_acpi            7540  0
rfkill                 10480  1 toshiba_acpi
backlight               4560  2 toshiba_acpi,video
input_polldev           4012  1 toshiba_acpi
{pts/2}% sudo fan
Fan is on.

So I have ACPI enabled kernel and toshiba seems to work in parallel with 
toshiba_acpi.

Are there specific models for which this does not work?

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2009-03-14 12:05 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-25 14:54 experimental patch for toshiba_acpi Charles
2009-02-25 15:24 ` Matthew Garrett
2009-02-25 16:18   ` Jonathan Buzzard
2009-02-25 16:51     ` Matthew Garrett
2009-02-25 17:12       ` Jonathan Buzzard
2009-02-25 17:28         ` Matthew Garrett
2009-02-25 17:53           ` Jonathan Buzzard
2009-02-25 17:59             ` Matthew Garrett
2009-02-25 20:18               ` Charles
2009-02-26  0:22               ` Jonathan Buzzard
2009-02-26  8:39                 ` Richard Hughes
2009-02-26 10:34                   ` Jonathan Buzzard
2009-02-26 12:52                     ` Richard Hughes
2009-02-26 13:27                       ` Jonathan Buzzard
2009-02-26 13:59                         ` Richard Hughes
2009-02-26 15:49                           ` Jonathan Buzzard
2009-02-25 17:33         ` Azael Avalos
2009-02-26 13:12       ` John Belmonte
2009-02-26 14:03         ` Richard Hughes
2009-02-26 15:51           ` Jonathan Buzzard
2009-02-26 16:01             ` Matthew Garrett
2009-02-27 16:49             ` Richard Hughes
2009-02-27 17:18               ` Jonathan Buzzard
2009-02-28 15:19               ` Henrique de Moraes Holschuh
2009-03-13  1:17                 ` Len Brown
2009-03-14  0:37                   ` Charles
2009-03-14  7:02                   ` Andrey Borzenkov
2009-03-14 12:05                     ` Matthew Garrett
2009-02-27 21:15 Andrey Borzenkov
2009-02-27 21:31 ` Charles
2009-02-28  6:13   ` Andrey Borzenkov
2009-02-28 18:00     ` Charles
2009-03-01  7:00       ` Andrey Borzenkov
2009-03-01 10:31         ` Charles

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.