[PATCH] gpu/drm/mgag200:remove break after return

[PATCH] gpu/drm/mgag200:remove break after return

[Bernard Zhao]

In function mgag200_set_pci_regs, there are some switch cases
returned, then break. These break will never run.
This patch is to make the code a bit readable.

Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 38672f9e5c4f..de873a5d276e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
 	case G200_SE_A:
 	case G200_SE_B:
 		return mga_g200se_set_plls(mdev, clock);
-		break;
 	case G200_WB:
 	case G200_EW3:
 		return mga_g200wb_set_plls(mdev, clock);
-		break;
 	case G200_EV:
 		return mga_g200ev_set_plls(mdev, clock);
-		break;
 	case G200_EH:
 	case G200_EH3:
 		return mga_g200eh_set_plls(mdev, clock);
-		break;
 	case G200_ER:
 		return mga_g200er_set_plls(mdev, clock);
+	default:
 		break;
 	}
 
-- 
2.28.0

[PATCH] gpu/drm/mgag200:remove break after return

[Bernard Zhao]

In function mgag200_set_pci_regs, there are some switch cases
returned, then break. These break will never run.
This patch is to make the code a bit readable.

Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 38672f9e5c4f..de873a5d276e 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
 	case G200_SE_A:
 	case G200_SE_B:
 		return mga_g200se_set_plls(mdev, clock);
-		break;
 	case G200_WB:
 	case G200_EW3:
 		return mga_g200wb_set_plls(mdev, clock);
-		break;
 	case G200_EV:
 		return mga_g200ev_set_plls(mdev, clock);
-		break;
 	case G200_EH:
 	case G200_EH3:
 		return mga_g200eh_set_plls(mdev, clock);
-		break;
 	case G200_ER:
 		return mga_g200er_set_plls(mdev, clock);
+	default:
 		break;
 	}
 
-- 
2.28.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu/drm/mgag200:remove break after return

[Thomas Zimmermann]

Hi

On 23.10.20 09:00, Bernard Zhao wrote:
> In function mgag200_set_pci_regs, there are some switch cases
> returned, then break. These break will never run.
> This patch is to make the code a bit readable.
> 
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 38672f9e5c4f..de873a5d276e 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>  	case G200_SE_A:
>  	case G200_SE_B:
>  		return mga_g200se_set_plls(mdev, clock);
> -		break;
>  	case G200_WB:
>  	case G200_EW3:
>  		return mga_g200wb_set_plls(mdev, clock);
> -		break;
>  	case G200_EV:
>  		return mga_g200ev_set_plls(mdev, clock);
> -		break;
>  	case G200_EH:
>  	case G200_EH3:
>  		return mga_g200eh_set_plls(mdev, clock);
> -		break;
>  	case G200_ER:
>  		return mga_g200er_set_plls(mdev, clock);
> +	default:

No default case here. If one of the enum values is not handled by the
switch, the compiler should warn about it.

Best regards
Thomas

>  		break;
>  	}
>  
> 

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

Re: [PATCH] gpu/drm/mgag200:remove break after return

[Thomas Zimmermann]

Hi

On 23.10.20 09:00, Bernard Zhao wrote:
> In function mgag200_set_pci_regs, there are some switch cases
> returned, then break. These break will never run.
> This patch is to make the code a bit readable.
> 
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 38672f9e5c4f..de873a5d276e 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>  	case G200_SE_A:
>  	case G200_SE_B:
>  		return mga_g200se_set_plls(mdev, clock);
> -		break;
>  	case G200_WB:
>  	case G200_EW3:
>  		return mga_g200wb_set_plls(mdev, clock);
> -		break;
>  	case G200_EV:
>  		return mga_g200ev_set_plls(mdev, clock);
> -		break;
>  	case G200_EH:
>  	case G200_EH3:
>  		return mga_g200eh_set_plls(mdev, clock);
> -		break;
>  	case G200_ER:
>  		return mga_g200er_set_plls(mdev, clock);
> +	default:

