Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-30 Thread Mark Jackson
Scott Wood wrote:
> On Tue, Jul 29, 2008 at 09:52:12AM +0100, Mark Jackson wrote:
>> I didn't want to use u-boot's "slient boot" options, since they're 
>> hard-coded at compile time.  The only place I could think to this was to 
>> modify the atmel_usart.c file as above.
> 
> It's not hard-coded at compile-time -- set GD_FLG_SILENT in gd->flags
> from early board code depending on the state of the GPIO pin.  For
> example, 8313erdb does this depending on whether it's booting or resuming
> from suspend.

Yes ... I can see that can be used to disable any console outputs.

*But* I'm also needing to disable any console *inputs* in a similar way.

Any ideas on how to do that ?

I'm guessing I need any extra "silent" check in console.c tstc() ?

Is it acceptable to modify console.c if I use a nicely generic #ifdef ?

e.g.

int tstc (void)
{
+#if defined(CONFIG_SILENT_CONSOLE) && defined(CONFIG_SILENT_CONSOLE_INPUT)
+   if (gd->flags & GD_FLG_SILENT)
+   return 0;
+#endif
+
if (gd->flags & GD_FLG_DEVINIT) {
/* Test the standard input */
return ftstc (stdin);
}

/* Send directly to the handler */
return serial_tstc ();
}

Mark

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-29 Thread Scott Wood
Wolfgang Denk wrote:
> In message <[EMAIL PROTECTED]> you wrote:
>> Aligning with TABs (or at all, in initializer lists) is not a good thing,
> 
> It is mandatory per Coding Style requirements.

Where?  I don't see any mention of alignment, and while there's no 
explicit definition of indentation, the rationale states that the choice 
of 8 characters for indentation is "to clearly define where
a block of control starts and ends", which has nothing to do with alignment.

>> IMO -- it screws things up when viewed with any other tab size (and why
>> else have tabs in the first place?).
> 
> What do you mean by "other tab size'? See Linux kernel coding style,
> Chapter 1: Indentation:
> 
>   Tabs are 8 characters, and thus indentations are also 8
>   characters.
> 
> Tabs are 8 characters. Full stop.

Yes, 8 is the "official" tab size for the Linux and U-Boot projects, for 
purposes of line length limits, etc.

That doesn't mean we should go out of our way to screw up alignment when 
a different size is used, when it's so easy to avoid.  What's the point 
of using TAB as a crappy compression scheme, rather than giving it 
semantic meaning as the block indentation level?

-Scott

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-29 Thread Wolfgang Denk
In message <[EMAIL PROTECTED]> you wrote:
>
> Aligning with TABs (or at all, in initializer lists) is not a good thing,

It is mandatory per Coding Style requirements.

> IMO -- it screws things up when viewed with any other tab size (and why
> else have tabs in the first place?).

What do you mean by "other tab size'? See Linux kernel coding style,
Chapter 1: Indentation:

Tabs are 8 characters, and thus indentations are also 8
characters.

Tabs are 8 characters. Full stop.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED]
What was sliced bread the greatest thing since?

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-29 Thread Scott Wood
On Tue, Jul 29, 2008 at 09:52:12AM +0100, Mark Jackson wrote:
> I didn't want to use u-boot's "slient boot" options, since they're 
> hard-coded at compile time.  The only place I could think to this was to 
> modify the atmel_usart.c file as above.

It's not hard-coded at compile-time -- set GD_FLG_SILENT in gd->flags
from early board code depending on the state of the GPIO pin.  For
example, 8313erdb does this depending on whether it's booting or resuming
from suspend.

-Scott

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-29 Thread Mark Jackson
Haavard Skinnemoen wrote:
> Mark Jackson <[EMAIL PROTECTED]> wrote:
>> The MIMC200 board is based on Atmel's NGW100 dev kit,
>> but with an extra 8MByte FLASH and 128KByte FRAM.
> 
> Do you have a link with some more information about the board?

Not at this moment ... it's still under development, so to speak.

> I also think your mailer mangles whitespace. Thunderbird can apparently
> be fixed; please see the instructions here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt

I'll fix this.

