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 sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Pavel Machek pavel@denx.de Cc: Dinh Nguyen dinguyen@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__ */