| Message ID | ae4d951d022e6c34b87ae46e15f1522f8d6d3480.1722355024.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | c6dbab46324b1742b50dc2fb5c1fee2c28129439 |
| Headers | show |
| Series | drm/radeon/r100: Handle unknown family in r100_cp_init_microcode() | expand |
Applied. Thanks! On Tue, Jul 30, 2024 at 12:05 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > > With -Werror: > > In function ‘r100_cp_init_microcode’, > inlined from ‘r100_cp_init’ at drivers/gpu/drm/radeon/r100.c:1136:7: > include/linux/printk.h:465:44: error: ‘%s’ directive argument is null [-Werror=format-overflow=] > 465 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__) > | ^ > include/linux/printk.h:437:17: note: in definition of macro ‘printk_index_wrap’ > 437 | _p_func(_fmt, ##__VA_ARGS__); \ > | ^~~~~~~ > include/linux/printk.h:508:9: note: in expansion of macro ‘printk’ > 508 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) > | ^~~~~~ > drivers/gpu/drm/radeon/r100.c:1062:17: note: in expansion of macro ‘pr_err’ > 1062 | pr_err("radeon_cp: Failed to load firmware \"%s\"\n", fw_name); > | ^~~~~~ > > Fix this by converting the if/else if/... construct into a proper > switch() statement with a default to handle the error case. > > As a bonus, the generated code is ca. 100 bytes smaller (with gcc 11.4.0 > targeting arm32). > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Compile-tested only. > --- > drivers/gpu/drm/radeon/r100.c | 70 ++++++++++++++++++++++------------- > 1 file changed, 45 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c > index 0b1e19345f43a771..bfd42e3e161e984f 100644 > --- a/drivers/gpu/drm/radeon/r100.c > +++ b/drivers/gpu/drm/radeon/r100.c > @@ -1016,45 +1016,65 @@ static int r100_cp_init_microcode(struct radeon_device *rdev) > > DRM_DEBUG_KMS("\n"); > > - if ((rdev->family == CHIP_R100) || (rdev->family == CHIP_RV100) || > - (rdev->family == CHIP_RV200) || (rdev->family == CHIP_RS100) || > - (rdev->family == CHIP_RS200)) { > + switch (rdev->family) { > + case CHIP_R100: > + case CHIP_RV100: > + case CHIP_RV200: > + case CHIP_RS100: > + case CHIP_RS200: > DRM_INFO("Loading R100 Microcode\n"); > fw_name = FIRMWARE_R100; > - } else if ((rdev->family == CHIP_R200) || > - (rdev->family == CHIP_RV250) || > - (rdev->family == CHIP_RV280) || > - (rdev->family == CHIP_RS300)) { > + break; > + > + case CHIP_R200: > + case CHIP_RV250: > + case CHIP_RV280: > + case CHIP_RS300: > DRM_INFO("Loading R200 Microcode\n"); > fw_name = FIRMWARE_R200; > - } else if ((rdev->family == CHIP_R300) || > - (rdev->family == CHIP_R350) || > - (rdev->family == CHIP_RV350) || > - (rdev->family == CHIP_RV380) || > - (rdev->family == CHIP_RS400) || > - (rdev->family == CHIP_RS480)) { > + break; > + > + case CHIP_R300: > + case CHIP_R350: > + case CHIP_RV350: > + case CHIP_RV380: > + case CHIP_RS400: > + case CHIP_RS480: > DRM_INFO("Loading R300 Microcode\n"); > fw_name = FIRMWARE_R300; > - } else if ((rdev->family == CHIP_R420) || > - (rdev->family == CHIP_R423) || > - (rdev->family == CHIP_RV410)) { > + break; > + > + case CHIP_R420: > + case CHIP_R423: > + case CHIP_RV410: > DRM_INFO("Loading R400 Microcode\n"); > fw_name = FIRMWARE_R420; > - } else if ((rdev->family == CHIP_RS690) || > - (rdev->family == CHIP_RS740)) { > + break; > + > + case CHIP_RS690: > + case CHIP_RS740: > DRM_INFO("Loading RS690/RS740 Microcode\n"); > fw_name = FIRMWARE_RS690; > - } else if (rdev->family == CHIP_RS600) { > + break; > + > + case CHIP_RS600: > DRM_INFO("Loading RS600 Microcode\n"); > fw_name = FIRMWARE_RS600; > - } else if ((rdev->family == CHIP_RV515) || > - (rdev->family == CHIP_R520) || > - (rdev->family == CHIP_RV530) || > - (rdev->family == CHIP_R580) || > - (rdev->family == CHIP_RV560) || > - (rdev->family == CHIP_RV570)) { > + break; > + > + case CHIP_RV515: > + case CHIP_R520: > + case CHIP_RV530: > + case CHIP_R580: > + case CHIP_RV560: > + case CHIP_RV570: > DRM_INFO("Loading R500 Microcode\n"); > fw_name = FIRMWARE_R520; > + break; > + > + default: > + DRM_ERROR("Unsupported Radeon family %u\n", rdev->family); > + return -EINVAL; > } > > err = request_firmware(&rdev->me_fw, fw_name, rdev->dev); > -- > 2.34.1 >
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index 0b1e19345f43a771..bfd42e3e161e984f 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -1016,45 +1016,65 @@ static int r100_cp_init_microcode(struct radeon_device *rdev) DRM_DEBUG_KMS("\n"); - if ((rdev->family == CHIP_R100) || (rdev->family == CHIP_RV100) || - (rdev->family == CHIP_RV200) || (rdev->family == CHIP_RS100) || - (rdev->family == CHIP_RS200)) { + switch (rdev->family) { + case CHIP_R100: + case CHIP_RV100: + case CHIP_RV200: + case CHIP_RS100: + case CHIP_RS200: DRM_INFO("Loading R100 Microcode\n"); fw_name = FIRMWARE_R100; - } else if ((rdev->family == CHIP_R200) || - (rdev->family == CHIP_RV250) || - (rdev->family == CHIP_RV280) || - (rdev->family == CHIP_RS300)) { + break; + + case CHIP_R200: + case CHIP_RV250: + case CHIP_RV280: + case CHIP_RS300: DRM_INFO("Loading R200 Microcode\n"); fw_name = FIRMWARE_R200; - } else if ((rdev->family == CHIP_R300) || - (rdev->family == CHIP_R350) || - (rdev->family == CHIP_RV350) || - (rdev->family == CHIP_RV380) || - (rdev->family == CHIP_RS400) || - (rdev->family == CHIP_RS480)) { + break; + + case CHIP_R300: + case CHIP_R350: + case CHIP_RV350: + case CHIP_RV380: + case CHIP_RS400: + case CHIP_RS480: DRM_INFO("Loading R300 Microcode\n"); fw_name = FIRMWARE_R300; - } else if ((rdev->family == CHIP_R420) || - (rdev->family == CHIP_R423) || - (rdev->family == CHIP_RV410)) { + break; + + case CHIP_R420: + case CHIP_R423: + case CHIP_RV410: DRM_INFO("Loading R400 Microcode\n"); fw_name = FIRMWARE_R420; - } else if ((rdev->family == CHIP_RS690) || - (rdev->family == CHIP_RS740)) { + break; + + case CHIP_RS690: + case CHIP_RS740: DRM_INFO("Loading RS690/RS740 Microcode\n"); fw_name = FIRMWARE_RS690; - } else if (rdev->family == CHIP_RS600) { + break; + + case CHIP_RS600: DRM_INFO("Loading RS600 Microcode\n"); fw_name = FIRMWARE_RS600; - } else if ((rdev->family == CHIP_RV515) || - (rdev->family == CHIP_R520) || - (rdev->family == CHIP_RV530) || - (rdev->family == CHIP_R580) || - (rdev->family == CHIP_RV560) || - (rdev->family == CHIP_RV570)) { + break; + + case CHIP_RV515: + case CHIP_R520: + case CHIP_RV530: + case CHIP_R580: + case CHIP_RV560: + case CHIP_RV570: DRM_INFO("Loading R500 Microcode\n"); fw_name = FIRMWARE_R520; + break; + + default: + DRM_ERROR("Unsupported Radeon family %u\n", rdev->family); + return -EINVAL; } err = request_firmware(&rdev->me_fw, fw_name, rdev->dev);
With -Werror: In function ‘r100_cp_init_microcode’, inlined from ‘r100_cp_init’ at drivers/gpu/drm/radeon/r100.c:1136:7: include/linux/printk.h:465:44: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 465 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__) | ^ include/linux/printk.h:437:17: note: in definition of macro ‘printk_index_wrap’ 437 | _p_func(_fmt, ##__VA_ARGS__); \ | ^~~~~~~ include/linux/printk.h:508:9: note: in expansion of macro ‘printk’ 508 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) | ^~~~~~ drivers/gpu/drm/radeon/r100.c:1062:17: note: in expansion of macro ‘pr_err’ 1062 | pr_err("radeon_cp: Failed to load firmware \"%s\"\n", fw_name); | ^~~~~~ Fix this by converting the if/else if/... construct into a proper switch() statement with a default to handle the error case. As a bonus, the generated code is ca. 100 bytes smaller (with gcc 11.4.0 targeting arm32). Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Compile-tested only. --- drivers/gpu/drm/radeon/r100.c | 70 ++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 25 deletions(-)