> > No, move this to mxs_gpio.c and simply make the function not static. > > I too find the construction a little strange, but I copied it from the > Blackfin implementation. > > But after taking a second look at it, it made sense: It makes the file > pulling including it, define it statically locally. The macro below > exports it to the macro name space and cmd_gpio checks for the existence > of that makro. If it doesn't exist there, cmd_gpio defines it as > simple_strtoul. I suppose this is created that way to allow diversity > between platforms with and without name support. > > Currently, Blackfin is the only platform supporting named pins. I'd be > happy to change it, but then Blackfin needs some work too and I don't have > the hardware to test it.
I don't think I follow ... don't you only have to define the function and that's it? > > > So if I pass name == NULL, we're done for here ;-) > > We would, but it's a static function, so it's unavailable to the rest of > the world. What? Which one is? The name_to_gpio_number() function, which I assume is used by the cmd_gpio must exactly be NON-static! Or prove me wrong please. > And name won't be NULL unless the command line argument parsing > is borked. I know what you mean, but if even all static functions start > schizophrenically checking all their parameters we'd be doing that half > the CPU cycles. > > > Braces missing around this return statement. > > Will do! I actually do that for all of my code, but this is how it was in > Blackfin (and in drivers/gpio/mxs_gpio.c, and in common/cmd_gpio.c, for > that matter). > > > Also, why not do something like: > > > > if (tolower(name[0]) != 'b') > > > > return -EINVAL; > > > > name++; > > pinnr = ... > > > > if (tolower(name[0]) != 'p') > > > > return -EINVAL; > > > > name++; > > ... > > > > It seems more explicit to me that way. > > Agreed; Will do! > > > What's this define for ? > > See above. > > > Do you even need this if CONFIG_CMD_GPIO is undefined? Move the function > > to mxs_gpio.c and make it not static should work for you. > > Nope, I don't, but I didn't put it there. It was already there, so somebody > must have approved (of overlooked) it ;-) Don't rely on other code too much, improvise and bring progress :) M _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