No default case here. If one of the enum values is not handled by the
switch, the compiler should warn about it.

Best regards
Thomas

>  		break;
>  	}
>  
> 

-- 
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re:Re: [PATCH] gpu/drm/mgag200:remove break after return

[Bernard]

From: Thomas Zimmermann <tzimmermann@suse.de>
Date: 2020-10-23 15:13:30
To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
Cc:  opensource.kernel@vivo.com
Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
>
>On 23.10.20 09:00, Bernard Zhao wrote:
>> In function mgag200_set_pci_regs, there are some switch cases
>> returned, then break. These break will never run.
>> This patch is to make the code a bit readable.
>> 
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 38672f9e5c4f..de873a5d276e 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>>  	case G200_SE_A:
>>  	case G200_SE_B:
>>  		return mga_g200se_set_plls(mdev, clock);
>> -		break;
>>  	case G200_WB:
>>  	case G200_EW3:
>>  		return mga_g200wb_set_plls(mdev, clock);
>> -		break;
>>  	case G200_EV:
>>  		return mga_g200ev_set_plls(mdev, clock);
>> -		break;
>>  	case G200_EH:
>>  	case G200_EH3:
>>  		return mga_g200eh_set_plls(mdev, clock);
>> -		break;
>>  	case G200_ER:
>>  		return mga_g200er_set_plls(mdev, clock);
>> +	default:
>
>No default case here. If one of the enum values is not handled by the
>switch, the compiler should warn about it.

Hi

For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
How does the compiler warn this("If one of the enum values is not handled") before the code runs?

BR//Bernard

>Best regards
>Thomas
>
>>  		break;
>>  	}
>>  
>> 
>
>-- 
>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

Re:Re: [PATCH] gpu/drm/mgag200:remove break after return

[Bernard]

From: Thomas Zimmermann <tzimmermann@suse.de>
Date: 2020-10-23 15:13:30
To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
Cc:  opensource.kernel@vivo.com
Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
>
>On 23.10.20 09:00, Bernard Zhao wrote:
>> In function mgag200_set_pci_regs, there are some switch cases
>> returned, then break. These break will never run.
>> This patch is to make the code a bit readable.
>> 
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> index 38672f9e5c4f..de873a5d276e 100644
>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>>  	case G200_SE_A:
>>  	case G200_SE_B:
>>  		return mga_g200se_set_plls(mdev, clock);
>> -		break;
>>  	case G200_WB:
>>  	case G200_EW3:
>>  		return mga_g200wb_set_plls(mdev, clock);
>> -		break;
>>  	case G200_EV:
>>  		return mga_g200ev_set_plls(mdev, clock);
>> -		break;
>>  	case G200_EH:
>>  	case G200_EH3:
>>  		return mga_g200eh_set_plls(mdev, clock);
>> -		break;
>>  	case G200_ER:
>>  		return mga_g200er_set_plls(mdev, clock);
>> +	default:
>
>No default case here. If one of the enum values is not handled by the
>switch, the compiler should warn about it.

Hi

For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
How does the compiler warn this("If one of the enum values is not handled") before the code runs?

BR//Bernard

>Best regards
>Thomas
>
>>  		break;
>>  	}
>>  
>> 
>
>-- 
>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


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] gpu/drm/mgag200:remove break after return

[Thomas Zimmermann]

Hi

