Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs

[Aditya Garg <gargaditya08@live.com>]

Hi

> On 19 Feb 2025, at 1:27 PM, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
> Hi
> 
> Am 18.02.25 um 21:12 schrieb Aditya Garg:
>> Hi
>> 
>> In continuation to my previous mail.
>> 
>>>> +
>>>> +static int appletbdrm_send_msg(struct appletbdrm_device *adev, u32 msg)
>>>> +{
>>>> + struct appletbdrm_msg_simple_request *request;
>>>> + int ret;
>>>> +
>>>> + request = kzalloc(sizeof(*request), GFP_KERNEL);
>>>> + if (!request)
>>>> + return -ENOMEM;
>>>> +
>>>> + request->header.unk_00 = cpu_to_le16(2);
>>>> + request->header.unk_02 = cpu_to_le16(0x1512);
>>>> + request->header.size = cpu_to_le32(sizeof(*request) - sizeof(request->header));
>>>> + request->msg = msg;
>>>> + request->size = request->header.size;
>>>> +
>>>> + ret = appletbdrm_send_request(adev, &request->header, sizeof(*request));
>>>> +
>>>> + kfree(request);
>>> This is temporary data for the send operation and save to free here?
>> Probably yes. If I understand correctly, it’s needed to make the touchbar go into the display mode, from the hid keyboard mode.
>> 
>> We here are doing the same as the Windows driver [1] for this does.
>> 
>> [1] https://github.com/imbushuo/DFRDisplayKm/blob/master/src/DFRDisplayKm/include/Dfr.h#L3
> 
> Yeah. My concern was that request is being freed while the USB send operation is still using it. But in the USB code, it doesn't look like that.
> 

For this, we are using usb_bulk_msg [1] which is a synchronous function, so it only returns once the data is completely sent.

So IMO its safe to kfree(request); here.

[1] https://manpages.debian.org/jessie-backports/linux-manual-4.8/usb_bulk_msg.9