[PATCH v2 0/3] drm/bridge: sii9234: use extcon to detect cable attachment

[PATCH v2 0/3] drm/bridge: sii9234: use extcon to detect cable attachment

[Henrik Grimler]

Hi,

This series fixes so HDMI through the sii9234 MHL chip works when
cable is hotplugged, by making the MHL chip use extcon cable detection
functions. Patch 3, that actually implements the extcon parts, is heavily
inspired by commit 688838442147 ("drm/bridge/sii8620: use micro-USB
cable detection logic to detect MHL") by Maciej Purski.

Before these changes, HDMI only worked if cable was plugged in before
booting. If no cable was connected, then wlr-randr still showed HDMI
as connected, with 0x0 px, which confused at least some UIs (phosh)
and caused problems:
https://gitlab.gnome.org/World/Phosh/phosh/-/issues/828

Tested on exynos4412-i9305.

Best regards,
Henrik Grimler

Signed-off-by: Henrik Grimler <henrik@grimler.se>
---
Changes in v2:
- Add dependency on extcon in patch 3. Issue reported by kernel test robot <lkp@intel.com>
- Link to v1: https://lore.kernel.org/r/20250721-exynos4-sii9234-driver-v1-0-2e47ed02f677@grimler.se

---
Henrik Grimler (3):
      drm/bridge: sii9234: fix some typos in comments and messages
      drm/bridge: sii9234: use dev_err_probe where applicable
      drm/bridge: sii9234: use extcon cable detection logic to detect MHL

 drivers/gpu/drm/bridge/Kconfig   |   1 +
 drivers/gpu/drm/bridge/sii9234.c | 121 ++++++++++++++++++++++++++++++++-------
 2 files changed, 101 insertions(+), 21 deletions(-)
---
base-commit: e48123c607a0db8b9ad02f83c8c3d39918dbda06
change-id: 20231218-exynos4-sii9234-driver-d817d4b511d5

Best regards,
-- 
Henrik Grimler <henrik@grimler.se>

[PATCH v2 1/3] drm/bridge: sii9234: fix some typos in comments and messages

[Henrik Grimler]

Fix spelling and formatting so that the code is easier to follow, and
so that it is more searchable.

Signed-off-by: Henrik Grimler <henrik@grimler.se>
---
v2: no changes
---
 drivers/gpu/drm/bridge/sii9234.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index bb1bed03eb5b7ae67f752c0d593dc54131e9e370..930117bbba87285e62107389606897740516eb0a 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -339,7 +339,7 @@ static int sii9234_cbus_reset(struct sii9234 *ctx)
 	return sii9234_clear_error(ctx);
 }
 
