On Fri, 08 Jun 2012, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Within drm_fill_in_dev we call drm_lastclose to clean up the dirt in
> case of failures. But the callers of drm_fill_in_dev simply don't do
> anything. Now from a cursory look drm_lastclose doesn't look like the
> best cleanup function in itself, but whom am I to judge the drm error
> paths. Imo before we could tackle this we need to move more of the
> legacy drm services setup and teardown into the respective drivers,
> that way drm_lastclose would become more manageble.
>
> Anyway, make things at least consistent by adding drm_lastclose at the
> right places in the error paths for all callers of drm_fill_in_dev.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/drm_pci.c | 8 +++++---
> drivers/gpu/drm/drm_platform.c | 8 +++++---
> drivers/gpu/drm/drm_usb.c | 8 +++++---
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 73218ac..40737a8 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -334,14 +334,14 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>
> if ((ret = drm_fill_in_dev(dev, ent, driver))) {
> printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
> - goto err_g2;
> + goto err_pci;
> }
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> pci_set_drvdata(pdev, dev);
> ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
> if (ret)
> - goto err_g2;
> + goto err_drm_fill_in;
> }
>
> if ((ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY)))
> @@ -375,7 +375,9 @@ err_g4:
> err_g3:
> if (drm_core_check_feature(dev, DRIVER_MODESET))
> drm_put_minor(&dev->control);
> -err_g2:
> +err_drm_fill_in:
> + drm_lastclose(dev);
> +err_pci:
> pci_disable_device(pdev);
> err_g1:
> kfree(dev);
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index 82431dc..177893a 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -60,14 +60,14 @@ int drm_get_platform_dev(struct platform_device *platdev,
>
> if (ret) {
> printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
> - goto err_g1;
> + goto err_kfree;
> }
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> dev_set_drvdata(&platdev->dev, dev);
> ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
> if (ret)
> - goto err_g1;
> + goto err_drm_fill_in;
> }
>
> ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
> @@ -103,7 +103,9 @@ err_g3:
> err_g2:
> if (drm_core_check_feature(dev, DRIVER_MODESET))
> drm_put_minor(&dev->control);
> -err_g1:
> +err_drm_fill_in:
> + drm_lastclose(dev);
> +err_kfree:
> kfree(dev);
> mutex_unlock(&drm_global_mutex);
> return ret;
> diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
> index 37c9a52..2e179f1 100644
> --- a/drivers/gpu/drm/drm_usb.c
> +++ b/drivers/gpu/drm/drm_usb.c
> @@ -25,13 +25,13 @@ int drm_get_usb_dev(struct usb_interface *interface,
> ret = drm_fill_in_dev(dev, NULL, driver);
> if (ret) {
> printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
> - goto err_g1;
> + goto err_drm_fill_in;
> }
>
> usb_set_intfdata(interface, dev);
> ret = drm_get_minor(dev, &dev->control, DRM_MINOR_CONTROL);
> if (ret)
> - goto err_g1;
> + goto err_kfree;
The above gotos err_drm_fill_in/err_kfree should switch places.
BR,
Jani.
>
> ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
> if (ret)
> @@ -63,7 +63,9 @@ err_g3:
> drm_put_minor(&dev->primary);
> err_g2:
> drm_put_minor(&dev->control);
> -err_g1:
> +err_drm_fill_in:
> + drm_lastclose(dev);
> +err_kfree:
> kfree(dev);
> mutex_unlock(&drm_global_mutex);
> return ret;
> --
> 1.7.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
