Dear Eric Benard,
In message <1247947830-22602-1-git-send-email-e...@eukrea.com> you wrote:
> CPUAT91 is built around Atmel's AT91RM9200 and has up to 16MB of NOR
> flash, up to 128MB of SDRAM, and includes a Micrel KS8721 PHY in RMII
> mode.
>
> Signed-off-by: Eric Benard <e...@eukrea.com>
> ---
Your subject line reads:
[PATCH 1/1] Add support for Eukrea CPUAT91 SBC
Note that the "1/1" part is bogus as you submit just a single patch -
please omit it.
Second, it seems that this is a resubmit of an earlier patch. You are
suppposed to mark it as such, for example by using something like
"[PATCH v2]" in the subject, and you are also supposed to include at
least a short summary what has been changed compared to your previous
version (add this here, i. e. below the "---" line).
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 575a7ec..da64571 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1,4 +1,4 @@
> -#########################################################################
> +9#########################################################################
^^^^^
Here you inserted garbage. Please fix.
> --- /dev/null
> +++ b/board/eukrea/cpuat91/cpuat91.c
> +int board_init(void)
> +{
> + /* Enable Ctrlc */
> + console_init_f();
> +
> + /* memory and cpu-speed are setup before relocation */
> + /* so we do _nothing_ here */
Incorrect multi-line comment style.
> + /* arch number of CPUAT91-Board */
> + gd->bd->bi_arch_number = MACH_TYPE_CPUAT91;
> + /* adress of boot parameters */
> + gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
> +
> + return 0;
> +}
> +
> +int dram_init(void)
> +{
> + gd->bd->bi_dram[0].start = PHYS_SDRAM;
> + gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
> + return 0;
> +}
> +
> +#if defined(CONFIG_DRIVER_ETHER)
> +#if defined(CONFIG_CMD_NET)
> +
> +/*
> + * Name:
> + * at91rm9200_GetPhyInterface
> + * Description:
> + * Initialise the interface functions to the PHY
> + * Arguments:
> + * None
> + * Return value:
> + * None
> + */
> +void at91rm9200_GetPhyInterface(AT91PS_PhyOps p_phyops)
> +{
> + p_phyops->Init = ks8721_InitPhy;
> + p_phyops->IsPhyConnected = ks8721_IsPhyConnected;
> + p_phyops->GetLinkSpeed = ks8721_GetLinkSpeed;
> + p_phyops->AutoNegotiate = ks8721_AutoNegotiate;
> +}
> +
> +#endif /* CONFIG_CMD_NET */
> +#endif /* CONFIG_DRIVER_ETHER */
> diff --git a/cpu/arm920t/at91rm9200/Makefile b/cpu/arm920t/at91rm9200/Makefile
> index 73aeeac..114d8ad 100644
> --- a/cpu/arm920t/at91rm9200/Makefile
> +++ b/cpu/arm920t/at91rm9200/Makefile
> @@ -31,14 +31,15 @@ COBJS += bcm5221.o
> COBJS += dm9161.o
> COBJS += ether.o
> COBJS += i2c.o
> +COBJS-$(CONFIG_KS8721_PHY) += ks8721.o
> COBJS += lxt972.o
> COBJS += reset.o
> COBJS += spi.o
> COBJS += timer.o
> COBJS += usb.o
>
> -SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c)
> -OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))
> +SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) $(COBJS-y:.o=.c)
> +OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS) $(COBJS-y))
>
> all: $(obj).depend $(LIB)
>
> diff --git a/cpu/arm920t/at91rm9200/ks8721.c b/cpu/arm920t/at91rm9200/ks8721.c
> new file mode 100644
> index 0000000..703e58c
> --- /dev/null
> +++ b/cpu/arm920t/at91rm9200/ks8721.c
...
> + * Return value:
> + * TRUE - if id read successfully
> + * FALSE- if error
> + */
Please use 0 and 1 (or -1) as return values. We don't use TRUE and
FALSE here.
...
> + if (stat1 & KS8721_10BASE_T_HD) {
> + /*set MII for 10BaseT and Half Duplex */
> + printf("10BT HD\n");
> + p_mac->EMAC_CFG &= ~(AT91C_EMAC_SPD | AT91C_EMAC_FD);
> + return TRUE;
> + }
> + return FALSE;
> +}
> +
> +
Please use only a single blank line here (and in similar places).
> +/*
> + * Name:
> + * ks8721_InitPhy
> + * Description:
> + * MAC starts checking its link by using parallel detection and
> + * Autonegotiation and the same is set in the MAC configuration registers
> + * Arguments:
> + * p_mac - pointer to struct AT91S_EMAC
> + * Return value:
> + * TRUE - if link status set succesfully
> + * FALSE - if link status not set
> + */
> +UCHAR ks8721_InitPhy(AT91PS_EMAC p_mac)
> +{
> + UCHAR ret = TRUE;
Please get rid of UCHAR an the like.
> + unsigned short IntValue;
> +
> + at91rm9200_EmacEnableMDIO(p_mac);
Please do not use CamelCase identifiers.
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: w...@denx.de
Our business is run on trust. We trust you will pay in advance.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