-/* Require to chek mhl imformation of samsung in cbus_init_register */
+/* Require to check mhl information of samsung in cbus_init_register */
 static int sii9234_cbus_init(struct sii9234 *ctx)
 {
 	cbus_writeb(ctx, 0x07, 0xF2);
@@ -614,7 +614,7 @@ static void sii9234_cable_out(struct sii9234 *ctx)
 
 	disable_irq(to_i2c_client(ctx->dev)->irq);
 	tpi_writeb(ctx, TPI_DPD_REG, 0);
-	/* Turn on&off hpd festure for only QCT HDMI */
+	/* Turn on&off hpd feature for only QCT HDMI */
 	sii9234_hw_off(ctx);
 
 	ctx->state = ST_OFF;
@@ -708,7 +708,7 @@ static enum sii9234_state sii9234_rsen_change(struct sii9234 *ctx)
 {
 	int value;
 
-	/* Work_around code to handle wrong interrupt */
+	/* Workaround code to handle wrong interrupt */
 	if (ctx->state != ST_RGND_1K) {
 		dev_err(ctx->dev, "RSEN_HIGH without RGND_1K\n");
 		return ST_FAILURE;
@@ -723,9 +723,9 @@ static enum sii9234_state sii9234_rsen_change(struct sii9234 *ctx)
 	}
 	dev_dbg(ctx->dev, "RSEN lost\n");
 	/*
-	 * Once RSEN loss is confirmed,we need to check
-	 * based on cable status and chip power status,whether
-	 * it is SINK Loss(HDMI cable not connected, TV Off)
+	 * Once RSEN loss is confirmed, we need to check
+	 * based on cable status and chip power status, whether
+	 * it is SINK Loss (HDMI cable not connected, TV Off)
 	 * or MHL cable disconnection
 	 * TODO: Define the below mhl_disconnection()
 	 */
@@ -820,7 +820,7 @@ static int sii9234_init_resources(struct sii9234 *ctx,
 	int ret;
 
 	if (!ctx->dev->of_node) {
-		dev_err(ctx->dev, "not DT device\n");
+		dev_err(ctx->dev, "no DT device\n");
 		return -ENODEV;
 	}
 

-- 
2.50.1

[PATCH v2 2/3] drm/bridge: sii9234: use dev_err_probe where applicable

[Henrik Grimler]

In case of error during resource acquisition the driver should print
an error message only if it is not deferred probe. Use dev_err_probe
helper to handle this, which will also record defer probe reason for
debugging.

Signed-off-by: Henrik Grimler <henrik@grimler.se>
---
v2: no changes
---
 drivers/gpu/drm/bridge/sii9234.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
index 930117bbba87285e62107389606897740516eb0a..0e0bb1bf71fdcef788715cfd6fa158a6992def33 100644
--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -825,10 +825,9 @@ static int sii9234_init_resources(struct sii9234 *ctx,
 	}
 
 	ctx->gpio_reset = devm_gpiod_get(ctx->dev, "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(ctx->gpio_reset)) {
-		dev_err(ctx->dev, "failed to get reset gpio from DT\n");
-		return PTR_ERR(ctx->gpio_reset);
-	}
+	if (IS_ERR(ctx->gpio_reset))
+		return dev_err_probe(ctx->dev, PTR_ERR(ctx->gpio_reset),
+				     "failed to get reset gpio from DT\n");
 
 	ctx->supplies[0].supply = "avcc12";
 	ctx->supplies[1].supply = "avcc33";
@@ -836,9 +835,7 @@ static int sii9234_init_resources(struct sii9234 *ctx,
 	ctx->supplies[3].supply = "cvcc12";
 	ret = devm_regulator_bulk_get(ctx->dev, 4, ctx->supplies);
 	if (ret) {
-		if (ret != -EPROBE_DEFER)
-			dev_err(ctx->dev, "regulator_bulk failed\n");
-		return ret;
+		dev_err_probe(ctx->dev, ret, "regulator_bulk failed\n");
 	}
 
 	ctx->client[I2C_MHL] = client;
@@ -911,10 +908,9 @@ static int sii9234_probe(struct i2c_client *client)
 					sii9234_irq_thread,
 					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
 					"sii9234", ctx);
-	if (ret < 0) {
-		dev_err(dev, "failed to install IRQ handler\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "failed to install IRQ handler\n");
 
 	ret = sii9234_init_resources(ctx, client);
 	if (ret < 0)

-- 
2.50.1

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

[Henrik Grimler]

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@grimler.se>
---
v2: add dependency on extcon. Issue reported by kernel test robot
    <lkp@intel.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
@@ -19,6 +19,7 @@
 
 #include <linux/delay.h>
 #include <linux/err.h>
+#include <linux/extcon.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -26,6 +27,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of_graph.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -170,8 +172,12 @@ struct sii9234 {
 	struct drm_bridge bridge;
 	struct device *dev;
 	struct gpio_desc *gpio_reset;
-	int i2c_error;
 	struct regulator_bulk_data supplies[4];
+	struct extcon_dev *extcon;
+	struct notifier_block extcon_nb;
+	struct work_struct extcon_wq;
+	int cable_state;
+	int i2c_error;
 
 	struct mutex lock; /* Protects fields below and device registers */
 	enum sii9234_state state;
@@ -864,6 +870,70 @@ static int sii9234_init_resources(struct sii9234 *ctx,
 	return 0;
 }
 
+static void sii9234_extcon_work(struct work_struct *work)
+{
+	struct sii9234 *ctx =
+		container_of(work, struct sii9234, extcon_wq);
+	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
+
+	if (state == ctx->cable_state)
+		return;
+
+	ctx->cable_state = state;
+
+	if (state > 0)
+		sii9234_cable_in(ctx);
+	else
+		sii9234_cable_out(ctx);
+}
+
+static int sii9234_extcon_notifier(struct notifier_block *self,
+			unsigned long event, void *ptr)
+{
+	struct sii9234 *ctx =
+		container_of(self, struct sii9234, extcon_nb);
+
+	schedule_work(&ctx->extcon_wq);
+
+	return NOTIFY_DONE;
+}
+
+static int sii9234_extcon_init(struct sii9234 *ctx)
+{
+	struct extcon_dev *edev;
+	struct device_node *musb, *muic;
+	int ret;
+
+	/* Get micro-USB connector node */
+	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
+	/* Then get micro-USB Interface Controller node */
+	muic = of_get_next_parent(musb);
+
+	if (!muic) {
+		dev_info(ctx->dev,
+			 "no extcon found, switching to 'always on' mode\n");
+		return 0;
+	}
+
+	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");
+	}
+
+	ctx->extcon = edev;
+	ctx->extcon_nb.notifier_call = sii9234_extcon_notifier;
+	INIT_WORK(&ctx->extcon_wq, sii9234_extcon_work);
+	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
+	if (ret) {
+		dev_err(ctx->dev, "failed to register notifier for MHL\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
 					 const struct drm_display_info *info,
 					 const struct drm_display_mode *mode)
@@ -916,12 +986,17 @@ static int sii9234_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = sii9234_extcon_init(ctx);
+	if (ret < 0)
+		return ret;
+
 	i2c_set_clientdata(client, ctx);
 
 	ctx->bridge.of_node = dev->of_node;
 	drm_bridge_add(&ctx->bridge);
 
-	sii9234_cable_in(ctx);
+	if (!ctx->extcon)
+		sii9234_cable_in(ctx);
 
 	return 0;
 }
@@ -930,7 +1005,15 @@ static void sii9234_remove(struct i2c_client *client)
 {
 	struct sii9234 *ctx = i2c_get_clientdata(client);
 
-	sii9234_cable_out(ctx);
+	if (ctx->extcon) {
+		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
+					   &ctx->extcon_nb);
+		flush_work(&ctx->extcon_wq);
+		if (ctx->cable_state > 0)
+			sii9234_cable_out(ctx);
+	} else {
+		sii9234_cable_out(ctx);
+	}
 	drm_bridge_remove(&ctx->bridge);
 }
 

-- 
2.50.1

Re: [PATCH v2 1/3] drm/bridge: sii9234: fix some typos in comments and messages

[Dmitry Baryshkov]

On Thu, Jul 24, 2025 at 08:50:51PM +0200, Henrik Grimler wrote:
> Fix spelling and formatting so that the code is easier to follow, and
> so that it is more searchable.
> 
> Signed-off-by: Henrik Grimler <henrik@grimler.se>
> ---
> v2: no changes
> ---
>  drivers/gpu/drm/bridge/sii9234.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] drm/bridge: sii9234: use dev_err_probe where applicable

[Dmitry Baryshkov]

On Thu, Jul 24, 2025 at 08:50:52PM +0200, Henrik Grimler wrote:
> In case of error during resource acquisition the driver should print
> an error message only if it is not deferred probe. Use dev_err_probe
> helper to handle this, which will also record defer probe reason for
> debugging.
> 
> Signed-off-by: Henrik Grimler <henrik@grimler.se>
> ---
> v2: no changes
> ---
>  drivers/gpu/drm/bridge/sii9234.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii9234.c b/drivers/gpu/drm/bridge/sii9234.c
> index 930117bbba87285e62107389606897740516eb0a..0e0bb1bf71fdcef788715cfd6fa158a6992def33 100644
> --- a/drivers/gpu/drm/bridge/sii9234.c
> +++ b/drivers/gpu/drm/bridge/sii9234.c
> @@ -825,10 +825,9 @@ static int sii9234_init_resources(struct sii9234 *ctx,
>  	}
>  
>  	ctx->gpio_reset = devm_gpiod_get(ctx->dev, "reset", GPIOD_OUT_LOW);
> -	if (IS_ERR(ctx->gpio_reset)) {
> -		dev_err(ctx->dev, "failed to get reset gpio from DT\n");
> -		return PTR_ERR(ctx->gpio_reset);
> -	}
> +	if (IS_ERR(ctx->gpio_reset))
> +		return dev_err_probe(ctx->dev, PTR_ERR(ctx->gpio_reset),
> +				     "failed to get reset gpio from DT\n");
>  
>  	ctx->supplies[0].supply = "avcc12";
>  	ctx->supplies[1].supply = "avcc33";
> @@ -836,9 +835,7 @@ static int sii9234_init_resources(struct sii9234 *ctx,
>  	ctx->supplies[3].supply = "cvcc12";
>  	ret = devm_regulator_bulk_get(ctx->dev, 4, ctx->supplies);
>  	if (ret) {
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(ctx->dev, "regulator_bulk failed\n");
> -		return ret;
> +		dev_err_probe(ctx->dev, ret, "regulator_bulk failed\n");

Drop the braces, use 'return dev_err_probe(...)'

>  	}
>  
>  	ctx->client[I2C_MHL] = client;
> @@ -911,10 +908,9 @@ static int sii9234_probe(struct i2c_client *client)
>  					sii9234_irq_thread,
>  					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>  					"sii9234", ctx);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to install IRQ handler\n");
> -		return ret;
> -	}
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "failed to install IRQ handler\n");
>  
>  	ret = sii9234_init_resources(ctx, client);
>  	if (ret < 0)
> 
> -- 
> 2.50.1
> 

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

On Thu, Jul 24, 2025 at 08:50:53PM +0200, 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.

Does HPD GPIO reflect the correct state of the cable? What is the order
of events in such a case? Should the sii9234 signal to Exynos HDMI that
the link is established?

> 
> Signed-off-by: Henrik Grimler <henrik@grimler.se>
> ---
> v2: add dependency on extcon. Issue reported by kernel test robot
>     <lkp@intel.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

Either this or 'depends on EXTCON || !EXTCON'

>  	help
>  	  Say Y here if you want support for the MHL interface.
>  	  It is an I2C driver, that detects connection of MHL bridge

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] drm/bridge: sii9234: use dev_err_probe where applicable

