[PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Sau Dan Lee]

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

>>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:

    Dmitry> On Wednesday 02 June 2004 04:49 am, Sau Dan Lee wrote:

    >>  So, only AUX ports can be directly accessed?  No direct access
    >> to keyboard port?  Why?

    Dmitry> Keyboards are to be handled in-kernel, at least for
    Dmitry> now. If there really a need we can enable grabbing
    Dmitry> keyboard as well, no big deal.

My new patch should  be able to connect the PC AT  or PS/2 keyboard to
your serio_raw thing.  See below.


    >> 2) I hate this black magic, in which the input "devices"
    >> (i.e. drivers) kidnap the serio ports they like according to
    >> the port type SERIO_8042_RAW, etc.  That's a kind of hardcoding
    >> the binding between ports and drivers.

    Dmitry> We do have some hardcoding so atkbd does not try to grab a
    Dmitry> serio linked to a serial port and sermouse does not try
    Dmitry> grabbing keyboard port, etc..  There is nothing new.

Of course, it is completely OK  for the serio device code to check the
serio port type, and refuse to work with incompatible ports.  However,
that  doesn't mean  it  should rob  all  the ports  it likes.   That's
selfish.  There could be other  serio devices that are also interested
in the  same kinds of ports  as yours!  Loving  chocolate doesn't mean
you should  steal all the  chocolates in the  world and eat  them all.
Learn to share!  :)

======================================================================

I've  added procfs  support to  serio.c, so  that we  can  now control
dynamically which serio ports connect  to which serio devices.  I call
it "serio_switch", because it conceptually works like a patch panel or
a switch with which you can rewire the connections.

If  you  ignore  this feature,  that  it  work  as in  vanilla  2.6.*.
However,  you can  now  go to  /proc/bus/input/serio_switch/ and  look
around.   There   are  2   directories  there:  "ports/"   with  files
representing  all registered  serio port.   Each file  is a  r/w file.
Upon reading,  it shows  the "name"  of the serio  device which  it is
connected to, or  "(unconnected)".  Write to it the  "name" of a serio
device, and that port will be connected to the device.  The write will
fail with a  kernel log message if the device name  is invalid, or the
device REFUSES  to connected to  the port (e.g. connecting  'atkbd' to
the  AUX port  attached to  a  mouse), leaving  the port  unconnected.
Write "(unconnected)" or  simply "" to the file, and  the port will be
disconnected.   (Feature: "cat  port_file  > port_file"  will cause  a
disconnection followed immediately by a connection!)

The other directory is "devices/",  with one file per registered serio
device.  "cat"  the file and  you see the  names of serio ports  it is
handling.  The file is read-only.


Problem: the  modules providing serio  devices do not  appropriate set
the name  field in the  struct serio_dev.  So,  my patch cannot  get a
meaningful serio  device name.   Right now, the  code checks  for this
problem,  and would  manufacture  a (hopefully  unique)  name for  the
device.   Once the  modules are  fixed  w.r.t. this  issue, the  serio
device names will be more user-friendly.



I think the same thing would be useful for input.c.  I choose to do it
for serio.c because it is  simpler (serio_port -> serio_device is 1->n
mapping), and because  it is a module.  (I can't  compile input.c as a
module unless I give up virtual terminals.)  It is useful for input.c,
because when debugging  input modules, it'd be better  to have 'evbug'
to monitor *only* the device being debugged.
 


    >> Isn't it better to leave the AUX ports as SERIO_8042, and let
    >> the user dynamically change this port<-->driver binding?  Then,
    >> we don't even need that ugly "i8042.raw" boot parameter or
    >> i8042_aux_raw option.  The user can decide which ports are
    >> connected to your serio_raw driver, and which ports are
    >> connected to psmouse.ko.  That would also allow multiple
    >> drivers driving the ports of the same type.
    >> 

    Dmitry> Yes, that's the perfect solution, but I believe we need
    Dmitry> sysfs for that and at least I started working on it, but
    Dmitry> it takes time. 

I've been studying and experimenting  with sysfs and kobject for a few
hours.  But  I'm still  so confused  that I decided  to go  the procfs
route.  At least, I can code it with procfs in a few hours.


    Dmitry> In the meantime i8042.raw should alleviate most of the
    Dmitry> user's troubles with their input devices no longer
    Dmitry> working.

Use my patch!   :D With my patch, it should be  possible to rewire any
serio port to your raw-device.  Instead of your i8042.raw option, It'd
be better to  have a "manual_connect" option that  tells the module to
refrain from  connecting to any devices upon  module loading.  Without
this option, it works like before: hunt for all its favourite preys.



Finally, here is  the patch.  It is based on  2.6.7-rc1.  (But I don't
think anything has changed in serio.c since 2.6.0.)

I actually developed this  patch upon a SERIO_USERDEV-patched version.
So, Tuukka, if  you want it, I  can give a patch on  patch.  Thanks to
the tool  'patch' with the  -F option and  rcsdiff, it took me  only 5
minutes to backport the changes to the serio.c in stock 2.6.7-rc1.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch against 2.6.7-rc1 --]
[-- Type: text/x-patch, Size: 10319 bytes --]

