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

[Javier Carrasco]

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
>