[Henrik Grimler]

Hi Dmitry,

On Sun, Jul 27, 2025 at 07:38:12PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jul 24, 2025 at 08:50:52PM +0200, Henrik Grimler wrote:
> > In case of error during resource acquisition the driver should print
> > an error message only if it is not deferred probe. Use dev_err_probe
> > helper to handle this, which will also record defer probe reason for
> > debugging.
[...]
> > @@ -836,9 +835,7 @@ static int sii9234_init_resources(struct sii9234 *ctx,
> >  	ctx->supplies[3].supply = "cvcc12";
> >  	ret = devm_regulator_bulk_get(ctx->dev, 4, ctx->supplies);
> >  	if (ret) {
> > -		if (ret != -EPROBE_DEFER)
> > -			dev_err(ctx->dev, "regulator_bulk failed\n");
> > -		return ret;
> > +		dev_err_probe(ctx->dev, ret, "regulator_bulk failed\n");
> 
> Drop the braces, use 'return dev_err_probe(...)'
> 
> >  	}
> >  
> >  	ctx->client[I2C_MHL] = client;

Thanks for reviewing and spotting that, will fix in next version!

Best regards,
Henrik Grimler

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

[Henrik Grimler]

Hi Dmitry,

On Sun, Jul 27, 2025 at 08:07:37PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jul 24, 2025 at 08:50:53PM +0200, 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.
> 
> Does HPD GPIO reflect the correct state of the cable?

Yes, the HPD gpio pin changes state from low to high when a mhl cable is
connected:

$ sudo cat /sys/kernel/debug/gpio|grep gpio-755
 gpio-755 (                    |hpd                 ) in  lo IRQ
$ sudo cat /sys/kernel/debug/gpio|grep gpio-755
 gpio-755 (                    |hpd                 ) in  hi IRQ

so that is described correctly.

> What is the order of events in such a case?

Order of events as in function tracing log? I enabled function tracing
with these filters:

  'max77693_muic_*' 'samsung_dsim_*' 'drm_bridge_*'
  'drm_atomic_bridge_*' 'exynos_irq_*' 'hdmi*' 'sii9234_*'

and captured the calls when I connect cable. dmesg with debug output
at the same time gives:

[ 6568.462521] max77693-muic max77693-muic: external connector is attached (adc:0x00, prev_adc:0x0)
[ 6568.470575] max77693-charger max77693-charger: not charging. connector type: 13
[ 6568.491722] sii9234 15-0039: sii9234: detection started d3
[ 6569.398148] i2c i2c-15: sendbytes: NAK bailout.
[ 6569.401477] sii9234 15-0039: writebm:  TPI[0x3d] <- 0x3e
[ 6569.408403] max77693-muic max77693-muic: external connector is attached (adc:0x00, prev_adc:0x0)
[ 6569.422638] max77693-charger max77693-charger: not charging. connector type: 13
[ 6569.570615] sii9234 15-0039: sii9234_irq_thread
[ 6569.622846] sii9234 15-0039: irq 00/00 42/5c 00/00
[ 6569.626182] sii9234 15-0039: RGND_READY_INT
[ 6570.592367] sii9234 15-0039: sii9234_irq_thread
[ 6570.644626] sii9234 15-0039: irq 00/60 40/5c 00/00
[ 6570.656185] sii9234 15-0039: RGND 1K!!
[ 6570.937165] sii9234 15-0039: sii9234_irq_thread
[ 6570.989467] sii9234 15-0039: irq 20/60 00/5c 00/0c
[ 6571.000986] sii9234 15-0039: MHL cable connected.. RSEN High
[ 6571.222655] sii9234 15-0039: sii9234_irq_thread
[ 6571.274884] sii9234 15-0039: irq 00/60 04/5c 00/04
[ 6571.278219] sii9234 15-0039: mhl est interrupt
[ 6571.408117] sii9234 15-0039: sii9234_irq_thread
[ 6571.460346] sii9234 15-0039: irq 40/60 00/5c 00/04

and in captured trace I see that on cable connect we get an irq that
is handled through:
1. max77693_muic_irq_handler
2. max77693_muic_irq_work
3. max77693_muic_adc_handler
4. sii9234_extcon_notifier
5. sii9234_extcon_work
6. sii9234_cable_in
7. hdmi_irq_thread

Raw captured trace dat file can be found here:
https://grimler.se/files/sii9234-mhl-connect-trace.dat

Maybe you were asking for some other type of order of events log
though, please let me know if I misunderstand.

> Should the sii9234 signal to Exynos HDMI that the link is established?

Maybe.. Sorry, I do not know enough about extcon and drm yet. I assume
you mean through drm_helper_hpd_irq_event() and
drm_bridge_hpd_notify(), I will experiment a bit and add it to the
driver and see if this improves it.

There is currently (as I wrote to Marek Szyprowski in a response in
v1) an issue where device screen stops working if cable is connected
when device screen is off, maybe proper notification would help..

> > Signed-off-by: Henrik Grimler <henrik@grimler.se>
> > ---
> > v2: add dependency on extcon. Issue reported by kernel test robot
> >     <lkp@intel.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
> 
> Either this or 'depends on EXTCON || !EXTCON'

Feels like depends is a better description so will change to it,
thanks!

Best regards,
Henrik Grimler

> >  	help
> >  	  Say Y here if you want support for the MHL interface.
> >  	  It is an I2C driver, that detects connection of MHL bridge
> 
> -- 
> With best wishes
> Dmitry
> 

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

[Marek Szyprowski]

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@grimler.se>
> ---
> v2: add dependency on extcon. Issue reported by kernel test robot
>      <lkp@intel.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
> @@ -19,6 +19,7 @@
>   
>   #include <linux/delay.h>
>   #include <linux/err.h>
> +#include <linux/extcon.h>
>   #include <linux/gpio/consumer.h>
>   #include <linux/i2c.h>
>   #include <linux/interrupt.h>
> @@ -26,6 +27,7 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/mutex.h>
> +#include <linux/of_graph.h>
>   #include <linux/regulator/consumer.h>
>   #include <linux/slab.h>
>   
> @@ -170,8 +172,12 @@ struct sii9234 {
>   	struct drm_bridge bridge;
>   	struct device *dev;
>   	struct gpio_desc *gpio_reset;
> -	int i2c_error;
>   	struct regulator_bulk_data supplies[4];
> +	struct extcon_dev *extcon;
> +	struct notifier_block extcon_nb;
> +	struct work_struct extcon_wq;
> +	int cable_state;
> +	int i2c_error;
>   
>   	struct mutex lock; /* Protects fields below and device registers */
>   	enum sii9234_state state;
> @@ -864,6 +870,70 @@ static int sii9234_init_resources(struct sii9234 *ctx,
>   	return 0;
>   }
>   
> +static void sii9234_extcon_work(struct work_struct *work)
> +{
> +	struct sii9234 *ctx =
> +		container_of(work, struct sii9234, extcon_wq);
> +	int state = extcon_get_state(ctx->extcon, EXTCON_DISP_MHL);
> +
> +	if (state == ctx->cable_state)
> +		return;
> +
> +	ctx->cable_state = state;
> +
> +	if (state > 0)
> +		sii9234_cable_in(ctx);
> +	else
> +		sii9234_cable_out(ctx);
> +}
> +
> +static int sii9234_extcon_notifier(struct notifier_block *self,
> +			unsigned long event, void *ptr)
> +{
> +	struct sii9234 *ctx =
> +		container_of(self, struct sii9234, extcon_nb);
> +
> +	schedule_work(&ctx->extcon_wq);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int sii9234_extcon_init(struct sii9234 *ctx)
> +{
> +	struct extcon_dev *edev;
> +	struct device_node *musb, *muic;
> +	int ret;
> +
> +	/* Get micro-USB connector node */
> +	musb = of_graph_get_remote_node(ctx->dev->of_node, 1, -1);
> +	/* Then get micro-USB Interface Controller node */
> +	muic = of_get_next_parent(musb);
> +
> +	if (!muic) {
> +		dev_info(ctx->dev,
> +			 "no extcon found, switching to 'always on' mode\n");
> +		return 0;
> +	}
> +
> +	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);
         }


> +
> +	ctx->extcon = edev;
> +	ctx->extcon_nb.notifier_call = sii9234_extcon_notifier;
> +	INIT_WORK(&ctx->extcon_wq, sii9234_extcon_work);
> +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
> +	if (ret) {
> +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
>   					 const struct drm_display_info *info,
>   					 const struct drm_display_mode *mode)
> @@ -916,12 +986,17 @@ static int sii9234_probe(struct i2c_client *client)
>   	if (ret < 0)
>   		return ret;
>   
> +	ret = sii9234_extcon_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +
>   	i2c_set_clientdata(client, ctx);
>   
>   	ctx->bridge.of_node = dev->of_node;
>   	drm_bridge_add(&ctx->bridge);
>   
> -	sii9234_cable_in(ctx);
> +	if (!ctx->extcon)
> +		sii9234_cable_in(ctx);
>   
>   	return 0;
>   }
> @@ -930,7 +1005,15 @@ static void sii9234_remove(struct i2c_client *client)
>   {
>   	struct sii9234 *ctx = i2c_get_clientdata(client);
>   
> -	sii9234_cable_out(ctx);
> +	if (ctx->extcon) {
> +		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
> +					   &ctx->extcon_nb);
> +		flush_work(&ctx->extcon_wq);
> +		if (ctx->cable_state > 0)
> +			sii9234_cable_out(ctx);
> +	} else {
> +		sii9234_cable_out(ctx);
> +	}
>   	drm_bridge_remove(&ctx->bridge);
>   }
>   
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Dmitry Baryshkov]

