Touchscreen support for sharp sl-5500

This adds support for reading ADCs (etc), neccessary to operate touch screen on Sharp Zaurus sl-5500.

Quoted 2 lines · Pavel Machek

This adds support for reading ADCs (etc), neccessary to operate touch screen on Sharp Zaurus sl-5500.

I would like to know what the diffs are between my version (attached) and this version before they get applied.

The only reason my version has not been submitted is because it lives in the drivers/misc directory, and mainline kernel folk don't like drivers which clutter up that directory. In fact, I had been told that drivers/misc should remain completely empty - which makes this set of miscellaneous drivers homeless.

Quoted 2 lines · Pavel Machek

This adds support for reading ADCs (etc), neccessary to operate touch screen on Sharp Zaurus sl-5500.

I would like to know what the diffs are between my version (attached) and this version before they get applied.

The only reason my version has not been submitted is because it lives in the drivers/misc directory, and mainline kernel folk don't like drivers which clutter up that directory. In fact, I had been told that drivers/misc should remain completely empty - which makes this set of miscellaneous drivers homeless.

Quoted 2 lines

This adds support for reading ADCs (etc), neccessary to operate touch screen on Sharp Zaurus sl-5500.

I would like to know what the diffs are between my version (attached) and this version before they get applied.

The only reason my version has not been submitted is because it lives in the drivers/misc directory, and mainline kernel folk don't like drivers which clutter up that directory. In fact, I had been told that drivers/misc should remain completely empty - which makes this set of miscellaneous drivers homeless.

I have made quite a lot of cleanups to touchscreen part, and it seems to be acceptable by input people. I think it should go into drivers/input/touchscreen/collie_ts.c... Also it looks to me like mcp.h should go into asm/arch-sa1100, so that other drivers can use it...

If the problem is that you have a single piece of hardware you need to bind several drivers to - I guess you will have to create a new sub-device bus for that. Or just register sub-devices on the same bus the parent device is registered on - I am not sure what is best in this particular case - I am not familiar with the arch.

Can we change this to "while (!kthread_should_stop())" to make me completely happy?

Quoted 5 lines · Dmitry Torokhov

If the problem is that you have a single piece of hardware you need to bind several drivers to - I guess you will have to create a new sub-device bus for that. Or just register sub-devices on the same bus the parent device is registered on - I am not sure what is best in this particular case - I am not familiar with the arch.

That is exactly the problem - these kinds of devices do _not_ fit well into the device model. A struct device for every different possible sub-unit is completely overkill.

For instance, you may logically use one ADC and some GPIO lines on the device for X and something else for Y and they logically end up in different drivers.

The problem is that the parent doesn't actually know how many devices to create nor what to call them, and they're logically indistinguishable from each other so there's no logical naming system.

Quoted 11 lines · Russell King

Quoted 5 lines · Dmitry Torokhov

If the problem is that you have a single piece of hardware you need to bind several drivers to - I guess you will have to create a new sub-device bus for that. Or just register sub-devices on the same bus the parent device is registered on - I am not sure what is best in this particular case - I am not familiar with the arch.

That is exactly the problem - these kinds of devices do _not_ fit well into the device model. A struct device for every different possible sub-unit is completely overkill.

Quoted 9 lines

For instance, you may logically use one ADC and some GPIO lines on the device for X and something else for Y and they logically end up in different drivers.

The problem is that the parent doesn't actually know how many devices to create nor what to call them, and they're logically indistinguishable from each other so there's no logical naming system.

Then we should probably not try to force them into driver model. Have parent device register struct device and when sub-drivers register they could attach class devices (like input devices) directly to the "main" device thus hiding presence of sub-sections of the chip from sysfs completely. My point is that we should not be using class_interface here - its purpose is diferent.

Quoted 22 lines · Dmitry Torokhov

Quoted 11 lines · Russell King

Quoted 5 lines · Dmitry Torokhov

