[RFT PATCH 01/15] drm/armada: Call drm_atomic_helper_shutdown() at shutdown time

Maxime Ripard mripard at kernel.org
Mon Sep 4 07:36:10 UTC 2023


On Sun, Sep 03, 2023 at 04:53:42PM +0100, Russell King (Oracle) wrote:
> On Fri, Sep 01, 2023 at 04:41:12PM -0700, Douglas Anderson wrote:
> > Based on grepping through the source code this driver appears to be
> > missing a call to drm_atomic_helper_shutdown() at system shutdown
> > time. Among other things, this means that if a panel is in use that it
> > won't be cleanly powered off at system shutdown time.
> > 
> > The fact that we should call drm_atomic_helper_shutdown() in the case
> > of OS shutdown/restart comes straight out of the kernel doc "driver
> > instance overview" in drm_drv.c.
> > 
> > This driver was fairly easy to update. The drm_device is stored in the
> > drvdata so we just have to make sure the drvdata is NULL whenever the
> > device is not bound.
> 
> ... and there I think you have a misunderstanding of the driver model.
> Please have a look at device_unbind_cleanup() which will be called if
> probe fails, or when the device is removed (in other words, when it is
> not bound to a driver.)
> 
> Also, devices which aren't bound to a driver won't have their shutdown
> method called (because there is no driver currently bound to that
> device.) So, ->probe must have completed successfully, and ->remove
> must not have been called for that device.
> 
> So, I think that all these dev_set_drvdata(dev, NULL) that you're
> adding are just asking for a kernel janitor to come along later and
> remove them because they serve no purpose... so best not introduce
> them in the first place.

What would that hypothetical janitor clean up exactly? Code making sure
that there's no dangling pointer? Doesn't look very wise to me.

Maxime


More information about the dri-devel mailing list