>> board/atmel/mimc200/Makefile   |   40 +
>> board/atmel/mimc200/config.mk  |3 +
>> board/atmel/mimc200/mimc200.c  |  158 
>> board/atmel/mimc200/u-boot.lds |   73 +
> 
> Is this really an Atmel board? Note that the directory under "board" is
> supposed to indicate the _board_ vendor, not the chip vendor.

Urm ... no, I'll create a new vendor directory.

>> --- a/cpu/at32ap/at32ap700x/clk.c
>> +++ b/cpu/at32ap/at32ap700x/clk.c
>> @@ -65,4 +65,12 @@ void clk_init(void)
>> /* Use PLL0 as main clock */
>> sm_writel(PM_MCCTRL, SM_BIT(PLLSEL));
>> #endif
>> +
>> +#ifdef CONFIG_MIMC200
>> +// enable gclk outputs
>> +//AVR32_PM.gcctrl[0] = 0x0004; /* LVDS at 10MHz */
>> +sm_writel(PM_GCCTRL, 0x0004);
>> +//AVR32_PM.gcctrl[1] = 0x0216; /* Ethernet at 25MHz if PLL running */
>> +//sm_writel(PM_GCCTRL + 4, 0x0216);
>> +#endif
> 
> Please define a gclk_init() function in your board file and move this
> stuff there. That's what Hammerhead ended up doing.
> 
> The gclk_init() hook isn't in mainline yet. I'll create a "next" branch
> that you can base your work on in the mean time.

I've just moved it to my main board setup code.

>> +#ifndef CONFIG_MIMC200
>> gpio_select_periph_A(GPIO_PIN_PC18, 0);/* SPD*/
>> #endif
>> +#endif
>> }
>>
>> void gpio_enable_macb1(void)
>> @@ -129,8 +131,10 @@ void gpio_enable_macb1(void)
>> gpio_select_periph_B(GPIO_PIN_PC29, 0);/* RXD2*/
>> gpio_select_periph_B(GPIO_PIN_PC30, 0);/* RXD3*/
>> gpio_select_periph_B(GPIO_PIN_PC24, 0);/* RXCK*/
>> +#ifndef CONFIG_MIMC200
>> gpio_select_periph_B(GPIO_PIN_PD15, 0);/* SPD*/
>> #endif
>> +#endif
> 
> I'd prefer a more generic define here...or possibly some sort of
> parameter that can be passed from the board code. Let's leave that for
> later though -- this is fine for now.

Again, I've moved this to my board setup code.

>> +#ifdef CONFIG_MIMC200
>> +// setup Data Flash chip select (NCS2)
>> +hsmc3_writel(MODE2, 0x20121003);
>> +hsmc3_writel(CYCLE2, 0x000a0009);
>> +hsmc3_writel(PULSE2, 0x0a060806);
>> +hsmc3_writel(SETUP2, 0x00030102);
>> +
>> +// setup FRAM chip select (NCS3)
>> +hsmc3_writel(MODE3, 0x10120001);
>> +hsmc3_writel(CYCLE3, 0x001e001d);
>> +hsmc3_writel(PULSE3, 0x08040704);
>> +hsmc3_writel(SETUP3, 0x02050204);
>> +#endif
> 
> Hmm, ok, I guess you currently don't have much choice but put to those
> here. Let's make a mental note that this should be improved later.

I've actually removed this, since the extra chips contain no boot-related info 
/ code.