--- linux-2.6.7-rc1/drivers/input/serio/serio.c	2004/06/02 16:47:33	1.1.1.1
+++ linux-2.6.7-rc1.serio_switch/drivers/input/serio/serio.c	2004/06/02 16:55:34	1.1.1.2
@@ -35,20 +35,22 @@
 #include <linux/stddef.h>
 #include <linux/module.h>
 #include <linux/serio.h>
 #include <linux/errno.h>
 #include <linux/wait.h>
 #include <linux/completion.h>
 #include <linux/sched.h>
 #include <linux/smp_lock.h>
 #include <linux/suspend.h>
 #include <linux/slab.h>
+#include <linux/proc_fs.h>
+#include <asm/uaccess.h>
 
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@ucw.cz>");
 MODULE_DESCRIPTION("Serio abstraction core");
 MODULE_LICENSE("GPL");
 
 EXPORT_SYMBOL(serio_interrupt);
 EXPORT_SYMBOL(serio_register_port);
 EXPORT_SYMBOL(serio_register_port_delayed);
 EXPORT_SYMBOL(__serio_register_port);
 EXPORT_SYMBOL(serio_unregister_port);
@@ -66,20 +68,26 @@
 	struct serio *serio;
 	struct list_head node;
 };
 
 static DECLARE_MUTEX(serio_sem);
 static LIST_HEAD(serio_list);
 static LIST_HEAD(serio_dev_list);
 static LIST_HEAD(serio_event_list);
 static int serio_pid;
 
+#ifdef CONFIG_PROC_FS
+static struct proc_dir_entry *proc_bus_input_ss_dir;
+static struct proc_dir_entry *proc_bus_input_ss_ports_dir;
+static struct proc_dir_entry *proc_bus_input_ss_devices_dir;
+#endif
+
 static void serio_find_dev(struct serio *serio)
 {
 	struct serio_dev *dev;
 
 	list_for_each_entry(dev, &serio_dev_list, node) {
 		if (serio->dev)
 			break;
 		if (dev->connect)
 			dev->connect(serio, dev);
 	}
@@ -215,28 +223,50 @@
 /*
  * Submits register request to kseriod for subsequent execution.
  * Can be used when it is not obvious whether the serio_sem is
  * taken or not and when delayed execution is feasible.
  */
 void serio_register_port_delayed(struct serio *serio)
 {
 	serio_queue_event(serio, SERIO_REGISTER_PORT);
 }
 
+#ifdef CONFIG_PROC_FS
+static 
+int serio_switch_port_proc_file_read(char *page, char **start, off_t off,
+				     int count, int *eof, void *data);
+static
+int serio_switch_port_proc_file_write(struct file *file,
+				      const char __user *buffer,
+				      unsigned long count, void *data);
+#endif
+
 /*
  * Should only be called directly if serio_sem has already been taken,
  * for example when unregistering a serio from other input device's
  * connect() function.
  */
 void __serio_register_port(struct serio *serio)
 {
 	list_add_tail(&serio->node, &serio_list);
+
+#ifdef CONFIG_PROC_FS
+	if (proc_bus_input_ss_ports_dir) {
+		struct proc_dir_entry *proc_entry;
+
+		proc_entry = create_proc_read_entry(serio->name, S_IWUSR|S_IRUSR|S_IRGRP|S_IROTH, proc_bus_input_ss_ports_dir,
+						    serio_switch_port_proc_file_read, serio);
+		proc_entry->owner = THIS_MODULE;
+		proc_entry->write_proc = serio_switch_port_proc_file_write;
+	}
+#endif
+
 	serio_find_dev(serio);
 }
 
 void serio_unregister_port(struct serio *serio)
 {
 	down(&serio_sem);
 	__serio_unregister_port(serio);
 	up(&serio_sem);
 }
 
@@ -251,40 +281,85 @@
 }
 
 /*
  * Should only be called directly if serio_sem has already been taken,
  * for example when unregistering a serio from other input device's
  * disconnect() function.
  */
 void __serio_unregister_port(struct serio *serio)
 {
 	serio_invalidate_pending_events(serio);
+#ifdef CONFIG_PROC_FS
+	if (proc_bus_input_ss_ports_dir) {
+		remove_proc_entry(serio->name, proc_bus_input_ss_ports_dir);
+	}
+#endif
 	list_del_init(&serio->node);
 	if (serio->dev && serio->dev->disconnect)
 		serio->dev->disconnect(serio);
 }
 
