[U-Boot] [PATCH 2/2] ARM: omap3: add support to Technexion twister board

[Igor Grinberg]

On 11/23/11 16:41, Stefano Babic wrote:
> On 23/11/2011 15:18, Igor Grinberg wrote:
>>
>> Sorry, missed the below in the first reply
>>
> 
> It does not matter...
> 
>>> +int board_init(void)
>>> +{
>>> +	gpmc_init(); /* in SRAM or SDRAM, finish GPMC */
>>> +
>>> +	/* boot param addr */
>>> +	gd->bd->bi_boot_params = (OMAP34XX_SDRC_CS0 + 0x100);
>>> +
>>> +	writel(0x00000000, &gpmc_cfg->cs[1].config1);
>>> +	writel(0x001e1e01, &gpmc_cfg->cs[1].config2);
>>> +	writel(0x00080300, &gpmc_cfg->cs[1].config3);
>>> +	writel(0x1c091c09, &gpmc_cfg->cs[1].config4);
>>> +	writel(0x04181f1f, &gpmc_cfg->cs[1].config5);
>>> +	writel(0x00000FCF, &gpmc_cfg->cs[1].config6);
>>> +	writel(0x00000f61, &gpmc_cfg->cs[1].config7);
>>> +
>>> +	writel(0x00000000, &gpmc_cfg->cs[3].config1);
>>> +	writel(0x001e1e01, &gpmc_cfg->cs[3].config2);
>>> +	writel(0x00080300, &gpmc_cfg->cs[3].config3);
>>> +	writel(0x1c091c09, &gpmc_cfg->cs[3].config4);
>>> +	writel(0x04181f1f, &gpmc_cfg->cs[3].config5);
>>> +	writel(0x00000FCF, &gpmc_cfg->cs[3].config6);
>>> +	writel(0x00000f63, &gpmc_cfg->cs[3].config7);
>>
>> Can there be an explanation of what's that and why is it needed?
> 
> They are for a UART Controller (XR16L2751) and for the seconfd ethernet
> controller (SMC911X). I will explain this.
> 
>> Also, it looks like you are writing the same values to both chip selects.
> 
> Why not ? The setup sets some relaxing time for both of them

I don't mean you should not do this - it is perfectly fine.
I mean, you should define those values and reuse the defines or
consolidate it in some other way...
Anyway, if you use the enable_gpmc_cs_config() function, it gets
an array of those values as one of the parameters.

> 
>> Can enable_gpmc_cs_config() function be used here as well and
>> the values documented (e.g. NET_GPMC_CONFIGx)?
> 
> Sure...the name NET_GPMC_CONFIG is quite misleading, the setup for a
> chip select has nothing to do with the NET. I see that on other boards
> this defines are always used to set the chipselect for an ethernet
> controller, but it is not a rule..

I've just used NET_... as an example. I did not mean you should use it.
Now when I know, that's XR16L2751 and an Ethernet controller,
I can propose a better name - XR16L2751_GPMC_CONFIG for the UART.
For the second Ethernet controller, are these the same values as
those in NET_GPMC_CONFIG? If yes, then why not use it?


-- 
Regards,
Igor.