On Fri, Aug 08, 2025 at 11:52:59AM +0200, Henrik Grimler wrote:
> Hi Dmitry,
> 
> On Sun, Jul 27, 2025 at 08:07:37PM +0300, Dmitry Baryshkov wrote:
> > On Thu, Jul 24, 2025 at 08:50:53PM +0200, 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.
> > 
> > Does HPD GPIO reflect the correct state of the cable?
> 
> Yes, the HPD gpio pin changes state from low to high when a mhl cable is
> connected:
> 
> $ sudo cat /sys/kernel/debug/gpio|grep gpio-755
>  gpio-755 (                    |hpd                 ) in  lo IRQ
> $ sudo cat /sys/kernel/debug/gpio|grep gpio-755
>  gpio-755 (                    |hpd                 ) in  hi IRQ
> 
> so that is described correctly.
> 

Ack.

> 
> and in captured trace I see that on cable connect we get an irq that
> is handled through:
> 1. max77693_muic_irq_handler
> 2. max77693_muic_irq_work
> 3. max77693_muic_adc_handler
> 4. sii9234_extcon_notifier
> 5. sii9234_extcon_work
> 6. sii9234_cable_in
> 7. hdmi_irq_thread
> 
> Raw captured trace dat file can be found here:
> https://grimler.se/files/sii9234-mhl-connect-trace.dat
> 
> Maybe you were asking for some other type of order of events log
> though, please let me know if I misunderstand.
> 
> > Should the sii9234 signal to Exynos HDMI that the link is established?
> 
> Maybe.. Sorry, I do not know enough about extcon and drm yet. I assume
> you mean through drm_helper_hpd_irq_event() and
> drm_bridge_hpd_notify(), I will experiment a bit and add it to the
> driver and see if this improves it.

If you are getting the HDMI IRQ event, then I'd suggest checking that
you are actually getting the 'plugged' event, etc. I was worried that
you are hijacking the DRM chain. But if you are getting hotplug events,
then it's fine (and most likely correct).

> 
> There is currently (as I wrote to Marek Szyprowski in a response in
> v1) an issue where device screen stops working if cable is connected
> when device screen is off, maybe proper notification would help..
> 
> > > Signed-off-by: Henrik Grimler <henrik@grimler.se>
> > > ---
> > > v2: add dependency on extcon. Issue reported by kernel test robot
> > >     <lkp@intel.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
> > 
> > Either this or 'depends on EXTCON || !EXTCON'
> 
> Feels like depends is a better description so will change to it,
> thanks!
> 
> Best regards,
> Henrik Grimler
> 
> > >  	help
> > >  	  Say Y here if you want support for the MHL interface.
> > >  	  It is an I2C driver, that detects connection of MHL bridge
> > 
> > -- 
> > With best wishes
> > Dmitry
> > 

-- 
With best wishes
Dmitry

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

[Henrik Grimler]

Hi Marek,

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

Best regards,
Henrik Grimler

> > +
> > +	ctx->extcon = edev;
> > +	ctx->extcon_nb.notifier_call = sii9234_extcon_notifier;
> > +	INIT_WORK(&ctx->extcon_wq, sii9234_extcon_work);
> > +	ret = extcon_register_notifier(edev, EXTCON_DISP_MHL, &ctx->extcon_nb);
> > +	if (ret) {
> > +		dev_err(ctx->dev, "failed to register notifier for MHL\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   static enum drm_mode_status sii9234_mode_valid(struct drm_bridge *bridge,
> >   					 const struct drm_display_info *info,
> >   					 const struct drm_display_mode *mode)
> > @@ -916,12 +986,17 @@ static int sii9234_probe(struct i2c_client *client)
> >   	if (ret < 0)
> >   		return ret;
> >   
> > +	ret = sii9234_extcon_init(ctx);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >   	i2c_set_clientdata(client, ctx);
> >   
> >   	ctx->bridge.of_node = dev->of_node;
> >   	drm_bridge_add(&ctx->bridge);
> >   
> > -	sii9234_cable_in(ctx);
> > +	if (!ctx->extcon)
> > +		sii9234_cable_in(ctx);
> >   
> >   	return 0;
> >   }
> > @@ -930,7 +1005,15 @@ static void sii9234_remove(struct i2c_client *client)
> >   {
> >   	struct sii9234 *ctx = i2c_get_clientdata(client);
> >   
> > -	sii9234_cable_out(ctx);
> > +	if (ctx->extcon) {
> > +		extcon_unregister_notifier(ctx->extcon, EXTCON_DISP_MHL,
> > +					   &ctx->extcon_nb);
> > +		flush_work(&ctx->extcon_wq);
> > +		if (ctx->cable_state > 0)
> > +			sii9234_cable_out(ctx);
> > +	} else {
> > +		sii9234_cable_out(ctx);
> > +	}
> >   	drm_bridge_remove(&ctx->bridge);
> >   }
> >   
> >
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 

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

