From: Thierry Reding treding@nvidia.com

The ->alloc() callback in struct falcon_ops returns an ERR_PTR()-encoded error code on failure, so it needs to be properly checked for, otherwise subsequent code may dereference an invalid pointer.

Signed-off-by: Thierry Reding treding@nvidia.com --- drivers/gpu/drm/tegra/falcon.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c index f685e72949d1..78c7a0156601 100644 --- a/drivers/gpu/drm/tegra/falcon.c +++ b/drivers/gpu/drm/tegra/falcon.c @@ -141,9 +141,9 @@ int falcon_load_firmware(struct falcon *falcon) /* allocate iova space for the firmware */ falcon->firmware.vaddr = falcon->ops->alloc(falcon, firmware->size, &falcon->firmware.paddr); - if (!falcon->firmware.vaddr) { - dev_err(falcon->dev, "dma memory mapping failed\n"); - return -ENOMEM; + if (IS_ERR(falcon->firmware.vaddr)) { + dev_err(falcon->dev, "DMA memory mapping failed\n"); + return PTR_ERR(falcon->firmware.vaddr); }

/* copy firmware image into local area. this also ensures endianness */

On 23/11/2018 12:06, Thierry Reding wrote:

From: Thierry Reding treding@nvidia.com

Tegra supports generic PM domains on 64-bit ARM, and if that is enabled, the power domain code will make sure that resets are asserted and deasserted at appropriate points in time.

If generic PM domains are not implemented, such as on 32-bit Tegra, the resets need to be asserted and deasserted explicitly by the driver.

Signed-off-by: Thierry Reding treding@nvidia.com

drivers/gpu/drm/tegra/vic.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c index 9fa77405db01..23f530db45ad 100644 --- a/drivers/gpu/drm/tegra/vic.c +++ b/drivers/gpu/drm/tegra/vic.c @@ -38,6 +38,7 @@ struct vic { struct iommu_domain *domain; struct device *dev; struct clk *clk;

• struct reset_control *rst;

  /* Platform configuration */ const struct vic_config *config;

@@ -56,13 +57,37 @@ static void vic_writel(struct vic *vic, u32 value, unsigned int offset) static int vic_runtime_resume(struct device *dev) { struct vic *vic = dev_get_drvdata(dev);

• int err;
• err = clk_prepare_enable(vic->clk);
• if (err < 0)

• return err;
  

• usleep_range(2000, 4000);

The Tegra genpd code has a usleep_range(10, 20), is that not sufficient here? If it is, it would be good to be consistent.

• err = reset_control_deassert(vic->rst);
• if (err < 0)

• goto disable;
  

• usleep_range(2000, 4000);
• return 0;

• return clk_prepare_enable(vic->clk);

+disable:

• clk_disable_unprepare(vic->clk);
• return err;

}

static int vic_runtime_suspend(struct device *dev) { struct vic *vic = dev_get_drvdata(dev);

• int err;

• err = reset_control_assert(vic->rst);

• if (err < 0)

• return err;
  

• usleep_range(2000, 4000);

  clk_disable_unprepare(vic->clk);

@@ -324,6 +349,14 @@ static int vic_probe(struct platform_device *pdev) return PTR_ERR(vic->clk); }

• if (!dev->pm_domain) {

• vic->rst = devm_reset_control_get(dev, "vic");
  

• if (IS_ERR(vic->rst)) {
  

• 	dev_err(&pdev->dev, "failed to get reset\n");
  

• 	return PTR_ERR(vic->rst);
  

• }
  

• }
• vic->falcon.dev = dev; vic->falcon.regs = vic->regs; vic->falcon.ops = &vic_falcon_ops;

Cheers Jon