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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250823192219.GA127144@grimfrac.localdomain>
Date: Sat, 23 Aug 2025 21:22:19 +0200
From: Henrik Grimler <henrik@...mler.se>
To: Marek Szyprowski <m.szyprowski@...sung.com>
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

Hi Marek,

On Fri, Aug 22, 2025 at 09:37:25AM +0200, Marek Szyprowski wrote:
> 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.

Thanks for the insights! Will leave that for now then and send a v3.

Best regards,
Henrik Grimler

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