[PATCH] drm/mgag200: Initialize delta variable in PLL compute function

[PATCH] drm/mgag200: Initialize delta variable in PLL compute function

[Thomas Zimmermann]

Initialize delta variable in PLL compute function for G200SE, rev 00 to
the maximum value. Fixes uninitialized usage.

  smatch warnings:
  drivers/gpu/drm/mgag200/mgag200_pll.c:142 mgag200_pixpll_compute_g200se_00() \
  error: uninitialized symbol 'delta'.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 2545ac960364 ("drm/mgag200: Abstract pixel PLL via struct mgag200_pll")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/mgag200/mgag200_pll.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c b/drivers/gpu/drm/mgag200/mgag200_pll.c
index 7c903cf19c0d..e9ae22b4f813 100644
--- a/drivers/gpu/drm/mgag200/mgag200_pll.c
+++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
@@ -124,6 +124,7 @@ static int mgag200_pixpll_compute_g200se_00(struct mgag200_pll *pixpll, long clo
 	unsigned int computed;

 	m = n = p = s = 0;
+	delta = 0xffffffff;
 	permitteddelta = clock * 5 / 1000;

 	for (testp = 8; testp > 0; testp /= 2) {
--
2.32.0

Re: [PATCH] drm/mgag200: Initialize delta variable in PLL compute function

[Daniel Vetter]

On Tue, Aug 17, 2021 at 8:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Initialize delta variable in PLL compute function for G200SE, rev 00 to
> the maximum value. Fixes uninitialized usage.
>
>   smatch warnings:
>   drivers/gpu/drm/mgag200/mgag200_pll.c:142 mgag200_pixpll_compute_g200se_00() \
>   error: uninitialized symbol 'delta'.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 2545ac960364 ("drm/mgag200: Abstract pixel PLL via struct mgag200_pll")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: dri-devel@lists.freedesktop.org

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I checked a bit your integer types, and there's a fair bit of
inconsistency for what you're using for clock. unsigned int, long, int
can all be found somewhere. But that's  different story.
-Daniel

> ---
>  drivers/gpu/drm/mgag200/mgag200_pll.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c b/drivers/gpu/drm/mgag200/mgag200_pll.c
> index 7c903cf19c0d..e9ae22b4f813 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_pll.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
> @@ -124,6 +124,7 @@ static int mgag200_pixpll_compute_g200se_00(struct mgag200_pll *pixpll, long clo
>         unsigned int computed;
>
>         m = n = p = s = 0;
> +       delta = 0xffffffff;
>         permitteddelta = clock * 5 / 1000;
>
>         for (testp = 8; testp > 0; testp /= 2) {
> --
> 2.32.0
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/mgag200: Initialize delta variable in PLL compute function

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2462 bytes --]

Hi

Am 17.08.21 um 21:50 schrieb Daniel Vetter:
> On Tue, Aug 17, 2021 at 8:16 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Initialize delta variable in PLL compute function for G200SE, rev 00 to
>> the maximum value. Fixes uninitialized usage.
>>
>>    smatch warnings:
>>    drivers/gpu/drm/mgag200/mgag200_pll.c:142 mgag200_pixpll_compute_g200se_00() \
>>    error: uninitialized symbol 'delta'.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: 2545ac960364 ("drm/mgag200: Abstract pixel PLL via struct mgag200_pll")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Sam Ravnborg <sam@ravnborg.org>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: dri-devel@lists.freedesktop.org
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I checked a bit your integer types, and there's a fair bit of
> inconsistency for what you're using for clock. unsigned int, long, int
> can all be found somewhere. But that's  different story.

Better not look at the PLL code if you value your eyesight. I untangled 
the computation of the values from the register setup, but I don't dare 
to change the algorithms.

In principle, most devices should be able to share a single algorithm 
for computing the values and writing them out. But somehow they all 
differ. Maybe some minimal cleanup can be done. Without actual HW for 
testing, I'd rather not touch the logic.

Best regards
Thomas

> -Daniel
> 
>> ---
>>   drivers/gpu/drm/mgag200/mgag200_pll.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_pll.c b/drivers/gpu/drm/mgag200/mgag200_pll.c
>> index 7c903cf19c0d..e9ae22b4f813 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_pll.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_pll.c
>> @@ -124,6 +124,7 @@ static int mgag200_pixpll_compute_g200se_00(struct mgag200_pll *pixpll, long clo
>>          unsigned int computed;
>>
>>          m = n = p = s = 0;
>> +       delta = 0xffffffff;
>>          permitteddelta = clock * 5 / 1000;
>>
>>          for (testp = 8; testp > 0; testp /= 2) {
>> --
>> 2.32.0
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]