Re: [PATCH] drm/tilcdc: Allocate register storage based on the actual number registers

Reviewed-By: Michael Bode michael@bumbleB.de

Jyri,

good idea to allocate the space for registers dynamically. This eliminates the need of synchronizing driver and header file in an elegant way.

Only thing I personally don't like, is potentially passing a NULL pointer to kfree. But I learned that this is valid kernel code.

Br, Michael

On 07/03/2015 12:45 PM, Jyri Sarha wrote:

Allocate suspend/resume register storage based on the actual number registers the driver is aware of. The static allocation for register storage had falen behind badly.

Reported-by: Michael Bode michael@bumbleB.de Signed-off-by: Jyri Sarha jsarha@ti.com

---

drivers/gpu/drm/tilcdc/tilcdc_drv.c | 20 +++++++++++++++++++- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 4908c1f..2f87263 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -139,11 +139,14 @@ static int tilcdc_unload(struct drm_device *dev)

pm_runtime_disable(dev->dev);

• kfree(priv->saved_register); kfree(priv);

  return 0; }

+static size_t tilcdc_num_regs(void);

• static int tilcdc_load(struct drm_device *dev, unsigned long flags) { struct platform_device *pdev = dev->platformdev;

@@ -155,7 +158,11 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) int ret;

priv = kzalloc(sizeof(*priv), GFP_KERNEL);

• if (!priv) {

• if (priv)

• priv->saved_register = kcalloc(sizeof(*priv->saved_register),
  

• 			       tilcdc_num_regs(), GFP_KERNEL);
  

• if (!priv || !priv->saved_register) {

• kfree(priv);
  

  dev_err(dev->dev, "failed to allocate private data\n"); return -ENOMEM; }

@@ -345,6 +352,7 @@ fail_free_wq:

fail_free_priv: dev->dev_private = NULL;

• kfree(priv->saved_register); kfree(priv); return ret; }

@@ -467,6 +475,16 @@ static const struct { REG(2, true, LCDC_INT_ENABLE_SET_REG), #undef REG };

+static size_t tilcdc_num_regs(void) +{

• return ARRAY_SIZE(registers);

+} +#else +static size_t tilcdc_num_regs(void) +{

• return 0;

+} #endif

#ifdef CONFIG_DEBUG_FS diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h index e863ad0..bc94835 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -67,7 +67,7 @@ struct tilcdc_drm_private { uint32_t max_width;

/* register contents saved across suspend/resume: */

• u32 saved_register[12];

• u32 *saved_register;

  #ifdef CONFIG_CPU_FREQ struct notifier_block freq_transition;