diff mbox series

drm/radeon/r100: Handle unknown family in r100_cp_init_microcode()

Message ID ae4d951d022e6c34b87ae46e15f1522f8d6d3480.1722355024.git.geert+renesas@glider.be (mailing list archive)
State New, archived
Headers show
Series drm/radeon/r100: Handle unknown family in r100_cp_init_microcode() | expand

Commit Message

Geert Uytterhoeven July 30, 2024, 3:58 p.m. UTC 
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(-)

Comments

Alex Deucher Aug. 5, 2024, 8:56 p.m. UTC | #1 
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 mbox series

Patch

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);