[U-Boot] Blackfin: cmd_gpio port/pin naming
Dear Mr. Frysinger,
the Blackfin U-Boot GPIO command (see "arch/blackfin/cpu/cmd_gpio.c") specifies the port/pin naming in the form "[p][port]<#>", e.g. PF11. The pin portion of the specified GPIO is limited to 0..15. While this is correct for Blackfins with different bank names (e.g. BF537 with PF, PG, PH), it's not sufficient for Blackfins with linear naming like the BF561 (PF0..PF47).
Based on the port name and the pin number, it's transformed into linear GPIO numbering, which is passed to the GPIO routines (e.g. PG1 @ BF537 => 16+1).
Therefore simply changing the limit from 15 to MAX_BLACKFIN_GPIOS resp. MAX_RESOURCES is not an option (for all Blackfins).
I see three possible solutions: - Introduce a new define MAX_BLACKFIN_GPIO_PORT_NUMBER or similar in all arch/blackfin/include/asm/mach-bfxxx/gpio.h and use that for the check. Name suggestions are welcome. - #ifdef the check based on BFxxx_FAMILY resp. CONFIG_BFxxx. Not sure about the best defines here. And it's ugly. - Add another port naming specification to cmd_gpio like "P42".
I'd opt for the first approach, but before submitting a patch, I'd like to hear your opinion first.
N.B.: Didn't have a closer look at the portmux stuff.
Best regards, A. Pretzsch
On Friday, January 07, 2011 15:50:30 Andreas Pretzsch wrote:
the Blackfin U-Boot GPIO command (see "arch/blackfin/cpu/cmd_gpio.c") specifies the port/pin naming in the form "[p][port]<#>", e.g. PF11. The pin portion of the specified GPIO is limited to 0..15. While this is correct for Blackfins with different bank names (e.g. BF537 with PF, PG, PH), it's not sufficient for Blackfins with linear naming like the BF561 (PF0..PF47).
if we cut out the friendly port naming, the rest of the code is no longer Blackfin specific. so what i had been thinking of doing at some point was dropping that as a requirement and making it a "nice" feature so it could be moved into common/cmd_gpio.c for everyone to use.
so if we rip out that part and just make it something like: ... #ifndef name_to_gpio #define name_to_gpio(name) simple_strtoul(name, NULL, 10) #endif ... ulong pin = name_to_gpio(argv[2]); if (!gpio_is_valid(pin)) goto usage; ...
although perhaps in your case, we can just change "if (pin > 15)" to "if (!gpio_is_valid(pin))" and forget about the people who do PF34 on parts that only have PF0..PF15 (they'll instead get like PH4 or whatever). -mike
Am Freitag, den 07.01.2011, 17:38 -0500 schrieb Mike Frysinger:
On Friday, January 07, 2011 15:50:30 Andreas Pretzsch wrote:
the Blackfin U-Boot GPIO command (see "arch/blackfin/cpu/cmd_gpio.c") specifies the port/pin naming in the form "[p][port]<#>", e.g. PF11. The pin portion of the specified GPIO is limited to 0..15. While this is correct for Blackfins with different bank names (e.g. BF537 with PF, PG, PH), it's not sufficient for Blackfins with linear naming like the BF561 (PF0..PF47).
if we cut out the friendly port naming, the rest of the code is no longer Blackfin specific. so what i had been thinking of doing at some point was dropping that as a requirement and making it a "nice" feature so it could be moved into common/cmd_gpio.c for everyone to use.
Sounds like a good idea to me. As your code already uses the Linux GPIO conventions and naming (without the gpio_chip stuff, which is not necessary for a bootloader IMHO), it'd be a solid base for that.
Mr. Denk, are there any plans for a generic GPIO layer in U-Boot ?
so if we rip out that part and just make it something like: ... #ifndef name_to_gpio #define name_to_gpio(name) simple_strtoul(name, NULL, 10) #endif
Personally, I'd go with arch specific functions, following the Linux codebase, something like: #ifdef CONFIG_GENERIC_GPIO #include <asm/gpio.h> #else static inline int name_to_gpio(char *name) { return simple_strtoul(name, NULL, 10); } ... #endif
... ulong pin = name_to_gpio(argv[2]); if (!gpio_is_valid(pin)) goto usage; ...
although perhaps in your case, we can just change "if (pin > 15)" to "if (!gpio_is_valid(pin))" and forget about the people who do PF34 on parts that only have PF0..PF15 (they'll instead get like PH4 or whatever).
Would ACK that. Won't break any (correct) scripts out in the field and solves the BF561 gpio issue with minimal effort. I'll send a patch the next couple of minutes.
On Mon, Jan 10, 2011 at 10:59 AM, Andreas Pretzsch wrote:
As your code already uses the Linux GPIO conventions and naming (without the gpio_chip stuff, which is not necessary for a bootloader IMHO), it'd be a solid base for that.
Mr. Denk, are there any plans for a generic GPIO layer in U-Boot ?
if we arent supporting gpio_chips, then there really isnt much need for common GPIO code. there is already a generic GPIO layer in U-Boot just like in Linux -- include asm/gpio.h and use the normal gpio_xxx set of functions. -mike
Am Montag, den 10.01.2011, 11:28 -0500 schrieb Mike Frysinger:
On Mon, Jan 10, 2011 at 10:59 AM, Andreas Pretzsch wrote:
As your code already uses the Linux GPIO conventions and naming (without the gpio_chip stuff, which is not necessary for a bootloader IMHO), it'd be a solid base for that.
Mr. Denk, are there any plans for a generic GPIO layer in U-Boot ?
if we arent supporting gpio_chips, then there really isnt much need for common GPIO code. there is already a generic GPIO layer in U-Boot just like in Linux -- include asm/gpio.h and use the normal gpio_xxx set of functions.
Well, partly. True for Blackfin and a few ARM/AVR32/NIOS2, albeit the latter have a mixture of specific names like at91_get_gpio_value(), pin_to_controller(), kw_gpio_get_value(), etc.
Therefore unifying them to common names and syntax would be the first step. Of course, the gpio_chip approach of Linux is the cleaner way, adding only a few extra bytes of code/ram. I'm fine with both. As time permits, I'm willing to help here.
participants (2)
-
Andreas Pretzsch -
Mike Frysinger