On 23.10.20 09:48, Bernard wrote:
> 
> 
> From: Thomas Zimmermann <tzimmermann@suse.de>
> Date: 2020-10-23 15:13:30
> To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
> Cc:  opensource.kernel@vivo.com
> Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
>>
>> On 23.10.20 09:00, Bernard Zhao wrote:
>>> In function mgag200_set_pci_regs, there are some switch cases
>>> returned, then break. These break will never run.
>>> This patch is to make the code a bit readable.
>>>
>>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>>> ---
>>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> index 38672f9e5c4f..de873a5d276e 100644
>>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>>>  	case G200_SE_A:
>>>  	case G200_SE_B:
>>>  		return mga_g200se_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_WB:
>>>  	case G200_EW3:
>>>  		return mga_g200wb_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_EV:
>>>  		return mga_g200ev_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_EH:
>>>  	case G200_EH3:
>>>  		return mga_g200eh_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_ER:
>>>  		return mga_g200er_set_plls(mdev, clock);
>>> +	default:
>>
>> No default case here. If one of the enum values is not handled by the
>> switch, the compiler should warn about it.
> 
> Hi
> 
> For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
> How does the compiler warn this("If one of the enum values is not handled") before the code runs?

The enum mga_type {} has values (G200_ER, G200_WB, etc) that are known
to the compiler. If one of those values does not show up in the switch
statement, the compiler warns. Adding default would silence this warning.

In principle, mdev->type could contain any value that fits into an int.
But assigning anything that is not listed in enum mga_type {} is
considered an error as well.

Best regards
Thomas

> 
> BR//Bernard
> 
>> Best regards
>> Thomas
>>
>>>  		break;
>>>  	}
>>>  
>>>
>>
>> -- 
>> 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
> 
> 

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

Re: [PATCH] gpu/drm/mgag200:remove break after return

[Thomas Zimmermann]

Hi

On 23.10.20 09:48, Bernard wrote:
> 
> 
> From: Thomas Zimmermann <tzimmermann@suse.de>
> Date: 2020-10-23 15:13:30
> To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
> Cc:  opensource.kernel@vivo.com
> Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
>>
>> On 23.10.20 09:00, Bernard Zhao wrote:
>>> In function mgag200_set_pci_regs, there are some switch cases
>>> returned, then break. These break will never run.
>>> This patch is to make the code a bit readable.
>>>
>>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>>> ---
>>>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> index 38672f9e5c4f..de873a5d276e 100644
>>> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>>> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>>>  	case G200_SE_A:
>>>  	case G200_SE_B:
>>>  		return mga_g200se_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_WB:
>>>  	case G200_EW3:
>>>  		return mga_g200wb_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_EV:
>>>  		return mga_g200ev_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_EH:
>>>  	case G200_EH3:
>>>  		return mga_g200eh_set_plls(mdev, clock);
>>> -		break;
>>>  	case G200_ER:
>>>  		return mga_g200er_set_plls(mdev, clock);
>>> +	default:
>>
>> No default case here. If one of the enum values is not handled by the
>> switch, the compiler should warn about it.
> 
> Hi
> 
> For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
> How does the compiler warn this("If one of the enum values is not handled") before the code runs?

The enum mga_type {} has values (G200_ER, G200_WB, etc) that are known
to the compiler. If one of those values does not show up in the switch
statement, the compiler warns. Adding default would silence this warning.

In principle, mdev->type could contain any value that fits into an int.
But assigning anything that is not listed in enum mga_type {} is
considered an error as well.

Best regards
Thomas

> 
> BR//Bernard
> 
>> Best regards
>> Thomas
>>
>>>  		break;
>>>  	}
>>>  
>>>
>>
>> -- 
>> 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
> 
> 

-- 
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Re: [PATCH] gpu/drm/mgag200:remove break after return

[Sam Ravnborg]

Hi Bernard.

On Fri, Oct 23, 2020 at 03:48:49PM +0800, Bernard wrote:
> 
> 
> From: Thomas Zimmermann <tzimmermann@suse.de>
> Date: 2020-10-23 15:13:30
> To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
> Cc:  opensource.kernel@vivo.com
> Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
> >
> >On 23.10.20 09:00, Bernard Zhao wrote:
> >> In function mgag200_set_pci_regs, there are some switch cases
> >> returned, then break. These break will never run.
> >> This patch is to make the code a bit readable.
> >> 
> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> >> ---
> >>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> >> index 38672f9e5c4f..de873a5d276e 100644
> >> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> >> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> >> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
> >>  	case G200_SE_A:
> >>  	case G200_SE_B:
> >>  		return mga_g200se_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_WB:
> >>  	case G200_EW3:
> >>  		return mga_g200wb_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_EV:
> >>  		return mga_g200ev_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_EH:
> >>  	case G200_EH3:
> >>  		return mga_g200eh_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_ER:
> >>  		return mga_g200er_set_plls(mdev, clock);
> >> +	default:
> >
> >No default case here. If one of the enum values is not handled by the
> >switch, the compiler should warn about it.
> 
> Hi
> 
> For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
> How does the compiler warn this("If one of the enum values is not handled") before the code runs?