If the problem is that you have a single piece of hardware you need to bind several drivers to - I guess you will have to create a new sub-device bus for that. Or just register sub-devices on the same bus the parent device is registered on - I am not sure what is best in this particular case - I am not familiar with the arch.

That is exactly the problem - these kinds of devices do _not_ fit well into the device model. A struct device for every different possible sub-unit is completely overkill.

Quoted 9 lines

For instance, you may logically use one ADC and some GPIO lines on the device for X and something else for Y and they logically end up in different drivers.

The problem is that the parent doesn't actually know how many devices to create nor what to call them, and they're logically indistinguishable from each other so there's no logical naming system.

Quoted 6 lines

Then we should probably not try to force them into driver model. Have parent device register struct device and when sub-drivers register they could attach class devices (like input devices) directly to the "main" device thus hiding presence of sub-sections of the chip from sysfs completely. My point is that we should not be using class_interface here - its purpose is diferent.

If you look at _my_ version, you'll notice that it doesn't use the class interface stuff. A previous version of it did, and this seems to be what the collie stuff is based upon.

What I suggest is that the collie folk need to update their driver to my version so that we don't have two different forks of the same driver in existance. Then we can start discussing whether things should be using kthreads or not.

Quoted 3 lines · Russell King

If you look at _my_ version, you'll notice that it doesn't use the class interface stuff. A previous version of it did, and this seems to be what the collie stuff is based upon.

I was only commenting on something that was posted on LKML for inclusion into input subtree that I am interested in. I don't track ARM development that closely. Where can we see your version, please?

Quoted 4 lines

What I suggest is that the collie folk need to update their driver to my version so that we don't have two different forks of the same driver in existance. Then we can start discussing whether things should be using kthreads or not.

Do you have any reason why, generally speaking, threads should not be used? They seem to clean up code in drivers quite a bit.

Quoted 11 lines

Quoted 4 lines

The problem is that the parent doesn't actually know how many devices to create nor what to call them, and they're logically indistinguishable from each other so there's no logical naming system.

Then we should probably not try to force them into driver model. Have parent device register struct device and when sub-drivers register they could attach class devices (like input devices) directly to the "main" device thus hiding presence of sub-sections of the chip from sysfs completely. My point is that we should not be using class_interface here - its purpose is diferent.

If you look at _my_ version, you'll notice that it doesn't use the class interface stuff. A previous version of it did, and this seems to be what the collie stuff is based upon.

What I suggest is that the collie folk need to update their driver to my version so that we don't have two different forks of the same

Quoted 2 lines

Can we change this to "while (!kthread_should_stop())" to make me completely happy?

I still ask, and I'll keep repeating this. What is the difference between this and the reference implementation which is known to work on other hardware.

Let's not go all out on one implementation for one set of hardware, but try to work out what we need to do to the generic reference implementation to make it work on this hardware.

Quoted 4 lines

I have made quite a lot of cleanups to touchscreen part, and it seems to be acceptable by input people. I think it should go into drivers/input/touchscreen/collie_ts.c... Also it looks to me like mcp.h should go into asm/arch-sa1100, so that other drivers can use it...

I have couple of nitpicks (below) and one bigger concern - I am surprised that a driver for a physical device is implemented as an interface to a class device. This precludes implementing any kind of power management in the driver and pushes it into the parent and is generally speaking is a wrong thing to do (IMHO).

Review

Quoted 7 lines

(quoted diff) +7 -0

