All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] drm/panel-edp: take i2c adapter module reference
@ 2026-07-17 14:31 Johan Hovold
  2026-07-17 14:31 ` [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure Johan Hovold
  2026-07-17 14:31 ` [PATCH v2 2/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
  0 siblings, 2 replies; 6+ messages in thread
From: Johan Hovold @ 2026-07-17 14:31 UTC (permalink / raw)
  To: Douglas Anderson, Neil Armstrong, Jessica Zhang,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
  Cc: David Airlie, Simona Vetter, dri-devel, linux-kernel,
	Johan Hovold

The i2c subsystem currently blocks during adapter deregistration
whenever there are consumers holding a reference.
        
Switch to using of_get_i2c_adapter_by_node() which also takes a
reference to the adapter module so that an attempt to unload the module
while in use fails gracefully instead of blocking uninterruptibly.

Johan


Changes in v2:
 - fix a pre-existing leak in a corner case flagged by Sashiko that
   Doug thought was worth fixing
 - rebase module reference patch on top of that fix


Johan Hovold (2):
  drm/panel-edp: fix i2c adapter leak on probe failure
  drm/panel-edp: take i2c adapter module reference

 drivers/gpu/drm/panel/panel-edp.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure
  2026-07-17 14:31 [PATCH v2 0/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
@ 2026-07-17 14:31 ` Johan Hovold
  2026-07-17 14:43   ` sashiko-bot
  2026-07-17 17:03   ` Doug Anderson
  2026-07-17 14:31 ` [PATCH v2 2/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
  1 sibling, 2 replies; 6+ messages in thread
From: Johan Hovold @ 2026-07-17 14:31 UTC (permalink / raw)
  To: Douglas Anderson, Neil Armstrong, Jessica Zhang,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
  Cc: David Airlie, Simona Vetter, dri-devel, linux-kernel,
	Johan Hovold, stable

Make sure to drop the i2c adapter reference on probe failure (e.g.
probe deferral) and on driver unbind also if a devicetree redundantly
uses the 'ddc-i2c-bus' property to point to the aux ddc bus.

Fixes: cc5a3fc041f0 ("drm/panel: panel-simple: Stash DP AUX bus; allow using it for DDC")
Cc: stable@vger.kernel.org	# 5.15
Reported-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/CAD=FV=VZPhzHU+Pet2m3L+Pqc7mOPfZC-f5p0OuNL79wNZPxRg@mail.gmail.com
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/gpu/drm/panel/panel-edp.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index aa27d6cd932e..ac815729aa83 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -831,6 +831,13 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
 	return 0;
 }
 
+static void panel_edp_put_adapter(void *_adap)
+{
+	struct i2c_adapter *adap = _adap;
+
+	put_device(&adap->dev);
+}
+
 static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
 			   struct drm_dp_aux *aux)
 {
@@ -878,6 +885,11 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
 
 		if (!panel->ddc)
 			return -EPROBE_DEFER;
+
+		err = devm_add_action_or_reset(dev, panel_edp_put_adapter,
+					       panel->ddc);
+		if (err)
+			return err;
 	} else if (aux) {
 		panel->ddc = &aux->ddc;
 	}
@@ -889,7 +901,7 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
 
 	err = drm_panel_of_backlight(&panel->base);
 	if (err)
-		goto err_finished_ddc_init;
+		return err;
 
 	/*
 	 * We use runtime PM for prepare / unprepare since those power the panel
@@ -936,9 +948,6 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
 err_finished_pm_runtime:
 	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_disable(dev);
-err_finished_ddc_init:
-	if (panel->ddc && (!panel->aux || panel->ddc != &panel->aux->ddc))
-		put_device(&panel->ddc->dev);
 
 	return err;
 }
@@ -982,8 +991,6 @@ static void panel_edp_remove(struct device *dev)
 
 	pm_runtime_dont_use_autosuspend(dev);
 	pm_runtime_disable(dev);
-	if (panel->ddc && (!panel->aux || panel->ddc != &panel->aux->ddc))
-		put_device(&panel->ddc->dev);
 
 	drm_edid_free(panel->drm_edid);
 	panel->drm_edid = NULL;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] drm/panel-edp: take i2c adapter module reference
  2026-07-17 14:31 [PATCH v2 0/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
  2026-07-17 14:31 ` [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure Johan Hovold
@ 2026-07-17 14:31 ` Johan Hovold
  2026-07-17 17:04   ` Doug Anderson
  1 sibling, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2026-07-17 14:31 UTC (permalink / raw)
  To: Douglas Anderson, Neil Armstrong, Jessica Zhang,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
  Cc: David Airlie, Simona Vetter, dri-devel, linux-kernel,
	Johan Hovold

The i2c subsystem currently blocks during adapter deregistration
whenever there are consumers holding a reference.

Switch to using of_get_i2c_adapter_by_node() which also takes a
reference to the adapter module so that an attempt to unload the module
while in use fails gracefully instead of blocking uninterruptibly.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/gpu/drm/panel/panel-edp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
index ac815729aa83..522bc665a11a 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -835,7 +835,7 @@ static void panel_edp_put_adapter(void *_adap)
 {
 	struct i2c_adapter *adap = _adap;
 
-	put_device(&adap->dev);
+	i2c_put_adapter(adap);
 }
 
 static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
@@ -880,7 +880,7 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
 
 	ddc = of_parse_phandle(dev->of_node, "ddc-i2c-bus", 0);
 	if (ddc) {
-		panel->ddc = of_find_i2c_adapter_by_node(ddc);
+		panel->ddc = of_get_i2c_adapter_by_node(ddc);
 		of_node_put(ddc);
 
 		if (!panel->ddc)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure
  2026-07-17 14:31 ` [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure Johan Hovold
@ 2026-07-17 14:43   ` sashiko-bot
  2026-07-17 17:03   ` Doug Anderson
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-17 14:43 UTC (permalink / raw)
  To: Johan Hovold; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The device can be left permanently powered on if the driver is unbound while an asynchronous autosuspend is pending.
--

--- Patch [1]: [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure ---
commit 471fbc83c1856bd9eec16a225d0ec84f3032c113
Author: Johan Hovold <johan@kernel.org>

drm/panel-edp: fix i2c adapter leak on probe failure

This commit fixes a reference leak on an i2c adapter during probe failure
or driver unbind. It introduces a devres action to automatically drop the
adapter reference, streamlining the cleanup path and removing manual puts.

Link: https://lore.kernel.org/r/CAD=FV=VZPhzHU+Pet2m3L+Pqc7mOPfZC-f5p0OuNL79wNZPxRg@mail.gmail.com

> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index f66474d3ef65e..775bd6af31ae7 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c

[ ... ]

> @@ -983,8 +992,6 @@ static void panel_edp_remove(struct device *dev)
>  
>  	pm_runtime_dont_use_autosuspend(dev);
>  	pm_runtime_disable(dev);

[Severity: High]
This is a pre-existing issue, but does this sequence leave the device
permanently powered on if unbound while an autosuspend is pending?

During probe, pm_runtime_get_sync() is followed by
pm_runtime_put_autosuspend() (e.g., in generic_edp_panel_probe()), leaving
the device in RPM_ACTIVE for 1000ms. If the driver is unbound before this
timer fires, pm_runtime_disable() cancels the pending autosuspend work
without suspending the device.

This can leave the panel regulators and GPIOs permanently enabled, and
trigger a kernel WARN_ON from devres during regulator_put() due to an
unbalanced enable count. Should there be a synchronous suspend call prior
to disabling runtime PM in the removal path?

> -	if (panel->ddc && (!panel->aux || panel->ddc != &panel->aux->ddc))
> -		put_device(&panel->ddc->dev);
>  
>  	drm_edid_free(panel->drm_edid);
>  	panel->drm_edid = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717143119.1815106-1-johan@kernel.org?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure
  2026-07-17 14:31 ` [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure Johan Hovold
  2026-07-17 14:43   ` sashiko-bot
@ 2026-07-17 17:03   ` Doug Anderson
  1 sibling, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2026-07-17 17:03 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, stable

Hi,

On Fri, Jul 17, 2026 at 7:32 AM Johan Hovold <johan@kernel.org> wrote:
>
> Make sure to drop the i2c adapter reference on probe failure (e.g.
> probe deferral) and on driver unbind also if a devicetree redundantly
> uses the 'ddc-i2c-bus' property to point to the aux ddc bus.
>
> Fixes: cc5a3fc041f0 ("drm/panel: panel-simple: Stash DP AUX bus; allow using it for DDC")
> Cc: stable@vger.kernel.org      # 5.15
> Reported-by: Douglas Anderson <dianders@chromium.org>
> Link: https://lore.kernel.org/r/CAD=FV=VZPhzHU+Pet2m3L+Pqc7mOPfZC-f5p0OuNL79wNZPxRg@mail.gmail.com
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/gpu/drm/panel/panel-edp.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
> index aa27d6cd932e..ac815729aa83 100644
> --- a/drivers/gpu/drm/panel/panel-edp.c
> +++ b/drivers/gpu/drm/panel/panel-edp.c
> @@ -831,6 +831,13 @@ static int generic_edp_panel_probe(struct device *dev, struct panel_edp *panel)
>         return 0;
>  }
>
> +static void panel_edp_put_adapter(void *_adap)
> +{
> +       struct i2c_adapter *adap = _adap;
> +
> +       put_device(&adap->dev);
> +}
> +
>  static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
>                            struct drm_dp_aux *aux)
>  {
> @@ -878,6 +885,11 @@ static int panel_edp_probe(struct device *dev, const struct panel_desc *desc,
>
>                 if (!panel->ddc)
>                         return -EPROBE_DEFER;
> +
> +               err = devm_add_action_or_reset(dev, panel_edp_put_adapter,
> +                                              panel->ddc);
> +               if (err)
> +                       return err;

Yeah, devm makes this so much nicer. Thanks for fixing!

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'll plan to apply this, probably to drm-misc-next next week. Unless
someone objects, I won't land it -fixes because it's such a corner
case.

-Doug

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] drm/panel-edp: take i2c adapter module reference
  2026-07-17 14:31 ` [PATCH v2 2/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
@ 2026-07-17 17:04   ` Doug Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2026-07-17 17:04 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel

Hi,

On Fri, Jul 17, 2026 at 7:32 AM Johan Hovold <johan@kernel.org> wrote:
>
> The i2c subsystem currently blocks during adapter deregistration
> whenever there are consumers holding a reference.
>
> Switch to using of_get_i2c_adapter_by_node() which also takes a
> reference to the adapter module so that an attempt to unload the module
> while in use fails gracefully instead of blocking uninterruptibly.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/gpu/drm/panel/panel-edp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-17 17:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 14:31 [PATCH v2 0/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
2026-07-17 14:31 ` [PATCH v2 1/2] drm/panel-edp: fix i2c adapter leak on probe failure Johan Hovold
2026-07-17 14:43   ` sashiko-bot
2026-07-17 17:03   ` Doug Anderson
2026-07-17 14:31 ` [PATCH v2 2/2] drm/panel-edp: take i2c adapter module reference Johan Hovold
2026-07-17 17:04   ` Doug Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.