[Marek Szyprowski]

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

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

[Henrik Grimler]

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

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

[Henrik Grimler]

Hi Dmitry,

On Mon, Aug 18, 2025 at 03:42:07AM +0300, Dmitry Baryshkov wrote:
> On Fri, Aug 08, 2025 at 11:52:59AM +0200, Henrik Grimler wrote:
> > Hi Dmitry,
> > 
> > On Sun, Jul 27, 2025 at 08:07:37PM +0300, Dmitry Baryshkov wrote:
> > > On Thu, Jul 24, 2025 at 08:50:53PM +0200, 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.
> > > 
> > > Does HPD GPIO reflect the correct state of the cable?
> > 
> > Yes, the HPD gpio pin changes state from low to high when a mhl cable is
> > connected:
> > 
> > $ sudo cat /sys/kernel/debug/gpio|grep gpio-755
> >  gpio-755 (                    |hpd                 ) in  lo IRQ
> > $ sudo cat /sys/kernel/debug/gpio|grep gpio-755
> >  gpio-755 (                    |hpd                 ) in  hi IRQ
> > 
> > so that is described correctly.
> > 
> 
> Ack.
> 
> > 
> > and in captured trace I see that on cable connect we get an irq that
> > is handled through:
> > 1. max77693_muic_irq_handler
> > 2. max77693_muic_irq_work
> > 3. max77693_muic_adc_handler
> > 4. sii9234_extcon_notifier
> > 5. sii9234_extcon_work
> > 6. sii9234_cable_in
> > 7. hdmi_irq_thread
> > 
> > Raw captured trace dat file can be found here:
> > https://grimler.se/files/sii9234-mhl-connect-trace.dat
> > 
> > Maybe you were asking for some other type of order of events log
> > though, please let me know if I misunderstand.
> > 
> > > Should the sii9234 signal to Exynos HDMI that the link is established?
> > 
> > Maybe.. Sorry, I do not know enough about extcon and drm yet. I assume
> > you mean through drm_helper_hpd_irq_event() and
> > drm_bridge_hpd_notify(), I will experiment a bit and add it to the
> > driver and see if this improves it.
> 
> If you are getting the HDMI IRQ event, then I'd suggest checking that
> you are actually getting the 'plugged' event, etc. I was worried that
> you are hijacking the DRM chain. But if you are getting hotplug events,
> then it's fine (and most likely correct).

With some debugging in sii9234_extcon_notifier added:

--- a/drivers/gpu/drm/bridge/sii9234.c
+++ b/drivers/gpu/drm/bridge/sii9234.c
@@ -892,6 +892,8 @@ static int sii9234_extcon_notifier(struct notifier_block *self,
        struct sii9234 *ctx =
                container_of(self, struct sii9234, extcon_nb);
 
+       dev_info(ctx->dev, "extcon event %lu\n", event);
+
        schedule_work(&ctx->extcon_wq);
 
        return NOTIFY_DONE;

I see that sii9234 receives the hotplug event. On plug in:

[  532.132981] sii9234 15-0039: extcon event 0
[  532.136601] max77693-muic max77693-muic: external connector is attached (adc:0x00, prev_adc:0x0)
[  532.142777] sii9234 15-0039: RSEN_HIGH without RGND_1K
[  532.149815] sii9234 15-0039: extcon event 1
[  532.155662] max77693-charger max77693-charger: not charging. connector type: 13
[  532.164801] sii9234 15-0039: extcon event 0
[  532.168371] max77693-muic max77693-muic: external connector is detached(chg_type:0x0, prev_chg_type:0x0)
[  532.178370] sii9234 15-0039: extcon event 0
[  532.188250] max77693-charger max77693-charger: not charging. connector type: 13
[  533.097415] i2c i2c-15: sendbytes: NAK bailout.
[  533.100735] sii9234 15-0039: writebm:  TPI[0x3d] <- 0x3e
[  533.115161] sii9234 15-0039: writeb:  TPI[0x3d] <- 0x00

and disconnect:

[  547.195219] dwc2 12480000.usb: new device is full-speed
[  547.204912] max77693-muic max77693-muic: external connector is attached (adc:0x00, prev_adc:0x0)
[  547.212629] sii9234 15-0039: extcon event 1
[  547.218304] max77693-charger max77693-charger: not charging. connector type: 13
[  548.159257] i2c i2c-15: sendbytes: NAK bailout.
[  548.162602] sii9234 15-0039: writebm:  TPI[0x3d] <- 0x3e
[  548.167990] sii9234 15-0039: extcon event 0
[  548.172788] max77693-muic max77693-muic: external connector is attached (adc:0x00, prev_adc:0x0)
[  548.181336] sii9234 15-0039: extcon event 1
[  548.186510] max77693-charger max77693-charger: not charging. connector type: 13

It seems a bit weird to me that it receives multiple events, but maybe
that is expected. Will send a v3 shortly, thank you!

Best regards,
Henrik Grimler