+#ifdef CONFIG_PROC_FS
+static 
+int serio_switch_device_proc_file_read(char *page, char **start, off_t off,
+				       int count, int *eof, void *data);
+#endif
+
 void serio_register_device(struct serio_dev *dev)
 {
 	struct serio *serio;
 	down(&serio_sem);
 	list_add_tail(&dev->node, &serio_dev_list);
 	list_for_each_entry(serio, &serio_list, node)
 		if (!serio->dev && dev->connect)
 			dev->connect(serio, dev);
 	up(&serio_sem);
+
+#ifdef CONFIG_PROC_FS
+	if (!dev->name) {
+	  /* some modules do not set the dev->name!  :( */
+	  dev->name = kmalloc(16, GFP_KERNEL);
+	  if (!dev->name)
+	  	dev->name = "out of memory";
+	  else
+	  	snprintf(dev->name, 16, "xx%p", dev);
+	}
+
+	if (proc_bus_input_ss_devices_dir) {
+		struct proc_dir_entry *proc_entry;
+
+		proc_entry = create_proc_read_entry(dev->name, 0, proc_bus_input_ss_devices_dir,
+						    serio_switch_device_proc_file_read, dev);
+		proc_entry->owner = THIS_MODULE;
+	}
+#endif
 }
 
 void serio_unregister_device(struct serio_dev *dev)
 {
 	struct serio *serio;
 
+#ifdef CONFIG_PROC_FS
+	if (proc_bus_input_ss_devices_dir) {
+		remove_proc_entry(dev->name, proc_bus_input_ss_devices_dir);
+	}
+
+	if (dev->name[0] == 'x' && dev->name[1] == 'x') {
+		/* assume this name was generated by us */
+		kfree(dev->name);
+		dev->name = NULL;
+	} else if (strcpy(dev->name, "out of memory")==0) {
+		/* couldn't kmalloc */
+		dev->name = NULL;
+	}
+#endif
+
 	down(&serio_sem);
 	list_del_init(&dev->node);
 
 	list_for_each_entry(serio, &serio_list, node) {
 		if (serio->dev == dev && dev->disconnect)
 			dev->disconnect(serio);
 		serio_find_dev(serio);
 	}
 	up(&serio_sem);
 }
@@ -300,34 +375,218 @@
 	return 0;
 }
 
 /* called from serio_dev->connect/disconnect methods under serio_sem */
 void serio_close(struct serio *serio)
 {
 	serio->close(serio);
 	serio->dev = NULL;
 }
 
+
+#ifdef CONFIG_PROC_FS
+static
+struct proc_dir_entry *get_proc_bus_input(void)
+{
+	struct proc_dir_entry *entry;
+	for (entry = proc_bus->subdir; entry; entry = entry->next)
+		if (strcmp(entry->name, "input")==0) {
+			return entry;
+		}
+	return NULL;
+}
+#endif
+
+#ifdef CONFIG_PROC_FS
+static 
+int serio_switch_port_proc_file_read(char *page, char **start, off_t off,
+				     int count, int *eof, void *data)
+{
+	struct serio *serio = data;
+	struct serio_dev *dev = serio->dev;
+	int len, c;
+
+	if (!dev)
+		strcpy(page, "(unconnected)\n");
+	else
+	  	strcat(strcpy(page, dev->name), "\n");
+
+	len = strlen(page);
+	c = len - off;
+	if (c<0) c = 0;
+	*start = page + off;
+	*eof = 1;
+	return count<c? count : c;
+}
+
+static
+int serio_switch_port_proc_file_write(struct file *file,
+				      const char __user *ubuffer,
+				      unsigned long count, void *data)
+{
+	struct serio *serio = data;
+	unsigned char *kbuffer;
+	int r;
+  
+	kbuffer = kmalloc(count+1, GFP_KERNEL);
+	if (!kbuffer) return -ENOMEM;
+	r=copy_from_user(kbuffer, ubuffer, count);
+	if (r) goto out;
+	kbuffer[count] = '\0';
+	if (kbuffer[count-1] == '\n') /* tolerate an EOL */
+		kbuffer[count-1] = '\0';
+
+	if (strlen(kbuffer)==0 ||
+	    strcmp(kbuffer, "(unconnected)")==0) { /* disconnect */
+		if (serio->dev && serio->dev->disconnect)
+			serio->dev->disconnect(serio);
+		goto success;
+	} else {
+		/* find the device to connect to */
+		struct serio_dev *dev;
+
+		down(&serio_sem);
+		list_for_each_entry(dev, &serio_dev_list, node) {
+			if (strcmp(kbuffer, dev->name) != 0)
+				continue; /* not match */
+
+			/* found */
+
+			/* disconnect old connect first */
+			if (serio->dev && serio->dev->disconnect)
+				serio->dev->disconnect(serio);
+
+			/* now, connect to the found device */
+			if (dev->connect)
+				dev->connect(serio, dev);
+
+			up(&serio_sem);
+			if (serio->dev == dev)
+				goto success; /* done */
+			else {
+				printk(KERN_ERR "Inappropriate serio device '%s'\n", kbuffer);
+				r = -EINVAL;
+				goto out;
+			}
+		}
+		up(&serio_sem);
+
+		/* not found */
+		printk(KERN_ERR "Bad serio device '%s'\n", kbuffer);
+		r = -EINVAL;
+		goto out;
+	}
+success:
+	r = count;
+out:
+	kfree(kbuffer);
+	return r;
+}
+
+
+static 
+int serio_switch_device_proc_file_read(char *page, char **start, off_t off,
+				       int count, int *eof, void *data)
+{
+	struct serio_dev *dev = data;
+	struct serio *serio;
+	int len, cnt = 0;
+	off_t at = 0;
+
+	down(&serio_sem);
+	list_for_each_entry(serio, &serio_list, node) {
+		if (serio->dev != dev)
+			continue;
+		len = sprintf(page, "%s\n", serio->name);
+		at += len;
+
+		if (at >= off) {
+			if (!*start) {
+				*start = page + (off - (at - len));
+				cnt = at - off;
+			} else  cnt += len;
+			page += len;
+			if (cnt >= count)
+				break;
+		}
+	}
+
+	if (&serio->node == &serio_list)
+		*eof = 1;
+	up(&serio_sem);
+
+	return (count > cnt) ? cnt : count;
+}
+#endif
+
+
 static int __init serio_init(void)
 {
 	int pid;
 
 	pid = kernel_thread(serio_thread, NULL, CLONE_KERNEL);
 
 	if (!pid) {
 		printk(KERN_WARNING "serio: Failed to start kseriod\n");
 		return -1;
 	}
 
 	serio_pid = pid;
 
+#ifdef CONFIG_PROC_FS
+	{
+		struct proc_dir_entry *proc_bus_input_dir = get_proc_bus_input();
+		if (proc_bus_input_dir) {
+			proc_bus_input_ss_dir = proc_mkdir("serio_switch", proc_bus_input_dir);
+			if (proc_bus_input_ss_dir) {
+				proc_bus_input_ss_dir->owner = THIS_MODULE;
+
+				proc_bus_input_ss_ports_dir = proc_mkdir("ports", proc_bus_input_ss_dir);
+				if (proc_bus_input_ss_ports_dir)
+					proc_bus_input_ss_ports_dir->owner = THIS_MODULE;
+				else
+					printk(KERN_WARNING "/proc/bus/input/serio/ports/ cannot be created\n");
+
+				proc_bus_input_ss_devices_dir = proc_mkdir("devices", proc_bus_input_ss_dir);
+				if (proc_bus_input_ss_devices_dir)
+					proc_bus_input_ss_devices_dir->owner = THIS_MODULE;
+				else
+					printk(KERN_WARNING "/proc/bus/input/serio/devices/ cannot be created\n");
+			} else {
+				printk(KERN_WARNING "/proc/bus/input/serio/ cannot be created\n");
+			}
+		} else {
+			printk(KERN_WARNING "/proc/bus/input/ not found\n");
+		}
+	}
+#endif
+
 	return 0;
 }
 
 static void __exit serio_exit(void)
 {
+#ifdef CONFIG_PROC_FS
+	{
+		if (proc_bus_input_ss_devices_dir) {
+			proc_bus_input_ss_devices_dir->owner = NULL;
+			remove_proc_entry(proc_bus_input_ss_devices_dir->name,
+					  proc_bus_input_ss_devices_dir->parent);
+		}
+		if (proc_bus_input_ss_ports_dir) {
+			proc_bus_input_ss_ports_dir->owner = NULL;
+			remove_proc_entry(proc_bus_input_ss_ports_dir->name,
+					  proc_bus_input_ss_ports_dir->parent);
+		}
+		if (proc_bus_input_ss_dir) {
+			proc_bus_input_ss_dir->owner = NULL;
+			remove_proc_entry(proc_bus_input_ss_dir->name,
+					  proc_bus_input_ss_dir->parent);
+		}
+	}
+#endif
 	kill_proc(serio_pid, SIGTERM, 1);
 	wait_for_completion(&serio_exited);
 }
 
 module_init(serio_init);
 module_exit(serio_exit);

[-- Attachment #3: Type: text/plain, Size: 195 bytes --]




-- 
Sau Dan LEE                     李守敦(Big5)                    ~{@nJX6X~}(HZ) 

E-mail: danlee@informatik.uni-freiburg.de
Home page: http://www.informatik.uni-freiburg.de/~danlee

RE: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Dmitry Torokhov]

Sau Dan Lee wrote:
> 
> I've  added procfs  support to  serio.c, so  that we  can  now control
> dynamically which serio ports connect  to which serio devices.  I call
> it "serio_switch", because it conceptually works like a patch panel or
> a switch with which you can rewire the connections.

Let me start with saying that this is a very good patch and that is
exactly what I have in mind with regard to serio port/device binding.
The only problem with the patch is that it uses wrong foundation, namely
procfs, because:
 
- procfs hierarchy is disconnected from the rest of the system. You
  cannot trace device ownership starting with root PCI bus down to your
  AUX port.
- there is no automatic hotplug notification to the userspace
- it is not flexible with regard to adding custom attributes to the
  drivers. I can see you adding rate and resolution to psmouse driver
  and repeat rate to atkbd. With sysfs drivers can themselves create
  attributes and they will be cleaned up once device disappears. With
  procfs you will have to export it form serio to be available to
  drivers and cleanup can be a nightmare.

Of course sysfs has its "problems" - it requires users to follow certain
lifetime rules which are different from what serio uses at the moment.

So we have several options - if we adopt procfs based solution now we
will have to maintain it for very long time, along with competing sysfs
implementation. Dropping one kernel parameter which will never be widely
used is much easier, IMO.

So I propose we all join our ranks and tame that sysfs together ;) I had
some patches that were converting drivers to the sysfs adding them to
serio bus, I probably should just send what I have as is for review
(I was going to rediff them as they are somewhat stale, I'll see what
shape they are later tonight).

--
Dmitry

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Sau Dan Lee]

>>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:

    Dmitry> Let me start with saying that this is a very good patch
    Dmitry> and that is exactly what I have in mind with regard to
    Dmitry> serio port/device binding.  The only problem with the
    Dmitry> patch is that it uses wrong foundation, namely procfs,
    Dmitry> because:
    ...
 
    Dmitry> So we have several options - if we adopt procfs based
    Dmitry> solution now we will have to maintain it for very long
    Dmitry> time, along with competing sysfs implementation. Dropping
    Dmitry> one kernel parameter which will never be widely used is
    Dmitry> much easier, IMO.

It's not just the matter of dropping one kernel parameter.  The procfs
support, _already  implemented_, allows  one to fine-tune  the binding
between serio  ports and  devices, which is  a new and  useful feature
that  your kernel  parameter  doesn't provide.   

Can you unbind the keyboard port?   Can you bind/unbind any of the AUX
ports  *dynamically*  without   reloading  the  i8042  module?   These
functionalities are  already there in the serio-related  code.  Just a
userland interface is missing.  My patch is to fill this gap.



    Dmitry> So I propose we all join our ranks and tame that sysfs
    Dmitry> together ;) I had some patches that were converting
    Dmitry> drivers to the sysfs adding them to serio bus, 

sysfs  looks  good  for  simple parameters:  integers,  strings.   For
anything more  complicated (sets, graphs),  I don't see it  fit (yet).
Unfortunately, the serio port<-->device relation is already a graph (1
to n).

I'd like  to see  how you implement  the device<-->handler  binding in
input.c using sysfs.  It'd be a nice feature.  Imagine how annoying it
is for 'evbug' to report your keypresses, when you're just debugging a
mouse driver.   Being able to adjust the  device<-->handler binding is
what I  want.  I don't  care whether it's  a procfs approach  or sysfs
approach, as long as it is  reasonably useable.  (You could even do it
with ioctl(), if you provide a  nice command line tool so that I don't
need  to care about  the ioctl  parameters.)  I'm  not going  to touch
input.c,  because  I  don't  want   to  reboot  everytime  to  test  a
modification.  It's hard to compile input.c as a module.



-- 
Sau Dan LEE                     李守敦(Big5)                    ~{@nJX6X~}(HZ) 

E-mail: danlee@informatik.uni-freiburg.de
Home page: http://www.informatik.uni-freiburg.de/~danlee

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Dmitry Torokhov]

On Thursday 03 June 2004 12:54 am, Sau Dan Lee wrote:
> >>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:
> 
>     Dmitry> Let me start with saying that this is a very good patch
>     Dmitry> and that is exactly what I have in mind with regard to
>     Dmitry> serio port/device binding.  The only problem with the
>     Dmitry> patch is that it uses wrong foundation, namely procfs,
>     Dmitry> because:
>     ...
>  
>     Dmitry> So we have several options - if we adopt procfs based
>     Dmitry> solution now we will have to maintain it for very long
>     Dmitry> time, along with competing sysfs implementation. Dropping
>     Dmitry> one kernel parameter which will never be widely used is
>     Dmitry> much easier, IMO.
> 
> It's not just the matter of dropping one kernel parameter.  The procfs
> support, _already  implemented_, allows  one to fine-tune  the binding
> between serio  ports and  devices, which is  a new and  useful feature
> that  your kernel  parameter  doesn't provide.   

What I was trying to say is serio and input system will have sysfs support,
there is no question about that because sysfs _is_ the new driver model. So
by adopting procfs based solution we'll get ourselves 2 competing interfaces
for the same thing, just one will be very limited.

> 
> Can you unbind the keyboard port?   Can you bind/unbind any of the AUX
> ports  *dynamically*  without   reloading  the  i8042  module?   These

No, and I was not trying to. It is just a stop-gap measure to help end
users to get their PS/2 devices working until we have proper infrastructure
in place.

