Archive-only list for patches
 help / color / mirror / Atom feed
* [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config()
@ 2025-04-23 17:41 Nathan Chancellor
  2025-04-24  7:23 ` AngeloGioacchino Del Regno
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Nathan Chancellor @ 2025-04-23 17:41 UTC (permalink / raw)
  To: Neil Armstrong, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann
  Cc: Jessica Zhang, AngeloGioacchino Del Regno, dri-devel, llvm,
	patches, Nathan Chancellor

Clang warns (or errors with CONFIG_WERROR=y):

  drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
    838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
    842 |         if (goa_odd_valid != goa_even_valid)
        |                              ^~~~~~~~~~~~~~
  drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
    838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    839 |                 goa_even_valid = false;
  drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
    818 |         bool goa_odd_valid, goa_even_valid;
        |                                           ^
        |                                            = 0

Even though only the even valid variable gets flagged, both valid
variables appear to have the same issue of possibly being used
uninitialized if the if statement initializing them to false is not
taken.

Turn the if statement then variable assignment into a single variable
assignment, which states that the configuration is valid when there are
not all zeros, clearing up the warning since the variable will always be
initialized.

Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Changes in v2:
- Initialize valid variables once using the inverse of the current
  condition (Angelo).
- Link to v1: https://lore.kernel.org/r/20250422-panel-himax-hx8279-fix-sometimes-uninitialized-v1-1-614dba12b30d@kernel.org
---
 drivers/gpu/drm/panel/panel-himax-hx8279.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-himax-hx8279.c b/drivers/gpu/drm/panel/panel-himax-hx8279.c
index b48b350b62da..fb302d1f91b9 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx8279.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx8279.c
@@ -825,8 +825,7 @@ static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
 			num_zero++;
 	}
 
-	if (num_zero == ARRAY_SIZE(desc->goa_odd_timing))
-		goa_odd_valid = false;
+	goa_odd_valid = (num_zero != ARRAY_SIZE(desc->goa_odd_timing));
 
 	/* Up to 3 zeroes is a valid config. Check them all. */
 	num_zero = 1;
@@ -835,8 +834,7 @@ static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
 			num_zero++;
 	}
 
-	if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
-		goa_even_valid = false;
+	goa_even_valid = (num_zero != ARRAY_SIZE(desc->goa_even_timing));
 
 	/* Programming one without the other would make no sense! */
 	if (goa_odd_valid != goa_even_valid)

---
base-commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
change-id: 20250422-panel-himax-hx8279-fix-sometimes-uninitialized-207354fb930c

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config()
  2025-04-23 17:41 [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config() Nathan Chancellor
@ 2025-04-24  7:23 ` AngeloGioacchino Del Regno
  2025-04-24  7:35 ` Neil Armstrong
  2025-04-24  8:22 ` Neil Armstrong
  2 siblings, 0 replies; 4+ messages in thread
From: AngeloGioacchino Del Regno @ 2025-04-24  7:23 UTC (permalink / raw)
  To: Nathan Chancellor, Neil Armstrong, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann
  Cc: Jessica Zhang, dri-devel, llvm, patches

Il 23/04/25 19:41, Nathan Chancellor ha scritto:
> Clang warns (or errors with CONFIG_WERROR=y):
> 
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>      838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>          |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
>      842 |         if (goa_odd_valid != goa_even_valid)
>          |                              ^~~~~~~~~~~~~~
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
>      838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      839 |                 goa_even_valid = false;
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
>      818 |         bool goa_odd_valid, goa_even_valid;
>          |                                           ^
>          |                                            = 0
> 
> Even though only the even valid variable gets flagged, both valid
> variables appear to have the same issue of possibly being used
> uninitialized if the if statement initializing them to false is not
> taken.
> 
> Turn the if statement then variable assignment into a single variable
> assignment, which states that the configuration is valid when there are
> not all zeros, clearing up the warning since the variable will always be
> initialized.
> 
> Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
> Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Actually, I prefer this one, as Arnd's commit fixes only one of the two, but this
initializes both :-)

