Hi Noralf,
On Thursday, 31 August 2017 20:16:42 EEST Noralf Trønnes wrote:
Den 31.08.2017 12.18, skrev Laurent Pinchart:
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 david@lechnology.com Signed-off-by: Noralf Trønnes noralf@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 ?
I rely on the fact that drm_device is the first member in the driver structure and thus it will be freed in drm_dev_release(). A later patch adds a drm_driver.release function though.
That's a bit hackish. As a later patch changes this I'd be OK with this one, but you should mention that you rely on the structure layout in the commit message.
*/
- 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]