unsigned int for gpio
Hello Simon,
in commit 95a260a9
dm: Enable gpio command to support driver model
Now that named GPIO banks are supported, along with a way of obtaining the status of a GPIO (input or output), we can provide an enhanced GPIO command for driver model. Where the driver provides its own operation for obtaining the GPIO state, this is used, otherwise a generic version is sufficient.
you made the following change:
- int gpio;
+ unsigned int gpio;
This breaks the code after it though:
/* turn the gpio name into a gpio number */
gpio = name_to_gpio(str_gpio);
if (gpio < 0)
goto show_usage;And causes warnings with clang like:
common/cmd_gpio.c:159:11: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
if (gpio < 0)
~~~~ ^ ~Do you recall why it is made unsigned?
Regards,
Jeroen
Hi Jeroen,
Quoted 25 lines · Jeroen Hofstee
Hello Simon,
in commit
95a260a9dm: Enable gpio command to support driver model
Now that named GPIO banks are supported, along with a way of obtaining the status of a GPIO (input or output), we can provide an enhanced GPIO command for driver model. Where the driver provides its own operation for obtaining the GPIO state, this is used, otherwise a generic version is sufficient.
you made the following change:
(quoted diff) +1
-1- int gpio; + unsigned int gpio;This breaks the code after it though:
/* turn the gpio name into a gpio number */ gpio = name_to_gpio(str_gpio); if (gpio < 0) goto show_usage;
Quoted 8 lines
And causes warnings with clang like:
common/cmd_gpio.c:159:11: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]if (gpio < 0) ~~~~ ^ ~Do you recall why it is made unsigned?
This is because gpio_lookup_name() needs an unsigned (-ve values have no meaning). I think that is what we want ultimately, but perhaps we need to go back to int in the meantime. The simplest fix would probably be to use a separate unsigned variable for the driver model code, and go back to an int for the current (old) code. The difference is that driver model returns a separate error code, whereas the old code combines the GPIO and error into a single value.
Regards,
Simon
Hello Simon,
Quoted 37 lines · Simon Glass
Hi Jeroen,
Quoted 25 lines · Jeroen Hofstee
Hello Simon,
in commit
95a260a9dm: Enable gpio command to support driver model
Now that named GPIO banks are supported, along with a way of obtaining the status of a GPIO (input or output), we can provide an enhanced GPIO command for driver model. Where the driver provides its own operation for obtaining the GPIO state, this is used, otherwise a generic version is sufficient.
you made the following change:
(quoted diff) +1
-1- int gpio; + unsigned int gpio;This breaks the code after it though:
/* turn the gpio name into a gpio number */ gpio = name_to_gpio(str_gpio); if (gpio < 0) goto show_usage;Quoted 8 lines
And causes warnings with clang like:
common/cmd_gpio.c:159:11: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]if (gpio < 0) ~~~~ ^ ~Do you recall why it is made unsigned?
Quoted 7 lines
This is because
gpio_lookup_name()needs an unsigned (-ve values have no meaning). I think that is what we want ultimately, but perhaps we need to go back to int in the meantime. The simplest fix would probably be to use a separate unsigned variable for the driver model code, and go back to an int for the current (old) code. The difference is that driver model returns a separate error code, whereas the old code combines the GPIO and error into a single value.
ok thanks. I will leave it alone then for now. Since the default lookup, simple_strtoul, is unsigned as well, it won't solve a common problem anyway.
Regards,
Jeroen