From: Osama Abdelkader <osama.abdelkader@gmail.com>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>
Cc: 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 19:25:16 +0200 [thread overview]
Message-ID: <aqrQ_BDn67QXG94p@osama> (raw)
In-Reply-To: <DLGKRRWANR4E.1AG4LI99DCJ2F@bootlin.com>
Hello Luca,
Thanks for the reply.
On Wed, Sep 16, 2026 at 09:50:35AM +0200, Luca Ceresoli wrote:
> 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:.
>
I see, I used:
git send-email --to-cmd='scripts/get_maintainer.pl --no-rolestats' --to=luca.ceresoli@bootlin.com v6-000*
get_maintainer.pl doesn't return maintainer for the cover letter.
Thanks for the notice, will fix that.
> * 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")
>
Sorry for the confusion, once I sent v5 sashiko-bot caught couple of issues
including compile errors, that's why I was in rush to fix that.
> * The panel-simple patch disappeared in v6, why? (see next question)
>
There was a concern from sashiko-bot on v5 that the devm_ changes in panel-simple
may lead to use-after-free window, that's why I excluded patch 4 in v6.
I have to write that in changelog, yes.
> * 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 see, I took sashiko report copy/paste. It's too verbose, will clean it up
and summarize it in next version.
>
> 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.
I will try it, I just didn't use it before and didn't want to make something
wrong so tried to stick to git send-email.
>
> 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.
>
Sure, thanks.
> Kind regards,
> Luca
>
Kind regards,
Osama
> > ---
> > 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
prev parent reply other threads:[~2026-09-16 17:25 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
2026-09-16 17:25 ` Osama Abdelkader [this message]
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=aqrQ_BDn67QXG94p@osama \
--to=osama.abdelkader@gmail.com \
--cc=airlied@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=jesszhan0024@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.ceresoli@bootlin.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=neil.armstrong@linaro.org \
--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.