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@xxxxxxxxxx>
> >>> ---
> >>> v2: add dependency on extcon. Issue reported by kernel test robot
> >>> <lkp@xxxxxxxxx>
> >>> ---
> >>> 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
>
