linux-kernel - Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 15 May 2023 11:08:10 +0200
From:   Javier Carrasco <javier.carrasco@...fvision.net>
To:     Krzysztof Kozlowski <krzk@...nel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Henrik Rydberg <rydberg@...math.org>,
        Bastian Hecht <hechtb@...il.com>,
        Michael Riesch <michael.riesch@...fvision.net>
Cc:     linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        oe-kbuild-all@...ts.linux.dev
Subject: Re: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons
 handling



On 15.05.23 10:41, Krzysztof Kozlowski wrote:
> On 15/05/2023 10:33, Javier Carrasco wrote:
>>>> All these functions are declared in the linux/input/ts-virtobj.h header
>>>> and also inline-defined there if ts-virtobj is not selected. If it is
>>>> selected (either y or M), the functions are exported from
>>>> driver/input/touchscreen/ts-virtobj.c. According to the error report,
>>>> ts-virtobj was selected as a module.
>>>>
>>>> I could build the kernel with the three possible configurations
>>>> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or
>>>> warnings repeatedly, so I am a bit confused now. I am probably
>>>> missing something, but I do not know what.
>>>>
>>>> I wonder if the new file where the functions are defined (ts-virtobj.c)
>>>> could not be found by some reason or if the test build does not like the
>>>> way I handled the function declaration/definition. Any hint or advice
>>>> would be more than welcome.
>>>
>>> The report is correct. Build driver builtin and your virtual as module.
>>
>> You are right, that was the configuration I was missing, which I
>> honestly did not even consider when I tested this feature.
>>
>> The problem seems to be that I am using the IS_ENABLED macro which
>> returns true if selected as a module, but the define is in that case
>> CONFIG_TOUCHSCREEN_TS_VIRTOBJ_MODULE instead of
>> CONFIG_TOUCHSCREEN_TS_VIRTOBJ. As I am using the latter define in the
>> Makefile, it does not get compiled.
> 
> This could be proper solution for build failure but does not look like
> the proper solution for entire choice/design. The questions are: why
> TOUCHSCREEN_TS_VIRTOBJ should be selectable by user? How can user know
> that he needs a driver with VIRTOBJ?
> 
> I think he cannot and your config help option suggests that:
> "you are using a touchscreen driver that supports printed overlays".
> What driver supports or not, is a job for kernel developers. Not for
> kernel configurators or distros. User should never investigate whether
> his touchscreen driver support printed overlays. Why would user like to
> dig into kernel to know that? Thus either your driver should select
> VIRTOBJ or depend on it.
> 
I was thinking about the minimal savings (inline functions that just
return a value and the size of the ts-virtobj object itself) that could
be obtained if the touchscreen does not need the functionality at all.
But probably those savings are negligible and there will not be many
cases where the user will know if that applies.

I agree, the feature will be selected by drivers that support it. That
actually simplifies the code a little bit.

> Best regards,
> Krzysztof
>