From: Thomas Zimmermann <tzimmermann@suse.de>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>,
Marek Vasut <marex@denx.de>, Stefan Agner <stefan@agner.ch>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
Shawn Guo <shawnguo@kernel.org>,
Sascha Hauer <s.hauer@pengutronix.de>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Fabio Estevam <festevam@gmail.com>,
Dario Binacchi <dario.binacchi@amarulasolutions.com>,
Michael Trimarchi <michael@amarulasolutions.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
dri-devel@lists.freedesktop.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm/mxsfb: fix missing rollback on failure in mxsfb_probe()
Date: Thu, 13 Mar 2025 17:02:47 +0100 [thread overview]
Message-ID: <a2f39979-459d-4581-baed-ff75e0af1bce@suse.de> (raw)
In-Reply-To: <20250313-mxsfb_probe-fix-rollback-on-error-v2-1-38374088e8c7@bootlin.com>
Hi
Am 13.03.25 um 16:21 schrieb Luca Ceresoli:
> When aperture_remove_all_conflicting_devices() fails, the current code
> returns without going through the rollback actions at the end of the
> function, thus the actions done by drm_dev_alloc() and mxsfb_load() are not
> undone.
>
> Fix by moving this call at the very beginning of the probe function, so
> that no rollback is needed if it fails. Conflicting drivers need to be
> kicked out before setting up DRM anyway.
>
> Fixes: c8e7b185d45b ("drm/mxsfb: Remove generic DRM drivers in probe function")
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
The patchdiff is somewhat non-intuitive, but looks correct.
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Thanks for the fix.
Best regards
Thomas
> ---
> The offending commit is not yet merged into master, and even less in a
> released kernel, so this does not need to go through stable.
>
> Changes in v2:
> - move this call at the beginning instead of adding a goto
> ---
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index c183b1112bc4e9fe4f3b048a2b6e4c98d1d47cb3..ee64053d381448360140c419fed1dc4fe9f7c68e 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -352,14 +352,6 @@ static int mxsfb_probe(struct platform_device *pdev)
> struct drm_device *drm;
> int ret;
>
> - drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);
> - if (IS_ERR(drm))
> - return PTR_ERR(drm);
> -
> - ret = mxsfb_load(drm, device_get_match_data(&pdev->dev));
> - if (ret)
> - goto err_free;
> -
> /*
> * Remove early framebuffers (ie. simplefb). The framebuffer can be
> * located anywhere in RAM
> @@ -369,6 +361,14 @@ static int mxsfb_probe(struct platform_device *pdev)
> return dev_err_probe(&pdev->dev, ret,
> "can't kick out existing framebuffers\n");
>
> + drm = drm_dev_alloc(&mxsfb_driver, &pdev->dev);
> + if (IS_ERR(drm))
> + return PTR_ERR(drm);
> +
> + ret = mxsfb_load(drm, device_get_match_data(&pdev->dev));
> + if (ret)
> + goto err_free;
> +
> ret = drm_dev_register(drm, 0);
> if (ret)
> goto err_unload;
>
> ---
> base-commit: f9f087d946266bc5da7c3a17bd8fd9d01969e3cf
> change-id: 20250313-mxsfb_probe-fix-rollback-on-error-3074b9080f34
>
> Best regards,
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
prev parent reply other threads:[~2025-03-13 16:02 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-13 15:21 Luca Ceresoli
2025-03-13 16:02 ` Thomas Zimmermann [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=a2f39979-459d-4581-baed-ff75e0af1bce@suse.de \
--to=tzimmermann@suse.de \
--cc=airlied@gmail.com \
--cc=dario.binacchi@amarulasolutions.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=festevam@gmail.com \
--cc=imx@lists.linux.dev \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luca.ceresoli@bootlin.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=marex@denx.de \
--cc=michael@amarulasolutions.com \
--cc=mripard@kernel.org \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=simona@ffwll.ch \
--cc=stefan@agner.ch \
--cc=thomas.petazzoni@bootlin.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®