public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register
@ 2026-02-09  9:05 Luca Leonardo Scorcia
  2026-02-09 11:40 ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 6+ messages in thread
From: Luca Leonardo Scorcia @ 2026-02-09  9:05 UTC (permalink / raw)
  To: linux-mediatek
  Cc: Luca Leonardo Scorcia, Chun-Kuang Hu, Philipp Zabel, David Airlie,
	Simona Vetter, Matthias Brugger, AngeloGioacchino Del Regno,
	dri-devel, linux-kernel, linux-arm-kernel

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
[...]
Modules linked in: mediatek_drm(+) drm_display_helper cec drm_client_lib
 drm_dma_helper drm_kms_helper panel_simple
[...]
Call trace:
 drm_mode_object_add+0x58/0x98 (P)
 __drm_encoder_init+0x48/0x140
 drm_encoder_init+0x6c/0xa0
 drm_simple_encoder_init+0x20/0x34 [drm_kms_helper]
 mtk_dsi_bind+0x34/0x13c [mediatek_drm]
 component_bind_all+0x120/0x280
 mtk_drm_bind+0x284/0x67c [mediatek_drm]
 try_to_bring_up_aggregate_device+0x23c/0x320
 __component_add+0xa4/0x198
 component_add+0x14/0x20
 mtk_dsi_host_attach+0x78/0x100 [mediatek_drm]
 mipi_dsi_attach+0x2c/0x50
 panel_simple_dsi_probe+0x4c/0x9c [panel_simple]
 mipi_dsi_drv_probe+0x1c/0x28
 really_probe+0xc0/0x3dc
 __driver_probe_device+0x80/0x160
 driver_probe_device+0x40/0x120
 __device_attach_driver+0xbc/0x17c
 bus_for_each_drv+0x88/0xf0
 __device_attach+0x9c/0x1cc
 device_initial_probe+0x54/0x60
 bus_probe_device+0x34/0xa0
 device_add+0x5b0/0x800
 mipi_dsi_device_register_full+0xdc/0x16c
 mipi_dsi_host_register+0xc4/0x17c
 mtk_dsi_probe+0x10c/0x260 [mediatek_drm]
 platform_probe+0x5c/0xa4
 really_probe+0xc0/0x3dc
 __driver_probe_device+0x80/0x160
 driver_probe_device+0x40/0x120
 __driver_attach+0xc8/0x1f8
 bus_for_each_dev+0x7c/0xe0
 driver_attach+0x24/0x30
 bus_add_driver+0x11c/0x240
 driver_register+0x68/0x130
 __platform_register_drivers+0x64/0x160
 mtk_drm_init+0x24/0x1000 [mediatek_drm]
 do_one_initcall+0x60/0x1d0
 do_init_module+0x54/0x240
 load_module+0x1838/0x1dc0
 init_module_from_file+0xd8/0xf0
 __arm64_sys_finit_module+0x1b4/0x428
 invoke_syscall.constprop.0+0x48/0xc8
 do_el0_svc+0x3c/0xb8
 el0_svc+0x34/0xe8
 el0t_64_sync_handler+0xa0/0xe4
 el0t_64_sync+0x198/0x19c
