Hello Christian,
Christian Riesch wrote:
> Hello Heiko,
> I hope this is the complete mail now :-/
seems so ...
> On Wed, Nov 16, 2011 at 7:49 AM, Heiko Schocher <h...@denx.de> wrote:
>> Hello Christian,
>>
>> Christian Riesch wrote:
[...]
>>> - da850_pinmux_ctl(18, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX18);
>>> - da850_pinmux_ctl(19, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX19);
>>> + /* setup serial port */
>>> + davinci_configure_pin_mux(uart_pins, ARRAY_SIZE(uart_pins));
>> Why only the uart pins? We could use here something like "board_pins"
>> and initialize here all pins for the board?
>
> Because only the UART pins are required here. Since the CPU has
And if you need some other pins, for example gpio?
> already loaded the SPL from SPI flash or is executing the SPL from NOR
> flash or whatever, the pins for memory access are already configured.
> Later the board specific file can do all the configuration that it
> actually needs, see board/davinci/da8xxevm/da850evm.c.
Yes, but, why not setup all pinmux settings immediately in the
spl code?
>> I reworked this for the enbw_cmc board too, and removed also the
>> CONFIG_SYS_DA850_PINMUX* defines complete ... but I am not really
>> happy with it. Why?
>>
>> We have for example on the am1808 19 * 8 = 152 pins to setup up
>>
>> If using the CONFIG_SYS_DA850_PINMUX* defines we have 19 register-
>> writes and have setup them all (And you must think about all
>> your pins, if we use such a struct, not defined pins are in
>> default state ... which is good or bad ...)
>>
>> With using davinci_configure_pin_mux() we have 152 * (read, write
>> and some logic operations) ...
>
> Actually the number of read, writes, logic operations will depend on
> the number of GPIO pins you use on your board. I guess you will not
> change the pinmux settings of pins you didn't connect on your board.
> But yes, these are a lot of operations that need to be done.
Not with the define approach! ... There we have only 19 register
writes.
>> and I have to code a "static const
>> struct pinmux_config board_pins" with 152 lines in the code ...
>
> How about using an approach like in board/davinci/da8xxevm/da850evm.c.
> There we have several structs like
>
> static const struct pinmux_config spi1_pins[] = {
> ...
> }
>
> that defines pinmux for groups of pins that are usually used together.
>
> Later, these groups are put together in
>
> static const struct pinmux_resource pinmuxes[] = {
> { DAVINCI_LPSC_AEMIF }, /* NAND, NOR */
> { DAVINCI_LPSC_SPI1 }, /* Serial Flash */
> { DAVINCI_LPSC_EMAC }, /* image download */
> { DAVINCI_LPSC_UART2 }, /* console */
> { DAVINCI_LPSC_GPIO },
> };
You mean here:
static const struct pinmux_resource pinmuxes[] = {
#ifdef CONFIG_SPI_FLASH
PINMUX_ITEM(spi1_pins),
#endif
PINMUX_ITEM(uart_pins),
PINMUX_ITEM(i2c_pins),
#ifdef CONFIG_NAND_DAVINCI
PINMUX_ITEM(nand_pins),
#elif defined(CONFIG_USE_NOR)
PINMUX_ITEM(nor_pins),
#endif
};
right?
> We could move the pinmux_config structs to a header file which would reduce
> the code in your board config file to a few lines, you only would need
> a pinmux_resource struct.
Yep, if we go this way, we should move them to a include
file, so we can use them for all da8xx boards.
> Still we need to do pinmuxing for UART (and maybe also for other pins) in
> the SPL. How do you like the approach in static void set_mux_conf_regs(void)
> in arch/arm/cpu/armv7/omap-common/hwinit-common.c? Depending on the
> context either the pins that are essential for the SPL or
> all other pins are configured.
Yes, looks good to me.
> This would at least reduce the number of code lines that you need for a
> new board. And this code would be much easier to understand than this
> list of magic numbers.
Yes. Don't understand me wrong against the "struct pinmux_resource"
approach, it looks good to me also, and I agree this is (maybe) better
to read (maybe, because if something is setup wrong, you need in both
approaches the help from the doc ...), but I didn't get the disadvantages
of "my" define approach setting up the pinmux in one place ...
Advantages of it I think:
- if settings are wrong i find it fast (because in one place)
- setup with a minimum nr of instructions.
- smaller code size
- if using the pinmux setup tool from TI
(URL: http://www-s.ti.com/sc/techlit/spraba2.zip)
you can easy setup all pins and gets a check for pinmux conflicts
for free ... and it exports a header file, from where you easy can
get the values for the CONFIG_SYS_DA850_PINMUX* defines ... if you
want to use this tool, it is more work to convert this to the
"struct pinmux_resource" approach ...
So let us now decide which way to go ... (BTW: If we decide to go the
"struct pinmux_resource" approach, can you provide a patch, which
moves the pinmux settings from the existing da8xx boards to an include
file (whats with arch/arm/include/asm/arch-davinci/da8xx_pinmux.h)?
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