>> void serial_putc(char c)
>> {
>> +#if defined(CONFIG_MIMC200_DBGLINK)
>> +// only output serial data if DEBUG link connected
>> +// this is connected to PIOE_21
>> +if (gpio_get_value(GPIO_PIN_PE21) == 0)
>> +{
>> +#endif
> 
> As others have noted, this is pretty ugly. There must be a better way
> to do this...but I don't know exactly how. Moving this test to a
> separate function and providing a dummy stub for the case when
> CONFIG_MIMC200_DBGLINK is not set might be a good first step. Something like
> 



> 
> Alternatively, we could do some tricks involving weak functions here
> and move the actual test into the board code.

Is there any way I can tell u-boot (on-the-fly) to enable / disable any comms 
output ?

If so, I could just check the "debug" link in my boot code, and adjust the 
u-boot comms accordingly.

> Thanks for posting this, but please Cc me when posting new avr32 board
> patches. I think I missed you first submission.

No problems.  Thanks for the comments.

Mark

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-29 Thread Mark Jackson
Ben Warren wrote:
> C++ style comments are not allowed.  Please fix them all.
>
> 
>   
Okay.
>
> The prototype is:
>
> int board_eth_init(bd_t *);
>
> You'll need to return 0 if you can't get anything useful from
> macb_eth_initialize();
>
> 
>   
Okay.
>
> It's frowned upon to conditionally compile board-specific code in CPU
> files.  Can this be done in your board code?
>   
I'll give it a go !!
>   
>> diff --git a/cpu/at32ap/at32ap700x/gpio.c b/cpu/at32ap/at32ap700x/gpio.c
>> index 56ba2f9..7c6679d 100644
>> --- a/cpu/at32ap/at32ap700x/gpio.c
>> +++ b/cpu/at32ap/at32ap700x/gpio.c
>> @@ -104,8 +104,10 @@ void gpio_enable_macb0(void)
>>gpio_select_periph_A(GPIO_PIN_PC11, 0);/* RXD2*/
>>gpio_select_periph_A(GPIO_PIN_PC12, 0);/* RXD3*/
>>gpio_select_periph_A(GPIO_PIN_PC14, 0);/* RXCK*/
>> +#ifndef CONFIG_MIMC200
>>gpio_select_periph_A(GPIO_PIN_PC18, 0);/* SPD*/
>> #endif
>> +#endif
>> }
>>
>> void gpio_enable_macb1(void)
>> @@ -129,8 +131,10 @@ void gpio_enable_macb1(void)
>>gpio_select_periph_B(GPIO_PIN_PC29, 0);/* RXD2*/
>>gpio_select_periph_B(GPIO_PIN_PC30, 0);/* RXD3*/
>>gpio_select_periph_B(GPIO_PIN_PC24, 0);/* RXCK*/
>> +#ifndef CONFIG_MIMC200
>>gpio_select_periph_B(GPIO_PIN_PD15, 0);/* SPD*/
>> #endif
>> +#endif
>> }
>> #endif
>> 
Should I just provide my own *complete* alternatives to these functions ?
>> diff --git a/cpu/at32ap/cpu.c b/cpu/at32ap/cpu.c
>> index 0ba8361..8985b68 100644
>> --- a/cpu/at32ap/cpu.c
>> +++ b/cpu/at32ap/cpu.c
>> @@ -56,6 +56,20 @@ int cpu_init(void)
>>hsmc3_writel(PULSE0, 0x0b0a0906);
>>hsmc3_writel(SETUP0, 0x00010002);
>>
>> +#ifdef CONFIG_MIMC200
>> +// setup Data Flash chip select (NCS2)
>> +hsmc3_writel(MODE2, 0x20121003);
>> +hsmc3_writel(CYCLE2, 0x000a0009);
>> +hsmc3_writel(PULSE2, 0x0a060806);
>> +hsmc3_writel(SETUP2, 0x00030102);
>> +
>> +// setup FRAM chip select (NCS3)
>> +hsmc3_writel(MODE3, 0x10120001);
>> +hsmc3_writel(CYCLE3, 0x001e001d);
>> +hsmc3_writel(PULSE3, 0x08040704);
>> +hsmc3_writel(SETUP3, 0x02050204);
>> +#endif
>> +
>>clk_init();
>> 
Okay ... this can probably go in my board setup file.
>>/* Update the CPU speed according to the PLL configuration */
>> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
>> index f35b997..9e131c0 100644
>> --- a/drivers/serial/atmel_usart.c
>> +++ b/drivers/serial/atmel_usart.c
>> @@ -21,6 +21,9 @@
>> #include 
>> #include 
>> #include 
>> +#if defined(CONFIG_MIMC200_DBGLINK)
>> +#include 
>> +#endif
>>
>> 
> This is definitely uncool.  Don't pollute common code with your debug stuff.
>   
Right ... this one's got me stumped.  This *isn't* debug code.

In standard operation, the standard u-boot comms port will drive (e.g.) 
a printer, and we don't want any u-boot messages being printed out every 
time the unit reboots.

*But* we do want to be able to use this port for normal u-boot operation 
when units are returned for calibration / testing / repair / etc,

So we have an external link that is meant to disconnect all comms 
traffic from u-boot.

I didn't want to use u-boot's "slient boot" options, since they're 
hard-coded at compile time.  The only place I could think to this was to 
modify the atmel_usart.c file as above.

