On Friday, October 23, 2015 at 09:26:53 AM, Stefan Roese wrote:
> The SR1500
Does SR mean Stefan Roese ? :-)
Anyway, shouldn't you place this device under board/vendorname/boardname
instead of plain board/boardname/ ?
And one more thing, would it be possible for you to do a short README on
adding a new board? That'd be real cool. Obviously, it's not something I
demand or that'd block this patch series, it'd be nice though.
> board is a CycloneV based board, similar to the EBV
> SoCrates, equipped with the following devices:
>
> - SPI NOR
> - eMMC
> - Ethernet
>
> Signed-off-by: Stefan Roese <s...@denx.de>
> Cc: Marek Vasut <ma...@denx.de>
> Cc: Pavel Machek <pa...@denx.de>
> Cc: Dinh Nguyen <dingu...@opensource.altera.com>
[...]
> +int board_early_init_f(void)
> +{
> + int ret;
> +
> + /* Reset the Marvell PHY 88E1510 */
> + ret = gpio_request(63, "PHY reset");
> + if (ret)
> + return ret;
> +
> + gpio_direction_output(63, 0);
> + mdelay(20);
> + gpio_set_value(63, 1);
Does the PHY come out of reset immediatelly after you deassert the nRESET
GPIO or not ? You might want to add a small delay here to bullet-proof the
code a bit more.
> + return 0;
> +}
> +
> +#define CONFIG_SYS_IDT_CLK_ADDR 0x6a
> +
> +static int do_clksave(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[]) +{
> + u8 buf[1];
> +
> + buf[0] = 0x01;
> + i2c_write(CONFIG_SYS_IDT_CLK_ADDR, 0, 0, buf, 1);
> +
> + return 0;
> +}
> +
> +U_BOOT_CMD(clksave, 1, 0, do_clksave,
> + "IDT 5V49EE702 Progsave command", "");
I am not convinced I should let this slide. Wouldn't it be better to just
have an environment script which sends 0x1 to this IDT Versaclock using the
i2c command ?
> +#define NET_DEV_NAME "ethernet@ff702000"
> +#define MII_MARVELL_PHY_PAGE 22
> +#define PHY_DIAG_START (1 << 15)
> +#define PHY_DIAG_BUSY (1 << 11)
> +
> +static char str[16];
Please move this static var into do_phytest() and pass it into pair_state()
as an argument.
> +static char *pair_state(int val)
> +{
> + switch (val) {
> + case 0x00:
> + strcpy(str, "Invalid");
> + break;
> + case 0x01:
> + strcpy(str, "Pair Ok");
> + break;
> + case 0x02:
> + strcpy(str, "Pair Open");
> + break;
> + case 0x03:
> + strcpy(str, "Same Pair Short");
> + break;
> + case 0x04:
> + strcpy(str, "Cross Pair Short");
Do I count correctly that you do strcpy() here on a string which is 16 byte
long + one trailing '\0' (total 17 bytes) and you strcpy() it into 16 byte
long buffer ? Well that's not good, this will overwrite one byte past the
$str buffer with '\0' :-)
> + break;
> + case 0x09:
> + strcpy(str, "Pair Busy");
> + break;
> + default:
> + strcpy(str, "Reserved");
> + break;
> + };
> +
> + return str;
> +}
> +
> +static int do_phytest(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[]) +{
> + char devname[] = NET_DEV_NAME;
const char * here ?
> + int addr = 0;
Looks like unsigned value to me, please make it so.
> + u16 data;
> + u16 status;
> + u16 oldpage;
> + int i;
> +
> + /* Save current page register */
> + miiphy_read(devname, addr, MII_MARVELL_PHY_PAGE, &oldpage);
> +
> + /*
> + * Run cable disgnostics
> + */
> + printf("Running cable diagnostic test...");
> + miiphy_write(devname, addr, MII_MARVELL_PHY_PAGE, 7);
> + miiphy_write(devname, addr, 21, PHY_DIAG_START);
> + miiphy_read(devname, addr, 21, &data);
> + while ((data & PHY_DIAG_BUSY) == PHY_DIAG_BUSY) {
> + miiphy_read(devname, addr, 21, &data);
> + mdelay(1);
Unbounded loop, do I need to say more ? ;-)
> + }
> + printf("done!\n");
[...]
> +/* Booting Linux */
> +#define CONFIG_BOOTDELAY 3
> +#define CONFIG_BOOTFILE "uImage"
> +#define CONFIG_BOOTARGS "console=ttyS0"
__stringify(CONFIG_BAUDRATE)
> +#ifdef CONFIG_SOCFPGA_VIRTUAL_TARGET
Do you really need socfpga_vt on your quite certainly physical hardware ?
> +#define CONFIG_BOOTCOMMAND "run ramboot"
> +#else
> +#define CONFIG_BOOTCOMMAND "run mmcload; run mmcboot"
> +#endif
> +#define CONFIG_LOADADDR 0x8000
> +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
> +#define CONFIG_SYS_CONSOLE_INFO_QUIET /* don't print console @
> startup
*/
> +
> +/* Ethernet on SoC (EMAC) */
> +#if defined(CONFIG_CMD_NET)
> +#define CONFIG_EMAC_BASE SOCFPGA_EMAC1_ADDRESS
This EMAC address is certainly not needed now, it should come from OF.
> +#define CONFIG_PHY_INTERFACE_MODE PHY_INTERFACE_MODE_RGMII
> +/* The PHY is autodetected, so no MII PHY address is needed here */
> +#define CONFIG_PHY_MARVELL
> +#define PHY_ANEG_TIMEOUT 8000
> +#endif
> +
> +/* Extra Environment */
> +#define CONFIG_HOSTNAME sr1500
> +
> +#define CONFIG_EXTRA_ENV_SETTINGS \
> + "verify=n\0" \
> + "loadaddr= " __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> + "ramboot=setenv bootargs " CONFIG_BOOTARGS ";" \
> + "bootm ${loadaddr} - ${fdt_addr}\0" \
> + "bootimage=zImage\0" \
> + "fdt_addr=100\0" \
> + "fdtimage=socfpga.dtb\0" \
> + "fsloadcmd=ext2load\0" \
> + "bootm ${loadaddr} - ${fdt_addr}\0" \
> + "mmcroot=/dev/mmcblk0p2\0" \
> + "mmcboot=setenv bootargs " CONFIG_BOOTARGS \
> + " root=${mmcroot} rw rootwait;" \
> + "bootz ${loadaddr} - ${fdt_addr}\0" \
> + "mmcload=mmc rescan;" \
> + "load mmc 0:1 ${loadaddr} ${bootimage};" \
> + "load mmc 0:1 ${fdt_addr} ${fdtimage}\0" \
> + "qspiroot=/dev/mtdblock0\0" \
> + "qspirootfstype=jffs2\0" \
> + "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
> + " root=${qspiroot} rw rootfstype=${qspirootfstype};"\
> + "bootm ${loadaddr} - ${fdt_addr}\0"
> +
> +/* Environment */
> +#define CONFIG_ENV_IS_IN_SPI_FLASH
> +
> +/* Enable SPI NOR flash reset, needed for SPI booting */
> +#define CONFIG_SPI_N25Q256A_RESET
> +
> +/* Environment setting for SPI flash */
> +#define CONFIG_SYS_REDUNDAND_ENVIRONMENT
> +#define CONFIG_ENV_SECT_SIZE (64 * 1024)
> +#define CONFIG_ENV_SIZE (16 * 1024)
> +#define CONFIG_ENV_OFFSET (0x00040000)
Parenthesis not needed.
> +#define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET +
> CONFIG_ENV_SECT_SIZE) +#define CONFIG_ENV_SPI_BUS 0
> +#define CONFIG_ENV_SPI_CS 0
> +#define CONFIG_ENV_SPI_MODE SPI_MODE_3
> +#define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED
> +
> +/* U-Boot payload is stored at offset 0x60000 */
> +#define CONFIG_SYS_SPI_U_BOOT_OFFS 0x60000
> +
> +/*
> + * Bootcounter
> + */
> +#define CONFIG_BOOTCOUNT_LIMIT
> +/* last 2 lwords in OCRAM */
> +#define CONFIG_SYS_BOOTCOUNT_ADDR 0xfffffff8
> +#define CONFIG_SYS_BOOTCOUNT_BE
It might be better to use some scratch registers for this, no ?
Especially since you can get SRAM corruption if you fiddle with
SRAM ECC configuration.
> +/* The rest of the configuration is shared */
> +#include <configs/socfpga_common.h>
> +
> +#endif /* __CONFIG_SOCFPGA_SR1500_H__ */
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