> functionalities are  already there in the serio-related  code.  Just a
> userland interface is missing.  My patch is to fill this gap.
> 
> 
> 
>     Dmitry> So I propose we all join our ranks and tame that sysfs
>     Dmitry> together ;) I had some patches that were converting
>     Dmitry> drivers to the sysfs adding them to serio bus, 
> 
> sysfs  looks  good  for  simple parameters:  integers,  strings.   For
> anything more  complicated (sets, graphs),  I don't see it  fit (yet).
> Unfortunately, the serio port<-->device relation is already a graph (1
> to n).
> 
> I'd like  to see  how you implement  the device<-->handler  binding in
> input.c using sysfs.

Sysfs provides all the same features as procfs (I mean you write read/write
methods and have them do whatever you please) but it also has benefits of
your stuff integrating with the rest of devices into a hierarchy.
 
-- 
Dmitry

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Sau Dan Lee]

>>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:

    Dmitry> So we have several options - if we adopt procfs based
    Dmitry> solution now we will have to maintain it for very long
    Dmitry> time, along with competing sysfs implementation. Dropping
    Dmitry> one kernel parameter which will never be widely used is
    Dmitry> much easier, IMO.

    >>  It's not just the matter of dropping one kernel parameter.
    >> The procfs support, _already implemented_, allows one to
    >> fine-tune the binding between serio ports and devices, which is
    >> a new and useful feature that your kernel parameter doesn't
    >> provide.

    Dmitry> What I was trying to say is serio and input system will
    Dmitry> have sysfs support, 

Then, why are you saying "dropping one kernel parameter"?



    >>  Can you unbind the keyboard port?  Can you bind/unbind any of
    >> the AUX ports *dynamically* without reloading the i8042 module?
    >> These

    Dmitry> No, and I was not trying to. It is just a stop-gap measure
    Dmitry> to help end users to get their PS/2 devices working until
    Dmitry> we have proper infrastructure in place.

I think  direct access to PS/2  devices must stay there  for the whole
2.6.x.  It's  unreasonable to assume  that all existing  _and working_
drivers will be kernelized.



    >>  sysfs looks good for simple parameters: integers, strings.
    >> For anything more complicated (sets, graphs), I don't see it
    >> fit (yet).  Unfortunately, the serio port<-->device relation is
    >> already a graph (1 to n).
    >> 
    >> I'd like to see how you implement the device<-->handler binding
    >> in input.c using sysfs.

    Dmitry> Sysfs provides all the same features as procfs (I mean you
    Dmitry> write read/write methods and have them do whatever you
    Dmitry> please) but it also has benefits of your stuff integrating
    Dmitry> with the rest of devices into a hierarchy.

It's  different.  Procfs is  more versatile.   I can  stuff in  my own
struct file_operations to do more than just read() and write().  I can
even stuff in my own struct inode_operations if I want more.

Another problem with sysfs is  the "social" discipline as mentioned in
Documentation/filesystems/sysfs.txt:

        Attributes should be ASCII text files, preferably with only
        one value per file. It is noted that it may not be efficient
        to contain only value per file, so it is socially acceptable
        to express an array of values of the same type.

        Mixing types, expressing multiple lines of data, and doing
        fancy formatting of data is heavily frowned upon. Doing these
        things may get you publically humiliated and your code
        rewritten without notice.

It is  common in procfs  to format the  output nicely, and  to display
screenfuls  of information.   This is  to  be frowned  upon in  sysfs.
Currently  implementations  of sysfs  interface  do  follow this  rule
nicely.

Unfortunately, the  connection between devices and  drivers (either in
the serio.c interface or in the  input.c interface) is a graph.  It is
more complicated than an array.  Yes, you can represent a graph with a
matrix or an  adjacency list, both representable as  arrays in one way
or another.  Nothing in a digital computer cannot be represented by an
array of  bits anyway!   But useability of  the interface must  not be
ignored.
 


-- 
Sau Dan LEE                     李守敦(Big5)                    ~{@nJX6X~}(HZ) 

E-mail: danlee@informatik.uni-freiburg.de
Home page: http://www.informatik.uni-freiburg.de/~danlee

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Dmitry Torokhov]

On Thursday 03 June 2004 01:45 am, Sau Dan Lee wrote:
> >>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:
> 
>     Dmitry> So we have several options - if we adopt procfs based
>     Dmitry> solution now we will have to maintain it for very long
>     Dmitry> time, along with competing sysfs implementation. Dropping
>     Dmitry> one kernel parameter which will never be widely used is
>     Dmitry> much easier, IMO.
> 
>     >>  It's not just the matter of dropping one kernel parameter.
>     >> The procfs support, _already implemented_, allows one to
>     >> fine-tune the binding between serio ports and devices, which is
>     >> a new and useful feature that your kernel parameter doesn't
>     >> provide.
> 
>     Dmitry> What I was trying to say is serio and input system will
>     Dmitry> have sysfs support, 
> 
> Then, why are you saying "dropping one kernel parameter"?

I am referring to possibly dropping i0842.raw once sysfs is implemeted
as then user will be able to rebind another driver to a port, like
your procfs patch does. 

