linux-kernel - Re: [PATCH v2 3/3] drm/bridge: sii9234: use extcon cable detection logic to detect MHL

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: <1979fe6e-7a54-4812-9878-b4ce286401b2@samsung.com>
Date: Fri, 22 Aug 2025 09:37:25 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Henrik Grimler <henrik@...mler.se>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong
	<neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, Laurent
	Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman
	<jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, Maarten
	Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
	<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
	<airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	dri-devel@...ts.freedesktop.org, linux-samsung-soc@...r.kernel.org,
	~postmarketos/upstreaming@...ts.sr.ht, replicant@...osl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] drm/bridge: sii9234: use extcon cable detection
 logic to detect MHL

On 18.08.2025 16:26, Henrik Grimler wrote:
> On Thu, Aug 14, 2025 at 01:26:33PM +0200, Marek Szyprowski wrote:
>> On 24.07.2025 20:50, Henrik Grimler wrote:
>>> To use MHL we currently need the MHL chip to be permanently on, which
>>> consumes unnecessary power. Let's use extcon attached to MUIC to enable
>>> the MHL chip only if it detects an MHL cable.
>>>
>>> Signed-off-by: Henrik Grimler <henrik@...mler.se>
>>> ---
>>> v2: add dependency on extcon. Issue reported by kernel test robot
>>>       <lkp@...el.com>
>>> ---
>>>    drivers/gpu/drm/bridge/Kconfig   |  1 +
>>>    drivers/gpu/drm/bridge/sii9234.c | 89 ++++++++++++++++++++++++++++++++++++++--
>>>    2 files changed, 87 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>>> index b9e0ca85226a603a24f90c6879d1499f824060cb..f18a083f6e1c6fe40bde5e65a1548acc61a162ae 100644
>>> --- a/drivers/gpu/drm/bridge/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/Kconfig
>>> @@ -303,6 +303,7 @@ config DRM_SII902X
>>>    config DRM_SII9234
>>>    	tristate "Silicon Image SII9234 HDMI/MHL bridge"
>>>    	depends on OF
>>> +	select EXTCON
>>>    	help
>>>    	  Say Y here if you want support for the MHL interface.
>>>    	  It is an I2C driver, that detects connection of MHL bridge
>>> diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
>>> index 0e0bb1bf71fdcef788715cfd6fa158a6992def33..4d84ba01ea76816bebdbc29d48a041c9c6cd508e 100644
>>> --- a/drivers/gpu/drm/bridge/sii9234.c
>>> +++ b/drivers/gpu/drm/bridge/sii9234.c
> [ ...]
>
>>> +
>>> +	edev = extcon_find_edev_by_node(muic);
>>> +	of_node_put(muic);
>>> +	if (IS_ERR(edev)) {
>>> +		dev_err_probe(ctx->dev, PTR_ERR(edev),
>>> +			      "invalid or missing extcon\n");
>>> +	}
>> It looks that the original logic got lost somehow in the above code
>> block, what causes kernel oops if compiled as module and loaded before
>> extcon provider. Please handle -EPROBE_DEFER and propagate error value,
>> like the original code did in sii8620 driver:
>>
>>           if (IS_ERR(edev)) {
>>                   if (PTR_ERR(edev) == -EPROBE_DEFER)
>>                           return -EPROBE_DEFER;
>>                   dev_err(ctx->dev, "Invalid or missing extcon\n");
>>                   return PTR_ERR(edev);
>>           }
> Thanks for detecting the issue! I think my code is just missing return
> before dev_err_probe (same mistake as I did on patch 2). With return
> added I have not been able to reproduce any kernel oops, but if
> CONFIG_DRM_SII9234=y and CONFIG_EXTCON_MAX77693=m then it seems like
> linux gets stuck probing sii9234 and waiting for the extcon provider
> (verified with some printf debugging). This happens for me both with:
>
> 	edev = extcon_find_edev_by_node(muic);
> 	of_node_put(muic);
> 	if (IS_ERR(edev)) {
> 		return dev_err_probe(ctx->dev, PTR_ERR(edev),
> 				     "Invalid or missing extcon\n");
> 	}
>
> and
>
> 	edev = extcon_find_edev_by_node(muic);
> 	of_node_put(muic);
> 	if (IS_ERR(edev)) {
> 		if (PTR_ERR(edev) == -EPROBE_DEFER)
> 			return -EPROBE_DEFER;
> 		dev_err(ctx->dev, "Invalid or missing extcon\n");
> 		return PTR_ERR(edev);
> 	}
>
> I am not sure what to do to fix the issue, as far as I can see probe
> logic and extcon handling is the same as in sil-sii8620 and ite-it6505
> (i.e. the other bridges that use extcon). Will investigate further.

Indeed your code lacked only the return directive, I've noticed that 
just after sending my reply.

I'm not sure if there is a simple way to solve the endless probe issue 
with sii9234=y and max77963=m. We have to rely on the user to either 
keep all drivers compiled-in or configured as modules here. Afair the 
same issue happens with sii8620 and max77843.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland