Re: [PATCH] drm/radeon/r100: enhance error handling in r100_cp_init_microcode

From: Christian König
Date: Mon May 27 2024 - 03:59:12 EST


Am 27.05.24 um 03:20 schrieb Zhouyi Zhou:
In r100_cp_init_microcode, if rdev->family don't match any of
if statement, fw_name will be NULL, which will cause
gcc (11.4.0 powerpc64le-linux-gnu) complain:

In function ‘r100_cp_init_microcode’,
inlined from ‘r100_cp_init’ at drivers/gpu/drm/radeon/r100.c:1136:7:
./include/linux/printk.h:457:44: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
457 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)

Above warning is emitted during the rcutorture test in
in PPC VM of Opensource Lab of Oregon State Univerisity.

Enhance error handling in r100_cp_init_microcode, let r100_cp_init_microcode
return with -EINVAL when none of chip families is matched.

Signed-off-by: Zhouyi Zhou <zhouzhouyi@xxxxxxxxx>

---
drivers/gpu/drm/radeon/r100.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 0b1e19345f43..4f8a1bdd9365 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -1055,6 +1055,11 @@ static int r100_cp_init_microcode(struct radeon_device *rdev)
(rdev->family == CHIP_RV570)) {
DRM_INFO("Loading R500 Microcode\n");
fw_name = FIRMWARE_R520;
+ } else {
+ pr_err("radeon_cp: Failed to load firmware \"%d\"\n",
+ rdev->family);
+ err = -EINVAL;
+ goto out;
}
err = request_firmware(&rdev->me_fw, fw_name, rdev->dev);
@@ -1067,6 +1072,8 @@ static int r100_cp_init_microcode(struct radeon_device *rdev)
release_firmware(rdev->me_fw);
rdev->me_fw = NULL;
}
+
+out:

That looks superfluous, just return -EINVAL directly in the else case above.

Apart from that this is for ~15year old hardware. I'm a bit reluctant adding code for something that old even when this change here looks harmless.

Is there a plan to complain about that in an automated checker? If yes then the change is probably justified, if no then I would rather not do it.

Regards,
Christian.

return err;
}