Two small fixes to sii902x for crashes.
Signed-off-by: Tomi Valkeinen <[email protected]>
---
Tomi Valkeinen (2):
drm/bridge: sii902x: Fix probing race issue
drm/bridge: sii902x: Fix audio codec unregistration
drivers/gpu/drm/bridge/sii902x.c | 42 +++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)
---
base-commit: 0c75d52190b8bfa22cdb66e07148aea599c4535d
change-id: 20240103-si902x-fixes-468d7153b8ee
Best regards,
--
Tomi Valkeinen <[email protected]>
A null pointer dereference crash has been observed rarely on TI
platforms using sii9022 bridge:
[ 53.271356] sii902x_get_edid+0x34/0x70 [sii902x]
[ 53.276066] sii902x_bridge_get_edid+0x14/0x20 [sii902x]
[ 53.281381] drm_bridge_get_edid+0x20/0x34 [drm]
[ 53.286305] drm_bridge_connector_get_modes+0x8c/0xcc [drm_kms_helper]
[ 53.292955] drm_helper_probe_single_connector_modes+0x190/0x538 [drm_kms_helper]
[ 53.300510] drm_client_modeset_probe+0x1f0/0xbd4 [drm]
[ 53.305958] __drm_fb_helper_initial_config_and_unlock+0x50/0x510 [drm_kms_helper]
[ 53.313611] drm_fb_helper_initial_config+0x48/0x58 [drm_kms_helper]
[ 53.320039] drm_fbdev_dma_client_hotplug+0x84/0xd4 [drm_dma_helper]
[ 53.326401] drm_client_register+0x5c/0xa0 [drm]
[ 53.331216] drm_fbdev_dma_setup+0xc8/0x13c [drm_dma_helper]
[ 53.336881] tidss_probe+0x128/0x264 [tidss]
[ 53.341174] platform_probe+0x68/0xc4
[ 53.344841] really_probe+0x188/0x3c4
[ 53.348501] __driver_probe_device+0x7c/0x16c
[ 53.352854] driver_probe_device+0x3c/0x10c
[ 53.357033] __device_attach_driver+0xbc/0x158
[ 53.361472] bus_for_each_drv+0x88/0xe8
[ 53.365303] __device_attach+0xa0/0x1b4
[ 53.369135] device_initial_probe+0x14/0x20
[ 53.373314] bus_probe_device+0xb0/0xb4
[ 53.377145] deferred_probe_work_func+0xcc/0x124
[ 53.381757] process_one_work+0x1f0/0x518
[ 53.385770] worker_thread+0x1e8/0x3dc
[ 53.389519] kthread+0x11c/0x120
[ 53.392750] ret_from_fork+0x10/0x20
The issue here is as follows:
- tidss probes, but is deferred as sii902x is still missing.
- sii902x starts probing and enters sii902x_init().
- sii902x calls drm_bridge_add(). Now the sii902x bridge is ready from
DRM's perspective.
- sii902x calls sii902x_audio_codec_init() and
platform_device_register_data()
- The registration of the audio platform device causes probing of the
deferred devices.
- tidss probes, which eventually causes sii902x_bridge_get_edid() to be
called.
- sii902x_bridge_get_edid() tries to use the i2c to read the edid.
However, the sii902x driver has not set up the i2c part yet, leading
to the crash.
Fix this by moving the drm_bridge_add() to the end of the
sii902x_init(), which is also at the very end of sii902x_probe().
Signed-off-by: Tomi Valkeinen <[email protected]>
Fixes: 21d808405fe4 ("drm/bridge/sii902x: Fix EDID readback")
---
drivers/gpu/drm/bridge/sii902x.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 2bdc5b439beb..69da73e414a9 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -1080,16 +1080,6 @@ static int sii902x_init(struct sii902x *sii902x)
return ret;
}
- sii902x->bridge.funcs = &sii902x_bridge_funcs;
- sii902x->bridge.of_node = dev->of_node;
- sii902x->bridge.timings = &default_sii902x_timings;
- sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
-
- if (sii902x->i2c->irq > 0)
- sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
-
- drm_bridge_add(&sii902x->bridge);
-
sii902x_audio_codec_init(sii902x, dev);
i2c_set_clientdata(sii902x->i2c, sii902x);
@@ -1102,7 +1092,21 @@ static int sii902x_init(struct sii902x *sii902x)
return -ENOMEM;
sii902x->i2cmux->priv = sii902x;
- return i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0);
+ ret = i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0);
+ if (ret)
+ return ret;
+
+ sii902x->bridge.funcs = &sii902x_bridge_funcs;
+ sii902x->bridge.of_node = dev->of_node;
+ sii902x->bridge.timings = &default_sii902x_timings;
+ sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
+
+ if (sii902x->i2c->irq > 0)
+ sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
+
+ drm_bridge_add(&sii902x->bridge);
+
+ return 0;
}
static int sii902x_probe(struct i2c_client *client)
@@ -1170,12 +1174,11 @@ static int sii902x_probe(struct i2c_client *client)
}
static void sii902x_remove(struct i2c_client *client)
-
{
struct sii902x *sii902x = i2c_get_clientdata(client);
- i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(&sii902x->bridge);
+ i2c_mux_del_adapters(sii902x->i2cmux);
}
static const struct of_device_id sii902x_dt_ids[] = {
--
2.34.1
The driver never unregisters the audio codec platform device, which can
lead to a crash on module reloading, nor does it handle the return value
from sii902x_audio_codec_init().
Signed-off-by: Tomi Valkeinen <[email protected]>
Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Cc: Jyri Sarha <[email protected]>
---
drivers/gpu/drm/bridge/sii902x.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 69da73e414a9..4560ae9cbce1 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -1080,7 +1080,9 @@ static int sii902x_init(struct sii902x *sii902x)
return ret;
}
- sii902x_audio_codec_init(sii902x, dev);
+ ret = sii902x_audio_codec_init(sii902x, dev);
+ if (ret)
+ return ret;
i2c_set_clientdata(sii902x->i2c, sii902x);
@@ -1088,13 +1090,15 @@ static int sii902x_init(struct sii902x *sii902x)
1, 0, I2C_MUX_GATE,
sii902x_i2c_bypass_select,
sii902x_i2c_bypass_deselect);
- if (!sii902x->i2cmux)
- return -ENOMEM;
+ if (!sii902x->i2cmux) {
+ ret = -ENOMEM;
+ goto err_unreg_audio;
+ }
sii902x->i2cmux->priv = sii902x;
ret = i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0);
if (ret)
- return ret;
+ goto err_unreg_audio;
sii902x->bridge.funcs = &sii902x_bridge_funcs;
sii902x->bridge.of_node = dev->of_node;
@@ -1107,6 +1111,12 @@ static int sii902x_init(struct sii902x *sii902x)
drm_bridge_add(&sii902x->bridge);
return 0;
+
+err_unreg_audio:
+ if (!PTR_ERR_OR_ZERO(sii902x->audio.pdev))
+ platform_device_unregister(sii902x->audio.pdev);
+
+ return ret;
}
static int sii902x_probe(struct i2c_client *client)
@@ -1179,6 +1189,9 @@ static void sii902x_remove(struct i2c_client *client)
drm_bridge_remove(&sii902x->bridge);
i2c_mux_del_adapters(sii902x->i2cmux);
+
+ if (!PTR_ERR_OR_ZERO(sii902x->audio.pdev))
+ platform_device_unregister(sii902x->audio.pdev);
}
static const struct of_device_id sii902x_dt_ids[] = {
--
2.34.1
On Wed, Jan 3, 2024 at 2:31 PM Tomi Valkeinen
<[email protected]> wrote:
> Two small fixes to sii902x for crashes.
>
> Signed-off-by: Tomi Valkeinen <[email protected]>
These look good to me!
Acked-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
Hi,
On Wed, 03 Jan 2024 15:31:06 +0200, Tomi Valkeinen wrote:
> Two small fixes to sii902x for crashes.
>
>
Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes)
[1/2] drm/bridge: sii902x: Fix probing race issue
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=08ac6f132dd77e40f786d8af51140c96c6d739c9
[2/2] drm/bridge: sii902x: Fix audio codec unregistration
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3fc6c76a8d208d3955c9e64b382d0ff370bc61fc
--
Neil
On 03/01/24 19:01, Tomi Valkeinen wrote:
> Two small fixes to sii902x for crashes.
>
> Signed-off-by: Tomi Valkeinen <[email protected]>
> ---
> Tomi Valkeinen (2):
> drm/bridge: sii902x: Fix probing race issue
> drm/bridge: sii902x: Fix audio codec unregistration
Stress-tested kernel boot on SK-AM62, SK-AM62-LP, SK-AM62A, and I
couldn't reproduce the issue. The kernel booted fine every time.
For the series,
Reviewed-by: Aradhya Bhatia <[email protected]>
>
> drivers/gpu/drm/bridge/sii902x.c | 42 +++++++++++++++++++++++++++-------------
> 1 file changed, 29 insertions(+), 13 deletions(-)
> ---
> base-commit: 0c75d52190b8bfa22cdb66e07148aea599c4535d
> change-id: 20240103-si902x-fixes-468d7153b8ee
>
> Best regards,
On Wed, 03 Jan 2024 15:31:06 +0200, Tomi Valkeinen wrote:
> Two small fixes to sii902x for crashes.
>
>
Applied, thanks!
[1/2] drm/bridge: sii902x: Fix probing race issue
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=dffdfb8f5de1
[2/2] drm/bridge: sii902x: Fix audio codec unregistration
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bc77bde2d3f0
Rob