+static int ucb1x00_thread(void *_ts)
+{
+       struct ucb1x00_ts *ts = _ts;
+       struct task_struct *tsk = current;
+       int valid;
+
+       ts->rtask = tsk;

Just move that assignment into ucb1x00_input_open and kill all this "current" stuff.

Review

Quoted 9 lines

(quoted diff) +9 -0

+       /*
+        * We run as a real-time thread.  However, thus far
+        * this doesn't seem to be necessary.
+        */
+       tsk->policy = SCHED_FIFO;
+       tsk->rt_priority = 1;
+
+       valid = 0;
+       for (;;) {

Can we change this to "while (!kthread_should_stop())" to make me completely happy?

Hi!

Quoted 5 lines

Quoted 2 lines

This adds support for reading ADCs (etc), neccessary to operate touch screen on Sharp Zaurus sl-5500.

I would like to know what the diffs are between my version (attached) and this version before they get applied.

Hmm, diff looks quite big (attached), and I got it from lenz for 99% part.

I have made quite a lot of cleanups to touchscreen part, and it seems to be acceptable by input people. I think it should go into drivers/input/touchscreen/collie_ts.c...

Also it looks to me like mcp.h should go into asm/arch-sa1100, so that other drivers can use it...

Quoted 5 lines

The only reason my version has not been submitted is because it lives in the drivers/misc directory, and mainline kernel folk don't like drivers which clutter up that directory. In fact, I had been told that drivers/misc should remain completely empty - which makes this set of miscellaneous drivers homeless.

Could they simply live in arch/arm/mach-sa1100? Or is arch/arm/soc better place?

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

This adds support for reading ADCs (etc), neccessary to operate touch screen on Sharp Zaurus sl-5500.

I would like to know what the diffs are between my version (attached) and this version before they get applied.

Hmm, diff looks quite big (attached), and I got it from lenz for 99% part.

It looks like John's version is actually based on a previous revision of this driver. 8/

Quoted 3 lines

I have made quite a lot of cleanups to touchscreen part, and it seems to be acceptable by input people. I think it should go into drivers/input/touchscreen/collie_ts.c...

Err, why should my assabet touchscreen be called "collie_ts" ? collie is just a platform which happens to use it - it's got no relevance to the driver naming at all.

Quoted 2 lines

Also it looks to me like mcp.h should go into asm/arch-sa1100, so that other drivers can use it...

That doesn't make sense when you have other non-SA1100 devices using mcp-core.c. Whether that happens or not I've no idea - I can't see what everyone's using out there (just like I've absolutely zero idea what collie folk are doing or not doing.)

Quoted 8 lines

Quoted 5 lines

The only reason my version has not been submitted is because it lives in the drivers/misc directory, and mainline kernel folk don't like drivers which clutter up that directory. In fact, I had been told that drivers/misc should remain completely empty - which makes this set of miscellaneous drivers homeless.

Could they simply live in arch/arm/mach-sa1100? Or is arch/arm/soc better place?

arch/arm/soc? That means that (a) we end up with another directory to accumulate crap, (b) it's not a SoC so doesn't belong in a directory named as such, (c) it means that the MCP and UCB drivers get their individual files scattered throughout the kernel tree, one in this directory, one in that directory, one in another random directory. That's far from ideal.

Anyway, summarising this, the results are that what we have here is a complete and utter mess. ;(

So, if the collie folk would like to clean their changes up and send them to me as the driver author, I'll see about integrating them into my version and we'll take it from there.

Quoted 7 lines

Quoted 2 lines

Also it looks to me like mcp.h should go into asm/arch-sa1100, so that other drivers can use it...

That doesn't make sense when you have other non-SA1100 devices using mcp-core.c. Whether that happens or not I've no idea - I can't see what everyone's using out there (just like I've absolutely zero idea what collie folk are doing or not doing.)

set_telecom_divisor relies on CONFIG_SA1100 being set (otherwise it breaks compilation, because struct members will not be available; at least in this version), so I doubt it has many non-SA1100 users...

Quoted 3 lines

So, if the collie folk would like to clean their changes up and send them to me as the driver author, I'll see about integrating them into my version and we'll take it from there.

Okay, will do. [Is there chance to pull your tree using git? It would help a bit...]

Quoted 6 lines

Quoted 3 lines

So, if the collie folk would like to clean their changes up and send them to me as the driver author, I'll see about integrating them into my version and we'll take it from there.

Okay, will do. [Is there chance to pull your tree using git? It would help a bit...]

However, if the UCB stuff is going to get worked on, I don't mind setting up, maintaining and publishing a git tree for that that, provided it then vanishes once merged into mainline. That falls within the "very limited purposes" clause above.