All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Osama Abdelkader" <osama.abdelkader@gmail.com>,
	<luca.ceresoli@bootlin.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Jessica Zhang" <jesszhan0024@gmail.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 4/4] drm/panel: simple: Use managed helpers in panel-simple
Date: Wed, 16 Sep 2026 09:50:35 +0200	[thread overview]
Message-ID: <DLGKRRWANR4E.1AG4LI99DCJ2F@bootlin.com> (raw)
In-Reply-To: <20260915145935.60397-5-osama.abdelkader@gmail.com>

Hello Osama,

On Tue Sep 15, 2026 at 4:59 PM CEST, Osama Abdelkader wrote:
> Use the managed panel add helper in panel-simple and drop the manual
> panel remove call from the remove path.
>
> Keep the MIPI DSI attach and detach manual, since panel-simple's
> remove callback also unprepares/disables the panel and releases the
> DDC adapter; deferring the detach to devm would run it after that
> cleanup instead of before, changing the teardown order.
>
> Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>

Thanks for continuing the cleanup.

However there is still some confusion in how you are sending patch series:

 * You sent the patches to various recipients including mailing lists
   (which is OK), but did not send the cover letter to mailing lists. I got
   the cover and it only has me in To: and nobody in Cc:.

 * You sent two iterations the same day without an explanation; this is
   normally a confusing behaviour, which makes sense only rarely and for a
   very good reason which is clearly explained in reply to the previous
   revision (e.g. "sorry, this series is broken, ignore it, sending a fixed
   one very soon")

 * The panel-simple patch disappeared in v6, why? (see next question)

 * The Changelog for v6 (not visible on mailing lists) is overly verbose,
   so I didn't even try reading it. Please use oneliners or so. And in case
   you did, don't uncritically copy-paste AI slop. Think about your readers
   and te minimum info they need.

I think I already suggested using b4 to send your patches, which would
handle the mechanical aspects for you. It prevent issues like the first
bullet above.

Can you please send a v7 with the above fixed, as well as the changelog
made concise (including the v5 and v6 entries)? I'll be glad to review
that.

Kind regards,
Luca

> ---
>  drivers/gpu/drm/panel/panel-simple.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index c95964ba25a9..02d21dde5766 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -737,7 +737,9 @@ static struct panel_simple *panel_simple_probe(struct device *dev)
>  		goto disable_pm_runtime;
>  	}
>
> -	drm_panel_add(&panel->base);
> +	err = devm_drm_panel_add(dev, &panel->base);
> +	if (err)
> +		goto disable_pm_runtime;
>
>  	return panel;
>
> @@ -785,7 +787,6 @@ static void panel_simple_remove(struct device *dev)
>  {
>  	struct panel_simple *panel = dev_get_drvdata(dev);
>
> -	drm_panel_remove(&panel->base);
>  	panel_simple_shutdown(dev);
>
>  	pm_runtime_dont_use_autosuspend(dev);
> @@ -6540,7 +6541,6 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
>  {
>  	const struct panel_desc_dsi *desc;
>  	struct panel_simple *panel;
> -	int err;
>
>  	panel = panel_simple_probe(&dsi->dev);
>  	if (IS_ERR(panel))
> @@ -6551,14 +6551,7 @@ static int panel_simple_dsi_probe(struct mipi_dsi_device *dsi)
>  	dsi->format = desc->format;
>  	dsi->lanes = desc->lanes;
>
> -	err = mipi_dsi_attach(dsi);
> -	if (err) {
> -		struct panel_simple *panel = mipi_dsi_get_drvdata(dsi);
> -
> -		drm_panel_remove(&panel->base);
> -	}
> -
> -	return err;
> +	return mipi_dsi_attach(dsi);
>  }
>
>  static void panel_simple_dsi_remove(struct mipi_dsi_device *dsi)





--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  parent reply	other threads:[~2026-09-16  7:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260915145935.60397-1-osama.abdelkader@gmail.com>
2026-09-15 14:59 ` [PATCH v5 1/4] drm/panel: Use devm_mipi_dsi_attach in BOE panels Osama Abdelkader
2026-09-15 14:59 ` [PATCH v5 2/4] drm/panel: Use managed helpers in Novatek and Raydium panels Osama Abdelkader
2026-09-15 14:59 ` [PATCH v5 3/4] drm/panel: Use managed helpers in assorted panels Osama Abdelkader
2026-09-15 15:11   ` sashiko-bot
2026-09-15 14:59 ` [PATCH v5 4/4] drm/panel: simple: Use managed helpers in panel-simple Osama Abdelkader
2026-09-15 15:08   ` sashiko-bot
2026-09-16  7:50   ` Luca Ceresoli [this message]
2026-09-16 17:25     ` Osama Abdelkader

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLGKRRWANR4E.1AG4LI99DCJ2F@bootlin.com \
    --to=luca.ceresoli@bootlin.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jesszhan0024@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=osama.abdelkader@gmail.com \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.