On 28/08/2023 19:51, Laurent Pinchart wrote:
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -592,15 +592,19 @@ int msm_csid_subdev_init(struct camss *camss, struct csid_device *csid,
csid->camss = camss;
csid->id = id;
- if (camss->res->version == CAMSS_8x16) {
+ switch (camss->res->version) {
+ case CAMSS_8x16:
csid->ops = &csid_ops_4_1;
- } else if (camss->res->version == CAMSS_8x96 ||
- camss->res->version == CAMSS_660) {
+ break;
+ case CAMSS_8x96:
+ case CAMSS_660:
csid->ops = &csid_ops_4_7;
- } else if (camss->res->version == CAMSS_845 ||
- camss->res->version == CAMSS_8250) {
+ break;
+ case CAMSS_845:
+ case CAMSS_8250:
csid->ops = &csid_ops_gen2;
- } else {
+ break;
+ default:
return -EINVAL;
This should never happen, as adding support for a new SoC should come
with an update for all the applicable switch/case statements. It's
useful to let the compiler complain if someone forgets to do so, but
with a default case, you will only see the issue at runtime. Could it be
caught at compile time ?
This can be done in fact.
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wswitch_002denum-303
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
typedef enum {
MO = 0,
LARRY,
CURLY,
BINGO,
}my_type;
int main (int argc, char *argv[])
{
my_type x;
time_t t;
srand((unsigned) time(&t));
x = rand() % BINGO;
switch(x) {
case MO:
printf("mo\n");
break;
case LARRY:
printf("larry\n");
break;
default:
printf("blargh\n");
break;
}
return 0;
}
gcc -o test test.c -Wswitch-enum
test.c: In function ‘main’:
test.c:38:9: warning: enumeration value ‘CURLY’ not handled in switch
[-Wswitch-enum]
38 | switch(x) {
| ^~~~~~
It looks like we only enable that switch for tools though
grep -r "Wswitch-enum" *
tools/scripts/Makefile.include:EXTRA_WARNINGS += -Wswitch-enum
tools/bpf/bpftool/Makefile:CFLAGS += $(filter-out -Wswitch-enum
-Wnested-externs,$(EXTRA_WARNINGS))
I'll still implement the code though, since if we do introduce the
switch for the kernel it would be caught.
---
bod
