There's a pair of "nub" devices on Pyra handheld PC (https://pyra-handheld.com/), and there's driver for nub, which is going to be reworked for upstreaming. While the device itself fits most to "joystick" category, the computer itself lacks touchpad and mouse buttons, and the existing driver is capable of switching between modes, in which it shows up like one of the following: - scrolling wheel, - mouse buttons set, - pointer updating its absolute position (graphic pad alike AFAIU), - pointer device behaving like actual joystick / pointing stick.
Currently modes switching happens through r/w file in /proc, which is of couse going to be changed.
I wonder if such mode switching mechanism is tolerable for inclusion to upstream kernel in this case. I'd like some advice how to rearrange the driver to save most of flexibility while matching upstream kernel conventions. I am especially interested in comments from subsystem maintainers.
Existing driver: http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=drivers/input/misc/as5...
On Tuesday, June 14, 2016 2:45:23 PM CEST Andrey Utkin wrote:
There's a pair of "nub" devices on Pyra handheld PC (https://pyra-handheld.com/), and there's driver for nub, which is going to be reworked for upstreaming. While the device itself fits most to "joystick" category, the computer itself lacks touchpad and mouse buttons, and the existing driver is capable of switching between modes, in which it shows up like one of the following:
- scrolling wheel,
- mouse buttons set,
- pointer updating its absolute position (graphic pad alike AFAIU),
- pointer device behaving like actual joystick / pointing stick.
Currently modes switching happens through r/w file in /proc, which is of couse going to be changed.
I wonder if such mode switching mechanism is tolerable for inclusion to upstream kernel in this case. I'd like some advice how to rearrange the driver to save most of flexibility while matching upstream kernel conventions. I am especially interested in comments from subsystem maintainers.
My best idea for this is that the device driver should just report one type of device, with the other modes being handled on top of that, by a generic kernel framework or in user space.
Code for registering the same inputs to all those frameworks doesn't seem to belong into a driver.
Arnd
Am 14.06.2016 um 19:02 schrieb Andrey Utkin andrey_utkin@fastmail.com:
On Tue, Jun 14, 2016 at 02:45:23PM +0300, Andrey Utkin wrote:
There's a pair of "nub" devices on Pyra handheld PC ...
Update: found drivers/input/joystick/as5011.c in mainline kernel, will look how it works with as5013 hardware.
Before you spend too much time on it, they have not much in common except the manufacturer and the as501x designation.
Register models (data sheet) are very different.
BR, Nikolaus
On Tue, Jun 14, 2016 at 07:09:50PM +0200, H. Nikolaus Schaller wrote:
Am 14.06.2016 um 19:02 schrieb Andrey Utkin andrey_utkin@fastmail.com: Update: found drivers/input/joystick/as5011.c in mainline kernel, will look how it works with as5013 hardware.
Before you spend too much time on it, they have not much in common except the manufacturer and the as501x designation.
Register models (data sheet) are very different.
Thanks for this hint, Nikolaus!
But still, apart from register models I guess as5011 driver is a good starting point for a fork. I am considering this because - as5011 driver has support for button push event, and as5013 doesn't; - as5011 driver conforms to mainline standards much better than existing out-of-tree as5013 driver. It also conforms to what reviewers expect to see: anyway as5013 won't pass review unless all tricky modes are dropped. This is according to Arnd reply; Pyra dev team member aTc also votes for userspace input management daemon.
I am considering forking instead of adding conditional logic to as5011 because I don't have a chance to test as5011 hardware, so I could break it unintentionally and never know about that. This is serious because I'm going to add support of all power modes and other parameters eventually.
There is one thing which both as5011 and as5013 existing drivers do: expose a header file in include/linux/input/, with some "struct blah_blah_platform_data" declaration. This header file is not included anywhere except for corresponding driver's .c file. So I think this header file is not needed, and all its contents should be inlined into single driver's source file. From asking on #kernelnewbies IRC chat, I gained some confidence that there's no subtleties regarding this.
I guess "struct drivername_platform_data" is something deprecated by now, as long as there are devicetree files and of_...() API, so I should port vsense_dt_init() logic from existing as5013 driver?
Also info about GPIO pin for button push interrupt should be added to devicetree files. Or please indicate that it's not going to have its GPIO pin for button interrupt - then driver should check button status at once with periodic position check.
Hi Andrey,
Am 16.06.2016 um 21:51 schrieb Andrey Utkin andrey_utkin@fastmail.com:
On Tue, Jun 14, 2016 at 07:09:50PM +0200, H. Nikolaus Schaller wrote:
Am 14.06.2016 um 19:02 schrieb Andrey Utkin andrey_utkin@fastmail.com: Update: found drivers/input/joystick/as5011.c in mainline kernel, will look how it works with as5013 hardware.
Before you spend too much time on it, they have not much in common except the manufacturer and the as501x designation.
Register models (data sheet) are very different.
Thanks for this hint, Nikolaus!
But still, apart from register models I guess as5011 driver is a good starting point for a fork. I am considering this because
- as5011 driver has support for button push event, and as5013 doesn't;
- as5011 driver conforms to mainline standards much better than existing out-of-tree as5013 driver. It also conforms to what reviewers expect to see: anyway as5013 won't pass review unless all tricky modes are dropped. This is according to Arnd reply; Pyra dev team member aTc also votes for userspace input management daemon.
Yes, that are strong arguments. We should also look for notaz' comments since he wrote the as5013 driver and might have some ideas what is important from user side to have reliable operation.
I am considering forking instead of adding conditional logic to as5011 because I don't have a chance to test as5011 hardware, so I could break it unintentionally and never know about that. This is serious because I'm going to add support of all power modes and other parameters eventually.
One thing which makes me wonder is that the as5011 driver appears to be used nowhere and did only have 2 patches since introduced in 2011:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/?id=refs...
I.e. it might be unused and untested for a long time while our as5013 driver works.
This could also mean the as5011 driver can potentially be removed from kernel and a new as5013 driver can start from scratch.
Anyways, you will be able to test a new driver on real hardware soon. So mix&merge is probably the best strategy.
There is one thing which both as5011 and as5013 existing drivers do: expose a header file in include/linux/input/, with some "struct blah_blah_platform_data" declaration. This header file is not included anywhere except for corresponding driver's .c file. So I think this header file is not needed, and all its contents should be inlined into single driver's source file. From asking on #kernelnewbies IRC chat, I gained some confidence that there's no subtleties regarding this.
I guess "struct drivername_platform_data" is something deprecated by now, as long as there are devicetree files and of_...() API, so I should port vsense_dt_init() logic from existing as5013 driver?
I recently got similar feedback when submitting another driver, that new drivers should be DT only and no longer need platform_data at all. So there is no need for an include file to expose pdata to board files.
Also info about GPIO pin for button push interrupt should be added to devicetree files. Or please indicate that it's not going to have its GPIO pin for button interrupt - then driver should check button status at once with periodic position check.
The gpio can also interrupt but we might also want to do some debouncing.
This is already solved by the gpio-button driver. I.e. we would more or less duplicate all code from there.
Logically it looks better to have the push button integrated in the driver and grouped with it in DT. From DT perspective it could suffice to make the gpio-button node a subnode of the as5013 node and keep separate drivers? Just an idea.
BR, Nikolaus