From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 18:38:48 +0200
Some update suggestions were taken into account from static source code analysis.
Markus Elfring (5): Use kmalloc_array() in amdgpu_debugfs_gca_config_read() Improve determination of sizes in two functions Rename a jump label in amdgpu_debugfs_regs_read() Rename a jump label in amdgpu_device_init() Adjust checks for null pointers in nine functions
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 68 +++++++++++++++--------------- 1 file changed, 33 insertions(+), 35 deletions(-)
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 17:00:52 +0200
A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index df7ab245..2709ebd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2438,7 +2438,7 @@ static ssize_t amdgpu_debugfs_gca_config_read(struct file *f, char __user *buf, if (size & 0x3 || *pos & 0x3) return -EINVAL;
- config = kmalloc(256 * sizeof(*config), GFP_KERNEL); + config = kmalloc_array(256, sizeof(*config), GFP_KERNEL); if (!config) return -ENOMEM;
On Sun, Sep 18, 2016 at 12:50 PM, SF Markus Elfring elfring@users.sourceforge.net wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 17:00:52 +0200
A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net
Applied. thanks.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index df7ab245..2709ebd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2438,7 +2438,7 @@ static ssize_t amdgpu_debugfs_gca_config_read(struct file *f, char __user *buf, if (size & 0x3 || *pos & 0x3) return -EINVAL;
config = kmalloc(256 * sizeof(*config), GFP_KERNEL);
config = kmalloc_array(256, sizeof(*config), GFP_KERNEL); if (!config) return -ENOMEM;
-- 2.10.0
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 17:24:47 +0200
Replace the specification of data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 2709ebd..96a2457 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -855,7 +855,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev) static int amdgpu_atombios_init(struct amdgpu_device *adev) { struct card_info *atom_card_info = - kzalloc(sizeof(struct card_info), GFP_KERNEL); + kzalloc(sizeof(*atom_card_info), GFP_KERNEL);
if (!atom_card_info) return -ENOMEM; @@ -1224,7 +1224,8 @@ static int amdgpu_early_init(struct amdgpu_device *adev) }
adev->ip_block_status = kcalloc(adev->num_ip_blocks, - sizeof(struct amdgpu_ip_block_status), GFP_KERNEL); + sizeof(*adev->ip_block_status), + GFP_KERNEL); if (adev->ip_block_status == NULL) return -ENOMEM;
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 17:35:24 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 96a2457..2b8ba97 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2208,13 +2208,13 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf, uint32_t value;
if (*pos > adev->rmmio_size) - goto end; + goto check_bank;
value = RREG32(*pos >> 2); r = put_user(value, (uint32_t *)buf); if (r) { result = r; - goto end; + goto check_bank; }
result += 4; @@ -2222,8 +2222,7 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf, *pos += 4; size -= 4; } - -end: + check_bank: if (use_bank) { amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff); mutex_unlock(&adev->grbm_idx_mutex);
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 17:50:09 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 2b8ba97..fed4854 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1566,18 +1566,18 @@ int amdgpu_device_init(struct amdgpu_device *adev, /* Read BIOS */ if (!amdgpu_get_bios(adev)) { r = -EINVAL; - goto failed; + goto check_runtime; } /* Must be an ATOMBIOS */ if (!adev->is_atom_bios) { dev_err(adev->dev, "Expecting atombios for GPU\n"); r = -EINVAL; - goto failed; + goto check_runtime; } r = amdgpu_atombios_init(adev); if (r) { dev_err(adev->dev, "amdgpu_atombios_init failed\n"); - goto failed; + goto check_runtime; }
/* See if the asic supports SR-IOV */ @@ -1595,7 +1595,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, if (!adev->bios) { dev_err(adev->dev, "Card not posted and no BIOS - ignoring\n"); r = -EINVAL; - goto failed; + goto check_runtime; } DRM_INFO("GPU not posted. posting now...\n"); amdgpu_atom_asic_init(adev->mode_info.atom_context); @@ -1605,7 +1605,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_atombios_get_clock_info(adev); if (r) { dev_err(adev->dev, "amdgpu_atombios_get_clock_info failed\n"); - goto failed; + goto check_runtime; } /* init i2c buses */ amdgpu_atombios_i2c_init(adev); @@ -1614,7 +1614,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_fence_driver_init(adev); if (r) { dev_err(adev->dev, "amdgpu_fence_driver_init failed\n"); - goto failed; + goto check_runtime; }
/* init the mode config */ @@ -1624,7 +1624,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, if (r) { dev_err(adev->dev, "amdgpu_init failed\n"); amdgpu_fini(adev); - goto failed; + goto check_runtime; }
adev->accel_working = true; @@ -1634,7 +1634,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_ib_pool_init(adev); if (r) { dev_err(adev->dev, "IB initialization failed (%d).\n", r); - goto failed; + goto check_runtime; }
r = amdgpu_ib_ring_tests(adev); @@ -1682,12 +1682,11 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_late_init(adev); if (r) { dev_err(adev->dev, "amdgpu_late_init failed\n"); - goto failed; + goto check_runtime; }
return 0; - -failed: + check_runtime: if (runtime) vga_switcheroo_fini_domain_pm_ops(adev->dev); return r;
-----Original Message----- From: SF Markus Elfring [mailto:elfring@users.sourceforge.net] Sent: Sunday, September 18, 2016 12:53 PM To: dri-devel@lists.freedesktop.org; Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); David Airlie; Liu, Monk; StDenis, Tom Cc: LKML; kernel-janitors@vger.kernel.org; Julia Lawall Subject: [PATCH 4/5] drm/amdgpu: Rename a jump label in amdgpu_device_init()
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 17:50:09 +0200
Adjust jump labels according to the current Linux coding style convention.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 21 ++++++++++----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 2b8ba97..fed4854 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1566,18 +1566,18 @@ int amdgpu_device_init(struct amdgpu_device *adev, /* Read BIOS */ if (!amdgpu_get_bios(adev)) { r = -EINVAL;
goto failed;
goto check_runtime;
NACK. Failed is a more appropriate label here. The runtime check is just part of the failure cleanup.
Alex
} /* Must be an ATOMBIOS */ if (!adev->is_atom_bios) { dev_err(adev->dev, "Expecting atombios for GPU\n"); r = -EINVAL;
goto failed;
} r = amdgpu_atombios_init(adev); if (r) { dev_err(adev->dev, "amdgpu_atombios_init failed\n");goto check_runtime;
goto failed;
goto check_runtime;
}
/* See if the asic supports SR-IOV */
@@ -1595,7 +1595,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, if (!adev->bios) { dev_err(adev->dev, "Card not posted and no BIOS - ignoring\n"); r = -EINVAL;
goto failed;
} DRM_INFO("GPU not posted. posting now...\n"); amdgpu_atom_asic_init(adev->mode_info.atom_context);goto check_runtime;
@@ -1605,7 +1605,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_atombios_get_clock_info(adev); if (r) { dev_err(adev->dev, "amdgpu_atombios_get_clock_info failed\n");
goto failed;
} /* init i2c buses */ amdgpu_atombios_i2c_init(adev);goto check_runtime;
@@ -1614,7 +1614,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_fence_driver_init(adev); if (r) { dev_err(adev->dev, "amdgpu_fence_driver_init failed\n");
goto failed;
goto check_runtime;
}
/* init the mode config */
@@ -1624,7 +1624,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, if (r) { dev_err(adev->dev, "amdgpu_init failed\n"); amdgpu_fini(adev);
goto failed;
goto check_runtime;
}
adev->accel_working = true;
@@ -1634,7 +1634,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_ib_pool_init(adev); if (r) { dev_err(adev->dev, "IB initialization failed (%d).\n", r);
goto failed;
goto check_runtime;
}
r = amdgpu_ib_ring_tests(adev);
@@ -1682,12 +1682,11 @@ int amdgpu_device_init(struct amdgpu_device *adev, r = amdgpu_late_init(adev); if (r) { dev_err(adev->dev, "amdgpu_late_init failed\n");
goto failed;
goto check_runtime;
}
return 0;
-failed:
- check_runtime: if (runtime) vga_switcheroo_fini_domain_pm_ops(adev->dev); return r;
-- 2.10.0
From: Markus Elfring elfring@users.sourceforge.net Date: Sun, 18 Sep 2016 18:32:28 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
* The script "checkpatch.pl" can point information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
* Do also not use curly brackets at corresponding source code places where a single statement should be sufficient.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index fed4854..b5b7cfb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -251,7 +251,7 @@ static int amdgpu_vram_scratch_init(struct amdgpu_device *adev) { int r;
- if (adev->vram_scratch.robj == NULL) { + if (!adev->vram_scratch.robj) { r = amdgpu_bo_create(adev, AMDGPU_GPU_PAGE_SIZE, PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_VRAM, AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED, @@ -283,9 +283,9 @@ static void amdgpu_vram_scratch_fini(struct amdgpu_device *adev) { int r;
- if (adev->vram_scratch.robj == NULL) { + if (!adev->vram_scratch.robj) return; - } + r = amdgpu_bo_reserve(adev->vram_scratch.robj, false); if (likely(r == 0)) { amdgpu_bo_kunmap(adev->vram_scratch.robj); @@ -359,9 +359,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev) return -EINVAL;
adev->doorbell.ptr = ioremap(adev->doorbell.base, adev->doorbell.num_doorbells * sizeof(u32)); - if (adev->doorbell.ptr == NULL) { + if (!adev->doorbell.ptr) return -ENOMEM; - } + DRM_INFO("doorbell mmio base: 0x%08X\n", (uint32_t)adev->doorbell.base); DRM_INFO("doorbell mmio size: %u\n", (unsigned)adev->doorbell.size);
@@ -456,7 +456,7 @@ static int amdgpu_wb_init(struct amdgpu_device *adev) { int r;
- if (adev->wb.wb_obj == NULL) { + if (!adev->wb.wb_obj) { r = amdgpu_bo_create(adev, AMDGPU_MAX_WB * 4, PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_GTT, 0, NULL, NULL, &adev->wb.wb_obj); @@ -657,7 +657,7 @@ int amdgpu_dummy_page_init(struct amdgpu_device *adev) if (adev->dummy_page.page) return 0; adev->dummy_page.page = alloc_page(GFP_DMA32 | GFP_KERNEL | __GFP_ZERO); - if (adev->dummy_page.page == NULL) + if (!adev->dummy_page.page) return -ENOMEM; adev->dummy_page.addr = pci_map_page(adev->pdev, adev->dummy_page.page, 0, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); @@ -679,7 +679,7 @@ int amdgpu_dummy_page_init(struct amdgpu_device *adev) */ void amdgpu_dummy_page_fini(struct amdgpu_device *adev) { - if (adev->dummy_page.page == NULL) + if (!adev->dummy_page.page) return; pci_unmap_page(adev->pdev, adev->dummy_page.addr, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); @@ -1226,10 +1226,10 @@ static int amdgpu_early_init(struct amdgpu_device *adev) adev->ip_block_status = kcalloc(adev->num_ip_blocks, sizeof(*adev->ip_block_status), GFP_KERNEL); - if (adev->ip_block_status == NULL) + if (!adev->ip_block_status) return -ENOMEM;
- if (adev->ip_blocks == NULL) { + if (!adev->ip_blocks) { DRM_ERROR("No IP blocks found!\n"); return r; } @@ -1525,9 +1525,9 @@ int amdgpu_device_init(struct amdgpu_device *adev, adev->rmmio_base = pci_resource_start(adev->pdev, 5); adev->rmmio_size = pci_resource_len(adev->pdev, 5); adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size); - if (adev->rmmio == NULL) { + if (!adev->rmmio) return -ENOMEM; - } + DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base); DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size);
@@ -1542,7 +1542,7 @@ int amdgpu_device_init(struct amdgpu_device *adev, break; } } - if (adev->rio_mem == NULL) + if (!adev->rio_mem) DRM_ERROR("Unable to find PCI I/O BAR\n");
/* early init functions */ @@ -1758,9 +1758,8 @@ int amdgpu_suspend_kms(struct drm_device *dev, bool suspend, bool fbcon) struct drm_connector *connector; int r;
- if (dev == NULL || dev->dev_private == NULL) { + if (!dev || !dev->dev_private) return -ENODEV; - }
adev = dev->dev_private;
@@ -1791,9 +1790,9 @@ int amdgpu_suspend_kms(struct drm_device *dev, bool suspend, bool fbcon) } }
- if (rfb == NULL || rfb->obj == NULL) { + if (!rfb || !rfb->obj) continue; - } + robj = gem_to_amdgpu_bo(rfb->obj); /* don't unpin kernel fb objects */ if (!amdgpu_fbdev_robj_is_fb(adev, robj)) {
dri-devel@lists.freedesktop.org