Hi Noralf,

Thank you for the patch.

On Monday, 28 August 2017 20:17:46 EEST Noralf Trønnes wrote:
> Might as well embed drm_device since tinydrm_device (embeds pipe struct
> and fbdev pointer) needs to stick around after driver-device unbind to
> handle open fd's after device removal.
> 
> Cc: David Lechner <da...@lechnology.com>
> Signed-off-by: Noralf Trønnes <nor...@tronnes.org>
> ---
>  drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 44  ++++++++++++-------------
>  drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  2 +-
>  drivers/gpu/drm/tinydrm/mi0283qt.c          |  8 +++---
>  drivers/gpu/drm/tinydrm/mipi-dbi.c          | 12 ++++----
>  drivers/gpu/drm/tinydrm/repaper.c           | 10 +++----
>  drivers/gpu/drm/tinydrm/st7586.c            | 16 +++++------
>  include/drm/tinydrm/tinydrm.h               |  9 +++++-
>  7 files changed, 50 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
> b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c index 551709e..f11f4cd 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c

[snip]

> @@ -142,23 +142,16 @@ static int tinydrm_init(struct device *parent, struct
> tinydrm_device *tdev, const struct drm_framebuffer_funcs *fb_funcs,
>                       struct drm_driver *driver)
>  {
> -     struct drm_device *drm;
> +     struct drm_device *drm = &tdev->drm;
> +     int ret;
> 
>       mutex_init(&tdev->dirty_lock);
>       tdev->fb_funcs = fb_funcs;
> 
> -     /*
> -      * We don't embed drm_device, because that prevent us from using
> -      * devm_kzalloc() to allocate tinydrm_device in the driver since
> -      * drm_dev_unref() frees the structure. The devm_ functions provide
> -      * for easy error handling.

Don't you then need a custom drm_driver.release handler to free the parent 
structure ?

> -      */
> -     drm = drm_dev_alloc(driver, parent);
> -     if (IS_ERR(drm))
> -             return PTR_ERR(drm);
> -
> -     tdev->drm = drm;
> -     drm->dev_private = tdev;
> +     ret = drm_dev_init(drm, driver, parent);
> +     if (ret)
> +             return ret;
> +
>       drm_mode_config_init(drm);
>       drm->mode_config.funcs = &tinydrm_mode_config_funcs;
> 

[snip]

> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c
> b/drivers/gpu/drm/tinydrm/mi0283qt.c index 7e5bb7d..77d40c9 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c

[snip]

> @@ -169,7 +169,7 @@ static int mi0283qt_probe(struct spi_device *spi)
>       u32 rotation = 0;
>       int ret;
> 
> -     mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> +     mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);

Where's the related kfree() ?

>       if (!mipi)
>               return -ENOMEM;

[snip]

> diff --git a/drivers/gpu/drm/tinydrm/repaper.c
> b/drivers/gpu/drm/tinydrm/repaper.c index 30dc97b..b8fc8eb 100644
> --- a/drivers/gpu/drm/tinydrm/repaper.c
> +++ b/drivers/gpu/drm/tinydrm/repaper.c

[snip]

> @@ -949,7 +949,7 @@ static int repaper_probe(struct spi_device *spi)
>               }
>       }
> 
> -     epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL);
> +     epd = kzalloc(sizeof(*epd), GFP_KERNEL);

Ditto.

>       if (!epd)
>               return -ENOMEM;

[snip]

> diff --git a/drivers/gpu/drm/tinydrm/st7586.c
> b/drivers/gpu/drm/tinydrm/st7586.c index b439956..bc2b905 100644
> --- a/drivers/gpu/drm/tinydrm/st7586.c
> +++ b/drivers/gpu/drm/tinydrm/st7586.c

[snip]

> @@ -349,7 +349,7 @@ static int st7586_probe(struct spi_device *spi)
>       u32 rotation = 0;
>       int ret;
> 
> -     mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> +     mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);

Ang here again.

>       if (!mipi)
>               return -ENOMEM;

[snip]

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to