Code: 52800022 941004ab 2a0003f3 37f80040 (29005a80)
---[ end trace 0000000000000000 ]---

Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
---
 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;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register
  2026-02-09  9:05 [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register Luca Leonardo Scorcia
@ 2026-02-09 11:40 ` AngeloGioacchino Del Regno
  2026-02-09 18:46   ` Luca Leonardo Scorcia
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-02-09 11:40 UTC (permalink / raw)
  To: Luca Leonardo Scorcia, linux-mediatek
  Cc: Chun-Kuang Hu, Philipp Zabel, David Airlie, Simona Vetter,
	Matthias Brugger, dri-devel, linux-kernel, linux-arm-kernel

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
> [...]
> Modules linked in: mediatek_drm(+) drm_display_helper cec drm_client_lib
>   drm_dma_helper drm_kms_helper panel_simple
> [...]
> Call trace:
>   drm_mode_object_add+0x58/0x98 (P)
>   __drm_encoder_init+0x48/0x140
>   drm_encoder_init+0x6c/0xa0
>   drm_simple_encoder_init+0x20/0x34 [drm_kms_helper]
>   mtk_dsi_bind+0x34/0x13c [mediatek_drm]
>   component_bind_all+0x120/0x280
>   mtk_drm_bind+0x284/0x67c [mediatek_drm]
>   try_to_bring_up_aggregate_device+0x23c/0x320
>   __component_add+0xa4/0x198
>   component_add+0x14/0x20
>   mtk_dsi_host_attach+0x78/0x100 [mediatek_drm]
>   mipi_dsi_attach+0x2c/0x50
>   panel_simple_dsi_probe+0x4c/0x9c [panel_simple]
>   mipi_dsi_drv_probe+0x1c/0x28
>   really_probe+0xc0/0x3dc
>   __driver_probe_device+0x80/0x160
>   driver_probe_device+0x40/0x120
>   __device_attach_driver+0xbc/0x17c
>   bus_for_each_drv+0x88/0xf0
>   __device_attach+0x9c/0x1cc
>   device_initial_probe+0x54/0x60
>   bus_probe_device+0x34/0xa0
>   device_add+0x5b0/0x800
>   mipi_dsi_device_register_full+0xdc/0x16c
>   mipi_dsi_host_register+0xc4/0x17c
>   mtk_dsi_probe+0x10c/0x260 [mediatek_drm]
>   platform_probe+0x5c/0xa4
>   really_probe+0xc0/0x3dc
>   __driver_probe_device+0x80/0x160
>   driver_probe_device+0x40/0x120
>   __driver_attach+0xc8/0x1f8
>   bus_for_each_dev+0x7c/0xe0
>   driver_attach+0x24/0x30
>   bus_add_driver+0x11c/0x240
>   driver_register+0x68/0x130
>   __platform_register_drivers+0x64/0x160
>   mtk_drm_init+0x24/0x1000 [mediatek_drm]
>   do_one_initcall+0x60/0x1d0
>   do_init_module+0x54/0x240
>   load_module+0x1838/0x1dc0
>   init_module_from_file+0xd8/0xf0
>   __arm64_sys_finit_module+0x1b4/0x428
>   invoke_syscall.constprop.0+0x48/0xc8
>   do_el0_svc+0x3c/0xb8
>   el0_svc+0x34/0xe8
>   el0t_64_sync_handler+0xa0/0xe4
>   el0t_64_sync+0x198/0x19c
> Code: 52800022 941004ab 2a0003f3 37f80040 (29005a80)
> ---[ end trace 0000000000000000 ]---
> 
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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?!?! :-)

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;
>   



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register
  2026-02-09 11:40 ` AngeloGioacchino Del Regno
@ 2026-02-09 18:46   ` Luca Leonardo Scorcia
  2026-02-10 11:31     ` AngeloGioacchino Del Regno
  2026-02-11  7:11   ` Claude review: " Claude Code Review Bot
  2026-02-11  7:11   ` Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Luca Leonardo Scorcia @ 2026-02-09 18:46 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-mediatek, Chun-Kuang Hu, Philipp Zabel, David Airlie,
	Simona Vetter, Matthias Brugger, dri-devel, linux-kernel,
	linux-arm-kernel

Il giorno lun 9 feb 2026 alle ore 12:40 AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.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@gmail.com>
>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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

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.

> 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;
> >
>
>


-- 
Luca Leonardo Scorcia
l.scorcia@gmail.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register
  2026-02-09 18:46   ` Luca Leonardo Scorcia
@ 2026-02-10 11:31     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-02-10 11:31 UTC (permalink / raw)
  To: Luca Leonardo Scorcia
  Cc: linux-mediatek, Chun-Kuang Hu, Philipp Zabel, David Airlie,
	Simona Vetter, Matthias Brugger, dri-devel, linux-kernel,
	linux-arm-kernel

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@collabora.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@gmail.com>
>>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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;
>>>
>>
>>
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register
  2026-02-09 11:40 ` AngeloGioacchino Del Regno
  2026-02-09 18:46   ` Luca Leonardo Scorcia
@ 2026-02-11  7:11   ` Claude Code Review Bot
  2026-02-11  7:11   ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-02-11  7:11 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking
 mipi_dsi_host_register
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Patches: 4
Reviewed: 2026-02-11T17:11:30.197586

---

This is a single-patch submission that fixes a critical NULL pointer dereference in the MediaTek DSI driver. The patch addresses an initialization ordering issue where `mipi_dsi_host_register()` is called before `platform_set_drvdata()`, causing a crash when the registration triggers callbacks that attempt to retrieve the driver data.

**Summary:**
- **Type:** Bug fix
- **Severity:** Critical (kernel crash/NULL pointer dereference)
- **Complexity:** Simple (2-line move)
- **Testing:** Verified on Xiaomi Smart Clock x04g (MT8167)

**Overall Assessment:** This is a correct fix for a real bug. The patch has already received a Reviewed-by tag from AngeloGioacchino Del Regno, a maintainer familiar with this subsystem. However, there are some issues with the patch metadata that should be addressed.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Claude review: Re: [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register
  2026-02-09 11:40 ` AngeloGioacchino Del Regno
  2026-02-09 18:46   ` Luca Leonardo Scorcia
  2026-02-11  7:11   ` Claude review: " Claude Code Review Bot
@ 2026-02-11  7:11   ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-02-11  7:11 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Commit Message Analysis:**