> 
> 
> 
>     >>  Can you unbind the keyboard port?  Can you bind/unbind any of
>     >> the AUX ports *dynamically* without reloading the i8042 module?
>     >> These
> 
>     Dmitry> No, and I was not trying to. It is just a stop-gap measure
>     Dmitry> to help end users to get their PS/2 devices working until
>     Dmitry> we have proper infrastructure in place.
> 
> I think  direct access to PS/2  devices must stay there  for the whole
> 2.6.x.  It's  unreasonable to assume  that all existing  _and working_
> drivers will be kernelized.
> 
> 
> 
>     >>  sysfs looks good for simple parameters: integers, strings.
>     >> For anything more complicated (sets, graphs), I don't see it
>     >> fit (yet).  Unfortunately, the serio port<-->device relation is
>     >> already a graph (1 to n).
>     >> 
>     >> I'd like to see how you implement the device<-->handler binding
>     >> in input.c using sysfs.
> 
>     Dmitry> Sysfs provides all the same features as procfs (I mean you
>     Dmitry> write read/write methods and have them do whatever you
>     Dmitry> please) but it also has benefits of your stuff integrating
>     Dmitry> with the rest of devices into a hierarchy.
> 
> It's  different.  Procfs is  more versatile.   I can  stuff in  my own
> struct file_operations to do more than just read() and write().  I can
> even stuff in my own struct inode_operations if I want more.
> 
> Another problem with sysfs is  the "social" discipline as mentioned in
> Documentation/filesystems/sysfs.txt:
> 
>         Attributes should be ASCII text files, preferably with only
>         one value per file. It is noted that it may not be efficient
>         to contain only value per file, so it is socially acceptable
>         to express an array of values of the same type.
> 
>         Mixing types, expressing multiple lines of data, and doing
>         fancy formatting of data is heavily frowned upon. Doing these
>         things may get you publically humiliated and your code
>         rewritten without notice.
> 
> It is  common in procfs  to format the  output nicely, and  to display
> screenfuls  of information.   This is  to  be frowned  upon in  sysfs.
> Currently  implementations  of sysfs  interface  do  follow this  rule
> nicely.
> 
> Unfortunately, the  connection between devices and  drivers (either in
> the serio.c interface or in the  input.c interface) is a graph.  It is
> more complicated than an array.  Yes, you can represent a graph with a
> matrix or an  adjacency list, both representable as  arrays in one way
> or another.  Nothing in a digital computer cannot be represented by an
> array of  bits anyway!   But useability of  the interface must  not be
> ignored.
>

I am not sure where you see the problem - consider a PCI bus and all PCI
devices and all drivers tyhat currently present in kernel. They are using
the new driver model and sysfs and they come together quite nicely.

-- 
Dmitry

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Sau Dan Lee]

>>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:

    >> Unfortunately, the connection between devices and drivers
    >> (either in the serio.c interface or in the input.c interface)
    >> is a graph.  It is more complicated than an array.  Yes, you
    >> can represent a graph with a matrix or an adjacency list, both
    >> representable as arrays in one way or another.  Nothing in a
    >> digital computer cannot be represented by an array of bits
    >> anyway!  But useability of the interface must not be ignored.
    >> 

    Dmitry> I am not sure where you see the problem - consider a PCI
    Dmitry> bus and all PCI devices and all drivers tyhat currently
    Dmitry> present in kernel. They are using the new driver model and
    Dmitry> sysfs and they come together quite nicely.

# ls -l /sys/bus/pci/devices/0000:01:00.0

        -r--r--r--    1 root     root          class
        -rw-r--r--    1 root     root          config
        -rw-r--r--    1 root     root          detach_state
        -r--r--r--    1 root     root          device
        -r--r--r--    1 root     root          irq
        drwxr-xr-x    2 root     root          power
        -r--r--r--    1 root     root          resource
        -r--r--r--    1 root     root          subsystem_device
        -r--r--r--    1 root     root          subsystem_vendor
        -r--r--r--    1 root     root          vendor

# ls -l /sys/bus/pci/drivers/PIIX\ IDE

        lrwxrwxrwx    1 root     root          17:20 0000:00:07.1 ->
                      ../../../../devices/pci0000:00/0000:00:07.1
        --w-------    1 root     root          17:20 new_id


The  info  are  binary  (shown  in 0x????  notation).   Each  reflects
directly the binary value of the corresponding 'attribute'.

1) None of these are arrays.  But in the input system, each device can
   be  attached to _multiple_  handlers, and  each handler  can handle
   _multiple_ devices.  That's an n-to-n relation.

2) I can't  find out  how to  dynamically change the  driver of  a PCI
   device.

3) PCI device<-->handler is a  many-to-one relation.  The input system
   is many-to-many.

4) How to  display/parse a device<-->handler connection?   You want to
   show and accept a pointer value?

The sysfs interface looks very confusing to me.




-- 
Sau Dan LEE                     李守敦(Big5)                    ~{@nJX6X~}(HZ) 

E-mail: danlee@informatik.uni-freiburg.de
Home page: http://www.informatik.uni-freiburg.de/~danlee

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Dmitry Torokhov]