Cheers,
Angelo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config()
  2025-04-23 17:41 [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config() Nathan Chancellor
  2025-04-24  7:23 ` AngeloGioacchino Del Regno
@ 2025-04-24  7:35 ` Neil Armstrong
  2025-04-24  8:22 ` Neil Armstrong
  2 siblings, 0 replies; 4+ messages in thread
From: Neil Armstrong @ 2025-04-24  7:35 UTC (permalink / raw)
  To: Nathan Chancellor, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann
  Cc: Jessica Zhang, AngeloGioacchino Del Regno, dri-devel, llvm,
	patches

On 23/04/2025 19:41, Nathan Chancellor wrote:
> Clang warns (or errors with CONFIG_WERROR=y):
> 
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>      838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>          |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
>      842 |         if (goa_odd_valid != goa_even_valid)
>          |                              ^~~~~~~~~~~~~~
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
>      838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      839 |                 goa_even_valid = false;
>    drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
>      818 |         bool goa_odd_valid, goa_even_valid;
>          |                                           ^
>          |                                            = 0
> 
> Even though only the even valid variable gets flagged, both valid
> variables appear to have the same issue of possibly being used
> uninitialized if the if statement initializing them to false is not
> taken.
> 
> Turn the if statement then variable assignment into a single variable
> assignment, which states that the configuration is valid when there are
> not all zeros, clearing up the warning since the variable will always be
> initialized.
> 
> Fixes: 38d42c261389 ("drm: panel: Add driver for Himax HX8279 DDIC panels")
> Suggested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Changes in v2:
> - Initialize valid variables once using the inverse of the current
>    condition (Angelo).
> - Link to v1: https://lore.kernel.org/r/20250422-panel-himax-hx8279-fix-sometimes-uninitialized-v1-1-614dba12b30d@kernel.org
> ---
>   drivers/gpu/drm/panel/panel-himax-hx8279.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx8279.c b/drivers/gpu/drm/panel/panel-himax-hx8279.c
> index b48b350b62da..fb302d1f91b9 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx8279.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx8279.c
> @@ -825,8 +825,7 @@ static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
>   			num_zero++;
>   	}
>   
> -	if (num_zero == ARRAY_SIZE(desc->goa_odd_timing))
> -		goa_odd_valid = false;
> +	goa_odd_valid = (num_zero != ARRAY_SIZE(desc->goa_odd_timing));
>   
>   	/* Up to 3 zeroes is a valid config. Check them all. */
>   	num_zero = 1;
> @@ -835,8 +834,7 @@ static int hx8279_check_goa_config(struct hx8279 *hx, struct device *dev)
>   			num_zero++;
>   	}
>   
> -	if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
> -		goa_even_valid = false;
> +	goa_even_valid = (num_zero != ARRAY_SIZE(desc->goa_even_timing));
>   
>   	/* Programming one without the other would make no sense! */
>   	if (goa_odd_valid != goa_even_valid)
> 
> ---
> base-commit: dcbd5dcc956e2331414fd7020b4655df08deeb87
> change-id: 20250422-panel-himax-hx8279-fix-sometimes-uninitialized-207354fb930c
> 
> Best regards,

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config()
  2025-04-23 17:41 [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config() Nathan Chancellor
  2025-04-24  7:23 ` AngeloGioacchino Del Regno
  2025-04-24  7:35 ` Neil Armstrong
@ 2025-04-24  8:22 ` Neil Armstrong
  2 siblings, 0 replies; 4+ messages in thread
From: Neil Armstrong @ 2025-04-24  8:22 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	Nathan Chancellor
  Cc: Jessica Zhang, AngeloGioacchino Del Regno, dri-devel, llvm,
	patches

Hi,

On Wed, 23 Apr 2025 10:41:41 -0700, Nathan Chancellor wrote:
> Clang warns (or errors with CONFIG_WERROR=y):
> 
>   drivers/gpu/drm/panel/panel-himax-hx8279.c:838:6: error: variable 'goa_even_valid' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>     838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/gpu/drm/panel/panel-himax-hx8279.c:842:23: note: uninitialized use occurs here
>     842 |         if (goa_odd_valid != goa_even_valid)
>         |                              ^~~~~~~~~~~~~~
>   drivers/gpu/drm/panel/panel-himax-hx8279.c:838:2: note: remove the 'if' if its condition is always true
>     838 |         if (num_zero == ARRAY_SIZE(desc->goa_even_timing))
>         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     839 |                 goa_even_valid = false;
>   drivers/gpu/drm/panel/panel-himax-hx8279.c:818:36: note: initialize the variable 'goa_even_valid' to silence this warning
>     818 |         bool goa_odd_valid, goa_even_valid;
>         |                                           ^
>         |                                            = 0
> 
> [...]

Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-next)

[1/1] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config()
      https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/0228687423587d8ef448fc1d81f96539507aa5bb

-- 
Neil


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-04-24  8:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 17:41 [PATCH v2] drm/panel: himax-hx8279: Always initialize goa_{even,odd}_valid in hx8279_check_goa_config() Nathan Chancellor
2025-04-24  7:23 ` AngeloGioacchino Del Regno
2025-04-24  7:35 ` Neil Armstrong
2025-04-24  8:22 ` Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox