[U-Boot] unsigned int for gpio

[U-Boot] unsigned int for gpio

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

-       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

[U-Boot] unsigned int for gpio

[Simon Glass]

Hi Jeroen,

On 11 June 2014 15:37, Jeroen Hofstee <jeroen@myspectrum.nl> wrote:
> 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?

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

[U-Boot] unsigned int for gpio

[Jeroen Hofstee]

Hello Simon,

On wo, 2014-06-11 at 17:12 -0600, Simon Glass wrote:
> Hi Jeroen,
> 
> On 11 June 2014 15:37, Jeroen Hofstee <jeroen@myspectrum.nl> wrote:
> > 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?
> 
> 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