If the switch/case does not include "G200_ER" then the compiler can see
one enum value is missing from the list and can warn.
As a test - Try to drop the default and drop G200_ER - then the
compiler (hopefully) will warn.

	Sam

Re: Re: [PATCH] gpu/drm/mgag200:remove break after return

[Sam Ravnborg]

Hi Bernard.

On Fri, Oct 23, 2020 at 03:48:49PM +0800, Bernard wrote:
> 
> 
> From: Thomas Zimmermann <tzimmermann@suse.de>
> Date: 2020-10-23 15:13:30
> To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
> Cc:  opensource.kernel@vivo.com
> Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
> >
> >On 23.10.20 09:00, Bernard Zhao wrote:
> >> In function mgag200_set_pci_regs, there are some switch cases
> >> returned, then break. These break will never run.
> >> This patch is to make the code a bit readable.
> >> 
> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> >> ---
> >>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
> >>  1 file changed, 1 insertion(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> >> index 38672f9e5c4f..de873a5d276e 100644
> >> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> >> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> >> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
> >>  	case G200_SE_A:
> >>  	case G200_SE_B:
> >>  		return mga_g200se_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_WB:
> >>  	case G200_EW3:
> >>  		return mga_g200wb_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_EV:
> >>  		return mga_g200ev_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_EH:
> >>  	case G200_EH3:
> >>  		return mga_g200eh_set_plls(mdev, clock);
> >> -		break;
> >>  	case G200_ER:
> >>  		return mga_g200er_set_plls(mdev, clock);
> >> +	default:
> >
> >No default case here. If one of the enum values is not handled by the
> >switch, the compiler should warn about it.
> 
> Hi
> 
> For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
> How does the compiler warn this("If one of the enum values is not handled") before the code runs?

If the switch/case does not include "G200_ER" then the compiler can see
one enum value is missing from the list and can warn.
As a test - Try to drop the default and drop G200_ER - then the
compiler (hopefully) will warn.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re:Re: Re: [PATCH] gpu/drm/mgag200:remove break after return

[Bernard]

From: Sam Ravnborg <sam@ravnborg.org>
Date: 2020-10-25 01:14:57
To:  Bernard <bernard@vivo.com>
Cc:  Thomas Zimmermann <tzimmermann@suse.de>,opensource.kernel@vivo.com,David Airlie <airlied@linux.ie>,linux-kernel@vger.kernel.org,dri-devel@lists.freedesktop.org,Dave Airlie <airlied@redhat.com>
Subject: Re: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi Bernard.
>
>On Fri, Oct 23, 2020 at 03:48:49PM +0800, Bernard wrote:
>> 
>> 
>> From: Thomas Zimmermann <tzimmermann@suse.de>
>> Date: 2020-10-23 15:13:30
>> To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
>> Cc:  opensource.kernel@vivo.com
>> Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
>> >
>> >On 23.10.20 09:00, Bernard Zhao wrote:
>> >> In function mgag200_set_pci_regs, there are some switch cases
>> >> returned, then break. These break will never run.
>> >> This patch is to make the code a bit readable.
>> >> 
>> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> >> ---
>> >>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>> >>  1 file changed, 1 insertion(+), 4 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> >> index 38672f9e5c4f..de873a5d276e 100644
>> >> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> >> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> >> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>> >>  	case G200_SE_A:
>> >>  	case G200_SE_B:
>> >>  		return mga_g200se_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_WB:
>> >>  	case G200_EW3:
>> >>  		return mga_g200wb_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_EV:
>> >>  		return mga_g200ev_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_EH:
>> >>  	case G200_EH3:
>> >>  		return mga_g200eh_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_ER:
>> >>  		return mga_g200er_set_plls(mdev, clock);
>> >> +	default:
>> >
>> >No default case here. If one of the enum values is not handled by the
>> >switch, the compiler should warn about it.
>> 
>> Hi
>> 
>> For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
>> How does the compiler warn this("If one of the enum values is not handled") before the code runs?
>
>If the switch/case does not include "G200_ER" then the compiler can see
>one enum value is missing from the list and can warn.
>As a test - Try to drop the default and drop G200_ER - then the
>compiler (hopefully) will warn.

Hi, Sam & Thomas：

  Thank you very much for your kind comments, I learned new useful knowledge.
  I will resubmit the patch.

BR//Bernard

>	Sam

Re:Re: Re: [PATCH] gpu/drm/mgag200:remove break after return

[Bernard]

From: Sam Ravnborg <sam@ravnborg.org>
Date: 2020-10-25 01:14:57
To:  Bernard <bernard@vivo.com>
Cc:  Thomas Zimmermann <tzimmermann@suse.de>,opensource.kernel@vivo.com,David Airlie <airlied@linux.ie>,linux-kernel@vger.kernel.org,dri-devel@lists.freedesktop.org,Dave Airlie <airlied@redhat.com>
Subject: Re: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi Bernard.
>
>On Fri, Oct 23, 2020 at 03:48:49PM +0800, Bernard wrote:
>> 
>> 
>> From: Thomas Zimmermann <tzimmermann@suse.de>
>> Date: 2020-10-23 15:13:30
>> To:  Bernard Zhao <bernard@vivo.com>,Dave Airlie <airlied@redhat.com>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,dri-devel@lists.freedesktop.org,linux-kernel@vger.kernel.org
>> Cc:  opensource.kernel@vivo.com
>> Subject: Re: [PATCH] gpu/drm/mgag200:remove break after return>Hi
>> >
>> >On 23.10.20 09:00, Bernard Zhao wrote:
>> >> In function mgag200_set_pci_regs, there are some switch cases
>> >> returned, then break. These break will never run.
>> >> This patch is to make the code a bit readable.
>> >> 
>> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> >> ---
>> >>  drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +----
>> >>  1 file changed, 1 insertion(+), 4 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> >> index 38672f9e5c4f..de873a5d276e 100644
>> >> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
>> >> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
>> >> @@ -794,20 +794,17 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev, long clock)
>> >>  	case G200_SE_A:
>> >>  	case G200_SE_B:
>> >>  		return mga_g200se_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_WB:
>> >>  	case G200_EW3:
>> >>  		return mga_g200wb_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_EV:
>> >>  		return mga_g200ev_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_EH:
>> >>  	case G200_EH3:
>> >>  		return mga_g200eh_set_plls(mdev, clock);
>> >> -		break;
>> >>  	case G200_ER:
>> >>  		return mga_g200er_set_plls(mdev, clock);
>> >> +	default:
>> >
>> >No default case here. If one of the enum values is not handled by the
>> >switch, the compiler should warn about it.
>> 
>> Hi
>> 
>> For this point I was a little confused, about this switch variable "mdev->type", my understanding is that this variable`s value can be certain only when the code is running.
>> How does the compiler warn this("If one of the enum values is not handled") before the code runs?
>
>If the switch/case does not include "G200_ER" then the compiler can see
>one enum value is missing from the list and can warn.
>As a test - Try to drop the default and drop G200_ER - then the
>compiler (hopefully) will warn.

Hi, Sam & Thomas：

  Thank you very much for your kind comments, I learned new useful knowledge.
  I will resubmit the patch.

BR//Bernard

>	Sam


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel