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]
Message-ID: <0960ece8-02e6-468f-9ee6-38e529939d35@collabora.com>
Date: Tue, 10 Feb 2026 12:31:29 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Luca Leonardo Scorcia <l.scorcia@...il.com>
Cc: linux-mediatek@...ts.infradead.org,
 Chun-Kuang Hu <chunkuang.hu@...nel.org>,
 Philipp Zabel <p.zabel@...gutronix.de>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Matthias Brugger <matthias.bgg@...il.com>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking
 mipi_dsi_host_register

Il 09/02/26 19:46, Luca Leonardo Scorcia ha scritto:
> Il giorno lun 9 feb 2026 alle ore 12:40 AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> ha scritto:
>>
>> Il 09/02/26 10:05, Luca Leonardo Scorcia ha scritto:
>>> The call to mipi_dsi_host_register triggers a callback to mtk_dsi_bind,
>>> which uses dev_get_drvdata to retrieve the mtk_dsi struct, so this
>>> structure needs to be stored inside the driver data before invoking it.
>>>
>>> As drvdata is currently uninitialized it leads to a crash when
>>> registering the DSI DRM encoder right after acquiring
>>> the mode_config.idr_mutex, blocking all subsequent DRM operations.
>>>
>>> Fixes the following crash during mediatek-drm probe (tested on Xiaomi
>>> Smart Clock x04g):
>>>
>>> Unable to handle kernel NULL pointer dereference at virtual address
>>>    0000000000000040
>>> [...]
>>> ---[ end trace 0000000000000000 ]---
>>>
>>> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@...il.com>
>>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>>
>> ... but wait, what have I just read? Xiaomi Smart Clock?! MT8167?
>>
>> Are you running upstream on this device?
>> If so, why don't you also contribute a nice devicetree to get this device finally
>> upstream?!?! :-)
> 
> That's correct! Most of the work had already been done by the
> wonderful people at postmarketOS, I just had to connect the dots. Some
> crucial patches (mt6392 regulator, mt8167 display nodes) were
> submitted in the past but weren't merged for various reasons.
> 
> Just rebased on v6.19: https://pasteboard.co/6bXQeWlwUqmz.jpg

Oh that's so cool!

> 
> I am trying to get it working as much as I can, then cleanup and
> submit. At the moment storage, usb networking, rtc, buttons, screen,
> touch, backlight, light/presence sensors, drm and gce are all working;
> still missing wifi/bt, audio and cpuidle.
> 

You got an astonishingly big amount of features working fine then.
That is practically almost everything.

I do encourage you to send all this good stuff upstream, as I'd anyway not really
expect your v1 to be perfect (however, I'm ready to get pleasantly surprised, so
please do?!).

Getting your current state correctly upstreamed makes you able to avoid refactors
during development and to actually get all the rest here upstream in relatively
less time.

After all, while it would be nice to have something complete and 100% working in
the first-ever iteration, you got really a lot of nice stuff there - and from an
upstream point of view, a device with this percentage of completion would be
perfectly acceptable.

Sending patches to add up more features later, even one by one, is just a regular
workflow, so that'd be perfectly acceptable as well.

What is not acceptable is non-functional placeholders that get deleted entirely
with subsequent patches (like adding a UART node with fake pinctrl because the
bootloader leaves that configured, but then deleting everything and refactoring
to make it proper ... bar exceptional cases, of course, everything is always
relative to specific situations).

Ultimately, it's your choice though, of course.

Keep up the good work!

Cheers,
Angelo

>> Cheers,
>> Angelo
>>
>>> ---
>>>    drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
>>> index 4c2983852596..2358407b8a6b 100644
>>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
>>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
>>> @@ -1232,6 +1232,8 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>>>
>>>        dsi->host.ops = &mtk_dsi_ops;
>>>        dsi->host.dev = dev;
>>> +     platform_set_drvdata(pdev, dsi);
>>> +
>>>        ret = mipi_dsi_host_register(&dsi->host);
>>>        if (ret < 0)
>>>                return dev_err_probe(dev, ret, "Failed to register DSI host\n");
>>> @@ -1254,8 +1256,6 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>>>
>>>        init_waitqueue_head(&dsi->irq_wait_queue);
>>>
>>> -     platform_set_drvdata(pdev, dsi);
>>> -
>>>        dsi->bridge.of_node = dev->of_node;
>>>        dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
>>>
>>
>>
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