How else should I do it ?
> Looking forward to V3
>   
Me too !!

Mark

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-29 Thread Haavard Skinnemoen
Mark Jackson <[EMAIL PROTECTED]> wrote:
> The MIMC200 board is based on Atmel's NGW100 dev kit,
> but with an extra 8MByte FLASH and 128KByte FRAM.

Do you have a link with some more information about the board?

I also think your mailer mangles whitespace. Thunderbird can apparently
be fixed; please see the instructions here:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt

> board/atmel/mimc200/Makefile   |   40 +
> board/atmel/mimc200/config.mk  |3 +
> board/atmel/mimc200/mimc200.c  |  158 
> board/atmel/mimc200/u-boot.lds |   73 +

Is this really an Atmel board? Note that the directory under "board" is
supposed to indicate the _board_ vendor, not the chip vendor.

> --- a/cpu/at32ap/at32ap700x/clk.c
> +++ b/cpu/at32ap/at32ap700x/clk.c
> @@ -65,4 +65,12 @@ void clk_init(void)
> /* Use PLL0 as main clock */
> sm_writel(PM_MCCTRL, SM_BIT(PLLSEL));
> #endif
> +
> +#ifdef CONFIG_MIMC200
> +// enable gclk outputs
> +//AVR32_PM.gcctrl[0] = 0x0004; /* LVDS at 10MHz */
> +sm_writel(PM_GCCTRL, 0x0004);
> +//AVR32_PM.gcctrl[1] = 0x0216; /* Ethernet at 25MHz if PLL running */
> +//sm_writel(PM_GCCTRL + 4, 0x0216);
> +#endif

Please define a gclk_init() function in your board file and move this
stuff there. That's what Hammerhead ended up doing.

The gclk_init() hook isn't in mainline yet. I'll create a "next" branch
that you can base your work on in the mean time.

> +#ifndef CONFIG_MIMC200
> gpio_select_periph_A(GPIO_PIN_PC18, 0);/* SPD*/
> #endif
> +#endif
> }
> 
> void gpio_enable_macb1(void)
> @@ -129,8 +131,10 @@ void gpio_enable_macb1(void)
> gpio_select_periph_B(GPIO_PIN_PC29, 0);/* RXD2*/
> gpio_select_periph_B(GPIO_PIN_PC30, 0);/* RXD3*/
> gpio_select_periph_B(GPIO_PIN_PC24, 0);/* RXCK*/
> +#ifndef CONFIG_MIMC200
> gpio_select_periph_B(GPIO_PIN_PD15, 0);/* SPD*/
> #endif
> +#endif

I'd prefer a more generic define here...or possibly some sort of
parameter that can be passed from the board code. Let's leave that for
later though -- this is fine for now.

> +#ifdef CONFIG_MIMC200
> +// setup Data Flash chip select (NCS2)
> +hsmc3_writel(MODE2, 0x20121003);
> +hsmc3_writel(CYCLE2, 0x000a0009);
> +hsmc3_writel(PULSE2, 0x0a060806);
> +hsmc3_writel(SETUP2, 0x00030102);
> +
> +// setup FRAM chip select (NCS3)
> +hsmc3_writel(MODE3, 0x10120001);
> +hsmc3_writel(CYCLE3, 0x001e001d);
> +hsmc3_writel(PULSE3, 0x08040704);
> +hsmc3_writel(SETUP3, 0x02050204);
> +#endif

Hmm, ok, I guess you currently don't have much choice but put to those
here. Let's make a mental note that this should be improved later.

