[U-Boot] [PATCH] M28: Added pin name support in GPIO driver

[Robert Deliën]

> I don't think I follow ... don't you only have to define the function and that's 
> it?

No, it's a bit more complicated than that. It wouldn't be my choice either, but now I'm familiar with it, I find it quite charming. Let me see if I can explain:

Check out common/cmd_gpio.c.
The first part pulls in platform specific headers and checks if the macro name_to_gpio is defined. In case of Blackfin (and with my patch for i.MX28) it is defined by the macro trailing the static function implementation. For all other platforms it's not defined and in that case a it will be defined with a generic strtoul implementation:
...
#include <asm/gpio.h>

#ifndef name_to_gpio
#define name_to_gpio(name) simple_strtoul(name, NULL, 10)
#endif
...

The (static) function do_gpio executing the gpio command uses the macro name_to_gpio to convert the 3rd argument string into a pin number, allowing the platform specific header to define a static inline for alternative translations.
...
static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
…
	/* turn the gpio name into a gpio number */
	gpio = name_to_gpio(str_gpio);
	if (gpio < 0)
		goto show_usage;
…

Of course name_to_gpio could be a non-static function, allowing it to be used by other parts of the code too. And for that reason do_gpio could be non-static too.  But that would still require an ifdef-orgy like this to make it work for different diversities:
…
	/* turn the gpio name into a gpio number */
#if defined (CONFIG_MX28) || defined (CONFIG_BLACKFIN)
	gpio = name_to_gpio(str_gpio);
#else
	gpio = simple_strtoul(str_gpio, NULL, 10);
#endif
	if (gpio < 0)
		goto show_usage;
…

I suppose it's all a matter of taste. And even though we would have made a different choice, I find the blackfin approach rather charming. Since they were the first ones introducing friendly pin names, they have set the standard and I'm merely adhering to 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.

I don't see much use for this function outside the scope of cmd_gpio.c. And somebody who does need it elsewhere is always free to make it non-static.
But I can agree with you here.

> Don't rely on other code too much, improvise and bring progress :)

I think the difference between both solutions is just taste. And since the blackfin solution was already there, I just adhered to it. I think having two different standards is not desirable, and changing the blackfin implementation may trigger that custodian (Mike Frysinger CC'd). I wonder if making changes to working code for a matter of taste is worth it

Robert.