On Thursday 03 June 2004 02:22 am, Sau Dan Lee wrote:
> >>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:
> 
>     >> Unfortunately, the connection between devices and drivers
>     >> (either in the serio.c interface or in the input.c interface)
>     >> is a graph.  It is more complicated than an array.  Yes, you
>     >> can represent a graph with a matrix or an adjacency list, both
>     >> representable as arrays in one way or another.  Nothing in a
>     >> digital computer cannot be represented by an array of bits
>     >> anyway!  But useability of the interface must not be ignored.
>     >> 
> 
>     Dmitry> I am not sure where you see the problem - consider a PCI
>     Dmitry> bus and all PCI devices and all drivers tyhat currently
>     Dmitry> present in kernel. They are using the new driver model and
>     Dmitry> sysfs and they come together quite nicely.
> 
> # ls -l /sys/bus/pci/devices/0000:01:00.0
> 
>         -r--r--r--    1 root     root          class
>         -rw-r--r--    1 root     root          config
>         -rw-r--r--    1 root     root          detach_state
>         -r--r--r--    1 root     root          device
>         -r--r--r--    1 root     root          irq
>         drwxr-xr-x    2 root     root          power
>         -r--r--r--    1 root     root          resource
>         -r--r--r--    1 root     root          subsystem_device
>         -r--r--r--    1 root     root          subsystem_vendor
>         -r--r--r--    1 root     root          vendor
> 
> # ls -l /sys/bus/pci/drivers/PIIX\ IDE
> 
>         lrwxrwxrwx    1 root     root          17:20 0000:00:07.1 ->
>                       ../../../../devices/pci0000:00/0000:00:07.1
>         --w-------    1 root     root          17:20 new_id
> 
> 
> The  info  are  binary  (shown  in 0x????  notation).   Each  reflects
> directly the binary value of the corresponding 'attribute'.
> 
> 1) None of these are arrays.  But in the input system, each device can
>    be  attached to _multiple_  handlers, and  each handler  can handle
>    _multiple_ devices.  That's an n-to-n relation.

And when they join together they form a new entity, a new device. And that's
input system, not serio.  

> 
> 2) I can't  find out  how to  dynamically change the  driver of  a PCI
>    device.

No need really. PCI devices are easily identifiable.

> 
> 3) PCI device<-->handler is a  many-to-one relation.  The input system
>    is many-to-many.
> 
> 4) How to  display/parse a device<-->handler connection?   You want to
>    show and accept a pointer value?

Strings are perfectly valid:

[dtor@core dtor]$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver
acpi-cpufreq
[dtor@core dtor]$ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors
powersave userspace performance

It just depends on implementation.

> 
> The sysfs interface looks very confusing to me.
> 
Yes, I haven't wrapped my mind around it completely either. But we have
Greg K-H and others who will make sure we chose the right path ;)

-- 
Dmitry

Re: [PATCH] serio.c: dynamically control serio ports bindings via procfs (Was: [RFC/RFT] Raw access to serio ports)

[Sau Dan Lee]

>>>>> "Dmitry" == Dmitry Torokhov <dtor_core@ameritech.net> writes:

    >> The info are binary (shown in 0x????  notation).  Each reflects
    >> directly the binary value of the corresponding 'attribute'.
    >> 
    >> 1) None of these are arrays.  But in the input system, each
    >> device can be attached to _multiple_ handlers, and each handler
    >> can handle _multiple_ devices.  That's an n-to-n relation.

    Dmitry> And when they join together they form a new entity, a new
    Dmitry> device. And that's input system, not serio.

No.  The AT keyboard is still ONE device (cat /proc/bus/input/devices)
after loading 'evbug'.  It is  just connected to two handlers: kbd and
evbug.  No new  device is created just because  the AT keyboard device
is joined to the evbug handler.


    >>  2) I can't find out how to dynamically change the driver of a
    >> PCI device.

    Dmitry> No need really. PCI devices are easily identifiable.

Suppose there  are 2  "competing" drivers for  one device.  I  want to
switch the device to driver 2 at 23:00, and switch it back to driver 1
at 08:00.  How to do that?


    >>  3) PCI device<-->handler is a many-to-one relation.  The input
    >> system is many-to-many.
    >> 
    >> 4) How to display/parse a device<-->handler connection?  You
    >> want to show and accept a pointer value?

    Dmitry> Strings are perfectly valid:

Of course.  What can't be represented as bit-strings?


    Dmitry> $ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver
    Dmitry> acpi-cpufreq 
    Dmitry> $ cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors
    Dmitry> powersave userspace performance

    Dmitry> It just depends on implementation.

Then, go ahead an implement it.  I have no interest in sysfs.


-- 
Sau Dan LEE                     李守敦(Big5)                    ~{@nJX6X~}(HZ) 

E-mail: danlee@informatik.uni-freiburg.de
Home page: http://www.informatik.uni-freiburg.de/~danlee