Skip to contentSkip to conversation

unsigned int for gpio

U-Boot Jeroen Hofstee 2 participants 3 messages Jun 11, 2014 – Jun 12, 2014

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

Hi Jeroen,

Quoted 25 lines · Jeroen Hofstee

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:

(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

#2 lore raw

Hello Simon,

Quoted 37 lines · Simon Glass

Hi Jeroen,

Quoted 25 lines · Jeroen Hofstee

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:

(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