* v4.1 to v4.7: regression in tsc2005 driver @ 2016-07-17 17:52 Pavel Machek 2016-07-17 18:24 ` Michael Welling 0 siblings, 1 reply; 44+ messages in thread From: Pavel Machek @ 2016-07-17 17:52 UTC (permalink / raw) To: kernel list, mwelling, dmitry.torokhov, linux-input Cc: pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge Hi! tsc2005 driver changed input device name, from drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 touchscreen"; to "TSC200X touchscreen". Unfortunately, X seems to propagate that name to userspace, where it is needed to be able to do xinput --set-prop --type=int ... with the right arguments to calibrate touchscreen. (Touchscreen is unusable without calibration). What to do with that? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-17 17:52 v4.1 to v4.7: regression in tsc2005 driver Pavel Machek @ 2016-07-17 18:24 ` Michael Welling 2016-07-17 18:42 ` Pavel Machek 0 siblings, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-17 18:24 UTC (permalink / raw) To: Pavel Machek Cc: kernel list, dmitry.torokhov, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > Hi! > > tsc2005 driver changed input device name, from > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > touchscreen"; > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > name to userspace, where it is needed to be able to do > > xinput --set-prop --type=int ... > > with the right arguments to calibrate touchscreen. (Touchscreen is > unusable without calibration). > > What to do with that? The input_dev name could be passed to the common probe function. http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 It could also be inferred from the bus_type or dev_name. http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc200x-core.c#L547 Pick a method and I will provide a patch. > > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-17 18:24 ` Michael Welling @ 2016-07-17 18:42 ` Pavel Machek 2016-07-17 18:51 ` Michael Welling 0 siblings, 1 reply; 44+ messages in thread From: Pavel Machek @ 2016-07-17 18:42 UTC (permalink / raw) To: Michael Welling Cc: kernel list, dmitry.torokhov, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Sun 2016-07-17 13:24:45, Michael Welling wrote: > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > Hi! > > > > tsc2005 driver changed input device name, from > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > touchscreen"; > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > name to userspace, where it is needed to be able to do > > > > xinput --set-prop --type=int ... > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > unusable without calibration). > > > > What to do with that? > > The input_dev name could be passed to the common probe function. > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 That would be preffered, I guess. How many stable releases are affected? Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-17 18:42 ` Pavel Machek @ 2016-07-17 18:51 ` Michael Welling 2016-07-17 20:03 ` Pavel Machek 0 siblings, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-17 18:51 UTC (permalink / raw) To: Pavel Machek Cc: kernel list, dmitry.torokhov, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > Hi! > > > > > > tsc2005 driver changed input device name, from > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > touchscreen"; > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > name to userspace, where it is needed to be able to do > > > > > > xinput --set-prop --type=int ... > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > unusable without calibration). > > > > > > What to do with that? > > > > The input_dev name could be passed to the common probe function. > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > That would be preffered, I guess. > > How many stable releases are affected? Well this patch is 9 months old now. Lets see. It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > Thanks, > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-17 18:51 ` Michael Welling @ 2016-07-17 20:03 ` Pavel Machek 2016-07-17 22:56 ` Michael Welling 0 siblings, 1 reply; 44+ messages in thread From: Pavel Machek @ 2016-07-17 20:03 UTC (permalink / raw) To: Michael Welling Cc: kernel list, dmitry.torokhov, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Sun 2016-07-17 13:51:34, Michael Welling wrote: > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > Hi! > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > touchscreen"; > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > name to userspace, where it is needed to be able to do > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > unusable without calibration). > > > > > > > > What to do with that? > > > > > > The input_dev name could be passed to the common probe function. > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > That would be preffered, I guess. > > > > How many stable releases are affected? > > Well this patch is 9 months old now. Lets see. > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. Ok, thanks for the information. I believe changing it back to "TSC2005" version makes sense (and then fixing it in stable). Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-17 20:03 ` Pavel Machek @ 2016-07-17 22:56 ` Michael Welling 2016-07-19 23:51 ` Dmitry Torokhov 0 siblings, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-17 22:56 UTC (permalink / raw) To: Pavel Machek Cc: kernel list, dmitry.torokhov, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > Hi! > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > touchscreen"; > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > name to userspace, where it is needed to be able to do > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > unusable without calibration). > > > > > > > > > > What to do with that? > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > That would be preffered, I guess. > > > > > > How many stable releases are affected? > > > > Well this patch is 9 months old now. Lets see. > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > Ok, thanks for the information. I believe changing it back to > "TSC2005" version makes sense (and then fixing it in stable). Lets see if the maintainer has any input before I submit a patch. > > Best regards, > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-17 22:56 ` Michael Welling @ 2016-07-19 23:51 ` Dmitry Torokhov 2016-07-20 0:39 ` Michael Welling 2016-07-20 6:25 ` Pavel Machek 0 siblings, 2 replies; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-19 23:51 UTC (permalink / raw) To: Michael Welling Cc: Pavel Machek, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > > Hi! > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > touchscreen"; > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > name to userspace, where it is needed to be able to do Technically X _is_ userspace. > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > unusable without calibration). > > > > > > > > > > > > What to do with that? Hmm, I do not think we ever committed for the device names to be stable. You are supposed to locate touchscreen device based on its properties and you might need some heuristic if you encounter a system with more than one such touchscreen. > > > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > > > That would be preffered, I guess. > > > > > > > > How many stable releases are affected? > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > Ok, thanks for the information. I believe changing it back to > > "TSC2005" version makes sense (and then fixing it in stable). Do we know how many users are affected? > > Lets see if the maintainer has any input before I submit a patch. I guess we could also pass the input_id structure to tsc200x_probe, fill the proper product ID (2004 or 2005) and derive the name form it. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-19 23:51 ` Dmitry Torokhov @ 2016-07-20 0:39 ` Michael Welling 2016-07-20 0:53 ` Dmitry Torokhov 2016-07-20 1:26 ` Aaro Koskinen 2016-07-20 6:25 ` Pavel Machek 1 sibling, 2 replies; 44+ messages in thread From: Michael Welling @ 2016-07-20 0:39 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > > > Hi! > > > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > touchscreen"; > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > name to userspace, where it is needed to be able to do > > Technically X _is_ userspace. > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > unusable without calibration). > > > > > > > > > > > > > > What to do with that? > > Hmm, I do not think we ever committed for the device names to be stable. > You are supposed to locate touchscreen device based on its properties > and you might need some heuristic if you encounter a system with more > than one such touchscreen. > > > > > > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > > > > > That would be preffered, I guess. > > > > > > > > > > How many stable releases are affected? > > > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > > > Ok, thanks for the information. I believe changing it back to > > > "TSC2005" version makes sense (and then fixing it in stable). > > Do we know how many users are affected? Anyone with an old N900 and the smarts to update the kernel. > > > > > Lets see if the maintainer has any input before I submit a patch. > > I guess we could also pass the input_id structure to tsc200x_probe, fill > the proper product ID (2004 or 2005) and derive the name form it. > Okay do you really think this should be considered a regression? Should I go ahead with the patch? > Thanks. > > -- > Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 0:39 ` Michael Welling @ 2016-07-20 0:53 ` Dmitry Torokhov 2016-07-20 1:34 ` Michael Welling 2016-07-20 16:33 ` v4.1 to v4.7: regression in tsc2005 driver Pali Rohár 2016-07-20 1:26 ` Aaro Koskinen 1 sibling, 2 replies; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 0:53 UTC (permalink / raw) To: Michael Welling Cc: Pavel Machek, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > > > > Hi! > > > > > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > name to userspace, where it is needed to be able to do > > > > Technically X _is_ userspace. > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > What to do with that? > > > > Hmm, I do not think we ever committed for the device names to be stable. > > You are supposed to locate touchscreen device based on its properties > > and you might need some heuristic if you encounter a system with more > > than one such touchscreen. > > > > > > > > > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > > > > > > > That would be preffered, I guess. > > > > > > > > > > > > How many stable releases are affected? > > > > > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > > > > > Ok, thanks for the information. I believe changing it back to > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > Do we know how many users are affected? > > Anyone with an old N900 and the smarts to update the kernel. Soo... only Pavel? ;) > > > > > > > > > Lets see if the maintainer has any input before I submit a patch. > > > > I guess we could also pass the input_id structure to tsc200x_probe, fill > > the proper product ID (2004 or 2005) and derive the name form it. > > > > Okay do you really think this should be considered a regression? > > Should I go ahead with the patch? To "fix" it is a small change so we might as well do that, but I probably leave it off stable for now. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 0:53 ` Dmitry Torokhov @ 2016-07-20 1:34 ` Michael Welling 2016-07-20 1:44 ` Dmitry Torokhov 2016-07-20 16:33 ` v4.1 to v4.7: regression in tsc2005 driver Pali Rohár 1 sibling, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-20 1:34 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Tue, Jul 19, 2016 at 05:53:07PM -0700, Dmitry Torokhov wrote: > On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > > > > > Hi! > > > > > > > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > Technically X _is_ userspace. > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > You are supposed to locate touchscreen device based on its properties > > > and you might need some heuristic if you encounter a system with more > > > than one such touchscreen. > > > > > > > > > > > > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > > > > > > > > > That would be preffered, I guess. > > > > > > > > > > > > > > How many stable releases are affected? > > > > > > > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > > > > > > > Ok, thanks for the information. I believe changing it back to > > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > > > Do we know how many users are affected? > > > > Anyone with an old N900 and the smarts to update the kernel. > > Soo... only Pavel? ;) > > > > > > > > > > > > > > Lets see if the maintainer has any input before I submit a patch. > > > > > > I guess we could also pass the input_id structure to tsc200x_probe, fill > > > the proper product ID (2004 or 2005) and derive the name form it. > > > If we passed the input_dev with the filled product idea it would have to be allocated in the calling probe functions instead of the common probe. This will lead to more duplicated code which we jumped through so many hoops to avoid. > > > > Okay do you really think this should be considered a regression? > > > > Should I go ahead with the patch? > > To "fix" it is a small change so we might as well do that, but I > probably leave it off stable for now. > > Thanks. > > -- > Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 1:34 ` Michael Welling @ 2016-07-20 1:44 ` Dmitry Torokhov 2016-07-20 2:09 ` Michael Welling 2016-07-20 3:49 ` [PATCH] Input: tsc200x - Report proper input_dev name Michael Welling 0 siblings, 2 replies; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 1:44 UTC (permalink / raw) To: Michael Welling Cc: Pavel Machek, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Tue, Jul 19, 2016 at 08:34:57PM -0500, Michael Welling wrote: > On Tue, Jul 19, 2016 at 05:53:07PM -0700, Dmitry Torokhov wrote: > > On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > > > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > > > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > > > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > > > > > > Hi! > > > > > > > > > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > > > Technically X _is_ userspace. > > > > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > > You are supposed to locate touchscreen device based on its properties > > > > and you might need some heuristic if you encounter a system with more > > > > than one such touchscreen. > > > > > > > > > > > > > > > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > > > > > > > > > > > That would be preffered, I guess. > > > > > > > > > > > > > > > > How many stable releases are affected? > > > > > > > > > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > > > > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > > > > > > > > > Ok, thanks for the information. I believe changing it back to > > > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > > > > > Do we know how many users are affected? > > > > > > Anyone with an old N900 and the smarts to update the kernel. > > > > Soo... only Pavel? ;) > > > > > > > > > > > > > > > > > > > Lets see if the maintainer has any input before I submit a patch. > > > > > > > > I guess we could also pass the input_id structure to tsc200x_probe, fill > > > > the proper product ID (2004 or 2005) and derive the name form it. > > > > > > If we passed the input_dev with the filled product idea it would have to be > allocated in the calling probe functions instead of the common probe. > > This will lead to more duplicated code which we jumped through so many hoops > to avoid. Not input_dev, input_id, like : static const struct input_id tsc2005_input_id = { .bustype = BUS_SPI, .product = 2005, }; static int tsc2005_probe(struct spi_device *spi) { ... return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, devm_regmap_init_spi(spi, &tsc200x_regmap_config), tsc2005_cmd); }; and in tsc200x_probe() you'd do: input_dev->name = dev_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", tsc_id->product); if (!input_dev->name) return -ENOMEM; ... input_dev->id = *tsc_id; Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 1:44 ` Dmitry Torokhov @ 2016-07-20 2:09 ` Michael Welling 2016-07-20 3:49 ` [PATCH] Input: tsc200x - Report proper input_dev name Michael Welling 1 sibling, 0 replies; 44+ messages in thread From: Michael Welling @ 2016-07-20 2:09 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Tue, Jul 19, 2016 at 06:44:24PM -0700, Dmitry Torokhov wrote: > On Tue, Jul 19, 2016 at 08:34:57PM -0500, Michael Welling wrote: > > On Tue, Jul 19, 2016 at 05:53:07PM -0700, Dmitry Torokhov wrote: > > > On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > > > > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > > > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > > > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > > > > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek wrote: > > > > > > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek wrote: > > > > > > > > > > > Hi! > > > > > > > > > > > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > > > > > Technically X _is_ userspace. > > > > > > > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > > > You are supposed to locate touchscreen device based on its properties > > > > > and you might need some heuristic if you encounter a system with more > > > > > than one such touchscreen. > > > > > > > > > > > > > > > > > > > > > > > > > The input_dev name could be passed to the common probe function. > > > > > > > > > > > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touchscreen/tsc2005.c#L65 > > > > > > > > > > > > > > > > > > That would be preffered, I guess. > > > > > > > > > > > > > > > > > > How many stable releases are affected? > > > > > > > > > > > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > > > > > > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > > > > > > > > > > > Ok, thanks for the information. I believe changing it back to > > > > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > > > > > > > Do we know how many users are affected? > > > > > > > > Anyone with an old N900 and the smarts to update the kernel. > > > > > > Soo... only Pavel? ;) > > > > > > > > > > > > > > > > > > > > > > > > Lets see if the maintainer has any input before I submit a patch. > > > > > > > > > > I guess we could also pass the input_id structure to tsc200x_probe, fill > > > > > the proper product ID (2004 or 2005) and derive the name form it. > > > > > > > > > If we passed the input_dev with the filled product idea it would have to be > > allocated in the calling probe functions instead of the common probe. > > > > This will lead to more duplicated code which we jumped through so many hoops > > to avoid. > > Not input_dev, input_id, like : > > static const struct input_id tsc2005_input_id = { > .bustype = BUS_SPI, > .product = 2005, > }; > > > > static int tsc2005_probe(struct spi_device *spi) > { > ... > return tsc200x_probe(&spi->dev, spi->irq, > &tsc2005_input_id, > devm_regmap_init_spi(spi, &tsc200x_regmap_config), > tsc2005_cmd); > }; > > and in tsc200x_probe() you'd do: > > input_dev->name = dev_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", > tsc_id->product); > if (!input_dev->name) > return -ENOMEM; > ... > input_dev->id = *tsc_id; > Okay. Easy enough. > Thanks. > > -- > Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 1:44 ` Dmitry Torokhov 2016-07-20 2:09 ` Michael Welling @ 2016-07-20 3:49 ` Michael Welling 2016-07-20 6:31 ` Pavel Machek 1 sibling, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-20 3:49 UTC (permalink / raw) To: kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Pavel Machek, Dmitry Torokhov, Aaro Koskinen Cc: Michael Welling Passes input_id struct to the the common probe function for the tsc200x drivers instead of just the bustype. This allows for the use of the product variable to set the input_dev->name variable according to the type of touchscreen used. Signed-off-by: Michael Welling <mwelling@ieee.org> --- drivers/input/touchscreen/tsc2004.c | 7 ++++++- drivers/input/touchscreen/tsc2005.c | 7 ++++++- drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- drivers/input/touchscreen/tsc200x-core.h | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c index 7295c19..6fe55d5 100644 --- a/drivers/input/touchscreen/tsc2004.c +++ b/drivers/input/touchscreen/tsc2004.c @@ -22,6 +22,11 @@ #include <linux/regmap.h> #include "tsc200x-core.h" +static const struct input_id tsc2004_input_id = { + .bustype = BUS_I2C, + .product = 2004, +}; + static int tsc2004_cmd(struct device *dev, u8 cmd) { u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, + return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id, devm_regmap_init_i2c(i2c, &tsc200x_regmap_config), tsc2004_cmd); } diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c index b9f593d..f2c5f0e 100644 --- a/drivers/input/touchscreen/tsc2005.c +++ b/drivers/input/touchscreen/tsc2005.c @@ -24,6 +24,11 @@ #include <linux/regmap.h> #include "tsc200x-core.h" +static const struct input_id tsc2005_input_id = { + .bustype = BUS_SPI, + .product = 2005, +}; + static int tsc2005_cmd(struct device *dev, u8 cmd) { u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi) if (error) return error; - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, + return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, devm_regmap_init_spi(spi, &tsc200x_regmap_config), tsc2005_cmd); } diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c index 26e81d1b..5e625c4 100644 --- a/drivers/input/touchscreen/tsc200x-core.c +++ b/drivers/input/touchscreen/tsc200x-core.c @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input) mutex_unlock(&ts->mutex); } -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, struct regmap *regmap, int (*tsc200x_cmd)(struct device *dev, u8 cmd)) { @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype, snprintf(ts->phys, sizeof(ts->phys), "%s/input-ts", dev_name(dev)); - input_dev->name = "TSC200X touchscreen"; + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", + tsc_id->product); input_dev->phys = ts->phys; - input_dev->id.bustype = bustype; + input_dev->id = *tsc_id; input_dev->dev.parent = dev; input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY); input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH); diff --git a/drivers/input/touchscreen/tsc200x-core.h b/drivers/input/touchscreen/tsc200x-core.h index 7a482d1..49a63a3 100644 --- a/drivers/input/touchscreen/tsc200x-core.h +++ b/drivers/input/touchscreen/tsc200x-core.h @@ -70,7 +70,7 @@ extern const struct regmap_config tsc200x_regmap_config; extern const struct dev_pm_ops tsc200x_pm_ops; -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, struct regmap *regmap, int (*tsc200x_cmd)(struct device *dev, u8 cmd)); int tsc200x_remove(struct device *dev); -- 2.1.4 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 3:49 ` [PATCH] Input: tsc200x - Report proper input_dev name Michael Welling @ 2016-07-20 6:31 ` Pavel Machek 2016-07-20 6:50 ` Michael Welling 0 siblings, 1 reply; 44+ messages in thread From: Pavel Machek @ 2016-07-20 6:31 UTC (permalink / raw) To: Michael Welling Cc: kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Dmitry Torokhov, Aaro Koskinen Hi! > Passes input_id struct to the the common probe function for the tsc200x drivers > instead of just the bustype. > > This allows for the use of the product variable to set the input_dev->name > variable according to the type of touchscreen used. > > Signed-off-by: Michael Welling <mwelling@ieee.org> > --- > drivers/input/touchscreen/tsc2004.c | 7 ++++++- > drivers/input/touchscreen/tsc2005.c | 7 ++++++- > drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- > drivers/input/touchscreen/tsc200x-core.h | 2 +- > 4 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c > index 7295c19..6fe55d5 100644 > --- a/drivers/input/touchscreen/tsc2004.c > +++ b/drivers/input/touchscreen/tsc2004.c > @@ -22,6 +22,11 @@ > #include <linux/regmap.h> > #include "tsc200x-core.h" > > +static const struct input_id tsc2004_input_id = { > + .bustype = BUS_I2C, > + .product = 2004, > +}; > + > static int tsc2004_cmd(struct device *dev, u8 cmd) > { > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > > { > - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, > + return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id, > devm_regmap_init_i2c(i2c, &tsc200x_regmap_config), > tsc2004_cmd); > } > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c > index b9f593d..f2c5f0e 100644 > --- a/drivers/input/touchscreen/tsc2005.c > +++ b/drivers/input/touchscreen/tsc2005.c > @@ -24,6 +24,11 @@ > #include <linux/regmap.h> > #include "tsc200x-core.h" > > +static const struct input_id tsc2005_input_id = { > + .bustype = BUS_SPI, > + .product = 2005, > +}; > + > static int tsc2005_cmd(struct device *dev, u8 cmd) > { > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi) > if (error) > return error; > > - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, > + return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, > devm_regmap_init_spi(spi, &tsc200x_regmap_config), > tsc2005_cmd); > } > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c > index 26e81d1b..5e625c4 100644 > --- a/drivers/input/touchscreen/tsc200x-core.c > +++ b/drivers/input/touchscreen/tsc200x-core.c > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input) > mutex_unlock(&ts->mutex); > } > > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, > struct regmap *regmap, > int (*tsc200x_cmd)(struct device *dev, u8 cmd)) > { > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > snprintf(ts->phys, sizeof(ts->phys), > "%s/input-ts", dev_name(dev)); > > - input_dev->name = "TSC200X touchscreen"; > + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", > + tsc_id->product); What about: if (tsc_id->product == 2005) input_dev->name = "TSC2005 touchscreen"; else input_dev->name = "TSC200X touchscreen"; We do want to use 'TSC2005' name for TSC2005, because compatibility, but you should keep TSC200X.... because compatibility. You don't want to break people's setups by going from TSC200X to TSC2004. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 6:31 ` Pavel Machek @ 2016-07-20 6:50 ` Michael Welling 2016-07-20 6:54 ` Pavel Machek 0 siblings, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-20 6:50 UTC (permalink / raw) To: Pavel Machek Cc: kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Dmitry Torokhov, Aaro Koskinen On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote: > Hi! > > > Passes input_id struct to the the common probe function for the tsc200x drivers > > instead of just the bustype. > > > > This allows for the use of the product variable to set the input_dev->name > > variable according to the type of touchscreen used. > > > > Signed-off-by: Michael Welling <mwelling@ieee.org> > > --- > > drivers/input/touchscreen/tsc2004.c | 7 ++++++- > > drivers/input/touchscreen/tsc2005.c | 7 ++++++- > > drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- > > drivers/input/touchscreen/tsc200x-core.h | 2 +- > > 4 files changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c > > index 7295c19..6fe55d5 100644 > > --- a/drivers/input/touchscreen/tsc2004.c > > +++ b/drivers/input/touchscreen/tsc2004.c > > @@ -22,6 +22,11 @@ > > #include <linux/regmap.h> > > #include "tsc200x-core.h" > > > > +static const struct input_id tsc2004_input_id = { > > + .bustype = BUS_I2C, > > + .product = 2004, > > +}; > > + > > static int tsc2004_cmd(struct device *dev, u8 cmd) > > { > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c, > > const struct i2c_device_id *id) > > > > { > > - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, > > + return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id, > > devm_regmap_init_i2c(i2c, &tsc200x_regmap_config), > > tsc2004_cmd); > > } > > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c > > index b9f593d..f2c5f0e 100644 > > --- a/drivers/input/touchscreen/tsc2005.c > > +++ b/drivers/input/touchscreen/tsc2005.c > > @@ -24,6 +24,11 @@ > > #include <linux/regmap.h> > > #include "tsc200x-core.h" > > > > +static const struct input_id tsc2005_input_id = { > > + .bustype = BUS_SPI, > > + .product = 2005, > > +}; > > + > > static int tsc2005_cmd(struct device *dev, u8 cmd) > > { > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi) > > if (error) > > return error; > > > > - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, > > + return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, > > devm_regmap_init_spi(spi, &tsc200x_regmap_config), > > tsc2005_cmd); > > } > > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c > > index 26e81d1b..5e625c4 100644 > > --- a/drivers/input/touchscreen/tsc200x-core.c > > +++ b/drivers/input/touchscreen/tsc200x-core.c > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input) > > mutex_unlock(&ts->mutex); > > } > > > > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, > > struct regmap *regmap, > > int (*tsc200x_cmd)(struct device *dev, u8 cmd)) > > { > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > > snprintf(ts->phys, sizeof(ts->phys), > > "%s/input-ts", dev_name(dev)); > > > > - input_dev->name = "TSC200X touchscreen"; > > + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", > > + tsc_id->product); > > What about: > > if (tsc_id->product == 2005) > input_dev->name = "TSC2005 touchscreen"; > else > input_dev->name = "TSC200X touchscreen"; > > We do want to use 'TSC2005' name for TSC2005, because compatibility, > but you should keep TSC200X.... because compatibility. You don't want > to break people's setups by going from TSC200X to TSC2004. By same logic we shouldn't change from TSC200X back to TSC2005 because of people's possibly new setup in the last 9 months. > > Best regards, > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 6:50 ` Michael Welling @ 2016-07-20 6:54 ` Pavel Machek 2016-07-20 7:06 ` Michael Welling 2016-07-20 16:45 ` Dmitry Torokhov 0 siblings, 2 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-20 6:54 UTC (permalink / raw) To: Michael Welling Cc: kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Dmitry Torokhov, Aaro Koskinen On Wed 2016-07-20 01:50:24, Michael Welling wrote: > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote: > > Hi! > > > > > Passes input_id struct to the the common probe function for the tsc200x drivers > > > instead of just the bustype. > > > > > > This allows for the use of the product variable to set the input_dev->name > > > variable according to the type of touchscreen used. > > > > > > Signed-off-by: Michael Welling <mwelling@ieee.org> > > > --- > > > drivers/input/touchscreen/tsc2004.c | 7 ++++++- > > > drivers/input/touchscreen/tsc2005.c | 7 ++++++- > > > drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- > > > drivers/input/touchscreen/tsc200x-core.h | 2 +- > > > 4 files changed, 17 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c > > > index 7295c19..6fe55d5 100644 > > > --- a/drivers/input/touchscreen/tsc2004.c > > > +++ b/drivers/input/touchscreen/tsc2004.c > > > @@ -22,6 +22,11 @@ > > > #include <linux/regmap.h> > > > #include "tsc200x-core.h" > > > > > > +static const struct input_id tsc2004_input_id = { > > > + .bustype = BUS_I2C, > > > + .product = 2004, > > > +}; > > > + > > > static int tsc2004_cmd(struct device *dev, u8 cmd) > > > { > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c, > > > const struct i2c_device_id *id) > > > > > > { > > > - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, > > > + return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id, > > > devm_regmap_init_i2c(i2c, &tsc200x_regmap_config), > > > tsc2004_cmd); > > > } > > > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c > > > index b9f593d..f2c5f0e 100644 > > > --- a/drivers/input/touchscreen/tsc2005.c > > > +++ b/drivers/input/touchscreen/tsc2005.c > > > @@ -24,6 +24,11 @@ > > > #include <linux/regmap.h> > > > #include "tsc200x-core.h" > > > > > > +static const struct input_id tsc2005_input_id = { > > > + .bustype = BUS_SPI, > > > + .product = 2005, > > > +}; > > > + > > > static int tsc2005_cmd(struct device *dev, u8 cmd) > > > { > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi) > > > if (error) > > > return error; > > > > > > - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, > > > + return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, > > > devm_regmap_init_spi(spi, &tsc200x_regmap_config), > > > tsc2005_cmd); > > > } > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c > > > index 26e81d1b..5e625c4 100644 > > > --- a/drivers/input/touchscreen/tsc200x-core.c > > > +++ b/drivers/input/touchscreen/tsc200x-core.c > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input) > > > mutex_unlock(&ts->mutex); > > > } > > > > > > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > > > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, > > > struct regmap *regmap, > > > int (*tsc200x_cmd)(struct device *dev, u8 cmd)) > > > { > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > > > snprintf(ts->phys, sizeof(ts->phys), > > > "%s/input-ts", dev_name(dev)); > > > > > > - input_dev->name = "TSC200X touchscreen"; > > > + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", > > > + tsc_id->product); > > > > What about: > > > > if (tsc_id->product == 2005) > > input_dev->name = "TSC2005 touchscreen"; > > else > > input_dev->name = "TSC200X touchscreen"; > > > > We do want to use 'TSC2005' name for TSC2005, because compatibility, > > but you should keep TSC200X.... because compatibility. You don't want > > to break people's setups by going from TSC200X to TSC2004. > > By same logic we shouldn't change from TSC200X back to TSC2005 because of > people's possibly new setup in the last 9 months. That's your, broken logic. TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005 for past few years. TSC200X->TSC2004 is just introducing regression, because noone ever seen TSC2004, AFAICT. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 6:54 ` Pavel Machek @ 2016-07-20 7:06 ` Michael Welling 2016-07-20 7:48 ` Pavel Machek 2016-07-20 16:45 ` Dmitry Torokhov 1 sibling, 1 reply; 44+ messages in thread From: Michael Welling @ 2016-07-20 7:06 UTC (permalink / raw) To: Pavel Machek Cc: kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Dmitry Torokhov, Aaro Koskinen On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote: > > By same logic we shouldn't change from TSC200X back to TSC2005 because of > > people's possibly new setup in the last 9 months. > > That's your, broken logic. > > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005 > for past few years. > > TSC200X->TSC2004 is just introducing regression, because noone ever > seen TSC2004, AFAICT. I am pretty sure no one cares. Escpecially me. > > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 7:06 ` Michael Welling @ 2016-07-20 7:48 ` Pavel Machek 0 siblings, 0 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-20 7:48 UTC (permalink / raw) To: Michael Welling Cc: kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Dmitry Torokhov, Aaro Koskinen On Wed 2016-07-20 02:06:17, Michael Welling wrote: > On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote: > > > By same logic we shouldn't change from TSC200X back to TSC2005 because of > > > people's possibly new setup in the last 9 months. > > > > That's your, broken logic. > > > > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005 > > for past few years. > > > > TSC200X->TSC2004 is just introducing regression, because noone ever > > seen TSC2004, AFAICT. > > I am pretty sure no one cares. Escpecially me. Dunno. Lets do the right thing: TSC2005 for 2005, TSC200X for everyone else. Interface issues are tricky to handle once created, so its best to be careful. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 6:54 ` Pavel Machek 2016-07-20 7:06 ` Michael Welling @ 2016-07-20 16:45 ` Dmitry Torokhov 2016-07-20 16:56 ` Pali Rohár 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 16:45 UTC (permalink / raw) To: Pavel Machek Cc: Michael Welling, kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge, Aaro Koskinen On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote: > On Wed 2016-07-20 01:50:24, Michael Welling wrote: > > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote: > > > Hi! > > > > > > > Passes input_id struct to the the common probe function for the tsc200x drivers > > > > instead of just the bustype. > > > > > > > > This allows for the use of the product variable to set the input_dev->name > > > > variable according to the type of touchscreen used. > > > > > > > > Signed-off-by: Michael Welling <mwelling@ieee.org> > > > > --- > > > > drivers/input/touchscreen/tsc2004.c | 7 ++++++- > > > > drivers/input/touchscreen/tsc2005.c | 7 ++++++- > > > > drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- > > > > drivers/input/touchscreen/tsc200x-core.h | 2 +- > > > > 4 files changed, 17 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c > > > > index 7295c19..6fe55d5 100644 > > > > --- a/drivers/input/touchscreen/tsc2004.c > > > > +++ b/drivers/input/touchscreen/tsc2004.c > > > > @@ -22,6 +22,11 @@ > > > > #include <linux/regmap.h> > > > > #include "tsc200x-core.h" > > > > > > > > +static const struct input_id tsc2004_input_id = { > > > > + .bustype = BUS_I2C, > > > > + .product = 2004, > > > > +}; > > > > + > > > > static int tsc2004_cmd(struct device *dev, u8 cmd) > > > > { > > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c, > > > > const struct i2c_device_id *id) > > > > > > > > { > > > > - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, > > > > + return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id, > > > > devm_regmap_init_i2c(i2c, &tsc200x_regmap_config), > > > > tsc2004_cmd); > > > > } > > > > diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c > > > > index b9f593d..f2c5f0e 100644 > > > > --- a/drivers/input/touchscreen/tsc2005.c > > > > +++ b/drivers/input/touchscreen/tsc2005.c > > > > @@ -24,6 +24,11 @@ > > > > #include <linux/regmap.h> > > > > #include "tsc200x-core.h" > > > > > > > > +static const struct input_id tsc2005_input_id = { > > > > + .bustype = BUS_SPI, > > > > + .product = 2005, > > > > +}; > > > > + > > > > static int tsc2005_cmd(struct device *dev, u8 cmd) > > > > { > > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi) > > > > if (error) > > > > return error; > > > > > > > > - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, > > > > + return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, > > > > devm_regmap_init_spi(spi, &tsc200x_regmap_config), > > > > tsc2005_cmd); > > > > } > > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c > > > > index 26e81d1b..5e625c4 100644 > > > > --- a/drivers/input/touchscreen/tsc200x-core.c > > > > +++ b/drivers/input/touchscreen/tsc200x-core.c > > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input) > > > > mutex_unlock(&ts->mutex); > > > > } > > > > > > > > -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > > > > +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, > > > > struct regmap *regmap, > > > > int (*tsc200x_cmd)(struct device *dev, u8 cmd)) > > > > { > > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype, > > > > snprintf(ts->phys, sizeof(ts->phys), > > > > "%s/input-ts", dev_name(dev)); > > > > > > > > - input_dev->name = "TSC200X touchscreen"; > > > > + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d touchscreen", > > > > + tsc_id->product); > > > > > > What about: > > > > > > if (tsc_id->product == 2005) > > > input_dev->name = "TSC2005 touchscreen"; > > > else > > > input_dev->name = "TSC200X touchscreen"; > > > > > > We do want to use 'TSC2005' name for TSC2005, because compatibility, > > > but you should keep TSC200X.... because compatibility. You don't want > > > to break people's setups by going from TSC200X to TSC2004. > > > > By same logic we shouldn't change from TSC200X back to TSC2005 because of > > people's possibly new setup in the last 9 months. > > That's your, broken logic. > > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005 > for past few years. People might have also seen TSC2005->TSC200X for the last few months. They could be dependent on this now and start complaining once we revert. I am sorely tempted to leave the name as is. Just adjust your script to check the name. Something like TSC_MODEL=`sed -ne 's/^N: Name="TSC\([0-9]\+\) .*/\1/p' /proc/bus/input/devices` xinput ... "TSC${TSC_MODEL} Touchscreen" Or fetch the current name from sysfs. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 16:45 ` Dmitry Torokhov @ 2016-07-20 16:56 ` Pali Rohár 2016-07-20 17:04 ` Dmitry Torokhov 0 siblings, 1 reply; 44+ messages in thread From: Pali Rohár @ 2016-07-20 16:56 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, Michael Welling, kernel list, linux-input, sre, ivo.g.dimitrov.75, patrikbachan, serge, Aaro Koskinen [-- Attachment #1: Type: Text/Plain, Size: 5984 bytes --] On Wednesday 20 July 2016 18:45:03 Dmitry Torokhov wrote: > On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote: > > On Wed 2016-07-20 01:50:24, Michael Welling wrote: > > > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote: > > > > Hi! > > > > > > > > > Passes input_id struct to the the common probe function for > > > > > the tsc200x drivers instead of just the bustype. > > > > > > > > > > This allows for the use of the product variable to set the > > > > > input_dev->name variable according to the type of > > > > > touchscreen used. > > > > > > > > > > Signed-off-by: Michael Welling <mwelling@ieee.org> > > > > > --- > > > > > > > > > > drivers/input/touchscreen/tsc2004.c | 7 ++++++- > > > > > drivers/input/touchscreen/tsc2005.c | 7 ++++++- > > > > > drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- > > > > > drivers/input/touchscreen/tsc200x-core.h | 2 +- > > > > > 4 files changed, 17 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/drivers/input/touchscreen/tsc2004.c > > > > > b/drivers/input/touchscreen/tsc2004.c index 7295c19..6fe55d5 > > > > > 100644 > > > > > --- a/drivers/input/touchscreen/tsc2004.c > > > > > +++ b/drivers/input/touchscreen/tsc2004.c > > > > > @@ -22,6 +22,11 @@ > > > > > > > > > > #include <linux/regmap.h> > > > > > #include "tsc200x-core.h" > > > > > > > > > > +static const struct input_id tsc2004_input_id = { > > > > > + .bustype = BUS_I2C, > > > > > + .product = 2004, > > > > > +}; > > > > > + > > > > > > > > > > static int tsc2004_cmd(struct device *dev, u8 cmd) > > > > > { > > > > > > > > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > > > > > > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client > > > > > *i2c, > > > > > > > > > > const struct i2c_device_id *id) > > > > > > > > > > { > > > > > > > > > > - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, > > > > > + return tsc200x_probe(&i2c->dev, i2c->irq, > > > > > &tsc2004_input_id, > > > > > > > > > > devm_regmap_init_i2c(i2c, > > > > > &tsc200x_regmap_config), > > > > > tsc2004_cmd); > > > > > > > > > > } > > > > > > > > > > diff --git a/drivers/input/touchscreen/tsc2005.c > > > > > b/drivers/input/touchscreen/tsc2005.c index b9f593d..f2c5f0e > > > > > 100644 > > > > > --- a/drivers/input/touchscreen/tsc2005.c > > > > > +++ b/drivers/input/touchscreen/tsc2005.c > > > > > @@ -24,6 +24,11 @@ > > > > > > > > > > #include <linux/regmap.h> > > > > > #include "tsc200x-core.h" > > > > > > > > > > +static const struct input_id tsc2005_input_id = { > > > > > + .bustype = BUS_SPI, > > > > > + .product = 2005, > > > > > +}; > > > > > + > > > > > > > > > > static int tsc2005_cmd(struct device *dev, u8 cmd) > > > > > { > > > > > > > > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > > > > > > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device > > > > > *spi) > > > > > > > > > > if (error) > > > > > > > > > > return error; > > > > > > > > > > - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, > > > > > + return tsc200x_probe(&spi->dev, spi->irq, > > > > > &tsc2005_input_id, > > > > > > > > > > devm_regmap_init_spi(spi, > > > > > &tsc200x_regmap_config), > > > > > tsc2005_cmd); > > > > > > > > > > } > > > > > > > > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c > > > > > b/drivers/input/touchscreen/tsc200x-core.c index > > > > > 26e81d1b..5e625c4 100644 > > > > > --- a/drivers/input/touchscreen/tsc200x-core.c > > > > > +++ b/drivers/input/touchscreen/tsc200x-core.c > > > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct > > > > > input_dev *input) > > > > > > > > > > mutex_unlock(&ts->mutex); > > > > > > > > > > } > > > > > > > > > > -int tsc200x_probe(struct device *dev, int irq, __u16 > > > > > bustype, +int tsc200x_probe(struct device *dev, int irq, > > > > > const struct input_id *tsc_id, > > > > > > > > > > struct regmap *regmap, > > > > > int (*tsc200x_cmd)(struct device *dev, u8 cmd)) > > > > > > > > > > { > > > > > > > > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, > > > > > int irq, __u16 bustype, > > > > > > > > > > snprintf(ts->phys, sizeof(ts->phys), > > > > > > > > > > "%s/input-ts", dev_name(dev)); > > > > > > > > > > - input_dev->name = "TSC200X touchscreen"; > > > > > + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d > > > > > touchscreen", + tsc_id->product); > > > > > > > > What about: > > > > if (tsc_id->product == 2005) > > > > > > > > input_dev->name = "TSC2005 touchscreen"; > > > > > > > > else > > > > > > > > input_dev->name = "TSC200X touchscreen"; > > > > > > > > We do want to use 'TSC2005' name for TSC2005, because > > > > compatibility, but you should keep TSC200X.... because > > > > compatibility. You don't want to break people's setups by > > > > going from TSC200X to TSC2004. > > > > > > By same logic we shouldn't change from TSC200X back to TSC2005 > > > because of people's possibly new setup in the last 9 months. > > > > That's your, broken logic. > > > > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005 > > for past few years. > > People might have also seen TSC2005->TSC200X for the last few > months. They could be dependent on this now and start complaining > once we revert. They could. But did it? Is there real breakage of real existing applications which are in use? I doubt. But there is for opposite scenario. Older applications (like Maemo mce) worked with older kernel version, but not with new due to this change TSC2005->TSC200X. So I still think this is a regression, which should be fixed... -- Pali Rohár pali.rohar@gmail.com [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 16:56 ` Pali Rohár @ 2016-07-20 17:04 ` Dmitry Torokhov 2016-07-20 17:14 ` Dmitry Torokhov 0 siblings, 1 reply; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 17:04 UTC (permalink / raw) To: Pali Rohár Cc: Pavel Machek, Michael Welling, kernel list, linux-input, sre, ivo.g.dimitrov.75, patrikbachan, serge, Aaro Koskinen On Wed, Jul 20, 2016 at 06:56:51PM +0200, Pali Rohár wrote: > On Wednesday 20 July 2016 18:45:03 Dmitry Torokhov wrote: > > On Wed, Jul 20, 2016 at 08:54:05AM +0200, Pavel Machek wrote: > > > On Wed 2016-07-20 01:50:24, Michael Welling wrote: > > > > On Wed, Jul 20, 2016 at 08:31:06AM +0200, Pavel Machek wrote: > > > > > Hi! > > > > > > > > > > > Passes input_id struct to the the common probe function for > > > > > > the tsc200x drivers instead of just the bustype. > > > > > > > > > > > > This allows for the use of the product variable to set the > > > > > > input_dev->name variable according to the type of > > > > > > touchscreen used. > > > > > > > > > > > > Signed-off-by: Michael Welling <mwelling@ieee.org> > > > > > > --- > > > > > > > > > > > > drivers/input/touchscreen/tsc2004.c | 7 ++++++- > > > > > > drivers/input/touchscreen/tsc2005.c | 7 ++++++- > > > > > > drivers/input/touchscreen/tsc200x-core.c | 7 ++++--- > > > > > > drivers/input/touchscreen/tsc200x-core.h | 2 +- > > > > > > 4 files changed, 17 insertions(+), 6 deletions(-) > > > > > > > > > > > > diff --git a/drivers/input/touchscreen/tsc2004.c > > > > > > b/drivers/input/touchscreen/tsc2004.c index 7295c19..6fe55d5 > > > > > > 100644 > > > > > > --- a/drivers/input/touchscreen/tsc2004.c > > > > > > +++ b/drivers/input/touchscreen/tsc2004.c > > > > > > @@ -22,6 +22,11 @@ > > > > > > > > > > > > #include <linux/regmap.h> > > > > > > #include "tsc200x-core.h" > > > > > > > > > > > > +static const struct input_id tsc2004_input_id = { > > > > > > + .bustype = BUS_I2C, > > > > > > + .product = 2004, > > > > > > +}; > > > > > > + > > > > > > > > > > > > static int tsc2004_cmd(struct device *dev, u8 cmd) > > > > > > { > > > > > > > > > > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > > > > > > > > > > @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client > > > > > > *i2c, > > > > > > > > > > > > const struct i2c_device_id *id) > > > > > > > > > > > > { > > > > > > > > > > > > - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, > > > > > > + return tsc200x_probe(&i2c->dev, i2c->irq, > > > > > > &tsc2004_input_id, > > > > > > > > > > > > devm_regmap_init_i2c(i2c, > > > > > > &tsc200x_regmap_config), > > > > > > tsc2004_cmd); > > > > > > > > > > > > } > > > > > > > > > > > > diff --git a/drivers/input/touchscreen/tsc2005.c > > > > > > b/drivers/input/touchscreen/tsc2005.c index b9f593d..f2c5f0e > > > > > > 100644 > > > > > > --- a/drivers/input/touchscreen/tsc2005.c > > > > > > +++ b/drivers/input/touchscreen/tsc2005.c > > > > > > @@ -24,6 +24,11 @@ > > > > > > > > > > > > #include <linux/regmap.h> > > > > > > #include "tsc200x-core.h" > > > > > > > > > > > > +static const struct input_id tsc2005_input_id = { > > > > > > + .bustype = BUS_SPI, > > > > > > + .product = 2005, > > > > > > +}; > > > > > > + > > > > > > > > > > > > static int tsc2005_cmd(struct device *dev, u8 cmd) > > > > > > { > > > > > > > > > > > > u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; > > > > > > > > > > > > @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device > > > > > > *spi) > > > > > > > > > > > > if (error) > > > > > > > > > > > > return error; > > > > > > > > > > > > - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, > > > > > > + return tsc200x_probe(&spi->dev, spi->irq, > > > > > > &tsc2005_input_id, > > > > > > > > > > > > devm_regmap_init_spi(spi, > > > > > > &tsc200x_regmap_config), > > > > > > tsc2005_cmd); > > > > > > > > > > > > } > > > > > > > > > > > > diff --git a/drivers/input/touchscreen/tsc200x-core.c > > > > > > b/drivers/input/touchscreen/tsc200x-core.c index > > > > > > 26e81d1b..5e625c4 100644 > > > > > > --- a/drivers/input/touchscreen/tsc200x-core.c > > > > > > +++ b/drivers/input/touchscreen/tsc200x-core.c > > > > > > @@ -450,7 +450,7 @@ static void tsc200x_close(struct > > > > > > input_dev *input) > > > > > > > > > > > > mutex_unlock(&ts->mutex); > > > > > > > > > > > > } > > > > > > > > > > > > -int tsc200x_probe(struct device *dev, int irq, __u16 > > > > > > bustype, +int tsc200x_probe(struct device *dev, int irq, > > > > > > const struct input_id *tsc_id, > > > > > > > > > > > > struct regmap *regmap, > > > > > > int (*tsc200x_cmd)(struct device *dev, u8 cmd)) > > > > > > > > > > > > { > > > > > > > > > > > > @@ -547,9 +547,10 @@ int tsc200x_probe(struct device *dev, > > > > > > int irq, __u16 bustype, > > > > > > > > > > > > snprintf(ts->phys, sizeof(ts->phys), > > > > > > > > > > > > "%s/input-ts", dev_name(dev)); > > > > > > > > > > > > - input_dev->name = "TSC200X touchscreen"; > > > > > > + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, "TSC%04d > > > > > > touchscreen", + tsc_id->product); > > > > > > > > > > What about: > > > > > if (tsc_id->product == 2005) > > > > > > > > > > input_dev->name = "TSC2005 touchscreen"; > > > > > > > > > > else > > > > > > > > > > input_dev->name = "TSC200X touchscreen"; > > > > > > > > > > We do want to use 'TSC2005' name for TSC2005, because > > > > > compatibility, but you should keep TSC200X.... because > > > > > compatibility. You don't want to break people's setups by > > > > > going from TSC200X to TSC2004. > > > > > > > > By same logic we shouldn't change from TSC200X back to TSC2005 > > > > because of people's possibly new setup in the last 9 months. > > > > > > That's your, broken logic. > > > > > > TSC200X->TSC2005 needs to be fixed, because people seen the TSC2005 > > > for past few years. > > > > People might have also seen TSC2005->TSC200X for the last few > > months. They could be dependent on this now and start complaining > > once we revert. > > They could. But did it? Is there real breakage of real existing > applications which are in use? I doubt. But there is for opposite > scenario. > > Older applications (like Maemo mce) worked with older kernel version, > but not with new due to this change TSC2005->TSC200X. > > So I still think this is a regression, which should be fixed... OK, fine, let's keep 200X for 2004, and real number for everyone else. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 17:04 ` Dmitry Torokhov @ 2016-07-20 17:14 ` Dmitry Torokhov 2016-07-20 20:25 ` Pavel Machek 2016-07-20 20:37 ` Pali Rohár 0 siblings, 2 replies; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 17:14 UTC (permalink / raw) To: Pali Rohár Cc: Pavel Machek, Michael Welling, kernel list, linux-input, sre, ivo.g.dimitrov.75, patrikbachan, serge, Aaro Koskinen Input: tsc200x - report proper input_dev name From: Michael Welling <mwelling@ieee.org> Passes input_id struct to the common probe function for the tsc200x drivers instead of just the bustype. This allows for the use of the product variable to set the input_dev->name variable according to the type of touchscreen used. Note that when we introduced support for TSC2004 we started calling everything TSC200X, so let's keep this quirk. Reported-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Michael Welling <mwelling@ieee.org> Cc: stable@vger.kernel.org Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/tsc2004.c | 7 ++++++- drivers/input/touchscreen/tsc2005.c | 7 ++++++- drivers/input/touchscreen/tsc200x-core.c | 15 ++++++++++++--- drivers/input/touchscreen/tsc200x-core.h | 2 +- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c index 7295c19..6fe55d5 100644 --- a/drivers/input/touchscreen/tsc2004.c +++ b/drivers/input/touchscreen/tsc2004.c @@ -22,6 +22,11 @@ #include <linux/regmap.h> #include "tsc200x-core.h" +static const struct input_id tsc2004_input_id = { + .bustype = BUS_I2C, + .product = 2004, +}; + static int tsc2004_cmd(struct device *dev, u8 cmd) { u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; @@ -42,7 +47,7 @@ static int tsc2004_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { - return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C, + return tsc200x_probe(&i2c->dev, i2c->irq, &tsc2004_input_id, devm_regmap_init_i2c(i2c, &tsc200x_regmap_config), tsc2004_cmd); } diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c index b9f593d..f2c5f0e 100644 --- a/drivers/input/touchscreen/tsc2005.c +++ b/drivers/input/touchscreen/tsc2005.c @@ -24,6 +24,11 @@ #include <linux/regmap.h> #include "tsc200x-core.h" +static const struct input_id tsc2005_input_id = { + .bustype = BUS_SPI, + .product = 2005, +}; + static int tsc2005_cmd(struct device *dev, u8 cmd) { u8 tx = TSC200X_CMD | TSC200X_CMD_12BIT | cmd; @@ -62,7 +67,7 @@ static int tsc2005_probe(struct spi_device *spi) if (error) return error; - return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI, + return tsc200x_probe(&spi->dev, spi->irq, &tsc2005_input_id, devm_regmap_init_spi(spi, &tsc200x_regmap_config), tsc2005_cmd); } diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c index 26e81d1b..b7059ed 100644 --- a/drivers/input/touchscreen/tsc200x-core.c +++ b/drivers/input/touchscreen/tsc200x-core.c @@ -450,7 +450,7 @@ static void tsc200x_close(struct input_dev *input) mutex_unlock(&ts->mutex); } -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, struct regmap *regmap, int (*tsc200x_cmd)(struct device *dev, u8 cmd)) { @@ -547,9 +547,18 @@ int tsc200x_probe(struct device *dev, int irq, __u16 bustype, snprintf(ts->phys, sizeof(ts->phys), "%s/input-ts", dev_name(dev)); - input_dev->name = "TSC200X touchscreen"; + if (tsc_id->product == 2004) { + input_dev->name = "TSC200X touchscreen"; + } else { + input_dev->name = devm_kasprintf(dev, GFP_KERNEL, + "TSC%04d touchscreen", + tsc_id->product); + if (!input_dev->name) + return -ENOMEM; + } + input_dev->phys = ts->phys; - input_dev->id.bustype = bustype; + input_dev->id = *tsc_id; input_dev->dev.parent = dev; input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY); input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH); diff --git a/drivers/input/touchscreen/tsc200x-core.h b/drivers/input/touchscreen/tsc200x-core.h index 7a482d1..49a63a3 100644 --- a/drivers/input/touchscreen/tsc200x-core.h +++ b/drivers/input/touchscreen/tsc200x-core.h @@ -70,7 +70,7 @@ extern const struct regmap_config tsc200x_regmap_config; extern const struct dev_pm_ops tsc200x_pm_ops; -int tsc200x_probe(struct device *dev, int irq, __u16 bustype, +int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id, struct regmap *regmap, int (*tsc200x_cmd)(struct device *dev, u8 cmd)); int tsc200x_remove(struct device *dev); ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 17:14 ` Dmitry Torokhov @ 2016-07-20 20:25 ` Pavel Machek 2016-07-20 20:37 ` Pali Rohár 1 sibling, 0 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-20 20:25 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pali Rohár, Michael Welling, kernel list, linux-input, sre, ivo.g.dimitrov.75, patrikbachan, serge, Aaro Koskinen On Wed 2016-07-20 10:14:49, Dmitry Torokhov wrote: > Input: tsc200x - report proper input_dev name > > From: Michael Welling <mwelling@ieee.org> > > Passes input_id struct to the common probe function for the tsc200x drivers > instead of just the bustype. > > This allows for the use of the product variable to set the input_dev->name > variable according to the type of touchscreen used. Note that when we > introduced support for TSC2004 we started calling everything TSC200X, so > let's keep this quirk. > > Reported-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Michael Welling <mwelling@ieee.org> > Cc: stable@vger.kernel.org > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Acked-by: Pavel Machek <pavel@ucw.cz> Thanks! Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Input: tsc200x - Report proper input_dev name 2016-07-20 17:14 ` Dmitry Torokhov 2016-07-20 20:25 ` Pavel Machek @ 2016-07-20 20:37 ` Pali Rohár 1 sibling, 0 replies; 44+ messages in thread From: Pali Rohár @ 2016-07-20 20:37 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, Michael Welling, kernel list, linux-input, sre, ivo.g.dimitrov.75, patrikbachan, serge, Aaro Koskinen [-- Attachment #1: Type: Text/Plain, Size: 813 bytes --] On Wednesday 20 July 2016 19:14:49 Dmitry Torokhov wrote: > Input: tsc200x - report proper input_dev name > > From: Michael Welling <mwelling@ieee.org> > > Passes input_id struct to the common probe function for the tsc200x > drivers instead of just the bustype. > > This allows for the use of the product variable to set the > input_dev->name variable according to the type of touchscreen used. > Note that when we introduced support for TSC2004 we started calling > everything TSC200X, so let's keep this quirk. > > Reported-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Michael Welling <mwelling@ieee.org> > Cc: stable@vger.kernel.org > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Acked-by: Pali Rohár <pali.rohar@gmail.com> -- Pali Rohár pali.rohar@gmail.com [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 0:53 ` Dmitry Torokhov 2016-07-20 1:34 ` Michael Welling @ 2016-07-20 16:33 ` Pali Rohár 1 sibling, 0 replies; 44+ messages in thread From: Pali Rohár @ 2016-07-20 16:33 UTC (permalink / raw) To: Dmitry Torokhov Cc: Michael Welling, Pavel Machek, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge [-- Attachment #1: Type: Text/Plain, Size: 2805 bytes --] On Wednesday 20 July 2016 02:53:07 Dmitry Torokhov wrote: > On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > > On Sun 2016-07-17 13:51:34, Michael Welling wrote: > > > > > > On Sun, Jul 17, 2016 at 08:42:09PM +0200, Pavel Machek > > > > > > wrote: > > > > > > > On Sun 2016-07-17 13:24:45, Michael Welling wrote: > > > > > > > > On Sun, Jul 17, 2016 at 07:52:57PM +0200, Pavel Machek > > > > > > > > wrote: > > > > > > > > > Hi! > > > > > > > > > > > > > > > > > > tsc2005 driver changed input device name, from > > > > > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: > > > > > > > > > input_dev->name = "TSC2005 touchscreen"; > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to > > > > > > > > > propagate that name to userspace, where it is needed > > > > > > > > > to be able to do > > > > > > Technically X _is_ userspace. > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. > > > > > > > > > (Touchscreen is unusable without calibration). > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > Hmm, I do not think we ever committed for the device names to be > > > stable. You are supposed to locate touchscreen device based on > > > its properties and you might need some heuristic if you > > > encounter a system with more than one such touchscreen. > > > > > > > > > > > The input_dev name could be passed to the common probe > > > > > > > > function. > > > > > > > > > > > > > > > > http://lxr.free-electrons.com/source/drivers/input/touc > > > > > > > > hscreen/tsc2005.c#L65 > > > > > > > > > > > > > > That would be preffered, I guess. > > > > > > > > > > > > > > How many stable releases are affected? > > > > > > > > > > > > Well this patch is 9 months old now. Lets see. > > > > > > > > > > > > It was introduced in v4.4-rc1. So v4.4, v4.5 and v4.6. > > > > > > > > > > Ok, thanks for the information. I believe changing it back to > > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > > > Do we know how many users are affected? > > > > Anyone with an old N900 and the smarts to update the kernel. > > Soo... only Pavel? ;) No, more are playing with upstream kernel and N900. Now I see that Maemo applications looks also for string "TSC2005 touchscreen"... I'm also for changing name back to "TSC2005 touchscreen" for TSC2005 touchscreen. -- Pali Rohár pali.rohar@gmail.com [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 0:39 ` Michael Welling 2016-07-20 0:53 ` Dmitry Torokhov @ 2016-07-20 1:26 ` Aaro Koskinen 2016-07-20 2:18 ` Michael Welling 1 sibling, 1 reply; 44+ messages in thread From: Aaro Koskinen @ 2016-07-20 1:26 UTC (permalink / raw) To: Michael Welling Cc: Dmitry Torokhov, Pavel Machek, kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > Ok, thanks for the information. I believe changing it back to > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > Do we know how many users are affected? > > Anyone with an old N900 and the smarts to update the kernel. I think tsc2005 could be also used on N810. A. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 1:26 ` Aaro Koskinen @ 2016-07-20 2:18 ` Michael Welling 0 siblings, 0 replies; 44+ messages in thread From: Michael Welling @ 2016-07-20 2:18 UTC (permalink / raw) To: Aaro Koskinen Cc: Dmitry Torokhov, Pavel Machek, kernel list, linux-input, pali.rohar, sre, ivo.g.dimitrov.75, patrikbachan, serge On Wed, Jul 20, 2016 at 04:26:44AM +0300, Aaro Koskinen wrote: > On Tue, Jul 19, 2016 at 07:39:08PM -0500, Michael Welling wrote: > > On Tue, Jul 19, 2016 at 04:51:20PM -0700, Dmitry Torokhov wrote: > > > On Sun, Jul 17, 2016 at 05:56:36PM -0500, Michael Welling wrote: > > > > On Sun, Jul 17, 2016 at 10:03:39PM +0200, Pavel Machek wrote: > > > > > Ok, thanks for the information. I believe changing it back to > > > > > "TSC2005" version makes sense (and then fixing it in stable). > > > > > > Do we know how many users are affected? > > > > Anyone with an old N900 and the smarts to update the kernel. > > I think tsc2005 could be also used on N810. The N810 uses the tsc2301 which appears to be the combination of the tsc2005 and a audio codec. It appears that the N810's touchscreen may not be properly supported in mainline anymore. Actually, I still have a N810 so I will take a look for historical reasons. > > A. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-19 23:51 ` Dmitry Torokhov 2016-07-20 0:39 ` Michael Welling @ 2016-07-20 6:25 ` Pavel Machek 2016-07-20 16:23 ` Dmitry Torokhov 1 sibling, 1 reply; 44+ messages in thread From: Pavel Machek @ 2016-07-20 6:25 UTC (permalink / raw) To: Dmitry Torokhov Cc: Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge Hi! > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > touchscreen"; > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > name to userspace, where it is needed to be able to do > > Technically X _is_ userspace. There's "userspace running as root" and "userspace userspace" :-). > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > unusable without calibration). > > > > > > > > > > > > > > What to do with that? > > Hmm, I do not think we ever committed for the device names to be stable. > You are supposed to locate touchscreen device based on its properties > and you might need some heuristic if you encounter a system with more > than one such touchscreen. Well, you are commited now, like it or not, X people did it for you :-(. Because there's no other reasonable way to use xinput --set-prop... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 6:25 ` Pavel Machek @ 2016-07-20 16:23 ` Dmitry Torokhov 2016-07-20 20:22 ` Pavel Machek 2016-07-20 21:47 ` Peter Hutterer 0 siblings, 2 replies; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 16:23 UTC (permalink / raw) To: Pavel Machek Cc: Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge, Peter Hutterer On Wed, Jul 20, 2016 at 08:25:58AM +0200, Pavel Machek wrote: > Hi! > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > name to userspace, where it is needed to be able to do > > > > Technically X _is_ userspace. > > There's "userspace running as root" and "userspace userspace" :-). I do not really see any difference form the kernel POW. > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > What to do with that? > > > > Hmm, I do not think we ever committed for the device names to be stable. > > You are supposed to locate touchscreen device based on its properties > > and you might need some heuristic if you encounter a system with more > > than one such touchscreen. > > Well, you are commited now, like it or not, X people did it for you > :-(. > > Because there's no other reasonable way to use xinput --set-prop... Well, X is going to have to fix it. How am I supposed to control my devices in multi-seat environment if I use the same hardware (or if I have device with multiple touchscreens)? They all will have the same name (well, all mice, then all keyboards, etc). Let's add Peter to the fold... In the mean time you can adjust the name or use XID instead. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 16:23 ` Dmitry Torokhov @ 2016-07-20 20:22 ` Pavel Machek 2016-07-20 21:47 ` Peter Hutterer 1 sibling, 0 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-20 20:22 UTC (permalink / raw) To: Dmitry Torokhov Cc: Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge, Peter Hutterer Hi! > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > Technically X _is_ userspace. > > > > There's "userspace running as root" and "userspace userspace" :-). > > I do not really see any difference form the kernel POW. > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > You are supposed to locate touchscreen device based on its properties > > > and you might need some heuristic if you encounter a system with more > > > than one such touchscreen. > > > > Well, you are commited now, like it or not, X people did it for you > > :-(. > > > > Because there's no other reasonable way to use xinput --set-prop... > > Well, X is going to have to fix it. How am I supposed to control my > devices in multi-seat environment if I use the same hardware (or if I > have device with multiple touchscreens)? They all will have the same > name (well, all mice, then all keyboards, etc). Let's add Peter to the > fold... Well, if someone has such a multiseat config, they have a problem. But in the meantime, I'd like to keep working system. http://who-t.blogspot.cz/2016/07/xinput-resolves-device-names-and.html And I see you replied in the blog. Good. Yes, X needs to be improved to work nicely with multiseat. Still kernel needs to play nice and not have regressions. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 16:23 ` Dmitry Torokhov 2016-07-20 20:22 ` Pavel Machek @ 2016-07-20 21:47 ` Peter Hutterer 2016-07-20 22:20 ` Dmitry Torokhov 2016-07-21 6:32 ` Pavel Machek 1 sibling, 2 replies; 44+ messages in thread From: Peter Hutterer @ 2016-07-20 21:47 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Wed, Jul 20, 2016 at 09:23:56AM -0700, Dmitry Torokhov wrote: > On Wed, Jul 20, 2016 at 08:25:58AM +0200, Pavel Machek wrote: > > Hi! > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > Technically X _is_ userspace. > > > > There's "userspace running as root" and "userspace userspace" :-). > > I do not really see any difference form the kernel POW. > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > You are supposed to locate touchscreen device based on its properties > > > and you might need some heuristic if you encounter a system with more > > > than one such touchscreen. > > > > Well, you are commited now, like it or not, X people did it for you > > :-(. > > > > Because there's no other reasonable way to use xinput --set-prop... > > Well, X is going to have to fix it. How am I supposed to control my > devices in multi-seat environment if I use the same hardware (or if I > have device with multiple touchscreens)? They all will have the same > name (well, all mice, then all keyboards, etc). Let's add Peter to the > fold... > > In the mean time you can adjust the name or use XID instead. X has partially fixed this a few years ago. All input drivers (that matter) export a Device Node property that sets the device node for each device. $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" Device Node (261): "/dev/input/event4" Based on that you can get the udev device and work your way into any of the sysfs tree. Or do whatever else you want. But other than that there isn't anything in X to fix. xinput is primarily a debugging tool and it does name resolution for convenience. But it's not a tool for complex configurations. It does exactly what it needs to do, if you need something that's more complicated and relies on information not available to the X device itself then you'll need to write a custom tool that does what you need. sorry. Cheers, Peter ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 21:47 ` Peter Hutterer @ 2016-07-20 22:20 ` Dmitry Torokhov 2016-07-20 22:55 ` Peter Hutterer 2016-07-21 6:32 ` Pavel Machek 1 sibling, 1 reply; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-20 22:20 UTC (permalink / raw) To: Peter Hutterer Cc: Pavel Machek, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thu, Jul 21, 2016 at 07:47:36AM +1000, Peter Hutterer wrote: > On Wed, Jul 20, 2016 at 09:23:56AM -0700, Dmitry Torokhov wrote: > > On Wed, Jul 20, 2016 at 08:25:58AM +0200, Pavel Machek wrote: > > > Hi! > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > > > Technically X _is_ userspace. > > > > > > There's "userspace running as root" and "userspace userspace" :-). > > > > I do not really see any difference form the kernel POW. > > > > > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > > You are supposed to locate touchscreen device based on its properties > > > > and you might need some heuristic if you encounter a system with more > > > > than one such touchscreen. > > > > > > Well, you are commited now, like it or not, X people did it for you > > > :-(. > > > > > > Because there's no other reasonable way to use xinput --set-prop... > > > > Well, X is going to have to fix it. How am I supposed to control my > > devices in multi-seat environment if I use the same hardware (or if I > > have device with multiple touchscreens)? They all will have the same > > name (well, all mice, then all keyboards, etc). Let's add Peter to the > > fold... > > > > In the mean time you can adjust the name or use XID instead. > > X has partially fixed this a few years ago. All input drivers (that > matter) export a Device Node property that sets the device node for each > device. > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > Device Node (261): "/dev/input/event4" > > Based on that you can get the udev device and work your way into any of the > sysfs tree. Or do whatever else you want. The issue is not that I can't figure out sysfs path for a device, the issue is that xinput does not accept anything but name or XID and I may have multiple devices with the same name in the system. > > But other than that there isn't anything in X to fix. xinput is primarily a > debugging tool and it does name resolution for convenience. But it's not a > tool for complex configurations. It does exactly what it needs to do, if you OK, I do not believe that this information was conveyed clearly enough. Apparently some setups use it for real configuration. > need something that's more complicated and relies on information not > available to the X device itself then you'll need to write a custom tool > that does what you need. sorry. Pavel, ^^^^ Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 22:20 ` Dmitry Torokhov @ 2016-07-20 22:55 ` Peter Hutterer 0 siblings, 0 replies; 44+ messages in thread From: Peter Hutterer @ 2016-07-20 22:55 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Wed, Jul 20, 2016 at 03:20:02PM -0700, Dmitry Torokhov wrote: > On Thu, Jul 21, 2016 at 07:47:36AM +1000, Peter Hutterer wrote: > > On Wed, Jul 20, 2016 at 09:23:56AM -0700, Dmitry Torokhov wrote: > > > On Wed, Jul 20, 2016 at 08:25:58AM +0200, Pavel Machek wrote: > > > > Hi! > > > > > > > > > > > > > > > drivers/input/touchscreen/tsc2005.c: input_dev->name = "TSC2005 > > > > > > > > > > > touchscreen"; > > > > > > > > > > > > > > > > > > > > > > to "TSC200X touchscreen". Unfortunately, X seems to propagate that > > > > > > > > > > > name to userspace, where it is needed to be able to do > > > > > > > > > > Technically X _is_ userspace. > > > > > > > > There's "userspace running as root" and "userspace userspace" :-). > > > > > > I do not really see any difference form the kernel POW. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > xinput --set-prop --type=int ... > > > > > > > > > > > > > > > > > > > > > > with the right arguments to calibrate touchscreen. (Touchscreen is > > > > > > > > > > > unusable without calibration). > > > > > > > > > > > > > > > > > > > > > > What to do with that? > > > > > > > > > > Hmm, I do not think we ever committed for the device names to be stable. > > > > > You are supposed to locate touchscreen device based on its properties > > > > > and you might need some heuristic if you encounter a system with more > > > > > than one such touchscreen. > > > > > > > > Well, you are commited now, like it or not, X people did it for you > > > > :-(. > > > > > > > > Because there's no other reasonable way to use xinput --set-prop... > > > > > > Well, X is going to have to fix it. How am I supposed to control my > > > devices in multi-seat environment if I use the same hardware (or if I > > > have device with multiple touchscreens)? They all will have the same > > > name (well, all mice, then all keyboards, etc). Let's add Peter to the > > > fold... > > > > > > In the mean time you can adjust the name or use XID instead. > > > > X has partially fixed this a few years ago. All input drivers (that > > matter) export a Device Node property that sets the device node for each > > device. > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > Device Node (261): "/dev/input/event4" > > > > Based on that you can get the udev device and work your way into any of the > > sysfs tree. Or do whatever else you want. > > The issue is not that I can't figure out sysfs path for a device, the > issue is that xinput does not accept anything but name or XID and I may > have multiple devices with the same name in the system. fwiw, the main reason why I don't want this in xinput is that anything sysfs related (or elsewhere) is platform specific. On BSD the Device Node isn't an evdev node and other efforts are required. xinput is an X tool itself and I don't want non-X functionality in it because you'll quickly unleash pandora's box here about stuffing custom features in that only apply to a tiny fraction of setups. even the case where you have more than one device with the same name is quite unusal (note: we do support a "pointer:" and "keyboard:" prefix for those where a device has a pointer and a keyboard device with the same name like many of the mouse/keyboard combos do). > > But other than that there isn't anything in X to fix. xinput is primarily a > > debugging tool and it does name resolution for convenience. But it's not a > > tool for complex configurations. It does exactly what it needs to do, if you > > OK, I do not believe that this information was conveyed clearly enough. > Apparently some setups use it for real configuration. yeah. I've been saying "get your DE to implement support" for 7-8 years now but saying things and being listened too are two different entities :) xinput's main problem is that it works for the majority of use-cases, so people use it. That's largely fine for most cases, but not when it comes to anything even remotely sophisticated. Cheers, Peter > > need something that's more complicated and relies on information not > > available to the X device itself then you'll need to write a custom tool > > that does what you need. sorry. > > Pavel, ^^^^ > > > Thanks. > > -- > Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-20 21:47 ` Peter Hutterer 2016-07-20 22:20 ` Dmitry Torokhov @ 2016-07-21 6:32 ` Pavel Machek 2016-07-21 6:42 ` Peter Hutterer 2016-07-25 14:56 ` Pali Rohár 1 sibling, 2 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-21 6:32 UTC (permalink / raw) To: Peter Hutterer Cc: Dmitry Torokhov, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge Hi! > > In the mean time you can adjust the name or use XID instead. > > X has partially fixed this a few years ago. All input drivers (that > matter) export a Device Node property that sets the device node for each > device. > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > Device Node (261): "/dev/input/event4" > > Based on that you can get the udev device and work your way into any of the > sysfs tree. Or do whatever else you want. > > But other than that there isn't anything in X to fix. xinput is primarily a > debugging tool and it does name resolution for convenience. But it's not a > tool for complex configurations. It does exactly what it needs to do, if you > need something that's more complicated and relies on information not > available to the X device itself then you'll need to write a custom tool > that does what you need. sorry. Ok.. so out of the box, touchscreen is "upside down" and miscalibrated on n900. So I need to run xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 xinput --set-prop --type=int 8 249 0 1 (or equivalent with names) so that I can use the touchscreen. (And that's quite important -- X is somehow unusable without pointing device). If xinput is not the right solution, what is the right solution? Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-21 6:32 ` Pavel Machek @ 2016-07-21 6:42 ` Peter Hutterer 2016-07-21 8:54 ` Pavel Machek 2016-07-25 14:56 ` Pali Rohár 1 sibling, 1 reply; 44+ messages in thread From: Peter Hutterer @ 2016-07-21 6:42 UTC (permalink / raw) To: Pavel Machek Cc: Dmitry Torokhov, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > Hi! > > > > In the mean time you can adjust the name or use XID instead. > > > > X has partially fixed this a few years ago. All input drivers (that > > matter) export a Device Node property that sets the device node for each > > device. > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > Device Node (261): "/dev/input/event4" > > > > Based on that you can get the udev device and work your way into any of the > > sysfs tree. Or do whatever else you want. > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > debugging tool and it does name resolution for convenience. But it's not a > > tool for complex configurations. It does exactly what it needs to do, if you > > need something that's more complicated and relies on information not > > available to the X device itself then you'll need to write a custom tool > > that does what you need. sorry. > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > on n900. So I need to run > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > xinput --set-prop --type=int 8 249 0 1 > > (or equivalent with names) so that I can use the touchscreen. (And > that's quite important -- X is somehow unusable without pointing > device). > > If xinput is not the right solution, what is the right solution? if it's reliably miscalibrated (i.e. the numbers don't change), use an xorg.conf snippet. If it needs some run-time changes add the hooks to whatever does the calibration. the X api itself is trivial, you can lift it from xinput. fwiw, you don't need to specify the type, in fact it's better not to because then libinput will just pick the right type anyway (or complain in case of mismatch). Cheers, Peter ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-21 6:42 ` Peter Hutterer @ 2016-07-21 8:54 ` Pavel Machek 2016-07-21 9:04 ` Pali Rohár 2016-07-22 0:10 ` Peter Hutterer 0 siblings, 2 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-21 8:54 UTC (permalink / raw) To: Peter Hutterer Cc: Dmitry Torokhov, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thu 2016-07-21 16:42:41, Peter Hutterer wrote: > On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > > Hi! > > > > > > In the mean time you can adjust the name or use XID instead. > > > > > > X has partially fixed this a few years ago. All input drivers (that > > > matter) export a Device Node property that sets the device node for each > > > device. > > > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > > Device Node (261): "/dev/input/event4" > > > > > > Based on that you can get the udev device and work your way into any of the > > > sysfs tree. Or do whatever else you want. > > > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > > debugging tool and it does name resolution for convenience. But it's not a > > > tool for complex configurations. It does exactly what it needs to do, if you > > > need something that's more complicated and relies on information not > > > available to the X device itself then you'll need to write a custom tool > > > that does what you need. sorry. > > > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > on n900. So I need to run > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > xinput --set-prop --type=int 8 249 0 1 > > > > (or equivalent with names) so that I can use the touchscreen. (And > > that's quite important -- X is somehow unusable without pointing > > device). > > > > If xinput is not the right solution, what is the right solution? > > if it's reliably miscalibrated (i.e. the numbers don't change), use an > xorg.conf snippet. If it needs some run-time changes add the hooks > to Does not change and is needed for all the N900's. Well. I guess xorg.conf snippet will do the trick, but that's hardly better. Should x.org have internal database saying "Nokia N900 with tsc2005 touchscreen means this calibration"? Should we have calibration info in the device tree, with kernel passing it to the x? Should kernel somehow do the calibration itself? Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-21 8:54 ` Pavel Machek @ 2016-07-21 9:04 ` Pali Rohár 2016-07-22 0:12 ` Peter Hutterer 2016-07-22 0:10 ` Peter Hutterer 1 sibling, 1 reply; 44+ messages in thread From: Pali Rohár @ 2016-07-21 9:04 UTC (permalink / raw) To: Pavel Machek Cc: Peter Hutterer, Dmitry Torokhov, Michael Welling, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thursday 21 July 2016 10:54:21 Pavel Machek wrote: > On Thu 2016-07-21 16:42:41, Peter Hutterer wrote: > > On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > > > Hi! > > > > > > > > In the mean time you can adjust the name or use XID instead. > > > > > > > > X has partially fixed this a few years ago. All input drivers (that > > > > matter) export a Device Node property that sets the device node for each > > > > device. > > > > > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > > > Device Node (261): "/dev/input/event4" > > > > > > > > Based on that you can get the udev device and work your way into any of the > > > > sysfs tree. Or do whatever else you want. > > > > > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > > > debugging tool and it does name resolution for convenience. But it's not a > > > > tool for complex configurations. It does exactly what it needs to do, if you > > > > need something that's more complicated and relies on information not > > > > available to the X device itself then you'll need to write a custom tool > > > > that does what you need. sorry. > > > > > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > > on n900. So I need to run > > > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > > xinput --set-prop --type=int 8 249 0 1 > > > > > > (or equivalent with names) so that I can use the touchscreen. (And > > > that's quite important -- X is somehow unusable without pointing > > > device). > > > > > > If xinput is not the right solution, what is the right solution? > > > > if it's reliably miscalibrated (i.e. the numbers don't change), use an > > xorg.conf snippet. If it needs some run-time changes add the hooks > > to > > Does not change and is needed for all the N900's. > > Well. I guess xorg.conf snippet will do the trick, but that's hardly > better. > > Should x.org have internal database saying "Nokia N900 with tsc2005 > touchscreen means this calibration"? > > Should we have calibration info in the device tree, with kernel > passing it to the x? > > Should kernel somehow do the calibration itself? >From my memory how this problem is solved on Maemo 5: There is XML snippet of HAL file which contains xorg properties for input driver. Xorg server loads from HAL xorg settings and somehow propagate them. That file is generated either from default system data (those comes from DEB package) or from user config file (that is generated from Settings application) or from CAL partition (NAND partition which contains device/product specific calibration data). Because it is read also from CAL, I need to say those data does not have to be same for all N900 devices. And because there is Settings application which can re-calibrate touchscreen, those data are not even static. Now HAL is deprecated, but I suspect that xorg.conf.d/ directory or UDEV can be used in same way to propagate device specific settings for touchscreen device... So... if we decide that it is good idea to put calibration data (either to kernel driver/N900 DTS or xorg project), first we need to somehow check and verify that those default calibration data are good for most (or all) N900. Or check on more N900 how differs data in CAL partition (maybe they are really static??). -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-21 9:04 ` Pali Rohár @ 2016-07-22 0:12 ` Peter Hutterer 2016-07-25 14:59 ` Pali Rohár 0 siblings, 1 reply; 44+ messages in thread From: Peter Hutterer @ 2016-07-22 0:12 UTC (permalink / raw) To: Pali Rohár Cc: Pavel Machek, Dmitry Torokhov, Michael Welling, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thu, Jul 21, 2016 at 11:04:29AM +0200, Pali Rohár wrote: > On Thursday 21 July 2016 10:54:21 Pavel Machek wrote: > > On Thu 2016-07-21 16:42:41, Peter Hutterer wrote: > > > On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > > > > Hi! > > > > > > > > > > In the mean time you can adjust the name or use XID instead. > > > > > > > > > > X has partially fixed this a few years ago. All input drivers (that > > > > > matter) export a Device Node property that sets the device node for each > > > > > device. > > > > > > > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > > > > Device Node (261): "/dev/input/event4" > > > > > > > > > > Based on that you can get the udev device and work your way into any of the > > > > > sysfs tree. Or do whatever else you want. > > > > > > > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > > > > debugging tool and it does name resolution for convenience. But it's not a > > > > > tool for complex configurations. It does exactly what it needs to do, if you > > > > > need something that's more complicated and relies on information not > > > > > available to the X device itself then you'll need to write a custom tool > > > > > that does what you need. sorry. > > > > > > > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > > > on n900. So I need to run > > > > > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > > > xinput --set-prop --type=int 8 249 0 1 > > > > > > > > (or equivalent with names) so that I can use the touchscreen. (And > > > > that's quite important -- X is somehow unusable without pointing > > > > device). > > > > > > > > If xinput is not the right solution, what is the right solution? > > > > > > if it's reliably miscalibrated (i.e. the numbers don't change), use an > > > xorg.conf snippet. If it needs some run-time changes add the hooks > > > to > > > > Does not change and is needed for all the N900's. > > > > Well. I guess xorg.conf snippet will do the trick, but that's hardly > > better. > > > > Should x.org have internal database saying "Nokia N900 with tsc2005 > > touchscreen means this calibration"? > > > > Should we have calibration info in the device tree, with kernel > > passing it to the x? > > > > Should kernel somehow do the calibration itself? > > From my memory how this problem is solved on Maemo 5: > > There is XML snippet of HAL file which contains xorg properties for > input driver. Xorg server loads from HAL xorg settings and somehow > propagate them. That file is generated either from default system data > (those comes from DEB package) or from user config file (that is > generated from Settings application) or from CAL partition (NAND > partition which contains device/product specific calibration data). > > Because it is read also from CAL, I need to say those data does not have > to be same for all N900 devices. > > And because there is Settings application which can re-calibrate > touchscreen, those data are not even static. > > Now HAL is deprecated, but I suspect that xorg.conf.d/ directory or UDEV > can be used in same way to propagate device specific settings for > touchscreen device... yes, xorg.conf.d snippets replaced HAL configuration, with pretty much the same functionality. Using udev is not generally recommended. Cheers, Peter > So... if we decide that it is good idea to put calibration data (either > to kernel driver/N900 DTS or xorg project), first we need to somehow > check and verify that those default calibration data are good for most > (or all) N900. Or check on more N900 how differs data in CAL partition > (maybe they are really static??). > > -- > Pali Rohár > pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-22 0:12 ` Peter Hutterer @ 2016-07-25 14:59 ` Pali Rohár 2016-07-31 21:28 ` Peter Hutterer 0 siblings, 1 reply; 44+ messages in thread From: Pali Rohár @ 2016-07-25 14:59 UTC (permalink / raw) To: Peter Hutterer Cc: Pavel Machek, Dmitry Torokhov, Michael Welling, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Friday 22 July 2016 10:12:19 Peter Hutterer wrote: > On Thu, Jul 21, 2016 at 11:04:29AM +0200, Pali Rohár wrote: > > On Thursday 21 July 2016 10:54:21 Pavel Machek wrote: > > > On Thu 2016-07-21 16:42:41, Peter Hutterer wrote: > > > > On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > > > > > Hi! > > > > > > > > > > > > In the mean time you can adjust the name or use XID instead. > > > > > > > > > > > > X has partially fixed this a few years ago. All input drivers (that > > > > > > matter) export a Device Node property that sets the device node for each > > > > > > device. > > > > > > > > > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > > > > > Device Node (261): "/dev/input/event4" > > > > > > > > > > > > Based on that you can get the udev device and work your way into any of the > > > > > > sysfs tree. Or do whatever else you want. > > > > > > > > > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > > > > > debugging tool and it does name resolution for convenience. But it's not a > > > > > > tool for complex configurations. It does exactly what it needs to do, if you > > > > > > need something that's more complicated and relies on information not > > > > > > available to the X device itself then you'll need to write a custom tool > > > > > > that does what you need. sorry. > > > > > > > > > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > > > > on n900. So I need to run > > > > > > > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > > > > xinput --set-prop --type=int 8 249 0 1 > > > > > > > > > > (or equivalent with names) so that I can use the touchscreen. (And > > > > > that's quite important -- X is somehow unusable without pointing > > > > > device). > > > > > > > > > > If xinput is not the right solution, what is the right solution? > > > > > > > > if it's reliably miscalibrated (i.e. the numbers don't change), use an > > > > xorg.conf snippet. If it needs some run-time changes add the hooks > > > > to > > > > > > Does not change and is needed for all the N900's. > > > > > > Well. I guess xorg.conf snippet will do the trick, but that's hardly > > > better. > > > > > > Should x.org have internal database saying "Nokia N900 with tsc2005 > > > touchscreen means this calibration"? > > > > > > Should we have calibration info in the device tree, with kernel > > > passing it to the x? > > > > > > Should kernel somehow do the calibration itself? > > > > From my memory how this problem is solved on Maemo 5: > > > > There is XML snippet of HAL file which contains xorg properties for > > input driver. Xorg server loads from HAL xorg settings and somehow > > propagate them. That file is generated either from default system data > > (those comes from DEB package) or from user config file (that is > > generated from Settings application) or from CAL partition (NAND > > partition which contains device/product specific calibration data). > > > > Because it is read also from CAL, I need to say those data does not have > > to be same for all N900 devices. > > > > And because there is Settings application which can re-calibrate > > touchscreen, those data are not even static. > > > > Now HAL is deprecated, but I suspect that xorg.conf.d/ directory or UDEV > > can be used in same way to propagate device specific settings for > > touchscreen device... > > yes, xorg.conf.d snippets replaced HAL configuration, with pretty much the > same functionality. Using udev is not generally recommended. In case that calibration data are stored in different format as xorg.conf.d accept (and these data can be changed), what is preferred way for pushing these calibration into X server? I thought that udev could be right way as it contains key/value properties in unified format (not X specific) and lot of other helpers fill these data for different devices. Why is is not generally recommended? -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-25 14:59 ` Pali Rohár @ 2016-07-31 21:28 ` Peter Hutterer 0 siblings, 0 replies; 44+ messages in thread From: Peter Hutterer @ 2016-07-31 21:28 UTC (permalink / raw) To: Pali Rohár Cc: Pavel Machek, Dmitry Torokhov, Michael Welling, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Mon, Jul 25, 2016 at 04:59:17PM +0200, Pali Rohár wrote: > On Friday 22 July 2016 10:12:19 Peter Hutterer wrote: > > On Thu, Jul 21, 2016 at 11:04:29AM +0200, Pali Rohár wrote: > > > On Thursday 21 July 2016 10:54:21 Pavel Machek wrote: > > > > On Thu 2016-07-21 16:42:41, Peter Hutterer wrote: > > > > > On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > > > > > > Hi! > > > > > > > > > > > > > > In the mean time you can adjust the name or use XID instead. > > > > > > > > > > > > > > X has partially fixed this a few years ago. All input drivers (that > > > > > > > matter) export a Device Node property that sets the device node for each > > > > > > > device. > > > > > > > > > > > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > > > > > > Device Node (261): "/dev/input/event4" > > > > > > > > > > > > > > Based on that you can get the udev device and work your way into any of the > > > > > > > sysfs tree. Or do whatever else you want. > > > > > > > > > > > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > > > > > > debugging tool and it does name resolution for convenience. But it's not a > > > > > > > tool for complex configurations. It does exactly what it needs to do, if you > > > > > > > need something that's more complicated and relies on information not > > > > > > > available to the X device itself then you'll need to write a custom tool > > > > > > > that does what you need. sorry. > > > > > > > > > > > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > > > > > on n900. So I need to run > > > > > > > > > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > > > > > xinput --set-prop --type=int 8 249 0 1 > > > > > > > > > > > > (or equivalent with names) so that I can use the touchscreen. (And > > > > > > that's quite important -- X is somehow unusable without pointing > > > > > > device). > > > > > > > > > > > > If xinput is not the right solution, what is the right solution? > > > > > > > > > > if it's reliably miscalibrated (i.e. the numbers don't change), use an > > > > > xorg.conf snippet. If it needs some run-time changes add the hooks > > > > > to > > > > > > > > Does not change and is needed for all the N900's. > > > > > > > > Well. I guess xorg.conf snippet will do the trick, but that's hardly > > > > better. > > > > > > > > Should x.org have internal database saying "Nokia N900 with tsc2005 > > > > touchscreen means this calibration"? > > > > > > > > Should we have calibration info in the device tree, with kernel > > > > passing it to the x? > > > > > > > > Should kernel somehow do the calibration itself? > > > > > > From my memory how this problem is solved on Maemo 5: > > > > > > There is XML snippet of HAL file which contains xorg properties for > > > input driver. Xorg server loads from HAL xorg settings and somehow > > > propagate them. That file is generated either from default system data > > > (those comes from DEB package) or from user config file (that is > > > generated from Settings application) or from CAL partition (NAND > > > partition which contains device/product specific calibration data). > > > > > > Because it is read also from CAL, I need to say those data does not have > > > to be same for all N900 devices. > > > > > > And because there is Settings application which can re-calibrate > > > touchscreen, those data are not even static. > > > > > > Now HAL is deprecated, but I suspect that xorg.conf.d/ directory or UDEV > > > can be used in same way to propagate device specific settings for > > > touchscreen device... > > > > yes, xorg.conf.d snippets replaced HAL configuration, with pretty much the > > same functionality. Using udev is not generally recommended. > > In case that calibration data are stored in different format as > xorg.conf.d accept (and these data can be changed), what is preferred > way for pushing these calibration into X server? write out an xorg.conf.d snippet file that contains the required options. this is what we already do for system-wide keyboard layouts (see localectl). > I thought that udev could be right way as it contains key/value > properties in unified format (not X specific) and lot of other helpers > fill these data for different devices. there is no "unified format" for configurations. you still need to agree on the key name and value format. e.g. for a calibration matrix: is it 6 numbers or all 9? is it in floats, percent, or device units? in the end what you'd be doing is just push the driver-specific configuration to udev. that's mostly fine for some values, not so great for others where the usage isn't as clear (see the commend about libinput below). > Why is is not generally recommended? because udev is not a storage mechanism for X. we made the mistake in earlier X server versions to use HAL as config storage but there is a distinct mismatch when you have xorg.conf and some external config data feeding into the server. since server 1.8 (~2008 or so) we don't use configuration data in udev. fwiw, libinput has a slightly different approach here, we do push *some* device configuration into udev but only those bits that are considered hardware properties and thus both immutable and required by anything else that wants to use the device. examples are fixes to the axis ranges, tags to identify the device type, etc. Cheers, Peter ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-21 8:54 ` Pavel Machek 2016-07-21 9:04 ` Pali Rohár @ 2016-07-22 0:10 ` Peter Hutterer 2016-07-22 0:57 ` Dmitry Torokhov 1 sibling, 1 reply; 44+ messages in thread From: Peter Hutterer @ 2016-07-22 0:10 UTC (permalink / raw) To: Pavel Machek Cc: Dmitry Torokhov, Michael Welling, kernel list, linux-input, pali.rohar, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thu, Jul 21, 2016 at 10:54:21AM +0200, Pavel Machek wrote: > On Thu 2016-07-21 16:42:41, Peter Hutterer wrote: > > On Thu, Jul 21, 2016 at 08:32:34AM +0200, Pavel Machek wrote: > > > Hi! > > > > > > > > In the mean time you can adjust the name or use XID instead. > > > > > > > > X has partially fixed this a few years ago. All input drivers (that > > > > matter) export a Device Node property that sets the device node for each > > > > device. > > > > > > > > $ xinput list-props "SynPS/2 Synaptics TouchPad" | grep "Device Node" > > > > Device Node (261): "/dev/input/event4" > > > > > > > > Based on that you can get the udev device and work your way into any of the > > > > sysfs tree. Or do whatever else you want. > > > > > > > > But other than that there isn't anything in X to fix. xinput is primarily a > > > > debugging tool and it does name resolution for convenience. But it's not a > > > > tool for complex configurations. It does exactly what it needs to do, if you > > > > need something that's more complicated and relies on information not > > > > available to the X device itself then you'll need to write a custom tool > > > > that does what you need. sorry. > > > > > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > > on n900. So I need to run > > > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > > xinput --set-prop --type=int 8 249 0 1 > > > > > > (or equivalent with names) so that I can use the touchscreen. (And > > > that's quite important -- X is somehow unusable without pointing > > > device). > > > > > > If xinput is not the right solution, what is the right solution? > > > > if it's reliably miscalibrated (i.e. the numbers don't change), use an > > xorg.conf snippet. If it needs some run-time changes add the hooks > > to > > Does not change and is needed for all the N900's. > > Well. I guess xorg.conf snippet will do the trick, but that's hardly > better. it's a lot better than running xinput. with the config snippet have the device ready to go at server startup, across VT switches, server restarts and device unplugs. for something static, especially if it's custom hardware like this shipping an xorg.conf file is always preferable over xinput scripts > Should x.org have internal database saying "Nokia N900 with tsc2005 > touchscreen means this calibration"? we sort-of do have this, look at xserver/config/10-quirks.conf problem is though that we'd need to match on the N900 too unless the tsc2005 is used nowhere else > Should we have calibration info in the device tree, with kernel > passing it to the x? > > Should kernel somehow do the calibration itself? if the kernel knows about the calibration it would make more sense to just do it in the kernel directly. but for anything even remotely run-time or user-configured the xorg.conf snippet is the best solution. Cheers, Peter ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-22 0:10 ` Peter Hutterer @ 2016-07-22 0:57 ` Dmitry Torokhov 0 siblings, 0 replies; 44+ messages in thread From: Dmitry Torokhov @ 2016-07-22 0:57 UTC (permalink / raw) To: Peter Hutterer Cc: Pavel Machek, Michael Welling, kernel list, linux-input, Pali Rohár, Sebastian Reichel, aaro.koskinen, Ivaylo Dimitrov, Patrik Bachan, serge On Thu, Jul 21, 2016 at 5:10 PM, Peter Hutterer <peter.hutterer@who-t.net> wrote: > On Thu, Jul 21, 2016 at 10:54:21AM +0200, Pavel Machek wrote: >> Should we have calibration info in the device tree, with kernel >> passing it to the x? >> >> Should kernel somehow do the calibration itself? > > if the kernel knows about the calibration it would make more sense to just > do it in the kernel directly. Historically we relied on userspace to perform calibration/transformation. Lately we've been introducing trivial transformations (inversion and axes swapping) and we allow plumbing this though DT; the rest I think is still better done in userspace (where you can do floating point, if desired). > but for anything even remotely run-time or > user-configured the xorg.conf snippet is the best solution. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-21 6:32 ` Pavel Machek 2016-07-21 6:42 ` Peter Hutterer @ 2016-07-25 14:56 ` Pali Rohár 2016-07-28 19:33 ` Pavel Machek 1 sibling, 1 reply; 44+ messages in thread From: Pali Rohár @ 2016-07-25 14:56 UTC (permalink / raw) To: Pavel Machek Cc: Peter Hutterer, Dmitry Torokhov, Michael Welling, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge On Thursday 21 July 2016 08:32:34 Pavel Machek wrote: > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > on n900. So I need to run > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > xinput --set-prop --type=int 8 249 0 1 What these commands do? Or how they change driver behavior? As Dmitry wrote, it is possible to move at list "upside down" switch to kernel/DTS? I think that every N900 touchscreen acts "upside down" by default... -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: v4.1 to v4.7: regression in tsc2005 driver 2016-07-25 14:56 ` Pali Rohár @ 2016-07-28 19:33 ` Pavel Machek 0 siblings, 0 replies; 44+ messages in thread From: Pavel Machek @ 2016-07-28 19:33 UTC (permalink / raw) To: Pali Rohár Cc: Peter Hutterer, Dmitry Torokhov, Michael Welling, kernel list, linux-input, sre, aaro.koskinen, ivo.g.dimitrov.75, patrikbachan, serge Hi! On Mon 2016-07-25 16:56:20, Pali Rohár wrote: > On Thursday 21 July 2016 08:32:34 Pavel Machek wrote: > > Ok.. so out of the box, touchscreen is "upside down" and miscalibrated > > on n900. So I need to run > > > > xinput --set-prop --type=float 8 115 1.10 0.00 -0.05 0.00 1.18 -0.10 0.00 0.00 1.00 > > xinput --set-prop --type=int 8 249 0 1 > > What these commands do? Or how they change driver behavior? > As Dmitry wrote, it is possible to move at list "upside down" switch to > kernel/DTS? I think that every N900 touchscreen acts "upside down" by > default... I don't think that's enough. Its upside down + slightly shifted. If you have time you can try, but... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2016-07-31 22:55 UTC | newest] Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-17 17:52 v4.1 to v4.7: regression in tsc2005 driver Pavel Machek 2016-07-17 18:24 ` Michael Welling 2016-07-17 18:42 ` Pavel Machek 2016-07-17 18:51 ` Michael Welling 2016-07-17 20:03 ` Pavel Machek 2016-07-17 22:56 ` Michael Welling 2016-07-19 23:51 ` Dmitry Torokhov 2016-07-20 0:39 ` Michael Welling 2016-07-20 0:53 ` Dmitry Torokhov 2016-07-20 1:34 ` Michael Welling 2016-07-20 1:44 ` Dmitry Torokhov 2016-07-20 2:09 ` Michael Welling 2016-07-20 3:49 ` [PATCH] Input: tsc200x - Report proper input_dev name Michael Welling 2016-07-20 6:31 ` Pavel Machek 2016-07-20 6:50 ` Michael Welling 2016-07-20 6:54 ` Pavel Machek 2016-07-20 7:06 ` Michael Welling 2016-07-20 7:48 ` Pavel Machek 2016-07-20 16:45 ` Dmitry Torokhov 2016-07-20 16:56 ` Pali Rohár 2016-07-20 17:04 ` Dmitry Torokhov 2016-07-20 17:14 ` Dmitry Torokhov 2016-07-20 20:25 ` Pavel Machek 2016-07-20 20:37 ` Pali Rohár 2016-07-20 16:33 ` v4.1 to v4.7: regression in tsc2005 driver Pali Rohár 2016-07-20 1:26 ` Aaro Koskinen 2016-07-20 2:18 ` Michael Welling 2016-07-20 6:25 ` Pavel Machek 2016-07-20 16:23 ` Dmitry Torokhov 2016-07-20 20:22 ` Pavel Machek 2016-07-20 21:47 ` Peter Hutterer 2016-07-20 22:20 ` Dmitry Torokhov 2016-07-20 22:55 ` Peter Hutterer 2016-07-21 6:32 ` Pavel Machek 2016-07-21 6:42 ` Peter Hutterer 2016-07-21 8:54 ` Pavel Machek 2016-07-21 9:04 ` Pali Rohár 2016-07-22 0:12 ` Peter Hutterer 2016-07-25 14:59 ` Pali Rohár 2016-07-31 21:28 ` Peter Hutterer 2016-07-22 0:10 ` Peter Hutterer 2016-07-22 0:57 ` Dmitry Torokhov 2016-07-25 14:56 ` Pali Rohár 2016-07-28 19:33 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).