The commit message adequately explains the issue and provides a detailed crash trace. However, it is missing critical metadata:

1. **Missing Fixes tag:** This is a bug fix and should include a `Fixes:` tag identifying which commit introduced the regression. Looking at the code, this would help with backporting to stable kernels.

2. **Missing Cc: stable tag:** Given this is a crash fix, it should include `Cc: stable@vger.kernel.org` to ensure it gets backported to stable kernel trees.

**Technical Analysis:**

```c
 	dsi->host.ops = &mtk_dsi_ops;
 	dsi->host.dev = dev;
+	platform_set_drvdata(pdev, dsi);
+
 	ret = mipi_dsi_host_register(&dsi->host);
```

**The fix is correct.** The issue stems from this call chain:
1. `mtk_dsi_probe()` calls `mipi_dsi_host_register()`
2. `mipi_dsi_host_register()` → `mipi_dsi_device_register_full()` → `device_add()`
3. `device_add()` → `bus_probe_device()` → driver binding occurs
4. Panel driver's `mipi_dsi_drv_probe()` → `panel_simple_dsi_probe()`
5. Panel calls `mipi_dsi_attach()` → `mtk_dsi_host_attach()`
6. `mtk_dsi_host_attach()` → `component_add()` → `try_to_bring_up_aggregate_device()`
7. `mtk_drm_bind()` → `component_bind_all()` → `mtk_dsi_bind()`
8. **`mtk_dsi_bind()` calls `dev_get_drvdata()` but drvdata was never set → NULL pointer dereference**

Moving `platform_set_drvdata()` before `mipi_dsi_host_register()` ensures the driver data is available when the callbacks are triggered.

**Code Correctness:**

The removed location (after `init_waitqueue_head()`) was too late because all the probe sequencing and component binding can happen synchronously during `mipi_dsi_host_register()`.

The new location (immediately before `mipi_dsi_host_register()`) is the earliest safe point where:
- The `dsi` structure is fully initialized for basic use
- The host ops and device are set
- No earlier position would make sense (drvdata shouldn't be set while struct is being populated)

**Potential Issues:**

Looking at the crash trace more carefully:

```
Call trace:
 drm_mode_object_add+0x58/0x98 (P)
 __drm_encoder_init+0x48/0x140
 drm_encoder_init+0x6c/0xa0
 drm_simple_encoder_init+0x20/0x34 [drm_kms_helper]
 mtk_dsi_bind+0x34/0x13c [mediatek_drm]
```

The crash happens in `mtk_dsi_bind()` at offset `+0x34`. The crash address `0x0000000000000040` (offset 0x40 from NULL) suggests dereferencing a field in a NULL structure. This aligns with `dev_get_drvdata()` returning NULL and then accessing a field at offset 0x40 in the `mtk_dsi` structure.

**Questions/Concerns:**

1. **Error path safety:** If `mipi_dsi_host_register()` fails, the driver returns early. At this point, drvdata is set but the device isn't fully initialized. However, this should be safe because:
   - The device probe failed, so nothing should be calling into the driver
   - The error path doesn't explicitly unset drvdata, but the device will be removed anyway

2. **Completeness:** Are there other fields in the `dsi` structure that need to be initialized before `mipi_dsi_host_register()`? Looking at the code:
   - Lines before the call: Basic host setup, IRQ registration, clocks, PHY - all completed ✓
   - Lines after the call: Wait queue, bridge setup - these are safe to do after

**Style/Formatting:**

The patch follows kernel coding style. The blank line added after `platform_set_drvdata()` is appropriate for visual separation.

**Recommendation:**

The patch should be accepted with the following amendments:

1. **Add Fixes tag:** Need to identify the commit that introduced this ordering issue (likely when the component framework integration was added or when the probe order changed)

2. **Add Cc: stable tag:** This crash fix should be backported

3. **Consider additional context:** The commit message could briefly mention that this issue manifests when DSI panels are probed synchronously during host registration

**Suggested improved commit message:**

```
drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register

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.

This manifests when the panel driver's probe is triggered synchronously
during mipi_dsi_host_register(), leading to component binding before
the DSI host driver has finished its probe sequence.

[crash trace as shown...]

Fixes: [commit-id] ("commit message of offending patch")
Cc: stable@vger.kernel.org
Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
```

**Verdict:** ✓ Correct fix, should be merged (with metadata improvements)

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-02-11  7:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-09  9:05 [PATCH] drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register Luca Leonardo Scorcia
2026-02-09 11:40 ` AngeloGioacchino Del Regno
2026-02-09 18:46   ` Luca Leonardo Scorcia
2026-02-10 11:31     ` AngeloGioacchino Del Regno
2026-02-11  7:11   ` Claude review: " Claude Code Review Bot
2026-02-11  7:11   ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox