Re: [PATCH 3/3] platform/surface: Add OF support

[Maximilian Luz <luzmaximilian@gmail.com>]

On 8/10/24 12:44 AM, Konrad Dybcio wrote:
> On 9.08.2024 8:09 PM, Maximilian Luz wrote:
>>>    +static int ssam_serdev_setup(struct acpi_device *ssh, struct serdev_device *serdev)
>>> +{
>>> +    if (ssh)
>>> +        return ssam_serdev_setup_via_acpi(serdev, ssh->handle);
>>> +
>>> +    /* TODO: these values may differ per board/implementation */
>>> +    serdev_device_set_baudrate(serdev, 4 * HZ_PER_MHZ);
>>
>> Isn't this defined in the DT spec that you're adding as "current-speed"
>> in patch 2? Why not load it from there?
> 
> Yeah and it's not under `required:`.. i added it for future proofing

Okay.

[...]

>>> +static const struct software_node *ssam_node_group_sl7[] = {
>>> +    &ssam_node_root,
>>> +    &ssam_node_bat_ac,
>>> +    &ssam_node_bat_main,
>>> +    &ssam_node_tmp_perf_profile_with_fan,
>>> +    &ssam_node_fan_speed,
>>> +    &ssam_node_hid_sam_keyboard,
>>
>> Did you check if there are any other HID devices connected? In the past,
>> keyboard and touchpad have been split into two separate devices, so is
>> it a combo keyboard + touchpad device this time? Some models also had
>> HID-based sensor and other devices.
> 
> No, touchpad is wired directly to the SoC via QSPI, same for the touch
> panel

Ah I see. So somewhat similar to the SLS2 I believe. Does it work with
multiple fingers out-of-the-box? Or does it send raw heatmaps like on
the SLS2 that require post-processing?

Maybe some background: Since quite some time, the touchscreens on
Surface devices operate in two modes: Basic single-touch no-post-
processing-required mode or multi-touch mode where it sends raw touch
heatmaps to be processed by some driver or user-space application. So
basically, the whole touch processing stack is shifted to the software
side. And with the SLS2, they now even applied that same thing to the
touchpad... We're trying to replicate that (user-space) processing
with the IPTSd daemon, but there's still a bit of work to be done to
make this reliable.

> 
>>
>> Would just be good to know if this can be assumed to be complete or if
>> we're maybe missing something here.
>>
>>> +    /* TODO: evaluate thermal sensors devices when we get a driver for that */
>>
>> FYI I've posted the driver at [1]. It needs a small Kbuild dependency
>> fix but apart from that I think it should be final, if you want to give
>> that a try.
>>
>> [1]: https://lore.kernel.org/lkml/20240804230832.247852-1-luzmaximilian@gmail.com/T/
> 
> I'll give it a shot, thanks
> 
>>
>> The rest looks fine. I'll try to find some time to update my SPX branch
>> this weekend and give it a spin.
> 
> About time that thing lands upstream ;)

Agreed :) Thanks for taking this up!

Best regards,
Max