Date:   Sun, 19 Feb 2017 23:01:20 +0100
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Pavel Machek <pavel@....cz>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Sebastian Reichel <sre@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Benoît Cousson <bcousson@...libre.com>,
        Tony Lindgren <tony@...mide.com>,
        Russell King <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        Michael Welling <mwelling@...e.org>,
        Mika Penttilä <mika.penttila@...tfour.com>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Igor Grinberg <grinberg@...pulab.co.il>,
        "Andrew F. Davis" <afd@...com>, Mark Brown <broonie@...nel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Alexander Stein <alexander.stein@...tec-electronic.com>,
        Eric Engestrom <eric@...estrom.ch>,
        Hans de Goede <hdegoede@...hat.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Petr Cvek <petr.cvek@....cz>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Nick Dyer <nick@...anahar.org>,
        Siebren Vroegindeweij <siebren.vroegindeweij@...mail.com>,
        Michel Verlaan <michel.verl@...il.com>,
        linux-input@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
        letux-kernel@...nphoenux.org, linux-iio@...r.kernel.org,
        kernel@...a-handheld.com, Aaro Koskinen <aaro.koskinen@...ia.com>,
        Pali Rohár <pali.rohar@...il.com>,
        Andrey Gelman <andrey.gelman@...pulab.co.il>,
        Haibo Chen <haibo.chen@...escale.com>
Subject: Re: [PATCH v9 1/8] drivers:input:tsc2007: add new common binding names, pre-calibration, flipping and rotation

Hi Pavel,

> Am 19.02.2017 um 21:57 schrieb Pavel Machek <pavel@....cz>:
> 
> Hi!
> 
> 
>> But as said I don't think we need float or fixed point for practical systems
>> at all.
> 
> So you are going to loose precision. And if userspace decides to
> calibrate it slightly differently from kernel, lost precision will
> matter.

Really?

Example:

ADC values go 100 .. 3995 (i.e. touch margin is 100 steps in pre-calibration)

This is scaled to let's say 0..640.

Now you find your touch is misaligned by 1% which is 6.4 pixels but barely
noticeable on a 5cm screen (1% is 0.5mm).

So you simply subtract 6 from all coordinates and the screen is aligned again.
Does the 0.4 pixel deviation in precision (= 0.2mm) matter?

If you do this scaling from ADC range 0..4095 to 0..640 in user-space, you will
find that you have to subtract 59 from the ADC values and then scale by 0.16431.
The result will be almost the same and deviate in the micrometer range. How big
is your stylus or finger?

We learn: a real world touch screen is not a high precision instrument.

> 
>>>>> and floating point in kernel is hard. Also
>>>>> you'd either have to invent new interface, or you'd break touchscreen
>>>>> for people that already have their touchscreens calibrated.
>>>> 
>>>> No, I don't break calibration for people using a different chip.
>>> 
>>> So you propose your touchscreen to behave differently from all other
>>> touchscreens in tree?
>> 
>> No. I only propose that my touch screen behaves properly and in the
>> best way it can. If others are worse, they should also be improved at
>> some time.
> 
> Different from all other drivers. Read: broken.

If something differs from requirements or a spec, then it is broken.

Not if it differs from other drivers (implementations). They may be broken
as well or follow different requirements.

Note:
1. the touch screen bindings do ask to scale to screen size (please read
http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt )

2. if you don't use these features the driver behaves exactly as it was
before by using defaults and like other drivers which do (not yet) have it

> 
> No. You have to design interface such that they _can_ be improved, and
> what you propose does not work that way.

It works. Please do real world tests...

> 
>> And note that I am not making things different from others in tree, I
>> am making the tsc2007 right (incl. following the touchscreen bindings
>> which define the touchscreen size in "Pixels").
> 
> Your touch screen is not in any way special, so it has to behave in
> the same way others do.

It does. Plus some new features which are missing...

> 
>>> That's just no-go.
>> 
>> In other words: you want to block any improvements unless your favourite
>> touchscreen is giving directions...
> 
> Yes. I want to prevent you from pushing crap into the kernel.

Crap? Well, we have discussed this driver for months here on the list and
after a lot of improvements we came up to v9.

And you still think it is crap and none of the other reviewers has noticed?

> 
> If you want to improve it in reasonable way, you know what to do.
> 
>>>> You seem to repeat yourself and just say which solution you prefer,
>>>> but I am missing the arguments why your solution (Pass calibration data
>>>> to userland) is right and the best one.
>>>> Which problems does it solve?
>>> 
>>> All you described.
>> 
>> I think you are missing one problem: providing already properly scaled touch
>> values to user space.
> 
> I'm not.

You are. Because you said: "All you described." And I had described that as well.

So you were either wrong before or now.

> Userspace has to know how to do the calibration _anyway_ (for
> other hardware),

What for? I do not understand which other hardware you are talking about.

On our devices there is only one touch glued to the panel and that one
has to be calibrated. Ideally before the user gets the device into his
hands => precalibration...

If you connect a digitizer, then that one has to be calibrated of course,
but it is not glued onto the display panel. Hence it is a different issue.

Please describe the use case / scenario you are thinking of.

> so giving scaled values to userspace is useless.

This is another example why I love to discuss with you :)

You are so wonderful in ignoring my explanations during the past 20 mails
where I described exactly and in detail why it is useful for me, for
others, for users of the GTA04 for the upcoming Pyra device... So at
least 2000 or more persons.

One daily user of these devices and kernel contributor already had commented his view.

IMHO, only Pavel Machek is thinking it is useless. Therefore he concludes it must be
useless for everybody else as well.

> 
>>>> How can you implement it in
>>>> a stable and portable way?
>>> 
>>> Easily.
>> 
>> Please go ahead and show code.
> 
> You don't get to tell me what to do, unless you pay me.

Same for me.

But: you get our contributions for free. Unpaid work, we did put into developing
these patches for the benefit of mankind.

> 
> You want to break kernel, you do coding. Or pay someone else,
> preferably someone who knows how to design kernel code.

Ah, so that's the way the wind is blowing...

You are trying to block community contributions to get a paid job by pretending to do
it better :)

This is a quite clever attempt to ruin the open source, volunteer and community idea.

It is defined as "building from and giving back to the community". We could easily run
our own fork of Linux and ignore upstreaming, but I believe that we should give back
something to those who have developed the other parts of Linux we simply use. And you
want to exclude us from this? Nonono.

It is really funny to discuss with you and I can still learn how to work around really
answering questions, accepting that other people have different requirements and ignoring
technical arguments and answers given to form a big picture.

So just derailing this discussion because you want to be paid is not at all fair.
But I can withstand this.

Please give me convincing technical arguments (which include our requirements and not
only yours) like other reviewers do and we will change code.

BR and thanks,
Nikolaus


Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)

Powered by blists - more mailing lists