> void serial_putc(char c)
> {
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +// only output serial data if DEBUG link connected
> +// this is connected to PIOE_21
> +if (gpio_get_value(GPIO_PIN_PE21) == 0)
> +{
> +#endif

As others have noted, this is pretty ugly. There must be a better way
to do this...but I don't know exactly how. Moving this test to a
separate function and providing a dummy stub for the case when
CONFIG_MIMC200_DBGLINK is not set might be a good first step. Something like

#if defined(CONFIG_MIMC200_DBGLINK)
static int usart_is_disabled(void)
{
return gpio_get_value(GPIO_PIN_PE21) != 0;
}
#else
static int usart_is_disabled(void)
{
return 0;
}
#endif

then you can simply do

void serial_putc(char c)
{
if (usart_is_disabled())
return;

/* do the usual stuff here */
}

Alternatively, we could do some tricks involving weak functions here
and move the actual test into the board code.

Thanks for posting this, but please Cc me when posting new avr32 board
patches. I think I missed you first submission.

Haavard

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-28 Thread Scott Wood
On Mon, Jul 28, 2008 at 10:03:43PM +0200, Jean-Christophe PLAGNIOL-VILLARD 
wrote:
> 
>  +
> > +static const struct sdram_config sdram_config = {
> > +.data_bits= SDRAM_DATA_16BIT,
> > +.row_bits= 13,
> > +.col_bits= 9,
> > +.bank_bits= 2,
> > +.cas= 3,
> > +.twr= 2,
> > +.trc= 6,
> > +.trp= 2,
> > +.trcd= 2,
> > +.tras= 6,
> > +.txsr= 6,
> 
> please indent with tab for all files

That's not indenting, that's alignment -- or would be, if it were
actually aligned. :-P

Aligning with TABs (or at all, in initializer lists) is not a good thing,
IMO -- it screws things up when viewed with any other tab size (and why
else have tabs in the first place?).

> > +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> > +{
> > +return bus == 0 && cs == 0;
> please replace by 
>   return (bus == 0) && (cs == 0);

Why?  This isn't typical coding style as far as I've seen.

-Scott

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-28 Thread Jean-Christophe PLAGNIOL-VILLARD
 +
> +static const struct sdram_config sdram_config = {
> +.data_bits= SDRAM_DATA_16BIT,
> +.row_bits= 13,
> +.col_bits= 9,
> +.bank_bits= 2,
> +.cas= 3,
> +.twr= 2,
> +.trc= 6,
> +.trp= 2,
> +.trcd= 2,
> +.tras= 6,
> +.txsr= 6,

please indent with tab for all files
> +/* 15.6 us */
> +.refresh_period= (156 * (SDRAMC_BUS_HZ / 1000)) / 1,
> +};
> +
> +int board_early_init_f(void)
> +{
> +/* Enable SDRAM in the EBI mux */
> +hmatrix_slave_write(EBI, SFR, HMATRIX_BIT(EBI_SDRAM_ENABLE));
> +
> +gpio_enable_ebi();
> +gpio_enable_usart1();
> +
> +// enable higher address lines for larger flash devices
please never use c++ comment style
> +gpio_select_periph_A(GPIO_PIN_PE16, 0);/* ADDR23 */
> +gpio_select_periph_A(GPIO_PIN_PE17, 0);/* ADDR24 */
> +gpio_select_periph_A(GPIO_PIN_PE18, 0);/* ADDR25 */
> +
> +// enable data flash chip select
> +gpio_select_periph_A(GPIO_PIN_PE25, 0);/* NCS2 */
 +
> +// reset phys
> +gpio_select_pio(GPIO_PIN_PE24, 0);
> +gpio_set_value(GPIO_PIN_PC18, 1);/* PHY RESET*/
> +gpio_select_pio(GPIO_PIN_PC18, GPIOF_OUTPUT);
> +
> +// GCLK0 - 10MHz clock
> +gpio_select_periph_A(GPIO_PIN_PA30,0);
please replace with tab or a simple space.
> +// GCLK1 - 25MHz clock
> +//gpio_select_periph_A(GPIO_PIN_PA31,0);
please remove dead code
> +
> +udelay(5000);
> +
> +// release phys reset
> +gpio_set_value(GPIO_PIN_PC18, 0);/* PHY RESET (Release)*/
> +
> +#if defined(CONFIG_MACB)
> +gpio_enable_macb0();
> +gpio_enable_macb1();
> +#endif
> +#if defined(CONFIG_MMC)
> +gpio_enable_mmci();
> +#endif
> +
> +return 0;
> +}
> +
> +phys_size_t initdram(int board_type)
> +{
> +unsigned long expected_size;
> +unsigned long actual_size;
> +void *sdram_base;
> +
> +sdram_base = map_physmem(EBI_SDRAM_BASE, EBI_SDRAM_SIZE, MAP_NOCACHE);
> +
> +expected_size = sdram_init(sdram_base, &sdram_config);
> +actual_size = get_ram_size(sdram_base, expected_size);
> +
> +unmap_physmem(sdram_base, EBI_SDRAM_SIZE);
> +
> +if (expected_size != actual_size)
> +printf("Warning: Only %lu of %lu MiB SDRAM is working\n",
> +actual_size >> 20, expected_size >> 20);
> +
> +return actual_size;
> +}
> +
> +void board_init_info(void)
> +{
> +gd->bd->bi_phy_id[0] = 0x01;
> +gd->bd->bi_phy_id[1] = 0x03;
> +}
> +
> +/* SPI chip select control */
> +#ifdef CONFIG_ATMEL_SPI
> +#include 
> +
> +int spi_cs_is_valid(unsigned int bus, unsigned int cs)
> +{
> +return bus == 0 && cs == 0;
please replace by 
return (bus == 0) && (cs == 0);
> +}
> +
> +void spi_cs_activate(struct spi_slave *slave)
> +{
> +}
> +
> +void spi_cs_deactivate(struct spi_slave *slave)
> +{
> +}
> +#endif /* CONFIG_ATMEL_SPI */
> +
> +#ifdef CONFIG_CMD_NET
> +extern int macb_eth_initialize(int id, void *regs, unsigned int phy_addr);
> +
> +void board_eth_init(bd_t *bi)
> +{
> +macb_eth_initialize(0, (void *)MACB0_BASE, bi->bi_phy_id[0]);
> +macb_eth_initialize(1, (void *)MACB1_BASE, bi->bi_phy_id[1]);
> +}
> +#endif

> --- a/drivers/serial/atmel_usart.c
> +++ b/drivers/serial/atmel_usart.c
> @@ -21,6 +21,9 @@
> #include 
> #include 
> #include 
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +#include 
> +#endif
> 
> #if defined(CONFIG_USART0)
> # define USART_ID0
> @@ -73,27 +76,67 @@ int serial_init(void)
> 
> void serial_putc(char c)
> {
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +// only output serial data if DEBUG link connected
> +// this is connected to PIOE_21
> +if (gpio_get_value(GPIO_PIN_PE21) == 0)
> +{
> +#endif
> +
> if (c == '\n')
> serial_putc('\r');
> 
> while (!(usart3_readl(CSR) & USART3_BIT(TXRDY))) ;
> usart3_writel(THR, c);
> +
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +}
> +#endif
> }
> 
> void serial_puts(const char *s)
> {
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +// only output serial data if DEBUG link connected
> +// this is connected to PIOE_21
> +if (gpio_get_value(GPIO_PIN_PE21) == 0)
> +{
> +#endif
> +
> while (*s)
> serial_putc(*s++);
> +
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +}
> +#endif
> }
> 
> int serial_getc(void)
> {
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +// only get serial data if DEBUG link connected
> +// this is connected to PIOE_21
> +if (gpio_get_value(GPIO_PIN_PE21) == 1)
> +{
> +return 0;
> +}
> +#endif
> +
> while (!(usart3_readl(CSR) & USART3_BIT(RXRDY))) ;
> return usart3_readl(RHR);
> }
> 
> int serial_tstc(void)
> {
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +// only get serial data if DEBUG link connected
> +// this is connected to PIOE_21
> +if (gpio_get_value(GPIO_PIN_PE21) == 1)
> +{
> +return (1 == 0);
why not return 0?

Re: [U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-28 Thread Ben Warren
Hi Mark,

On Mon, Jul 28, 2008 at 12:32 PM, Mark Jackson <[EMAIL PROTECTED]> wrote:
> The MIMC200 board is based on Atmel's NGW100 dev kit,
> but with an extra 8MByte FLASH and 128KByte FRAM.
>

No need to put the stuff about board_eth_init() in the subject.  Just
label this as [PATCH V2].



> +// enable higher address lines for larger flash devices
C++ style comments are not allowed.  Please fix them all.



> +#ifdef CONFIG_CMD_NET
> +extern int macb_eth_initialize(int id, void *regs, unsigned int phy_addr);
> +
> +void board_eth_init(bd_t *bi)
> +{
> +macb_eth_initialize(0, (void *)MACB0_BASE, bi->bi_phy_id[0]);
> +macb_eth_initialize(1, (void *)MACB1_BASE, bi->bi_phy_id[1]);
> +}

The prototype is:

int board_eth_init(bd_t *);

You'll need to return 0 if you can't get anything useful from
macb_eth_initialize();



> diff --git a/cpu/at32ap/at32ap700x/clk.c b/cpu/at32ap/at32ap700x/clk.c
> index b3aa034..73b859e 100644
> --- a/cpu/at32ap/at32ap700x/clk.c
> +++ b/cpu/at32ap/at32ap700x/clk.c
> @@ -65,4 +65,12 @@ void clk_init(void)
>/* Use PLL0 as main clock */
>sm_writel(PM_MCCTRL, SM_BIT(PLLSEL));
> #endif
> +
> +#ifdef CONFIG_MIMC200
> +// enable gclk outputs
> +//AVR32_PM.gcctrl[0] = 0x0004; /* LVDS at 10MHz */
> +sm_writel(PM_GCCTRL, 0x0004);
> +//AVR32_PM.gcctrl[1] = 0x0216; /* Ethernet at 25MHz if PLL running */
> +//sm_writel(PM_GCCTRL + 4, 0x0216);
> +#endif
> }

It's frowned upon to conditionally compile board-specific code in CPU
files.  Can this be done in your board code?

> diff --git a/cpu/at32ap/at32ap700x/gpio.c b/cpu/at32ap/at32ap700x/gpio.c
> index 56ba2f9..7c6679d 100644
> --- a/cpu/at32ap/at32ap700x/gpio.c
> +++ b/cpu/at32ap/at32ap700x/gpio.c
> @@ -104,8 +104,10 @@ void gpio_enable_macb0(void)
>gpio_select_periph_A(GPIO_PIN_PC11, 0);/* RXD2*/
>gpio_select_periph_A(GPIO_PIN_PC12, 0);/* RXD3*/
>gpio_select_periph_A(GPIO_PIN_PC14, 0);/* RXCK*/
> +#ifndef CONFIG_MIMC200
>gpio_select_periph_A(GPIO_PIN_PC18, 0);/* SPD*/
> #endif
> +#endif
> }
>
> void gpio_enable_macb1(void)
> @@ -129,8 +131,10 @@ void gpio_enable_macb1(void)
>gpio_select_periph_B(GPIO_PIN_PC29, 0);/* RXD2*/
>gpio_select_periph_B(GPIO_PIN_PC30, 0);/* RXD3*/
>gpio_select_periph_B(GPIO_PIN_PC24, 0);/* RXCK*/
> +#ifndef CONFIG_MIMC200
>gpio_select_periph_B(GPIO_PIN_PD15, 0);/* SPD*/
> #endif
> +#endif
> }
> #endif
>
> diff --git a/cpu/at32ap/cpu.c b/cpu/at32ap/cpu.c
> index 0ba8361..8985b68 100644
> --- a/cpu/at32ap/cpu.c
> +++ b/cpu/at32ap/cpu.c
> @@ -56,6 +56,20 @@ int cpu_init(void)
>hsmc3_writel(PULSE0, 0x0b0a0906);
>hsmc3_writel(SETUP0, 0x00010002);
>
> +#ifdef CONFIG_MIMC200
> +// setup Data Flash chip select (NCS2)
> +hsmc3_writel(MODE2, 0x20121003);
> +hsmc3_writel(CYCLE2, 0x000a0009);
> +hsmc3_writel(PULSE2, 0x0a060806);
> +hsmc3_writel(SETUP2, 0x00030102);
> +
> +// setup FRAM chip select (NCS3)
> +hsmc3_writel(MODE3, 0x10120001);
> +hsmc3_writel(CYCLE3, 0x001e001d);
> +hsmc3_writel(PULSE3, 0x08040704);
> +hsmc3_writel(SETUP3, 0x02050204);
> +#endif
> +
>clk_init();
>
>/* Update the CPU speed according to the PLL configuration */
> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
> index f35b997..9e131c0 100644
> --- a/drivers/serial/atmel_usart.c
> +++ b/drivers/serial/atmel_usart.c
> @@ -21,6 +21,9 @@
> #include 
> #include 
> #include 
> +#if defined(CONFIG_MIMC200_DBGLINK)
> +#include 
> +#endif
>
This is definitely uncool.  Don't pollute common code with your debug stuff.

Looking forward to V3

regards,
Ben

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
___
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users


[U-Boot-Users] [PATCH] Add MIMC200 board - now uses board_eth_init()

2008-07-28 Thread Mark Jackson
The MIMC200 board is based on Atmel's NGW100 dev kit,
but with an extra 8MByte FLASH and 128KByte FRAM.

---
CREDITS|4 +
MAINTAINERS|4 +
MAKEALL|1 +
Makefile   |3 +
board/atmel/mimc200/Makefile   |   40 +
board/atmel/mimc200/config.mk  |3 +
board/atmel/mimc200/mimc200.c  |  158 
board/atmel/mimc200/u-boot.lds |   73 +
cpu/at32ap/at32ap700x/clk.c|8 ++
cpu/at32ap/at32ap700x/gpio.c   |4 +
cpu/at32ap/cpu.c   |   14 +++
drivers/serial/atmel_usart.c   |   43 ++
include/configs/mimc200.h  |  173 
13 files changed, 528 insertions(+), 0 deletions(-)

diff --git a/CREDITS b/CREDITS
index 2b0dab7..5010c78 100644
--- a/CREDITS
+++ b/CREDITS
@@ -217,6 +217,10 @@ H: Rich Ireland
E: [EMAIL PROTECTED]
D: FPGA device configuration driver

+H: Mark Jackson
+E: [EMAIL PROTECTED]
+D: Port to MIMC200 board
+
N: Gary Jennejohn
E: [EMAIL PROTECTED], [EMAIL PROTECTED]
D: Support for Samsung ARM920T S3C2400X, ARM920T "TRAB"
diff --git a/MAINTAINERS b/MAINTAINERS
index cbe5c47..d4f6639 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -709,6 +709,10 @@ Haavard Skinnemoen <[EMAIL PROTECTED]>
ATSTK1006AT32AP7000
ATNGW100AT32AP7000

+Mark Jackson <[EMAIL PROTECTED]>
+
+MIMC200AT32AP7000
+
#
# SuperH Systems:#
##
diff --git a/MAKEALL b/MAKEALL
index c1a9c60..cfd376c 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -715,6 +715,7 @@ LIST_avr32="\
atstk1004\
atstk1006\
atngw100\
+mimc200\
"

#
diff --git a/Makefile b/Makefile
index 369bbd7..d85867c 100644
--- a/Makefile
+++ b/Makefile
@@ -2914,6 +2914,9 @@ atstk1004_config:unconfig
atstk1006_config:unconfig
@$(MKCONFIG) $(@:_config=) avr32 at32ap atstk1000 atmel at32ap700x

+mimc200_config:unconfig
+@$(MKCONFIG) $(@:_config=) avr32 at32ap mimc200 atmel at32ap700x
+
#
# SH3 (SuperH)
#
diff --git a/board/atmel/mimc200/Makefile b/board/atmel/mimc200/Makefile
new file mode 100644
index 000..9f3849f
--- /dev/null
+++ b/board/atmel/mimc200/Makefile
@@ -0,0 +1,40 @@
+#
+# Copyright (C) 2005-2006 Atmel Corporation
+#
+# See file CREDITS for list of people who contributed to this project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+
+include $(TOPDIR)/config.mk
+
+LIB:= $(obj)lib$(BOARD).a
+
+COBJS:= $(BOARD).o
+
+SRCS:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS:= $(addprefix $(obj),$(SOBJS) $(COBJS))
+
+$(LIB): $(obj).depend $(OBJS)
+$(AR) $(ARFLAGS) $@ $(OBJS)
+
+#
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#
diff --git a/board/atmel/mimc200/config.mk b/board/atmel/mimc200/config.mk
new file mode 100644
index 000..9a794e5
--- /dev/null
+++ b/board/atmel/mimc200/config.mk
@@ -0,0 +1,3 @@
+TEXT_BASE= 0x
+PLATFORM_RELFLAGS+= -ffunction-sections -fdata-sections
+PLATFORM_LDFLAGS+= --gc-sections
diff --git a/board/atmel/mimc200/mimc200.c b/board/atmel/mimc200/mimc200.c
new file mode 100644
index 000..3f89059
--- /dev/null
+++ b/board/atmel/mimc200/mimc200.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright (C) 2006 Atmel Corporation
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PAR