v4.1 to v4.7: regression in tsc2005 driver

v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Dmitry Torokhov]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Dmitry Torokhov]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Aaro Koskinen]

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.

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Dmitry Torokhov]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Michael Welling]

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.

[PATCH] Input: tsc200x - Report proper input_dev name

[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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Pavel Machek]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Michael Welling]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Pavel Machek]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Michael Welling]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Dmitry Torokhov]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

[-- 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 --]

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Dmitry Torokhov]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Pali Rohár]

[-- 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 --]

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Dmitry Torokhov]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Dmitry Torokhov]

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);

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Pavel Machek]

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

Re: [PATCH] Input: tsc200x - Report proper input_dev name

[Pali Rohár]

[-- 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 --]

Re: v4.1 to v4.7: regression in tsc2005 driver

[Peter Hutterer]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Dmitry Torokhov]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Peter Hutterer]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Peter Hutterer]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v4.1 to v4.7: regression in tsc2005 driver

[Peter Hutterer]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Peter Hutterer]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Dmitry Torokhov]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pali Rohár]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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

Re: v4.1 to v4.7: regression in tsc2005 driver

[Pavel Machek]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v4.1 to v4.7: regression in tsc2005 driver

[Peter Hutterer]

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