[U-Boot] [PATCH u-boot-marvell v3 01/10] board: turris_mox: Cosmetic restructurization

[U-Boot] [PATCH u-boot-marvell v3 01/10] board: turris_mox: Cosmetic restructurization

[Marek Behún]

Restructure the board initialization source.
Remove the module_topology environment variable since it won't be
needed.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 board/CZ.NIC/turris_mox/turris_mox.c | 136 ++++++++++++++++++---------
 1 file changed, 89 insertions(+), 47 deletions(-)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index c4622a49c2..415c462493 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -135,17 +135,15 @@ int board_init(void)
 	return 0;
 }
 
-int last_stage_init(void)
+static int mox_do_spi(u8 *in, u8 *out, size_t size)
 {
 	struct spi_slave *slave;
 	struct udevice *dev;
-	u8 din[10], dout[10];
-	int ret, i;
-	size_t len = 0;
-	char module_topology[128];
+	int ret;
 
-	ret = spi_get_bus_and_cs(0, 1, 20000000, SPI_CPHA, "spi_generic_drv",
-				 "mox-modules at 1", &dev, &slave);
+	ret = spi_get_bus_and_cs(0, 1, 1000000, SPI_CPHA | SPI_CPOL,
+				 "spi_generic_drv", "moxtet at 1", &dev,
+				 &slave);
 	if (ret)
 		goto fail;
 
@@ -153,57 +151,101 @@ int last_stage_init(void)
 	if (ret)
 		goto fail_free;
 
-	memset(din, 0, 10);
-	memset(dout, 0, 10);
+	ret = spi_xfer(slave, size * 8, out, in, SPI_XFER_ONCE);
+
+	spi_release_bus(slave);
+fail_free:
+	spi_free_slave(slave);
+fail:
+	return ret;
+}
+
+static int mox_get_topology(const u8 **ptopology, int *psize, int *pis_sd)
+{
+	static int is_sd;
+	static u8 topology[MAX_MOX_MODULES - 1];
+	static int size;
+	u8 din[MAX_MOX_MODULES], dout[MAX_MOX_MODULES];
+	int ret, i;
 
-	ret = spi_xfer(slave, 80, dout, din, SPI_XFER_ONCE);
+	if (size) {
+		if (ptopology)
+			*ptopology = topology;
+		if (psize)
+			*psize = size;
+		if (pis_sd)
+			*pis_sd = is_sd;
+		return 0;
+	}
+
+	memset(din, 0, MAX_MOX_MODULES);
+	memset(dout, 0, MAX_MOX_MODULES);
+
+	ret = mox_do_spi(din, dout, MAX_MOX_MODULES);
 	if (ret)
-		goto fail_release;
+		return ret;
+
+	if (din[0] == 0x10)
+		is_sd = 1;
+	else if (din[0] == 0x00)
+		is_sd = 0;
+	else
+		return -ENODEV;
+
+	for (i = 1; i < MAX_MOX_MODULES && din[i] != 0xff; ++i)
+		topology[i - 1] = din[i] & 0xf;
+	size = i - 1;
+
+	if (ptopology)
+		*ptopology = topology;
+	if (psize)
+		*psize = size;
+	if (pis_sd)
+		*pis_sd = is_sd;
+
+	return 0;
+}
 
-	if (din[0] != 0x00 && din[0] != 0xff)
-		goto fail_release;
+int last_stage_init(void)
+{
+	int ret, i;
+	const u8 *topology;
+	int module_count, is_sd;
+
+	ret = mox_get_topology(&topology, &module_count, &is_sd);
+	if (ret) {
+		printf("Cannot read module topology!\n");
+		return 0;
+	}
 
+	printf("Found Turris Mox %s version\n", is_sd ? "SD" : "eMMC");
 	printf("Module Topology:\n");
-	for (i = 1; i < 10 && din[i] != 0xff; ++i) {
-		u8 mid = din[i] & 0xf;
-		size_t mlen;
-		const char *mname = "";
-
-		switch (mid) {
-		case 0x1:
-			mname = "sfp-";
-			printf("% 4i: SFP Module\n", i);
+	for (i = 0; i < module_count; ++i) {
+		switch (topology[i]) {
+		case MOX_MODULE_SFP:
+			printf("% 4i: SFP Module\n", i + 1);
+			break;
+		case MOX_MODULE_PCI:
+			printf("% 4i: Mini-PCIe Module\n", i + 1);
+			break;
+		case MOX_MODULE_TOPAZ:
+			printf("% 4i: Topaz Switch Module (4-port)\n", i + 1);
 			break;
-		case 0x2:
-			mname = "pci-";
-			printf("% 4i: Mini-PCIe Module\n", i);
+		case MOX_MODULE_PERIDOT:
+			printf("% 4i: Peridot Switch Module (8-port)\n", i + 1);
 			break;
-		case 0x3:
-			mname = "topaz-";
-			printf("% 4i: Topaz Switch Module\n", i);
+		case MOX_MODULE_USB3:
+			printf("% 4i: USB 3.0 Module (4 ports)\n", i + 1);
+			break;
+		case MOX_MODULE_PASSPCI:
+			printf("% 4i: Passthrough Mini-PCIe Module\n", i + 1);
 			break;
 		default:
-			printf("% 4i: unknown (ID %i)\n", i, mid);
-		}
-
-		mlen = strlen(mname);
-		if (len + mlen < sizeof(module_topology)) {
-			strcpy(module_topology + len, mname);
-			len += mlen;
+			printf("% 4i: unknown (ID %i)\n", i + 1, topology[i]);
 		}
 	}
-	printf("\n");
-
-	module_topology[len > 0 ? len - 1 : 0] = '\0';
 
-	env_set("module_topology", module_topology);
+	printf("\n");
 
-fail_release:
-	spi_release_bus(slave);
-fail_free:
-	spi_free_slave(slave);
-fail:
-	if (ret)
-		printf("Cannot read module topology!\n");
-	return ret;
+	return 0;
 }
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 02/10] board: turris_mox: Change SERDES map depending on module topology

[Marek Behún]

When SFP module is connected directly to CPU module we want the SGMII
lane speed at 1.25 Gbps.

This is a temporary solution till there is a comphy driver in the kernel
capable of changing SGMII speed at runtime.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 board/CZ.NIC/turris_mox/turris_mox.c | 33 ++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index 415c462493..3c0ab58756 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -8,6 +8,7 @@
 #include <dm.h>
 #include <clk.h>
 #include <spi.h>
+#include <mvebu/comphy.h>
 #include <linux/string.h>
 #include <linux/libfdt.h>
 #include <fdt_support.h>
@@ -206,6 +207,38 @@ static int mox_get_topology(const u8 **ptopology, int *psize, int *pis_sd)
 	return 0;
 }
 
+int comphy_update_map(struct comphy_map *serdes_map, int count)
+{
+	int ret, i, size, sfpindex = -1, swindex = -1;
+	const u8 *topology;
+
+	ret = mox_get_topology(&topology, &size, NULL);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < size; ++i) {
+		if (topology[i] == MOX_MODULE_SFP && sfpindex == -1)
+			sfpindex = i;
+		else if ((topology[i] == MOX_MODULE_TOPAZ ||
+			  topology[i] == MOX_MODULE_PERIDOT) &&
+			 swindex == -1)
+			swindex = i;
+	}
+
+	if (sfpindex >= 0 && swindex >= 0) {
+		if (sfpindex < swindex)
+			serdes_map[0].speed = PHY_SPEED_1_25G;
+		else
+			serdes_map[0].speed = PHY_SPEED_3_125G;
+	} else if (sfpindex >= 0) {
+		serdes_map[0].speed = PHY_SPEED_1_25G;
+	} else if (swindex >= 0) {
+		serdes_map[0].speed = PHY_SPEED_3_125G;
+	}
+
+	return 0;
+}
+
 int last_stage_init(void)
 {
 	int ret, i;
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 02/10] board: turris_mox: Change SERDES map depending on module topology

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> When SFP module is connected directly to CPU module we want the SGMII
> lane speed at 1.25 Gbps.
> 
> This is a temporary solution till there is a comphy driver in the kernel
> capable of changing SGMII speed at runtime.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   board/CZ.NIC/turris_mox/turris_mox.c | 33 ++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
> index 415c462493..3c0ab58756 100644
> --- a/board/CZ.NIC/turris_mox/turris_mox.c
> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> @@ -8,6 +8,7 @@
>   #include <dm.h>
>   #include <clk.h>
>   #include <spi.h>
> +#include <mvebu/comphy.h>
>   #include <linux/string.h>
>   #include <linux/libfdt.h>
>   #include <fdt_support.h>
> @@ -206,6 +207,38 @@ static int mox_get_topology(const u8 **ptopology, int *psize, int *pis_sd)
>   	return 0;
>   }
>   
> +int comphy_update_map(struct comphy_map *serdes_map, int count)
> +{
> +	int ret, i, size, sfpindex = -1, swindex = -1;
> +	const u8 *topology;
> +
> +	ret = mox_get_topology(&topology, &size, NULL);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < size; ++i) {
> +		if (topology[i] == MOX_MODULE_SFP && sfpindex == -1)
> +			sfpindex = i;
> +		else if ((topology[i] == MOX_MODULE_TOPAZ ||
> +			  topology[i] == MOX_MODULE_PERIDOT) &&
> +			 swindex == -1)
> +			swindex = i;
> +	}
> +
> +	if (sfpindex >= 0 && swindex >= 0) {
> +		if (sfpindex < swindex)
> +			serdes_map[0].speed = PHY_SPEED_1_25G;
> +		else
> +			serdes_map[0].speed = PHY_SPEED_3_125G;
> +	} else if (sfpindex >= 0) {
> +		serdes_map[0].speed = PHY_SPEED_1_25G;
> +	} else if (swindex >= 0) {
> +		serdes_map[0].speed = PHY_SPEED_3_125G;
> +	}
> +
> +	return 0;
> +}
> +
>   int last_stage_init(void)
>   {
>   	int ret, i;
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 03/10] board: turris_mox: Check and configure modules

[Marek Behún]

Check if Mox modules are connected in supported mode, then configure
the MDIO addresses of switch modules.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/dts/armada-3720-turris-mox.dts |  11 ++
 board/CZ.NIC/turris_mox/turris_mox.c    | 251 +++++++++++++++++++++++-
 2 files changed, 261 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
index 7babc16679..9c96dd39a9 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -110,6 +110,17 @@
 		spi-max-frequency = <20000000>;
 		m25p,fast-read;
 	};
+
+	moxtet at 1 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "cznic,moxtet";
+		reg = <1>;
+		devrst-gpio = <&gpiosb 2 GPIO_ACTIVE_LOW>;
+		spi-max-frequency = <1000000>;
+		spi-cpol;
+		spi-cpha;
+	};
 };
 
 &uart0 {
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index 3c0ab58756..39c26416a7 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -4,11 +4,13 @@
  */
 
 #include <common.h>
+#include <asm/gpio.h>
 #include <asm/io.h>
 #include <dm.h>
 #include <clk.h>
 #include <spi.h>
 #include <mvebu/comphy.h>
+#include <miiphy.h>
 #include <linux/string.h>
 #include <linux/libfdt.h>
 #include <fdt_support.h>
@@ -239,11 +241,138 @@ int comphy_update_map(struct comphy_map *serdes_map, int count)
 	return 0;
 }
 
+#define SW_SMI_CMD_R(d, r)	(0x9800 | (((d) & 0x1f) << 5) | ((r) & 0x1f))
+#define SW_SMI_CMD_W(d, r)	(0x9400 | (((d) & 0x1f) << 5) | ((r) & 0x1f))
+
+static int sw_multi_read(struct mii_dev *bus, int sw, int dev, int reg)
+{
+	bus->write(bus, sw, 0, 0, SW_SMI_CMD_R(dev, reg));
+	mdelay(5);
+	return bus->read(bus, sw, 0, 1);
+}
+
+static void sw_multi_write(struct mii_dev *bus, int sw, int dev, int reg,
+			   u16 val)
+{
+	bus->write(bus, sw, 0, 1, val);
+	bus->write(bus, sw, 0, 0, SW_SMI_CMD_W(dev, reg));
+	mdelay(5);
+}
+
+static int sw_scratch_read(struct mii_dev *bus, int sw, int reg)
+{
+	sw_multi_write(bus, sw, 0x1c, 0x1a, (reg & 0x7f) << 8);
+	return sw_multi_read(bus, sw, 0x1c, 0x1a) & 0xff;
+}
+
+static void sw_led_write(struct mii_dev *bus, int sw, int port, int reg,
+			 u16 val)
+{
+	sw_multi_write(bus, sw, port, 0x16, 0x8000 | ((reg & 7) << 12)
+					    | (val & 0x7ff));
+}
+
+static void sw_blink_leds(struct mii_dev *bus, int peridot, int topaz)
+{
+	int i, p;
+	struct {
+		int port;
+		u16 val;
+		int wait;
+	} regs[] = {
+		{ 2, 0xef, 1 }, { 2, 0xfe, 1 }, { 2, 0x33, 0 },
+		{ 4, 0xef, 1 }, { 4, 0xfe, 1 }, { 4, 0x33, 0 },
+		{ 3, 0xfe, 1 }, { 3, 0xef, 1 }, { 3, 0x33, 0 },
+		{ 1, 0xfe, 1 }, { 1, 0xef, 1 }, { 1, 0x33, 0 }
+	};
+
+	for (i = 0; i < 12; ++i) {
+		for (p = 0; p < peridot; ++p) {
+			sw_led_write(bus, 0x10 + p, regs[i].port, 0,
+				     regs[i].val);
+			sw_led_write(bus, 0x10 + p, regs[i].port + 4, 0,
+				     regs[i].val);
+		}
+		if (topaz) {
+			sw_led_write(bus, 0x2, 0x10 + regs[i].port, 0,
+				     regs[i].val);
+		}
+
+		if (regs[i].wait)
+			mdelay(75);
+	}
+}
+
+static void check_switch_address(struct mii_dev *bus, int addr)
+{
+	if (sw_scratch_read(bus, addr, 0x70) >> 3 != addr)
+		printf("Check of switch MDIO address failed for 0x%02x\n",
+		       addr);
+}
+
+static int sfp, pci, topaz, peridot, usb, passpci;
+static int sfp_pos, peridot_pos[3];
+static int module_count;
+
+static int configure_peridots(struct gpio_desc *reset_gpio)
+{
+	int i, ret;
+	u8 dout[MAX_MOX_MODULES];
+
+	memset(dout, 0, MAX_MOX_MODULES);
+
+	/* set addresses of Peridot modules */
+	for (i = 0; i < peridot; ++i)
+		dout[module_count - peridot_pos[i]] = (~i) & 3;
+
+	/*
+	 * if there is a SFP module connected to the last Peridot module, set
+	 * the P10_SMODE to 1 for the Peridot module
+	 */
+	if (sfp)
+		dout[module_count - peridot_pos[i - 1]] |= 1 << 3;
+
+	dm_gpio_set_value(reset_gpio, 1);
+	mdelay(10);
+
+	ret = mox_do_spi(NULL, dout, module_count + 1);
+
+	mdelay(10);
+	dm_gpio_set_value(reset_gpio, 0);
+
+	mdelay(50);
+
+	return ret;
+}
+
+static int get_reset_gpio(struct gpio_desc *reset_gpio)
+{
+	int node;
+
+	node = fdt_node_offset_by_compatible(gd->fdt_blob, 0, "cznic,moxtet");
+	if (node < 0) {
+		printf("Cannot find Moxtet bus device node!\n");
+		return -1;
+	}
+
+	gpio_request_by_name_nodev(offset_to_ofnode(node), "devrst-gpio", 0,
+				   reset_gpio, GPIOD_IS_OUT);
+
+	if (!dm_gpio_is_valid(reset_gpio)) {
+		printf("Cannot find reset GPIO for Moxtet bus!\n");
+		return -1;
+	}
+
+	return 0;
+}
+
 int last_stage_init(void)
 {
 	int ret, i;
 	const u8 *topology;
-	int module_count, is_sd;
+	int is_sd;
+	struct mii_dev *bus;
+	struct gpio_desc reset_gpio = {};
 
 	ret = mox_get_topology(&topology, &module_count, &is_sd);
 	if (ret) {
@@ -278,6 +407,126 @@ int last_stage_init(void)
 		}
 	}
 
+	/* now check if modules are connected in supported mode */
+
+	for (i = 0; i < module_count; ++i) {
+		switch (topology[i]) {
+		case MOX_MODULE_SFP:
+			if (sfp) {
+				printf("Error: Only one SFP module is "
+				       "supported!\n");
+			} else if (topaz) {
+				printf("Error: SFP module cannot be connected "
+				       "after Topaz Switch module!\n");
+			} else {
+				sfp_pos = i;
+				++sfp;
+			}
+			break;
+		case MOX_MODULE_PCI:
+			if (pci) {
+				printf("Error: Only one Mini-PCIe module is "
+				       "supported!\n");
+			} else if (usb) {
+				printf("Error: Mini-PCIe module cannot come "
+				       "after USB 3.0 module!\n");
+			} else if (i && (i != 1 || !passpci)) {
+				printf("Error: Mini-PCIe module should be the "
+				       "first connected module or come right "
+				       "after Passthrough Mini-PCIe module!\n");
+			} else {
+				++pci;
+			}
+			break;
+		case MOX_MODULE_TOPAZ:
+			if (topaz) {
+				printf("Error: Only one Topaz module is "
+				       "supported!\n");
+			} else if (peridot >= 3) {
+				printf("Error: At most two Peridot modules "
+				       "can come before Topaz module!\n");
+			} else {
+				++topaz;
+			}
+			break;
+		case MOX_MODULE_PERIDOT:
+			if (sfp || topaz) {
+				printf("Error: Peridot module must come before "
+				       "SFP or Topaz module!\n");
+			} else if (peridot >= 3) {
+				printf("Error: At most three Peridot modules "
+				       "are supported!\n");
+			} else {
+				peridot_pos[peridot] = i;
+				++peridot;
+			}
+			break;
+		case MOX_MODULE_USB3:
+			if (pci) {
+				printf("Error: USB 3.0 module cannot come "
+				       "after Mini-PCIe module!\n");
+			} else if (usb) {
+				printf("Error: Only one USB 3.0 module is "
+				       "supported!\n");
+			} else if (i && (i != 1 || !passpci)) {
+				printf("Error: USB 3.0 module should be the "
+				       "first connected module or come right "
+				       "after Passthrough Mini-PCIe module!\n");
+			} else {
+				++usb;
+			}
+			break;
+		case MOX_MODULE_PASSPCI:
+			if (passpci) {
+				printf("Error: Only one Passthrough Mini-PCIe "
+				       "module is supported!\n");
+			} else if (i != 0) {
+				printf("Error: Passthrough Mini-PCIe module "
+				       "should be the first connected "
+				       "module!\n");
+			} else {
+				++passpci;
+			}
+		}
+	}
+
+	/* now configure modules */
+
+	if (get_reset_gpio(&reset_gpio) < 0)
+		return 0;
+
+	if (peridot > 0) {
+		if (configure_peridots(&reset_gpio) < 0) {
+			printf("Cannot configure Peridot modules!\n");
+			peridot = 0;
+		}
+	} else {
+		dm_gpio_set_value(&reset_gpio, 1);
+		mdelay(50);
+		dm_gpio_set_value(&reset_gpio, 0);
+		mdelay(50);
+	}
+
+	if (peridot || topaz) {
+		/*
+		 * now check if the addresses are set by reading Scratch & Misc
+		 * register 0x70 of Peridot (and potentially Topaz) modules
+		 */
+
+		bus = miiphy_get_dev_by_name("neta@30000");
+		if (!bus) {
+			printf("Cannot get MDIO bus device!\n");
+		} else {
+			for (i = 0; i < peridot; ++i)
+				check_switch_address(bus, 0x10 + i);
+
+			if (topaz)
+				check_switch_address(bus, 0x2);
+
+			sw_blink_leds(bus, peridot, topaz);
+		}
+	}
+
 	printf("\n");
 
 	return 0;
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 03/10] board: turris_mox: Check and configure modules

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> Check if Mox modules are connected in supported mode, then configure
> the MDIO addresses of switch modules.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>   arch/arm/dts/armada-3720-turris-mox.dts |  11 ++
>   board/CZ.NIC/turris_mox/turris_mox.c    | 251 +++++++++++++++++++++++-
>   2 files changed, 261 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
> index 7babc16679..9c96dd39a9 100644
> --- a/arch/arm/dts/armada-3720-turris-mox.dts
> +++ b/arch/arm/dts/armada-3720-turris-mox.dts
> @@ -110,6 +110,17 @@
>   		spi-max-frequency = <20000000>;
>   		m25p,fast-read;
>   	};
> +
> +	moxtet at 1 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "cznic,moxtet";
> +		reg = <1>;
> +		devrst-gpio = <&gpiosb 2 GPIO_ACTIVE_LOW>;
> +		spi-max-frequency = <1000000>;
> +		spi-cpol;
> +		spi-cpha;
> +	};
>   };
>   
>   &uart0 {
> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
> index 3c0ab58756..39c26416a7 100644
> --- a/board/CZ.NIC/turris_mox/turris_mox.c
> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> @@ -4,11 +4,13 @@
>    */
>   
>   #include <common.h>
> +#include <asm/gpio.h>
>   #include <asm/io.h>
>   #include <dm.h>
>   #include <clk.h>
>   #include <spi.h>
>   #include <mvebu/comphy.h>
> +#include <miiphy.h>
>   #include <linux/string.h>
>   #include <linux/libfdt.h>
>   #include <fdt_support.h>
> @@ -239,11 +241,138 @@ int comphy_update_map(struct comphy_map *serdes_map, int count)
>   	return 0;
>   }
>   
> +#define SW_SMI_CMD_R(d, r)	(0x9800 | (((d) & 0x1f) << 5) | ((r) & 0x1f))
> +#define SW_SMI_CMD_W(d, r)	(0x9400 | (((d) & 0x1f) << 5) | ((r) & 0x1f))
> +
> +static int sw_multi_read(struct mii_dev *bus, int sw, int dev, int reg)
> +{
> +	bus->write(bus, sw, 0, 0, SW_SMI_CMD_R(dev, reg));
> +	mdelay(5);
> +	return bus->read(bus, sw, 0, 1);
> +}
> +
> +static void sw_multi_write(struct mii_dev *bus, int sw, int dev, int reg,
> +			   u16 val)
> +{
> +	bus->write(bus, sw, 0, 1, val);
> +	bus->write(bus, sw, 0, 0, SW_SMI_CMD_W(dev, reg));
> +	mdelay(5);
> +}
> +
> +static int sw_scratch_read(struct mii_dev *bus, int sw, int reg)
> +{
> +	sw_multi_write(bus, sw, 0x1c, 0x1a, (reg & 0x7f) << 8);
> +	return sw_multi_read(bus, sw, 0x1c, 0x1a) & 0xff;
> +}
> +
> +static void sw_led_write(struct mii_dev *bus, int sw, int port, int reg,
> +			 u16 val)
> +{
> +	sw_multi_write(bus, sw, port, 0x16, 0x8000 | ((reg & 7) << 12)
> +					    | (val & 0x7ff));
> +}
> +
> +static void sw_blink_leds(struct mii_dev *bus, int peridot, int topaz)
> +{
> +	int i, p;
> +	struct {
> +		int port;
> +		u16 val;
> +		int wait;
> +	} regs[] = {
> +		{ 2, 0xef, 1 }, { 2, 0xfe, 1 }, { 2, 0x33, 0 },
> +		{ 4, 0xef, 1 }, { 4, 0xfe, 1 }, { 4, 0x33, 0 },
> +		{ 3, 0xfe, 1 }, { 3, 0xef, 1 }, { 3, 0x33, 0 },
> +		{ 1, 0xfe, 1 }, { 1, 0xef, 1 }, { 1, 0x33, 0 }
> +	};
> +
> +	for (i = 0; i < 12; ++i) {
> +		for (p = 0; p < peridot; ++p) {
> +			sw_led_write(bus, 0x10 + p, regs[i].port, 0,
> +				     regs[i].val);
> +			sw_led_write(bus, 0x10 + p, regs[i].port + 4, 0,
> +				     regs[i].val);
> +		}
> +		if (topaz) {
> +			sw_led_write(bus, 0x2, 0x10 + regs[i].port, 0,
> +				     regs[i].val);
> +		}
> +
> +		if (regs[i].wait)
> +			mdelay(75);
> +	}
> +}
> +
> +static void check_switch_address(struct mii_dev *bus, int addr)
> +{
> +	if (sw_scratch_read(bus, addr, 0x70) >> 3 != addr)
> +		printf("Check of switch MDIO address failed for 0x%02x\n",
> +		       addr);
> +}
> +
> +static int sfp, pci, topaz, peridot, usb, passpci;
> +static int sfp_pos, peridot_pos[3];
> +static int module_count;
> +
> +static int configure_peridots(struct gpio_desc *reset_gpio)
> +{
> +	int i, ret;
> +	u8 dout[MAX_MOX_MODULES];
> +
> +	memset(dout, 0, MAX_MOX_MODULES);
> +
> +	/* set addresses of Peridot modules */
> +	for (i = 0; i < peridot; ++i)
> +		dout[module_count - peridot_pos[i]] = (~i) & 3;
> +
> +	/*
> +	 * if there is a SFP module connected to the last Peridot module, set
> +	 * the P10_SMODE to 1 for the Peridot module
> +	 */
> +	if (sfp)
> +		dout[module_count - peridot_pos[i - 1]] |= 1 << 3;
> +
> +	dm_gpio_set_value(reset_gpio, 1);
> +	mdelay(10);
> +
> +	ret = mox_do_spi(NULL, dout, module_count + 1);
> +
> +	mdelay(10);
> +	dm_gpio_set_value(reset_gpio, 0);
> +
> +	mdelay(50);
> +
> +	return ret;
> +}
> +
> +static int get_reset_gpio(struct gpio_desc *reset_gpio)
> +{
> +	int node;
> +
> +	node = fdt_node_offset_by_compatible(gd->fdt_blob, 0, "cznic,moxtet");
> +	if (node < 0) {
> +		printf("Cannot find Moxtet bus device node!\n");
> +		return -1;
> +	}
> +
> +	gpio_request_by_name_nodev(offset_to_ofnode(node), "devrst-gpio", 0,
> +				   reset_gpio, GPIOD_IS_OUT);
> +
> +	if (!dm_gpio_is_valid(reset_gpio)) {
> +		printf("Cannot find reset GPIO for Moxtet bus!\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>   int last_stage_init(void)
>   {
>   	int ret, i;
>   	const u8 *topology;
> -	int module_count, is_sd;
> +	int is_sd;
> +	struct mii_dev *bus;
> +	struct gpio_desc reset_gpio = {};
>   
>   	ret = mox_get_topology(&topology, &module_count, &is_sd);
>   	if (ret) {
> @@ -278,6 +407,126 @@ int last_stage_init(void)
>   		}
>   	}
>   
> +	/* now check if modules are connected in supported mode */
> +
> +	for (i = 0; i < module_count; ++i) {
> +		switch (topology[i]) {
> +		case MOX_MODULE_SFP:
> +			if (sfp) {
> +				printf("Error: Only one SFP module is "
> +				       "supported!\n");

This gives checkpatch warnings:

WARNING: quoted string split across lines
#492: FILE: board/CZ.NIC/turris_mox/turris_mox.c:417:
+                               printf("Error: Only one SFP module is "
+                                      "supported!\n");

I don't have strong feeling here, but in general I also like the strings
not being split, as its better for grepping / searching the code.

BTW: Could you please change your patch subject for all those patches
to reflect the architecture. Something like this:

board: turris_mox: Check and configure modules
->
arm: mvebu: turris_mox: Check and configure modules

Thanks,
Stefan

> +			} else if (topaz) {
> +				printf("Error: SFP module cannot be connected "
> +				       "after Topaz Switch module!\n");
> +			} else {
> +				sfp_pos = i;
> +				++sfp;
> +			}
> +			break;
> +		case MOX_MODULE_PCI:
> +			if (pci) {
> +				printf("Error: Only one Mini-PCIe module is "
> +				       "supported!\n");
> +			} else if (usb) {
> +				printf("Error: Mini-PCIe module cannot come "
> +				       "after USB 3.0 module!\n");
> +			} else if (i && (i != 1 || !passpci)) {
> +				printf("Error: Mini-PCIe module should be the "
> +				       "first connected module or come right "
> +				       "after Passthrough Mini-PCIe module!\n");
> +			} else {
> +				++pci;
> +			}
> +			break;
> +		case MOX_MODULE_TOPAZ:
> +			if (topaz) {
> +				printf("Error: Only one Topaz module is "
> +				       "supported!\n");
> +			} else if (peridot >= 3) {
> +				printf("Error: At most two Peridot modules "
> +				       "can come before Topaz module!\n");
> +			} else {
> +				++topaz;
> +			}
> +			break;
> +		case MOX_MODULE_PERIDOT:
> +			if (sfp || topaz) {
> +				printf("Error: Peridot module must come before "
> +				       "SFP or Topaz module!\n");
> +			} else if (peridot >= 3) {
> +				printf("Error: At most three Peridot modules "
> +				       "are supported!\n");
> +			} else {
> +				peridot_pos[peridot] = i;
> +				++peridot;
> +			}
> +			break;
> +		case MOX_MODULE_USB3:
> +			if (pci) {
> +				printf("Error: USB 3.0 module cannot come "
> +				       "after Mini-PCIe module!\n");
> +			} else if (usb) {
> +				printf("Error: Only one USB 3.0 module is "
> +				       "supported!\n");
> +			} else if (i && (i != 1 || !passpci)) {
> +				printf("Error: USB 3.0 module should be the "
> +				       "first connected module or come right "
> +				       "after Passthrough Mini-PCIe module!\n");
> +			} else {
> +				++usb;
> +			}
> +			break;
> +		case MOX_MODULE_PASSPCI:
> +			if (passpci) {
> +				printf("Error: Only one Passthrough Mini-PCIe "
> +				       "module is supported!\n");
> +			} else if (i != 0) {
> +				printf("Error: Passthrough Mini-PCIe module "
> +				       "should be the first connected "
> +				       "module!\n");
> +			} else {
> +				++passpci;
> +			}
> +		}
> +	}
> +
> +	/* now configure modules */
> +
> +	if (get_reset_gpio(&reset_gpio) < 0)
> +		return 0;
> +
> +	if (peridot > 0) {
> +		if (configure_peridots(&reset_gpio) < 0) {
> +			printf("Cannot configure Peridot modules!\n");
> +			peridot = 0;
> +		}
> +	} else {
> +		dm_gpio_set_value(&reset_gpio, 1);
> +		mdelay(50);
> +		dm_gpio_set_value(&reset_gpio, 0);
> +		mdelay(50);
> +	}
> +
> +	if (peridot || topaz) {
> +		/*
> +		 * now check if the addresses are set by reading Scratch & Misc
> +		 * register 0x70 of Peridot (and potentially Topaz) modules
> +		 */
> +
> +		bus = miiphy_get_dev_by_name("neta at 30000");
> +		if (!bus) {
> +			printf("Cannot get MDIO bus device!\n");
> +		} else {
> +			for (i = 0; i < peridot; ++i)
> +				check_switch_address(bus, 0x10 + i);
> +
> +			if (topaz)
> +				check_switch_address(bus, 0x2);
> +
> +			sw_blink_leds(bus, peridot, topaz);
> +		}
> +	}
> +
>   	printf("\n");
>   
>   	return 0;
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 04/10] arch/arm/dts: Fix Turris Mox device tree

[Marek Behún]

DTC issues a warning because #address-cells and #size-cells properties
are not set in the mdio node.
Also add ethernet1 alias.
Also add RTC node.
Also fix USB3 regulator startup delay time.
Also fix PCI Express SERDES speed to 5 GHz (this is only cosmetic, the
speed value is not used byt the comphy driver for PCI Express, but
should be 5 GHz nonetheless).

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/dts/armada-3720-turris-mox.dts | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
index 9c96dd39a9..2a71da21bf 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -24,6 +24,7 @@
 
 	aliases {
 		ethernet0 = &eth0;
+		ethernet1 = &eth1;
 		i2c0 = &i2c0;
 		spi0 = &spi0;
 	};
@@ -38,12 +39,16 @@
 		regulator-name = "usb3-vbus";
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
+		startup-delay-us = <2000000>;
 		shutdown-delay-us = <1000000>;
 		gpio = <&gpiosb 0 GPIO_ACTIVE_HIGH>;
 		regulator-boot-on;
 	};
 
 	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		eth_phy1: ethernet-phy at 1 {
 			reg = <1>;
 		};
@@ -59,7 +64,7 @@
 
 	phy1 {
 		phy-type = <PHY_TYPE_PEX0>;
-		phy-speed = <PHY_SPEED_2_5G>;
+		phy-speed = <PHY_SPEED_5G>;
 	};
 
 	phy2 {
@@ -80,6 +85,11 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c1_pins>;
 	status = "okay";
+
+	rtc at 6f {
+		compatible = "microchip,mcp7941x";
+		reg = <0x6f>;
+	};
 };
 
 &sdhci1 {
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 04/10] arch/arm/dts: Fix Turris Mox device tree

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> DTC issues a warning because #address-cells and #size-cells properties
> are not set in the mdio node.
> Also add ethernet1 alias.
> Also add RTC node.
> Also fix USB3 regulator startup delay time.
> Also fix PCI Express SERDES speed to 5 GHz (this is only cosmetic, the
> speed value is not used byt the comphy driver for PCI Express, but
> should be 5 GHz nonetheless).
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   arch/arm/dts/armada-3720-turris-mox.dts | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
> index 9c96dd39a9..2a71da21bf 100644
> --- a/arch/arm/dts/armada-3720-turris-mox.dts
> +++ b/arch/arm/dts/armada-3720-turris-mox.dts
> @@ -24,6 +24,7 @@
>   
>   	aliases {
>   		ethernet0 = &eth0;
> +		ethernet1 = &eth1;
>   		i2c0 = &i2c0;
>   		spi0 = &spi0;
>   	};
> @@ -38,12 +39,16 @@
>   		regulator-name = "usb3-vbus";
>   		regulator-min-microvolt = <5000000>;
>   		regulator-max-microvolt = <5000000>;
> +		startup-delay-us = <2000000>;
>   		shutdown-delay-us = <1000000>;
>   		gpio = <&gpiosb 0 GPIO_ACTIVE_HIGH>;
>   		regulator-boot-on;
>   	};
>   
>   	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
>   		eth_phy1: ethernet-phy at 1 {
>   			reg = <1>;
>   		};
> @@ -59,7 +64,7 @@
>   
>   	phy1 {
>   		phy-type = <PHY_TYPE_PEX0>;
> -		phy-speed = <PHY_SPEED_2_5G>;
> +		phy-speed = <PHY_SPEED_5G>;
>   	};
>   
>   	phy2 {
> @@ -80,6 +85,11 @@
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&i2c1_pins>;
>   	status = "okay";
> +
> +	rtc at 6f {
> +		compatible = "microchip,mcp7941x";
> +		reg = <0x6f>;
> +	};
>   };
>   
>   &sdhci1 {
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 05/10] board: turris_mox: Update defconfig

[Marek Behún]

Add gpio command to defconfig - this can be used to detect whether the
button is pressed or light LEDs.
Add DS1307 RTC driver and the date command.
Add CONFIG_WATCHDOG, so that U-Boot calls watchdog_reset.
Add CONFIG_MISC_INIT_R so that ethernet addresses are read from OTP
before network controller is initialized.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 configs/turris_mox_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig
index 749ed31acd..e89e6617ca 100644
--- a/configs/turris_mox_defconfig
+++ b/configs/turris_mox_defconfig
@@ -13,7 +13,9 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_ARCH_EARLY_INIT_R=y
+CONFIG_MISC_INIT_R=y
 CONFIG_CMD_CLK=y
+CONFIG_CMD_GPIO=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
@@ -24,6 +26,7 @@ CONFIG_CMD_USB=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
+CONFIG_CMD_DATE=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_MVEBU_BUBT=y
 CONFIG_CMD_BTRFS=y
@@ -63,6 +66,8 @@ CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_MVEBU_A3700_UART=y
 CONFIG_MVEBU_A3700_SPI=y
+CONFIG_DM_RTC=y
+CONFIG_RTC_DS1307=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_XHCI_HCD=y
@@ -73,6 +78,7 @@ CONFIG_USB_ETHER_ASIX=y
 CONFIG_USB_ETHER_MCS7830=y
 CONFIG_USB_ETHER_RTL8152=y
 CONFIG_USB_ETHER_SMSC95XX=y
+CONFIG_WATCHDOG=y
 CONFIG_WDT=y
 CONFIG_WDT_ARMADA_37XX=y
 CONFIG_SHA1=y
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 05/10] board: turris_mox: Update defconfig

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> Add gpio command to defconfig - this can be used to detect whether the
> button is pressed or light LEDs.
> Add DS1307 RTC driver and the date command.
> Add CONFIG_WATCHDOG, so that U-Boot calls watchdog_reset.
> Add CONFIG_MISC_INIT_R so that ethernet addresses are read from OTP
> before network controller is initialized.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   configs/turris_mox_defconfig | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig
> index 749ed31acd..e89e6617ca 100644
> --- a/configs/turris_mox_defconfig
> +++ b/configs/turris_mox_defconfig
> @@ -13,7 +13,9 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
>   # CONFIG_DISPLAY_CPUINFO is not set
>   # CONFIG_DISPLAY_BOARDINFO is not set
>   CONFIG_ARCH_EARLY_INIT_R=y
> +CONFIG_MISC_INIT_R=y
>   CONFIG_CMD_CLK=y
> +CONFIG_CMD_GPIO=y
>   # CONFIG_CMD_FLASH is not set
>   CONFIG_CMD_I2C=y
>   CONFIG_CMD_MMC=y
> @@ -24,6 +26,7 @@ CONFIG_CMD_USB=y
>   # CONFIG_CMD_SETEXPR is not set
>   CONFIG_CMD_TFTPPUT=y
>   CONFIG_CMD_CACHE=y
> +CONFIG_CMD_DATE=y
>   CONFIG_CMD_TIME=y
>   CONFIG_CMD_MVEBU_BUBT=y
>   CONFIG_CMD_BTRFS=y
> @@ -63,6 +66,8 @@ CONFIG_DEBUG_UART_SHIFT=2
>   CONFIG_DEBUG_UART_ANNOUNCE=y
>   CONFIG_MVEBU_A3700_UART=y
>   CONFIG_MVEBU_A3700_SPI=y
> +CONFIG_DM_RTC=y
> +CONFIG_RTC_DS1307=y
>   CONFIG_USB=y
>   CONFIG_DM_USB=y
>   CONFIG_USB_XHCI_HCD=y
> @@ -73,6 +78,7 @@ CONFIG_USB_ETHER_ASIX=y
>   CONFIG_USB_ETHER_MCS7830=y
>   CONFIG_USB_ETHER_RTL8152=y
>   CONFIG_USB_ETHER_SMSC95XX=y
> +CONFIG_WATCHDOG=y
>   CONFIG_WDT=y
>   CONFIG_WDT_ARMADA_37XX=y
>   CONFIG_SHA1=y
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 06/10] watchdog: armada_37xx: Fix compliance with kernel's driver

[Marek Behún]

The Armada 37xx watchdog driver was recently accepted for mainline
kernel by watchdog subsystem maintainer, but the driver works a little
different than the one in U-Boot. This patch fixes this.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/watchdog/armada-37xx-wdt.c | 109 ++++++++++++++++++-----------
 1 file changed, 67 insertions(+), 42 deletions(-)

diff --git a/drivers/watchdog/armada-37xx-wdt.c b/drivers/watchdog/armada-37xx-wdt.c
index 0fa4fda4fc..91cd8a6e6a 100644
--- a/drivers/watchdog/armada-37xx-wdt.c
+++ b/drivers/watchdog/armada-37xx-wdt.c
@@ -22,42 +22,63 @@ struct a37xx_wdt {
 };
 
 /*
- * We use Counter 1 for watchdog timer, because so does Marvell's Linux by
- * default.
+ * We use Counter 1 as watchdog timer, and Counter 0 for re-triggering Counter 1
  */
 
-#define CNTR_CTRL			0x10
+#define CNTR_CTRL(id)			((id) * 0x10)
 #define CNTR_CTRL_ENABLE		0x0001
 #define CNTR_CTRL_ACTIVE		0x0002
 #define CNTR_CTRL_MODE_MASK		0x000c
 #define CNTR_CTRL_MODE_ONESHOT		0x0000
+#define CNTR_CTRL_MODE_HWSIG		0x000c
+#define CNTR_CTRL_TRIG_SRC_MASK		0x00f0
+#define CNTR_CTRL_TRIG_SRC_PREV_CNTR	0x0050
 #define CNTR_CTRL_PRESCALE_MASK		0xff00
 #define CNTR_CTRL_PRESCALE_MIN		2
 #define CNTR_CTRL_PRESCALE_SHIFT	8
 
-#define CNTR_COUNT_LOW			0x14
-#define CNTR_COUNT_HIGH			0x18
+#define CNTR_COUNT_LOW(id)		(CNTR_CTRL(id) + 0x4)
+#define CNTR_COUNT_HIGH(id)		(CNTR_CTRL(id) + 0x8)
 
-static void set_counter_value(struct a37xx_wdt *priv)
+static void set_counter_value(struct a37xx_wdt *priv, int id, u64 val)
 {
-	writel(priv->timeout & 0xffffffff, priv->reg + CNTR_COUNT_LOW);
-	writel(priv->timeout >> 32, priv->reg + CNTR_COUNT_HIGH);
+	writel(val & 0xffffffff, priv->reg + CNTR_COUNT_LOW(id));
+	writel(val >> 32, priv->reg + CNTR_COUNT_HIGH(id));
 }
 
-static void a37xx_wdt_enable(struct a37xx_wdt *priv)
+static void counter_enable(struct a37xx_wdt *priv, int id)
 {
-	u32 reg = readl(priv->reg + CNTR_CTRL);
+	setbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
+}
 
-	reg |= CNTR_CTRL_ENABLE;
-	writel(reg, priv->reg + CNTR_CTRL);
+static void counter_disable(struct a37xx_wdt *priv, int id)
+{
+	clrbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
 }
 
-static void a37xx_wdt_disable(struct a37xx_wdt *priv)
+static int init_counter(struct a37xx_wdt *priv, int id, u32 mode, u32 trig_src)
 {
-	u32 reg = readl(priv->reg + CNTR_CTRL);
+	u32 reg;
+
+	reg = readl(priv->reg + CNTR_CTRL(id));
+	if (reg & CNTR_CTRL_ACTIVE)
+		return -EBUSY;
+
+	reg &= ~(CNTR_CTRL_MODE_MASK | CNTR_CTRL_PRESCALE_MASK |
+		 CNTR_CTRL_TRIG_SRC_MASK);
+
+	/* set mode */
+	reg |= mode;
+
+	/* set prescaler to the min value */
+	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
+
+	/* set trigger source */
+	reg |= trig_src;
 
-	reg &= ~CNTR_CTRL_ENABLE;
-	writel(reg, priv->reg + CNTR_CTRL);
+	writel(reg, priv->reg + CNTR_CTRL(id));
+
+	return 0;
 }
 
 static int a37xx_wdt_reset(struct udevice *dev)
@@ -67,9 +88,9 @@ static int a37xx_wdt_reset(struct udevice *dev)
 	if (!priv->timeout)
 		return -EINVAL;
 
-	a37xx_wdt_disable(priv);
-	set_counter_value(priv);
-	a37xx_wdt_enable(priv);
+	/* counter 1 is retriggered by forcing end count on counter 0 */
+	counter_disable(priv, 0);
+	counter_enable(priv, 0);
 
 	return 0;
 }
@@ -78,10 +99,14 @@ static int a37xx_wdt_expire_now(struct udevice *dev, ulong flags)
 {
 	struct a37xx_wdt *priv = dev_get_priv(dev);
 
-	a37xx_wdt_disable(priv);
-	priv->timeout = 0;
-	set_counter_value(priv);
-	a37xx_wdt_enable(priv);
+	/* first we set timeout to 0 */
+	counter_disable(priv, 1);
+	set_counter_value(priv, 1, 0);
+	counter_enable(priv, 1);
+
+	/* and then we start counter 1 by forcing end count on counter 0 */
+	counter_disable(priv, 0);
+	counter_enable(priv, 0);
 
 	return 0;
 }
@@ -89,26 +114,25 @@ static int a37xx_wdt_expire_now(struct udevice *dev, ulong flags)
 static int a37xx_wdt_start(struct udevice *dev, u64 ms, ulong flags)
 {
 	struct a37xx_wdt *priv = dev_get_priv(dev);
-	u32 reg;
-
-	reg = readl(priv->reg + CNTR_CTRL);
-
-	if (reg & CNTR_CTRL_ACTIVE)
-		return -EBUSY;
+	int err;
 
-	/* set mode */
-	reg = (reg & ~CNTR_CTRL_MODE_MASK) | CNTR_CTRL_MODE_ONESHOT;
+	err = init_counter(priv, 0, CNTR_CTRL_MODE_ONESHOT, 0);
+	if (err < 0)
+		return err;
 
-	/* set prescaler to the min value */
-	reg &= ~CNTR_CTRL_PRESCALE_MASK;
-	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
+	err = init_counter(priv, 1, CNTR_CTRL_MODE_HWSIG,
+			   CNTR_CTRL_TRIG_SRC_PREV_CNTR);
+	if (err < 0)
+		return err;
 
 	priv->timeout = ms * priv->clk_rate / 1000 / CNTR_CTRL_PRESCALE_MIN;
 
-	writel(reg, priv->reg + CNTR_CTRL);
+	set_counter_value(priv, 0, 0);
+	set_counter_value(priv, 1, priv->timeout);
+	counter_enable(priv, 1);
 
-	set_counter_value(priv);
-	a37xx_wdt_enable(priv);
+	/* we have to force end count on counter 0 to start counter 1 */
+	counter_enable(priv, 0);
 
 	return 0;
 }
@@ -117,7 +141,9 @@ static int a37xx_wdt_stop(struct udevice *dev)
 {
 	struct a37xx_wdt *priv = dev_get_priv(dev);
 
-	a37xx_wdt_disable(priv);
+	counter_disable(priv, 1);
+	counter_disable(priv, 0);
+	writel(0, priv->sel_reg);
 
 	return 0;
 }
@@ -139,11 +165,10 @@ static int a37xx_wdt_probe(struct udevice *dev)
 
 	priv->clk_rate = (ulong)get_ref_clk() * 1000000;
 
-	a37xx_wdt_disable(priv);
-
 	/*
-	 * We use timer 1 as watchdog timer (because Marvell's Linux uses that
-	 * timer as default), therefore we only set bit TIMER1_IS_WCHDOG_TIMER.
+	 * We use counter 1 as watchdog timer, therefore we only set bit
+	 * TIMER1_IS_WCHDOG_TIMER. Counter 0 is only used to force re-trigger on
+	 * counter 1.
 	 */
 	writel(1 << 1, priv->sel_reg);
 
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 06/10] watchdog: armada_37xx: Fix compliance with kernel's driver

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> The Armada 37xx watchdog driver was recently accepted for mainline
> kernel by watchdog subsystem maintainer, but the driver works a little
> different than the one in U-Boot. This patch fixes this.

Out of curiosity: What is different and why does the "old" implementation
not work any more?
  
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>   drivers/watchdog/armada-37xx-wdt.c | 109 ++++++++++++++++++-----------
>   1 file changed, 67 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/watchdog/armada-37xx-wdt.c b/drivers/watchdog/armada-37xx-wdt.c
> index 0fa4fda4fc..91cd8a6e6a 100644
> --- a/drivers/watchdog/armada-37xx-wdt.c
> +++ b/drivers/watchdog/armada-37xx-wdt.c
> @@ -22,42 +22,63 @@ struct a37xx_wdt {
>   };
>   
>   /*
> - * We use Counter 1 for watchdog timer, because so does Marvell's Linux by
> - * default.
> + * We use Counter 1 as watchdog timer, and Counter 0 for re-triggering Counter 1
>    */
>   
> -#define CNTR_CTRL			0x10
> +#define CNTR_CTRL(id)			((id) * 0x10)
>   #define CNTR_CTRL_ENABLE		0x0001
>   #define CNTR_CTRL_ACTIVE		0x0002
>   #define CNTR_CTRL_MODE_MASK		0x000c
>   #define CNTR_CTRL_MODE_ONESHOT		0x0000
> +#define CNTR_CTRL_MODE_HWSIG		0x000c
> +#define CNTR_CTRL_TRIG_SRC_MASK		0x00f0
> +#define CNTR_CTRL_TRIG_SRC_PREV_CNTR	0x0050
>   #define CNTR_CTRL_PRESCALE_MASK		0xff00
>   #define CNTR_CTRL_PRESCALE_MIN		2
>   #define CNTR_CTRL_PRESCALE_SHIFT	8
>   
> -#define CNTR_COUNT_LOW			0x14
> -#define CNTR_COUNT_HIGH			0x18
> +#define CNTR_COUNT_LOW(id)		(CNTR_CTRL(id) + 0x4)
> +#define CNTR_COUNT_HIGH(id)		(CNTR_CTRL(id) + 0x8)
>   
> -static void set_counter_value(struct a37xx_wdt *priv)
> +static void set_counter_value(struct a37xx_wdt *priv, int id, u64 val)
>   {
> -	writel(priv->timeout & 0xffffffff, priv->reg + CNTR_COUNT_LOW);
> -	writel(priv->timeout >> 32, priv->reg + CNTR_COUNT_HIGH);
> +	writel(val & 0xffffffff, priv->reg + CNTR_COUNT_LOW(id));
> +	writel(val >> 32, priv->reg + CNTR_COUNT_HIGH(id));
>   }
>   
> -static void a37xx_wdt_enable(struct a37xx_wdt *priv)
> +static void counter_enable(struct a37xx_wdt *priv, int id)
>   {
> -	u32 reg = readl(priv->reg + CNTR_CTRL);
> +	setbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
> +}
>   
> -	reg |= CNTR_CTRL_ENABLE;
> -	writel(reg, priv->reg + CNTR_CTRL);
> +static void counter_disable(struct a37xx_wdt *priv, int id)
> +{
> +	clrbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
>   }
>   
> -static void a37xx_wdt_disable(struct a37xx_wdt *priv)
> +static int init_counter(struct a37xx_wdt *priv, int id, u32 mode, u32 trig_src)
>   {
> -	u32 reg = readl(priv->reg + CNTR_CTRL);
> +	u32 reg;
> +
> +	reg = readl(priv->reg + CNTR_CTRL(id));
> +	if (reg & CNTR_CTRL_ACTIVE)
> +		return -EBUSY;
> +
> +	reg &= ~(CNTR_CTRL_MODE_MASK | CNTR_CTRL_PRESCALE_MASK |
> +		 CNTR_CTRL_TRIG_SRC_MASK);
> +
> +	/* set mode */
> +	reg |= mode;
> +
> +	/* set prescaler to the min value */
> +	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
> +
> +	/* set trigger source */
> +	reg |= trig_src;
>   
> -	reg &= ~CNTR_CTRL_ENABLE;
> -	writel(reg, priv->reg + CNTR_CTRL);
> +	writel(reg, priv->reg + CNTR_CTRL(id));
> +
> +	return 0;
>   }
>   
>   static int a37xx_wdt_reset(struct udevice *dev)
> @@ -67,9 +88,9 @@ static int a37xx_wdt_reset(struct udevice *dev)
>   	if (!priv->timeout)
>   		return -EINVAL;
>   
> -	a37xx_wdt_disable(priv);
> -	set_counter_value(priv);
> -	a37xx_wdt_enable(priv);
> +	/* counter 1 is retriggered by forcing end count on counter 0 */
> +	counter_disable(priv, 0);
> +	counter_enable(priv, 0);
>   
>   	return 0;
>   }
> @@ -78,10 +99,14 @@ static int a37xx_wdt_expire_now(struct udevice *dev, ulong flags)
>   {
>   	struct a37xx_wdt *priv = dev_get_priv(dev);
>   
> -	a37xx_wdt_disable(priv);
> -	priv->timeout = 0;
> -	set_counter_value(priv);
> -	a37xx_wdt_enable(priv);
> +	/* first we set timeout to 0 */
> +	counter_disable(priv, 1);
> +	set_counter_value(priv, 1, 0);
> +	counter_enable(priv, 1);
> +
> +	/* and then we start counter 1 by forcing end count on counter 0 */
> +	counter_disable(priv, 0);
> +	counter_enable(priv, 0);
>   
>   	return 0;
>   }
> @@ -89,26 +114,25 @@ static int a37xx_wdt_expire_now(struct udevice *dev, ulong flags)
>   static int a37xx_wdt_start(struct udevice *dev, u64 ms, ulong flags)
>   {
>   	struct a37xx_wdt *priv = dev_get_priv(dev);
> -	u32 reg;
> -
> -	reg = readl(priv->reg + CNTR_CTRL);
> -
> -	if (reg & CNTR_CTRL_ACTIVE)
> -		return -EBUSY;
> +	int err;
>   
> -	/* set mode */
> -	reg = (reg & ~CNTR_CTRL_MODE_MASK) | CNTR_CTRL_MODE_ONESHOT;
> +	err = init_counter(priv, 0, CNTR_CTRL_MODE_ONESHOT, 0);
> +	if (err < 0)
> +		return err;
>   
> -	/* set prescaler to the min value */
> -	reg &= ~CNTR_CTRL_PRESCALE_MASK;
> -	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
> +	err = init_counter(priv, 1, CNTR_CTRL_MODE_HWSIG,
> +			   CNTR_CTRL_TRIG_SRC_PREV_CNTR);
> +	if (err < 0)
> +		return err;
>   
>   	priv->timeout = ms * priv->clk_rate / 1000 / CNTR_CTRL_PRESCALE_MIN;
>   
> -	writel(reg, priv->reg + CNTR_CTRL);
> +	set_counter_value(priv, 0, 0);
> +	set_counter_value(priv, 1, priv->timeout);
> +	counter_enable(priv, 1);
>   
> -	set_counter_value(priv);
> -	a37xx_wdt_enable(priv);
> +	/* we have to force end count on counter 0 to start counter 1 */
> +	counter_enable(priv, 0);
>   
>   	return 0;
>   }
> @@ -117,7 +141,9 @@ static int a37xx_wdt_stop(struct udevice *dev)
>   {
>   	struct a37xx_wdt *priv = dev_get_priv(dev);
>   
> -	a37xx_wdt_disable(priv);
> +	counter_disable(priv, 1);
> +	counter_disable(priv, 0);
> +	writel(0, priv->sel_reg);
>   
>   	return 0;
>   }
> @@ -139,11 +165,10 @@ static int a37xx_wdt_probe(struct udevice *dev)
>   
>   	priv->clk_rate = (ulong)get_ref_clk() * 1000000;
>   
> -	a37xx_wdt_disable(priv);
> -
>   	/*
> -	 * We use timer 1 as watchdog timer (because Marvell's Linux uses that
> -	 * timer as default), therefore we only set bit TIMER1_IS_WCHDOG_TIMER.
> +	 * We use counter 1 as watchdog timer, therefore we only set bit
> +	 * TIMER1_IS_WCHDOG_TIMER. Counter 0 is only used to force re-trigger on
> +	 * counter 1.
>   	 */
>   	writel(1 << 1, priv->sel_reg);
>   
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 06/10] watchdog: armada_37xx: Fix compliance with kernel's driver

[Marek Behún]

Hi Stefan,

I forgot to answer you to this email.
Guenter pointed out that in the previous implementation of the watchdog
there was a tiny period of time when the watchdog was disabled (during
pinging - disable, set timeout, enable) which made the system
vulnerable.
I therefore changed the watchdog to use 2 counters:
Counter 1 is the watchdog counter - on expiry, the system is reset.
Counter 0 is used to reset Counter 1 to start counting from the set
timeout again. So Counter 1 is set to be reset on Counter 0 expiry
event and pinging is done by forcing immediate expiry event on Counter
0.

Marek

On Thu, 29 Nov 2018 14:03:27 +0100
Stefan Roese <sr@denx.de> wrote:

> On 20.11.18 13:04, Marek Behún wrote:
> > The Armada 37xx watchdog driver was recently accepted for mainline
> > kernel by watchdog subsystem maintainer, but the driver works a
> > little different than the one in U-Boot. This patch fixes this.  
> 
> Out of curiosity: What is different and why does the "old"
> implementation not work any more?
>   
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > ---
> >   drivers/watchdog/armada-37xx-wdt.c | 109
> > ++++++++++++++++++----------- 1 file changed, 67 insertions(+), 42
> > deletions(-)
> > 
> > diff --git a/drivers/watchdog/armada-37xx-wdt.c
> > b/drivers/watchdog/armada-37xx-wdt.c index 0fa4fda4fc..91cd8a6e6a
> > 100644 --- a/drivers/watchdog/armada-37xx-wdt.c
> > +++ b/drivers/watchdog/armada-37xx-wdt.c
> > @@ -22,42 +22,63 @@ struct a37xx_wdt {
> >   };
> >   
> >   /*
> > - * We use Counter 1 for watchdog timer, because so does Marvell's
> > Linux by
> > - * default.
> > + * We use Counter 1 as watchdog timer, and Counter 0 for
> > re-triggering Counter 1 */
> >   
> > -#define CNTR_CTRL			0x10
> > +#define CNTR_CTRL(id)			((id) * 0x10)
> >   #define CNTR_CTRL_ENABLE		0x0001
> >   #define CNTR_CTRL_ACTIVE		0x0002
> >   #define CNTR_CTRL_MODE_MASK		0x000c
> >   #define CNTR_CTRL_MODE_ONESHOT		0x0000
> > +#define CNTR_CTRL_MODE_HWSIG		0x000c
> > +#define CNTR_CTRL_TRIG_SRC_MASK		0x00f0
> > +#define CNTR_CTRL_TRIG_SRC_PREV_CNTR	0x0050
> >   #define CNTR_CTRL_PRESCALE_MASK		0xff00
> >   #define CNTR_CTRL_PRESCALE_MIN		2
> >   #define CNTR_CTRL_PRESCALE_SHIFT	8
> >   
> > -#define CNTR_COUNT_LOW			0x14
> > -#define CNTR_COUNT_HIGH			0x18
> > +#define CNTR_COUNT_LOW(id)		(CNTR_CTRL(id) + 0x4)
> > +#define CNTR_COUNT_HIGH(id)		(CNTR_CTRL(id) + 0x8)
> >   
> > -static void set_counter_value(struct a37xx_wdt *priv)
> > +static void set_counter_value(struct a37xx_wdt *priv, int id, u64
> > val) {
> > -	writel(priv->timeout & 0xffffffff, priv->reg +
> > CNTR_COUNT_LOW);
> > -	writel(priv->timeout >> 32, priv->reg + CNTR_COUNT_HIGH);
> > +	writel(val & 0xffffffff, priv->reg + CNTR_COUNT_LOW(id));
> > +	writel(val >> 32, priv->reg + CNTR_COUNT_HIGH(id));
> >   }
> >   
> > -static void a37xx_wdt_enable(struct a37xx_wdt *priv)
> > +static void counter_enable(struct a37xx_wdt *priv, int id)
> >   {
> > -	u32 reg = readl(priv->reg + CNTR_CTRL);
> > +	setbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
> > +}
> >   
> > -	reg |= CNTR_CTRL_ENABLE;
> > -	writel(reg, priv->reg + CNTR_CTRL);
> > +static void counter_disable(struct a37xx_wdt *priv, int id)
> > +{
> > +	clrbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
> >   }
> >   
> > -static void a37xx_wdt_disable(struct a37xx_wdt *priv)
> > +static int init_counter(struct a37xx_wdt *priv, int id, u32 mode,
> > u32 trig_src) {
> > -	u32 reg = readl(priv->reg + CNTR_CTRL);
> > +	u32 reg;
> > +
> > +	reg = readl(priv->reg + CNTR_CTRL(id));
> > +	if (reg & CNTR_CTRL_ACTIVE)
> > +		return -EBUSY;
> > +
> > +	reg &= ~(CNTR_CTRL_MODE_MASK | CNTR_CTRL_PRESCALE_MASK |
> > +		 CNTR_CTRL_TRIG_SRC_MASK);
> > +
> > +	/* set mode */
> > +	reg |= mode;
> > +
> > +	/* set prescaler to the min value */
> > +	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
> > +
> > +	/* set trigger source */
> > +	reg |= trig_src;
> >   
> > -	reg &= ~CNTR_CTRL_ENABLE;
> > -	writel(reg, priv->reg + CNTR_CTRL);
> > +	writel(reg, priv->reg + CNTR_CTRL(id));
> > +
> > +	return 0;
> >   }
> >   
> >   static int a37xx_wdt_reset(struct udevice *dev)
> > @@ -67,9 +88,9 @@ static int a37xx_wdt_reset(struct udevice *dev)
> >   	if (!priv->timeout)
> >   		return -EINVAL;
> >   
> > -	a37xx_wdt_disable(priv);
> > -	set_counter_value(priv);
> > -	a37xx_wdt_enable(priv);
> > +	/* counter 1 is retriggered by forcing end count on
> > counter 0 */
> > +	counter_disable(priv, 0);
> > +	counter_enable(priv, 0);
> >   
> >   	return 0;
> >   }
> > @@ -78,10 +99,14 @@ static int a37xx_wdt_expire_now(struct udevice
> > *dev, ulong flags) {
> >   	struct a37xx_wdt *priv = dev_get_priv(dev);
> >   
> > -	a37xx_wdt_disable(priv);
> > -	priv->timeout = 0;
> > -	set_counter_value(priv);
> > -	a37xx_wdt_enable(priv);
> > +	/* first we set timeout to 0 */
> > +	counter_disable(priv, 1);
> > +	set_counter_value(priv, 1, 0);
> > +	counter_enable(priv, 1);
> > +
> > +	/* and then we start counter 1 by forcing end count on
> > counter 0 */
> > +	counter_disable(priv, 0);
> > +	counter_enable(priv, 0);
> >   
> >   	return 0;
> >   }
> > @@ -89,26 +114,25 @@ static int a37xx_wdt_expire_now(struct udevice
> > *dev, ulong flags) static int a37xx_wdt_start(struct udevice *dev,
> > u64 ms, ulong flags) {
> >   	struct a37xx_wdt *priv = dev_get_priv(dev);
> > -	u32 reg;
> > -
> > -	reg = readl(priv->reg + CNTR_CTRL);
> > -
> > -	if (reg & CNTR_CTRL_ACTIVE)
> > -		return -EBUSY;
> > +	int err;
> >   
> > -	/* set mode */
> > -	reg = (reg & ~CNTR_CTRL_MODE_MASK) |
> > CNTR_CTRL_MODE_ONESHOT;
> > +	err = init_counter(priv, 0, CNTR_CTRL_MODE_ONESHOT, 0);
> > +	if (err < 0)
> > +		return err;
> >   
> > -	/* set prescaler to the min value */
> > -	reg &= ~CNTR_CTRL_PRESCALE_MASK;
> > -	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
> > +	err = init_counter(priv, 1, CNTR_CTRL_MODE_HWSIG,
> > +			   CNTR_CTRL_TRIG_SRC_PREV_CNTR);
> > +	if (err < 0)
> > +		return err;
> >   
> >   	priv->timeout = ms * priv->clk_rate / 1000 /
> > CNTR_CTRL_PRESCALE_MIN; 
> > -	writel(reg, priv->reg + CNTR_CTRL);
> > +	set_counter_value(priv, 0, 0);
> > +	set_counter_value(priv, 1, priv->timeout);
> > +	counter_enable(priv, 1);
> >   
> > -	set_counter_value(priv);
> > -	a37xx_wdt_enable(priv);
> > +	/* we have to force end count on counter 0 to start
> > counter 1 */
> > +	counter_enable(priv, 0);
> >   
> >   	return 0;
> >   }
> > @@ -117,7 +141,9 @@ static int a37xx_wdt_stop(struct udevice *dev)
> >   {
> >   	struct a37xx_wdt *priv = dev_get_priv(dev);
> >   
> > -	a37xx_wdt_disable(priv);
> > +	counter_disable(priv, 1);
> > +	counter_disable(priv, 0);
> > +	writel(0, priv->sel_reg);
> >   
> >   	return 0;
> >   }
> > @@ -139,11 +165,10 @@ static int a37xx_wdt_probe(struct udevice
> > *dev) 
> >   	priv->clk_rate = (ulong)get_ref_clk() * 1000000;
> >   
> > -	a37xx_wdt_disable(priv);
> > -
> >   	/*
> > -	 * We use timer 1 as watchdog timer (because Marvell's
> > Linux uses that
> > -	 * timer as default), therefore we only set bit
> > TIMER1_IS_WCHDOG_TIMER.
> > +	 * We use counter 1 as watchdog timer, therefore we only
> > set bit
> > +	 * TIMER1_IS_WCHDOG_TIMER. Counter 0 is only used to force
> > re-trigger on
> > +	 * counter 1.
> >   	 */
> >   	writel(1 << 1, priv->sel_reg);
> >   
> >   
> 
> Viele Grüße,
> Stefan
> 

[U-Boot] [PATCH u-boot-marvell v3 06/10] watchdog: armada_37xx: Fix compliance with kernel's driver

[Stefan Roese]

Hi Marek,

On 11.12.18 13:15, Marek Behún wrote:
> Hi Stefan,
> 
> I forgot to answer you to this email.
> Guenter pointed out that in the previous implementation of the watchdog
> there was a tiny period of time when the watchdog was disabled (during
> pinging - disable, set timeout, enable) which made the system
> vulnerable.
> I therefore changed the watchdog to use 2 counters:
> Counter 1 is the watchdog counter - on expiry, the system is reset.
> Counter 0 is used to reset Counter 1 to start counting from the set
> timeout again. So Counter 1 is set to be reset on Counter 0 expiry
> event and pinging is done by forcing immediate expiry event on Counter
> 0.

Thanks for explanation. Could you please re-submit and add this
description to the commit text, so that this change is more clear?

Thanks,
Stefan
  
> Marek
> 
> On Thu, 29 Nov 2018 14:03:27 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
>> On 20.11.18 13:04, Marek Behún wrote:
>>> The Armada 37xx watchdog driver was recently accepted for mainline
>>> kernel by watchdog subsystem maintainer, but the driver works a
>>> little different than the one in U-Boot. This patch fixes this.
>>
>> Out of curiosity: What is different and why does the "old"
>> implementation not work any more?
>>    
>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>> ---
>>>    drivers/watchdog/armada-37xx-wdt.c | 109
>>> ++++++++++++++++++----------- 1 file changed, 67 insertions(+), 42
>>> deletions(-)
>>>
>>> diff --git a/drivers/watchdog/armada-37xx-wdt.c
>>> b/drivers/watchdog/armada-37xx-wdt.c index 0fa4fda4fc..91cd8a6e6a
>>> 100644 --- a/drivers/watchdog/armada-37xx-wdt.c
>>> +++ b/drivers/watchdog/armada-37xx-wdt.c
>>> @@ -22,42 +22,63 @@ struct a37xx_wdt {
>>>    };
>>>    
>>>    /*
>>> - * We use Counter 1 for watchdog timer, because so does Marvell's
>>> Linux by
>>> - * default.
>>> + * We use Counter 1 as watchdog timer, and Counter 0 for
>>> re-triggering Counter 1 */
>>>    
>>> -#define CNTR_CTRL			0x10
>>> +#define CNTR_CTRL(id)			((id) * 0x10)
>>>    #define CNTR_CTRL_ENABLE		0x0001
>>>    #define CNTR_CTRL_ACTIVE		0x0002
>>>    #define CNTR_CTRL_MODE_MASK		0x000c
>>>    #define CNTR_CTRL_MODE_ONESHOT		0x0000
>>> +#define CNTR_CTRL_MODE_HWSIG		0x000c
>>> +#define CNTR_CTRL_TRIG_SRC_MASK		0x00f0
>>> +#define CNTR_CTRL_TRIG_SRC_PREV_CNTR	0x0050
>>>    #define CNTR_CTRL_PRESCALE_MASK		0xff00
>>>    #define CNTR_CTRL_PRESCALE_MIN		2
>>>    #define CNTR_CTRL_PRESCALE_SHIFT	8
>>>    
>>> -#define CNTR_COUNT_LOW			0x14
>>> -#define CNTR_COUNT_HIGH			0x18
>>> +#define CNTR_COUNT_LOW(id)		(CNTR_CTRL(id) + 0x4)
>>> +#define CNTR_COUNT_HIGH(id)		(CNTR_CTRL(id) + 0x8)
>>>    
>>> -static void set_counter_value(struct a37xx_wdt *priv)
>>> +static void set_counter_value(struct a37xx_wdt *priv, int id, u64
>>> val) {
>>> -	writel(priv->timeout & 0xffffffff, priv->reg +
>>> CNTR_COUNT_LOW);
>>> -	writel(priv->timeout >> 32, priv->reg + CNTR_COUNT_HIGH);
>>> +	writel(val & 0xffffffff, priv->reg + CNTR_COUNT_LOW(id));
>>> +	writel(val >> 32, priv->reg + CNTR_COUNT_HIGH(id));
>>>    }
>>>    
>>> -static void a37xx_wdt_enable(struct a37xx_wdt *priv)
>>> +static void counter_enable(struct a37xx_wdt *priv, int id)
>>>    {
>>> -	u32 reg = readl(priv->reg + CNTR_CTRL);
>>> +	setbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
>>> +}
>>>    
>>> -	reg |= CNTR_CTRL_ENABLE;
>>> -	writel(reg, priv->reg + CNTR_CTRL);
>>> +static void counter_disable(struct a37xx_wdt *priv, int id)
>>> +{
>>> +	clrbits_le32(priv->reg + CNTR_CTRL(id), CNTR_CTRL_ENABLE);
>>>    }
>>>    
>>> -static void a37xx_wdt_disable(struct a37xx_wdt *priv)
>>> +static int init_counter(struct a37xx_wdt *priv, int id, u32 mode,
>>> u32 trig_src) {
>>> -	u32 reg = readl(priv->reg + CNTR_CTRL);
>>> +	u32 reg;
>>> +
>>> +	reg = readl(priv->reg + CNTR_CTRL(id));
>>> +	if (reg & CNTR_CTRL_ACTIVE)
>>> +		return -EBUSY;
>>> +
>>> +	reg &= ~(CNTR_CTRL_MODE_MASK | CNTR_CTRL_PRESCALE_MASK |
>>> +		 CNTR_CTRL_TRIG_SRC_MASK);
>>> +
>>> +	/* set mode */
>>> +	reg |= mode;
>>> +
>>> +	/* set prescaler to the min value */
>>> +	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
>>> +
>>> +	/* set trigger source */
>>> +	reg |= trig_src;
>>>    
>>> -	reg &= ~CNTR_CTRL_ENABLE;
>>> -	writel(reg, priv->reg + CNTR_CTRL);
>>> +	writel(reg, priv->reg + CNTR_CTRL(id));
>>> +
>>> +	return 0;
>>>    }
>>>    
>>>    static int a37xx_wdt_reset(struct udevice *dev)
>>> @@ -67,9 +88,9 @@ static int a37xx_wdt_reset(struct udevice *dev)
>>>    	if (!priv->timeout)
>>>    		return -EINVAL;
>>>    
>>> -	a37xx_wdt_disable(priv);
>>> -	set_counter_value(priv);
>>> -	a37xx_wdt_enable(priv);
>>> +	/* counter 1 is retriggered by forcing end count on
>>> counter 0 */
>>> +	counter_disable(priv, 0);
>>> +	counter_enable(priv, 0);
>>>    
>>>    	return 0;
>>>    }
>>> @@ -78,10 +99,14 @@ static int a37xx_wdt_expire_now(struct udevice
>>> *dev, ulong flags) {
>>>    	struct a37xx_wdt *priv = dev_get_priv(dev);
>>>    
>>> -	a37xx_wdt_disable(priv);
>>> -	priv->timeout = 0;
>>> -	set_counter_value(priv);
>>> -	a37xx_wdt_enable(priv);
>>> +	/* first we set timeout to 0 */
>>> +	counter_disable(priv, 1);
>>> +	set_counter_value(priv, 1, 0);
>>> +	counter_enable(priv, 1);
>>> +
>>> +	/* and then we start counter 1 by forcing end count on
>>> counter 0 */
>>> +	counter_disable(priv, 0);
>>> +	counter_enable(priv, 0);
>>>    
>>>    	return 0;
>>>    }
>>> @@ -89,26 +114,25 @@ static int a37xx_wdt_expire_now(struct udevice
>>> *dev, ulong flags) static int a37xx_wdt_start(struct udevice *dev,
>>> u64 ms, ulong flags) {
>>>    	struct a37xx_wdt *priv = dev_get_priv(dev);
>>> -	u32 reg;
>>> -
>>> -	reg = readl(priv->reg + CNTR_CTRL);
>>> -
>>> -	if (reg & CNTR_CTRL_ACTIVE)
>>> -		return -EBUSY;
>>> +	int err;
>>>    
>>> -	/* set mode */
>>> -	reg = (reg & ~CNTR_CTRL_MODE_MASK) |
>>> CNTR_CTRL_MODE_ONESHOT;
>>> +	err = init_counter(priv, 0, CNTR_CTRL_MODE_ONESHOT, 0);
>>> +	if (err < 0)
>>> +		return err;
>>>    
>>> -	/* set prescaler to the min value */
>>> -	reg &= ~CNTR_CTRL_PRESCALE_MASK;
>>> -	reg |= CNTR_CTRL_PRESCALE_MIN << CNTR_CTRL_PRESCALE_SHIFT;
>>> +	err = init_counter(priv, 1, CNTR_CTRL_MODE_HWSIG,
>>> +			   CNTR_CTRL_TRIG_SRC_PREV_CNTR);
>>> +	if (err < 0)
>>> +		return err;
>>>    
>>>    	priv->timeout = ms * priv->clk_rate / 1000 /
>>> CNTR_CTRL_PRESCALE_MIN;
>>> -	writel(reg, priv->reg + CNTR_CTRL);
>>> +	set_counter_value(priv, 0, 0);
>>> +	set_counter_value(priv, 1, priv->timeout);
>>> +	counter_enable(priv, 1);
>>>    
>>> -	set_counter_value(priv);
>>> -	a37xx_wdt_enable(priv);
>>> +	/* we have to force end count on counter 0 to start
>>> counter 1 */
>>> +	counter_enable(priv, 0);
>>>    
>>>    	return 0;
>>>    }
>>> @@ -117,7 +141,9 @@ static int a37xx_wdt_stop(struct udevice *dev)
>>>    {
>>>    	struct a37xx_wdt *priv = dev_get_priv(dev);
>>>    
>>> -	a37xx_wdt_disable(priv);
>>> +	counter_disable(priv, 1);
>>> +	counter_disable(priv, 0);
>>> +	writel(0, priv->sel_reg);
>>>    
>>>    	return 0;
>>>    }
>>> @@ -139,11 +165,10 @@ static int a37xx_wdt_probe(struct udevice
>>> *dev)
>>>    	priv->clk_rate = (ulong)get_ref_clk() * 1000000;
>>>    
>>> -	a37xx_wdt_disable(priv);
>>> -
>>>    	/*
>>> -	 * We use timer 1 as watchdog timer (because Marvell's
>>> Linux uses that
>>> -	 * timer as default), therefore we only set bit
>>> TIMER1_IS_WCHDOG_TIMER.
>>> +	 * We use counter 1 as watchdog timer, therefore we only
>>> set bit
>>> +	 * TIMER1_IS_WCHDOG_TIMER. Counter 0 is only used to force
>>> re-trigger on
>>> +	 * counter 1.
>>>    	 */
>>>    	writel(1 << 1, priv->sel_reg);
>>>    
>>>    
>>
>> Viele Grüße,
>> Stefan
>>
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 07/10] MAINTAINERS: Add entry for CZ.NIC's Turris project

[Marek Behún]

Add myself as the maintainer of CZ.NIC's Turris Omnia and Turris Mox
projects.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index abdb6dcdb5..57a3b35bad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -118,6 +118,14 @@ F:	doc/README.bcm7xxx
 F:	drivers/mmc/bcmstb_sdhci.c
 F:	drivers/spi/bcmstb_spi.c
 
+ARM/CZ.NIC TURRIS MOX SUPPORT
+M:	Marek Behun <marek.behun@nic.cz>
+S:	Maintained
+F:	arch/arm/dts/armada-3720-turris-mox.dts
+F:	board/CZ.NIC/
+F:	configs/turris_*_defconfig
+F:	include/configs/turris_*.h
+
 ARM FREESCALE IMX
 M:	Stefano Babic <sbabic@denx.de>
 M:	Fabio Estevam <fabio.estevam@nxp.com>
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 07/10] MAINTAINERS: Add entry for CZ.NIC's Turris project

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> Add myself as the maintainer of CZ.NIC's Turris Omnia and Turris Mox
> projects.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   MAINTAINERS | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index abdb6dcdb5..57a3b35bad 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -118,6 +118,14 @@ F:	doc/README.bcm7xxx
>   F:	drivers/mmc/bcmstb_sdhci.c
>   F:	drivers/spi/bcmstb_spi.c
>   
> +ARM/CZ.NIC TURRIS MOX SUPPORT
> +M:	Marek Behun <marek.behun@nic.cz>
> +S:	Maintained
> +F:	arch/arm/dts/armada-3720-turris-mox.dts
> +F:	board/CZ.NIC/
> +F:	configs/turris_*_defconfig
> +F:	include/configs/turris_*.h
> +
>   ARM FREESCALE IMX
>   M:	Stefano Babic <sbabic@denx.de>
>   M:	Fabio Estevam <fabio.estevam@nxp.com>
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 08/10] board: turris_mox: Read info (and ethaddrs) from OTP

[Marek Behún]

Add support for reading One-Time Programmable memory via mailbox, which
communicates with CZ.NIC's firmware on the Secure Processor (Cortex-M3)
of Armada 3720.

Display product serial number and additional info, and also set MAC
addresses.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 board/CZ.NIC/turris_mox/Makefile     |   2 +-
 board/CZ.NIC/turris_mox/mox_sp.c     | 133 +++++++++++++++++++++++++++
 board/CZ.NIC/turris_mox/mox_sp.h     |  15 +++
 board/CZ.NIC/turris_mox/turris_mox.c |  54 ++++++++++-
 4 files changed, 201 insertions(+), 3 deletions(-)
 create mode 100644 board/CZ.NIC/turris_mox/mox_sp.c
 create mode 100644 board/CZ.NIC/turris_mox/mox_sp.h

diff --git a/board/CZ.NIC/turris_mox/Makefile b/board/CZ.NIC/turris_mox/Makefile
index 619704288b..33a52b63d7 100644
--- a/board/CZ.NIC/turris_mox/Makefile
+++ b/board/CZ.NIC/turris_mox/Makefile
@@ -2,4 +2,4 @@
 #
 # Copyright (C) 2018 Marek Behun <marek.behun@nic.cz>
 
-obj-y	:= turris_mox.o
+obj-y	:= turris_mox.o mox_sp.o
diff --git a/board/CZ.NIC/turris_mox/mox_sp.c b/board/CZ.NIC/turris_mox/mox_sp.c
new file mode 100644
index 0000000000..78438227dc
--- /dev/null
+++ b/board/CZ.NIC/turris_mox/mox_sp.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marek Behun <marek.behun@nic.cz>
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#define RWTM_CMD_PARAM(i)	(size_t)(0xd00b0000 + (i) * 4)
+#define RWTM_CMD		0xd00b0040
+#define RWTM_CMD_RETSTATUS	0xd00b0080
+#define RWTM_CMD_STATUS(i)	(size_t)(0xd00b0084 + (i) * 4)
+
+#define RWTM_HOST_INT_RESET	0xd00b00c8
+#define RWTM_HOST_INT_MASK	0xd00b00cc
+#define SP_CMD_COMPLETE		BIT(0)
+
+#define MBOX_STS_SUCCESS		(0x0 << 30)
+#define MBOX_STS_FAIL			(0x1 << 30)
+#define MBOX_STS_BADCMD			(0x2 << 30)
+#define MBOX_STS_LATER			(0x3 << 30)
+#define MBOX_STS_ERROR(s)		((s) & (3 << 30))
+#define MBOX_STS_VALUE(s)		(((s) >> 10) & 0xfffff)
+#define MBOX_STS_CMD(s)			((s) & 0x3ff)
+
+enum mbox_cmd {
+	MBOX_CMD_GET_RANDOM	= 1,
+	MBOX_CMD_BOARD_INFO,
+	MBOX_CMD_ECDSA_PUB_KEY,
+	MBOX_CMD_ECDSA_SIGN,
+
+	MBOX_CMD_OTP_READ,
+	MBOX_CMD_OTP_WRITE
+};
+
+static int mbox_do_cmd(enum mbox_cmd cmd, u32 *out, int nout)
+{
+	int i;
+	u32 status;
+
+	clrbits_le32(RWTM_HOST_INT_MASK, SP_CMD_COMPLETE);
+
+	writel(cmd, RWTM_CMD);
+
+	for (i = 0; i < 10; ++i) {
+		mdelay(10);
+		if (readl(RWTM_HOST_INT_RESET) & SP_CMD_COMPLETE)
+			break;
+	}
+
+	if (i == 10) {
+		/* if timed out, don't read status */
+		setbits_le32(RWTM_HOST_INT_RESET, SP_CMD_COMPLETE);
+		return -ETIMEDOUT;
+	}
+
+	for (i = 0; i < nout; ++i)
+		out[i] = readl(RWTM_CMD_STATUS(i));
+	status = readl(RWTM_CMD_RETSTATUS);
+
+	setbits_le32(RWTM_HOST_INT_RESET, SP_CMD_COMPLETE);
+
+	if (MBOX_STS_CMD(status) != cmd)
+		return -EIO;
+	else if (MBOX_STS_ERROR(status) == MBOX_STS_FAIL)
+		return -(int)MBOX_STS_VALUE(status);
+	else if (MBOX_STS_ERROR(status) != MBOX_STS_SUCCESS)
+		return -EIO;
+	else
+		return MBOX_STS_VALUE(status);
+}
+
+const char *mox_sp_get_ecdsa_public_key(void)
+{
+	static char public_key[135];
+	u32 out[16];
+	int res;
+
+	if (public_key[0])
+		return public_key;
+
+	res = mbox_do_cmd(MBOX_CMD_ECDSA_PUB_KEY, out, 16);
+	if (res < 0)
+		return NULL;
+
+	sprintf(public_key,
+		"%06x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x",
+		(u32)res, out[0], out[1], out[2], out[3], out[4], out[5],
+		out[6], out[7], out[8], out[9], out[10], out[11], out[12],
+		out[13], out[14], out[15]);
+
+	return public_key;
+}
+
+static inline void res_to_mac(u8 *mac, u32 t1, u32 t2)
+{
+	mac[0] = t1 >> 8;
+	mac[1] = t1;
+	mac[2] = t2 >> 24;
+	mac[3] = t2 >> 16;
+	mac[4] = t2 >> 8;
+	mac[5] = t2;
+}
+
+int mbox_sp_get_board_info(u64 *sn, u8 *mac1, u8 *mac2, int *bv, int *ram)
+{
+	u32 out[8];
+	int res;
+
+	res = mbox_do_cmd(MBOX_CMD_BOARD_INFO, out, 8);
+	if (res < 0)
+		return res;
+
+	if (sn) {
+		*sn = out[1];
+		*sn <<= 32;
+		*sn |= out[0];
+	}
+
+	if (bv)
+		*bv = out[2];
+
+	if (ram)
+		*ram = out[3];
+
+	if (mac1)
+		res_to_mac(mac1, out[4], out[5]);
+
+	if (mac2)
+		res_to_mac(mac2, out[6], out[7]);
+
+	return 0;
+}
diff --git a/board/CZ.NIC/turris_mox/mox_sp.h b/board/CZ.NIC/turris_mox/mox_sp.h
new file mode 100644
index 0000000000..49a4ed80ea
--- /dev/null
+++ b/board/CZ.NIC/turris_mox/mox_sp.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2018 Marek Behun <marek.behun@nic.cz>
+ */
+
+#ifndef _BOARD_CZNIC_TURRIS_MOX_MOX_SP_H_
+#define _BOARD_CZNIC_TURRIS_MOX_MOX_SP_H_
+
+#include <common.h>
+
+const char *mox_sp_get_ecdsa_public_key(void);
+int mbox_sp_get_board_info(u64 *sn, u8 *mac1, u8 *mac2, int *bv,
+			   int *ram);
+
+#endif /* _BOARD_CZNIC_TURRIS_MOX_MOX_SP_H_ */
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index 39c26416a7..89b3cd2ce0 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -19,6 +19,8 @@
 #include <wdt.h>
 #endif
 
+#include "mox_sp.h"
+
 #define MAX_MOX_MODULES		10
 
 #define MOX_MODULE_SFP		0x1
@@ -366,6 +368,49 @@ static int get_reset_gpio(struct gpio_desc *reset_gpio)
 	return 0;
 }
 
+int misc_init_r(void)
+{
+	int ret;
+	u8 mac1[6], mac2[6];
+
+	ret = mbox_sp_get_board_info(NULL, mac1, mac2, NULL, NULL);
+	if (ret < 0) {
+		printf("Cannot read data from OTP!\n");
+		return 0;
+	}
+
+	if (is_valid_ethaddr(mac1) && !env_get("ethaddr"))
+		eth_env_set_enetaddr("ethaddr", mac1);
+
+	if (is_valid_ethaddr(mac2) && !env_get("eth1addr"))
+		eth_env_set_enetaddr("eth1addr", mac2);
+
+	return 0;
+}
+
+static void mox_print_info(void)
+{
+	int ret, board_version, ram_size;
+	u64 serial_number;
+	const char *pub_key;
+
+	ret = mbox_sp_get_board_info(&serial_number, NULL, NULL, &board_version,
+				     &ram_size);
+	if (ret < 0)
+		return;
+
+	printf("Turris Mox:\n");
+	printf("  Board version: %i\n", board_version);
+	printf("  RAM size: %i MiB\n", ram_size);
+	printf("  Serial Number: %016llX\n", serial_number);
+
+	pub_key = mox_sp_get_ecdsa_public_key();
+	if (pub_key)
+		printf("  ECDSA Public Key: %s\n", pub_key);
+	else
+		printf("Cannot read ECDSA Public Key\n");
+}
+
 int last_stage_init(void)
 {
 	int ret, i;
@@ -374,14 +419,19 @@ int last_stage_init(void)
 	struct mii_dev *bus;
 	struct gpio_desc reset_gpio = {};
 
+	mox_print_info();
+
 	ret = mox_get_topology(&topology, &module_count, &is_sd);
 	if (ret) {
 		printf("Cannot read module topology!\n");
 		return 0;
 	}
 
-	printf("Found Turris Mox %s version\n", is_sd ? "SD" : "eMMC");
-	printf("Module Topology:\n");
+	printf("  SD/eMMC version: %s\n", is_sd ? "SD" : "eMMC");
+
+	if (module_count)
+		printf("Module Topology:\n");
+
 	for (i = 0; i < module_count; ++i) {
 		switch (topology[i]) {
 		case MOX_MODULE_SFP:
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 08/10] board: turris_mox: Read info (and ethaddrs) from OTP

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> Add support for reading One-Time Programmable memory via mailbox, which
> communicates with CZ.NIC's firmware on the Secure Processor (Cortex-M3)
> of Armada 3720.
> 
> Display product serial number and additional info, and also set MAC
> addresses.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   board/CZ.NIC/turris_mox/Makefile     |   2 +-
>   board/CZ.NIC/turris_mox/mox_sp.c     | 133 +++++++++++++++++++++++++++
>   board/CZ.NIC/turris_mox/mox_sp.h     |  15 +++
>   board/CZ.NIC/turris_mox/turris_mox.c |  54 ++++++++++-
>   4 files changed, 201 insertions(+), 3 deletions(-)
>   create mode 100644 board/CZ.NIC/turris_mox/mox_sp.c
>   create mode 100644 board/CZ.NIC/turris_mox/mox_sp.h
> 
> diff --git a/board/CZ.NIC/turris_mox/Makefile b/board/CZ.NIC/turris_mox/Makefile
> index 619704288b..33a52b63d7 100644
> --- a/board/CZ.NIC/turris_mox/Makefile
> +++ b/board/CZ.NIC/turris_mox/Makefile
> @@ -2,4 +2,4 @@
>   #
>   # Copyright (C) 2018 Marek Behun <marek.behun@nic.cz>
>   
> -obj-y	:= turris_mox.o
> +obj-y	:= turris_mox.o mox_sp.o
> diff --git a/board/CZ.NIC/turris_mox/mox_sp.c b/board/CZ.NIC/turris_mox/mox_sp.c
> new file mode 100644
> index 0000000000..78438227dc
> --- /dev/null
> +++ b/board/CZ.NIC/turris_mox/mox_sp.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Marek Behun <marek.behun@nic.cz>
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +
> +#define RWTM_CMD_PARAM(i)	(size_t)(0xd00b0000 + (i) * 4)
> +#define RWTM_CMD		0xd00b0040
> +#define RWTM_CMD_RETSTATUS	0xd00b0080
> +#define RWTM_CMD_STATUS(i)	(size_t)(0xd00b0084 + (i) * 4)
> +
> +#define RWTM_HOST_INT_RESET	0xd00b00c8
> +#define RWTM_HOST_INT_MASK	0xd00b00cc
> +#define SP_CMD_COMPLETE		BIT(0)
> +
> +#define MBOX_STS_SUCCESS		(0x0 << 30)
> +#define MBOX_STS_FAIL			(0x1 << 30)
> +#define MBOX_STS_BADCMD			(0x2 << 30)
> +#define MBOX_STS_LATER			(0x3 << 30)
> +#define MBOX_STS_ERROR(s)		((s) & (3 << 30))
> +#define MBOX_STS_VALUE(s)		(((s) >> 10) & 0xfffff)
> +#define MBOX_STS_CMD(s)			((s) & 0x3ff)
> +
> +enum mbox_cmd {
> +	MBOX_CMD_GET_RANDOM	= 1,
> +	MBOX_CMD_BOARD_INFO,
> +	MBOX_CMD_ECDSA_PUB_KEY,
> +	MBOX_CMD_ECDSA_SIGN,
> +
> +	MBOX_CMD_OTP_READ,
> +	MBOX_CMD_OTP_WRITE
> +};
> +
> +static int mbox_do_cmd(enum mbox_cmd cmd, u32 *out, int nout)
> +{
> +	int i;
> +	u32 status;
> +
> +	clrbits_le32(RWTM_HOST_INT_MASK, SP_CMD_COMPLETE);
> +
> +	writel(cmd, RWTM_CMD);
> +
> +	for (i = 0; i < 10; ++i) {
> +		mdelay(10);
> +		if (readl(RWTM_HOST_INT_RESET) & SP_CMD_COMPLETE)
> +			break;
> +	}
> +
> +	if (i == 10) {
> +		/* if timed out, don't read status */
> +		setbits_le32(RWTM_HOST_INT_RESET, SP_CMD_COMPLETE);
> +		return -ETIMEDOUT;
> +	}
> +
> +	for (i = 0; i < nout; ++i)
> +		out[i] = readl(RWTM_CMD_STATUS(i));
> +	status = readl(RWTM_CMD_RETSTATUS);
> +
> +	setbits_le32(RWTM_HOST_INT_RESET, SP_CMD_COMPLETE);
> +
> +	if (MBOX_STS_CMD(status) != cmd)
> +		return -EIO;
> +	else if (MBOX_STS_ERROR(status) == MBOX_STS_FAIL)
> +		return -(int)MBOX_STS_VALUE(status);
> +	else if (MBOX_STS_ERROR(status) != MBOX_STS_SUCCESS)
> +		return -EIO;
> +	else
> +		return MBOX_STS_VALUE(status);
> +}
> +
> +const char *mox_sp_get_ecdsa_public_key(void)
> +{
> +	static char public_key[135];
> +	u32 out[16];
> +	int res;
> +
> +	if (public_key[0])
> +		return public_key;
> +
> +	res = mbox_do_cmd(MBOX_CMD_ECDSA_PUB_KEY, out, 16);
> +	if (res < 0)
> +		return NULL;
> +
> +	sprintf(public_key,
> +		"%06x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x%08x",
> +		(u32)res, out[0], out[1], out[2], out[3], out[4], out[5],
> +		out[6], out[7], out[8], out[9], out[10], out[11], out[12],
> +		out[13], out[14], out[15]);
> +
> +	return public_key;
> +}
> +
> +static inline void res_to_mac(u8 *mac, u32 t1, u32 t2)
> +{
> +	mac[0] = t1 >> 8;
> +	mac[1] = t1;
> +	mac[2] = t2 >> 24;
> +	mac[3] = t2 >> 16;
> +	mac[4] = t2 >> 8;
> +	mac[5] = t2;
> +}
> +
> +int mbox_sp_get_board_info(u64 *sn, u8 *mac1, u8 *mac2, int *bv, int *ram)
> +{
> +	u32 out[8];
> +	int res;
> +
> +	res = mbox_do_cmd(MBOX_CMD_BOARD_INFO, out, 8);
> +	if (res < 0)
> +		return res;
> +
> +	if (sn) {
> +		*sn = out[1];
> +		*sn <<= 32;
> +		*sn |= out[0];
> +	}
> +
> +	if (bv)
> +		*bv = out[2];
> +
> +	if (ram)
> +		*ram = out[3];
> +
> +	if (mac1)
> +		res_to_mac(mac1, out[4], out[5]);
> +
> +	if (mac2)
> +		res_to_mac(mac2, out[6], out[7]);
> +
> +	return 0;
> +}
> diff --git a/board/CZ.NIC/turris_mox/mox_sp.h b/board/CZ.NIC/turris_mox/mox_sp.h
> new file mode 100644
> index 0000000000..49a4ed80ea
> --- /dev/null
> +++ b/board/CZ.NIC/turris_mox/mox_sp.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2018 Marek Behun <marek.behun@nic.cz>
> + */
> +
> +#ifndef _BOARD_CZNIC_TURRIS_MOX_MOX_SP_H_
> +#define _BOARD_CZNIC_TURRIS_MOX_MOX_SP_H_
> +
> +#include <common.h>
> +
> +const char *mox_sp_get_ecdsa_public_key(void);
> +int mbox_sp_get_board_info(u64 *sn, u8 *mac1, u8 *mac2, int *bv,
> +			   int *ram);
> +
> +#endif /* _BOARD_CZNIC_TURRIS_MOX_MOX_SP_H_ */
> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
> index 39c26416a7..89b3cd2ce0 100644
> --- a/board/CZ.NIC/turris_mox/turris_mox.c
> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> @@ -19,6 +19,8 @@
>   #include <wdt.h>
>   #endif
>   
> +#include "mox_sp.h"
> +
>   #define MAX_MOX_MODULES		10
>   
>   #define MOX_MODULE_SFP		0x1
> @@ -366,6 +368,49 @@ static int get_reset_gpio(struct gpio_desc *reset_gpio)
>   	return 0;
>   }
>   
> +int misc_init_r(void)
> +{
> +	int ret;
> +	u8 mac1[6], mac2[6];
> +
> +	ret = mbox_sp_get_board_info(NULL, mac1, mac2, NULL, NULL);
> +	if (ret < 0) {
> +		printf("Cannot read data from OTP!\n");
> +		return 0;
> +	}
> +
> +	if (is_valid_ethaddr(mac1) && !env_get("ethaddr"))
> +		eth_env_set_enetaddr("ethaddr", mac1);
> +
> +	if (is_valid_ethaddr(mac2) && !env_get("eth1addr"))
> +		eth_env_set_enetaddr("eth1addr", mac2);
> +
> +	return 0;
> +}
> +
> +static void mox_print_info(void)
> +{
> +	int ret, board_version, ram_size;
> +	u64 serial_number;
> +	const char *pub_key;
> +
> +	ret = mbox_sp_get_board_info(&serial_number, NULL, NULL, &board_version,
> +				     &ram_size);
> +	if (ret < 0)
> +		return;
> +
> +	printf("Turris Mox:\n");
> +	printf("  Board version: %i\n", board_version);
> +	printf("  RAM size: %i MiB\n", ram_size);
> +	printf("  Serial Number: %016llX\n", serial_number);
> +
> +	pub_key = mox_sp_get_ecdsa_public_key();
> +	if (pub_key)
> +		printf("  ECDSA Public Key: %s\n", pub_key);
> +	else
> +		printf("Cannot read ECDSA Public Key\n");
> +}
> +
>   int last_stage_init(void)
>   {
>   	int ret, i;
> @@ -374,14 +419,19 @@ int last_stage_init(void)
>   	struct mii_dev *bus;
>   	struct gpio_desc reset_gpio = {};
>   
> +	mox_print_info();
> +
>   	ret = mox_get_topology(&topology, &module_count, &is_sd);
>   	if (ret) {
>   		printf("Cannot read module topology!\n");
>   		return 0;
>   	}
>   
> -	printf("Found Turris Mox %s version\n", is_sd ? "SD" : "eMMC");
> -	printf("Module Topology:\n");
> +	printf("  SD/eMMC version: %s\n", is_sd ? "SD" : "eMMC");
> +
> +	if (module_count)
> +		printf("Module Topology:\n");
> +
>   	for (i = 0; i < module_count; ++i) {
>   		switch (topology[i]) {
>   		case MOX_MODULE_SFP:
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Marek Behún]

Depending on the data in the OTP memory, differentiate between the
512 MiB and 1 GiB versions of Turris Mox and report these RAM sizes in
dram_init and dram_init_banksize.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
 board/CZ.NIC/turris_mox/turris_mox.c | 27 +++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c
index f47273fde9..5e6ac9fc4a 100644
--- a/arch/arm/mach-mvebu/arm64-common.c
+++ b/arch/arm/mach-mvebu/arm64-common.c
@@ -43,8 +43,12 @@ const struct mbus_dram_target_info *mvebu_mbus_dram_info(void)
 	return NULL;
 }
 
-/* DRAM init code ... */
+/*
+ * DRAM init code ...
+ * Turris Mox defines this itself, depending on data in burned eFuses
+ */
 
+#ifndef CONFIG_TARGET_TURRIS_MOX
 int dram_init_banksize(void)
 {
 	fdtdec_setup_memory_banksize();
@@ -59,6 +63,7 @@ int dram_init(void)
 
 	return 0;
 }
+#endif /* !CONFIG_TARGET_TURRIS_MOX */
 
 int arch_cpu_init(void)
 {
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
index 89b3cd2ce0..9aa2fc004d 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -14,6 +14,7 @@
 #include <linux/string.h>
 #include <linux/libfdt.h>
 #include <fdt_support.h>
+#include <environment.h>
 
 #ifdef CONFIG_WDT_ARMADA_37XX
 #include <wdt.h>
@@ -40,6 +41,32 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+int dram_init(void)
+{
+	int ret, ram_size;
+
+	gd->ram_base = 0;
+	gd->ram_size = (phys_size_t)0x20000000;
+
+	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL, &ram_size);
+	if (ret < 0) {
+		puts("Cannot read RAM size from OTP, defaulting to 512 MiB");
+	} else {
+		if (ram_size == 1024)
+			gd->ram_size = (phys_size_t)0x40000000;
+	}
+
+	return 0;
+}
+
+int dram_init_banksize(void)
+{
+	gd->bd->bi_dram[0].start = (phys_addr_t)0;
+	gd->bd->bi_dram[0].size = gd->ram_size;
+
+	return 0;
+}
+
 #if defined(CONFIG_OF_BOARD_FIXUP)
 int board_fix_fdt(void *blob)
 {
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> Depending on the data in the OTP memory, differentiate between the
> 512 MiB and 1 GiB versions of Turris Mox and report these RAM sizes in
> dram_init and dram_init_banksize.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>   arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
>   board/CZ.NIC/turris_mox/turris_mox.c | 27 +++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c
> index f47273fde9..5e6ac9fc4a 100644
> --- a/arch/arm/mach-mvebu/arm64-common.c
> +++ b/arch/arm/mach-mvebu/arm64-common.c
> @@ -43,8 +43,12 @@ const struct mbus_dram_target_info *mvebu_mbus_dram_info(void)
>   	return NULL;
>   }
>   
> -/* DRAM init code ... */
> +/*
> + * DRAM init code ...
> + * Turris Mox defines this itself, depending on data in burned eFuses
> + */
>   
> +#ifndef CONFIG_TARGET_TURRIS_MOX
>   int dram_init_banksize(void)
>   {
>   	fdtdec_setup_memory_banksize();
> @@ -59,6 +63,7 @@ int dram_init(void)
>   
>   	return 0;
>   }
> +#endif /* !CONFIG_TARGET_TURRIS_MOX */

2 Problems with this:

a)
This does not apply any more with the latest changes in mainline.

b)
I really don't like #ifdef's here in this common code. Can you not
get rid of this somehow? Isn't the turris_mox also using ATF and
will read the RAM size from there?

U-Boot still has the good old get_ram_size() function, which can
easily auto-detect 512MiB vs 1GiB when run with 1GiB as parameter.

Thanks,
Stefan

>   
>   int arch_cpu_init(void)
>   {
> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
> index 89b3cd2ce0..9aa2fc004d 100644
> --- a/board/CZ.NIC/turris_mox/turris_mox.c
> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> @@ -14,6 +14,7 @@
>   #include <linux/string.h>
>   #include <linux/libfdt.h>
>   #include <fdt_support.h>
> +#include <environment.h>
>   
>   #ifdef CONFIG_WDT_ARMADA_37XX
>   #include <wdt.h>
> @@ -40,6 +41,32 @@
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> +int dram_init(void)
> +{
> +	int ret, ram_size;
> +
> +	gd->ram_base = 0;
> +	gd->ram_size = (phys_size_t)0x20000000;
> +
> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL, &ram_size);
> +	if (ret < 0) {
> +		puts("Cannot read RAM size from OTP, defaulting to 512 MiB");
> +	} else {
> +		if (ram_size == 1024)
> +			gd->ram_size = (phys_size_t)0x40000000;
> +	}
> +
> +	return 0;
> +}
> +
> +int dram_init_banksize(void)
> +{
> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
> +	gd->bd->bi_dram[0].size = gd->ram_size;
> +
> +	return 0;
> +}
> +
>   #if defined(CONFIG_OF_BOARD_FIXUP)
>   int board_fix_fdt(void *blob)
>   {
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Marek Behún]

Hi Stefan,

get_ram_size does not work correctly on Mox. On a 512 MiB board it
detects 1024 MiB of RAM, because on the 512 MiB RAM chip the topmost
address bit is simply ignored and the RAM wraps - on
0x20000000-0x40000000 CPU sees the same data as on 0x0-0x20000000.

ATF does not run RAM size determining code either, it just gets RAM
size from a register, this register is written before ATF by BootROM
and we have done it so that there is always 1 GB so that we could use
same secure firmware image for all Moxes. I tried to change this
register in secure firmware, but this lead to Synchornous Abort events
in U-Boot.

Maybe we could move the dram_init funcitons from arm64-common.c to
specific board files, or maybe we could declare them __weak in
arm64-common.c and turris_mox can then redefine them.

Would that be OK with you?

Marek

On Thu, 29 Nov 2018 14:07:59 +0100
Stefan Roese <sr@denx.de> wrote:

> On 20.11.18 13:04, Marek Behún wrote:
> > Depending on the data in the OTP memory, differentiate between the
> > 512 MiB and 1 GiB versions of Turris Mox and report these RAM sizes
> > in dram_init and dram_init_banksize.
> > 
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > ---
> >   arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
> >   board/CZ.NIC/turris_mox/turris_mox.c | 27
> > +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1
> > deletion(-)
> > 
> > diff --git a/arch/arm/mach-mvebu/arm64-common.c
> > b/arch/arm/mach-mvebu/arm64-common.c index f47273fde9..5e6ac9fc4a
> > 100644 --- a/arch/arm/mach-mvebu/arm64-common.c
> > +++ b/arch/arm/mach-mvebu/arm64-common.c
> > @@ -43,8 +43,12 @@ const struct mbus_dram_target_info
> > *mvebu_mbus_dram_info(void) return NULL;
> >   }
> >   
> > -/* DRAM init code ... */
> > +/*
> > + * DRAM init code ...
> > + * Turris Mox defines this itself, depending on data in burned
> > eFuses
> > + */
> >   
> > +#ifndef CONFIG_TARGET_TURRIS_MOX
> >   int dram_init_banksize(void)
> >   {
> >   	fdtdec_setup_memory_banksize();
> > @@ -59,6 +63,7 @@ int dram_init(void)
> >   
> >   	return 0;
> >   }
> > +#endif /* !CONFIG_TARGET_TURRIS_MOX */  
> 
> 2 Problems with this:
> 
> a)
> This does not apply any more with the latest changes in mainline.
> 
> b)
> I really don't like #ifdef's here in this common code. Can you not
> get rid of this somehow? Isn't the turris_mox also using ATF and
> will read the RAM size from there?
> 
> U-Boot still has the good old get_ram_size() function, which can
> easily auto-detect 512MiB vs 1GiB when run with 1GiB as parameter.
> 
> Thanks,
> Stefan
> 
> >   
> >   int arch_cpu_init(void)
> >   {
> > diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
> > b/board/CZ.NIC/turris_mox/turris_mox.c index 89b3cd2ce0..9aa2fc004d
> > 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c
> > +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> > @@ -14,6 +14,7 @@
> >   #include <linux/string.h>
> >   #include <linux/libfdt.h>
> >   #include <fdt_support.h>
> > +#include <environment.h>
> >   
> >   #ifdef CONFIG_WDT_ARMADA_37XX
> >   #include <wdt.h>
> > @@ -40,6 +41,32 @@
> >   
> >   DECLARE_GLOBAL_DATA_PTR;
> >   
> > +int dram_init(void)
> > +{
> > +	int ret, ram_size;
> > +
> > +	gd->ram_base = 0;
> > +	gd->ram_size = (phys_size_t)0x20000000;
> > +
> > +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
> > &ram_size);
> > +	if (ret < 0) {
> > +		puts("Cannot read RAM size from OTP, defaulting to
> > 512 MiB");
> > +	} else {
> > +		if (ram_size == 1024)
> > +			gd->ram_size = (phys_size_t)0x40000000;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int dram_init_banksize(void)
> > +{
> > +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
> > +	gd->bd->bi_dram[0].size = gd->ram_size;
> > +
> > +	return 0;
> > +}
> > +
> >   #if defined(CONFIG_OF_BOARD_FIXUP)
> >   int board_fix_fdt(void *blob)
> >   {
> >   
> 
> Viele Grüße,
> Stefan
> 

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Stefan Roese]

Hi Marek,

On 11.12.18 14:59, Marek Behún wrote:
> get_ram_size does not work correctly on Mox. On a 512 MiB board it
> detects 1024 MiB of RAM, because on the 512 MiB RAM chip the topmost
> address bit is simply ignored and the RAM wraps - on
> 0x20000000-0x40000000 CPU sees the same data as on 0x0-0x20000000.

That's what get_ram_size() does: It does detect such aliases when
the same memory is mapped at multiple areas (power of 2). Did you
give it a try with a max value of 1024 MiB? It should return
512 on such boards.
  
> ATF does not run RAM size determining code either, it just gets RAM
> size from a register, this register is written before ATF by BootROM
> and we have done it so that there is always 1 GB so that we could use
> same secure firmware image for all Moxes. I tried to change this
> register in secure firmware, but this lead to Synchornous Abort events
> in U-Boot.
> 
> Maybe we could move the dram_init funcitons from arm64-common.c to
> specific board files, or maybe we could declare them __weak in
> arm64-common.c and turris_mox can then redefine them.
> 
> Would that be OK with you?

Please fist check if get_ram_size() can't be used.

Thanks,
Stefan
  
> Marek
> 
> On Thu, 29 Nov 2018 14:07:59 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
>> On 20.11.18 13:04, Marek Behún wrote:
>>> Depending on the data in the OTP memory, differentiate between the
>>> 512 MiB and 1 GiB versions of Turris Mox and report these RAM sizes
>>> in dram_init and dram_init_banksize.
>>>
>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>> ---
>>>    arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
>>>    board/CZ.NIC/turris_mox/turris_mox.c | 27
>>> +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1
>>> deletion(-)
>>>
>>> diff --git a/arch/arm/mach-mvebu/arm64-common.c
>>> b/arch/arm/mach-mvebu/arm64-common.c index f47273fde9..5e6ac9fc4a
>>> 100644 --- a/arch/arm/mach-mvebu/arm64-common.c
>>> +++ b/arch/arm/mach-mvebu/arm64-common.c
>>> @@ -43,8 +43,12 @@ const struct mbus_dram_target_info
>>> *mvebu_mbus_dram_info(void) return NULL;
>>>    }
>>>    
>>> -/* DRAM init code ... */
>>> +/*
>>> + * DRAM init code ...
>>> + * Turris Mox defines this itself, depending on data in burned
>>> eFuses
>>> + */
>>>    
>>> +#ifndef CONFIG_TARGET_TURRIS_MOX
>>>    int dram_init_banksize(void)
>>>    {
>>>    	fdtdec_setup_memory_banksize();
>>> @@ -59,6 +63,7 @@ int dram_init(void)
>>>    
>>>    	return 0;
>>>    }
>>> +#endif /* !CONFIG_TARGET_TURRIS_MOX */
>>
>> 2 Problems with this:
>>
>> a)
>> This does not apply any more with the latest changes in mainline.
>>
>> b)
>> I really don't like #ifdef's here in this common code. Can you not
>> get rid of this somehow? Isn't the turris_mox also using ATF and
>> will read the RAM size from there?
>>
>> U-Boot still has the good old get_ram_size() function, which can
>> easily auto-detect 512MiB vs 1GiB when run with 1GiB as parameter.
>>
>> Thanks,
>> Stefan
>>
>>>    
>>>    int arch_cpu_init(void)
>>>    {
>>> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
>>> b/board/CZ.NIC/turris_mox/turris_mox.c index 89b3cd2ce0..9aa2fc004d
>>> 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c
>>> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
>>> @@ -14,6 +14,7 @@
>>>    #include <linux/string.h>
>>>    #include <linux/libfdt.h>
>>>    #include <fdt_support.h>
>>> +#include <environment.h>
>>>    
>>>    #ifdef CONFIG_WDT_ARMADA_37XX
>>>    #include <wdt.h>
>>> @@ -40,6 +41,32 @@
>>>    
>>>    DECLARE_GLOBAL_DATA_PTR;
>>>    
>>> +int dram_init(void)
>>> +{
>>> +	int ret, ram_size;
>>> +
>>> +	gd->ram_base = 0;
>>> +	gd->ram_size = (phys_size_t)0x20000000;
>>> +
>>> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
>>> &ram_size);
>>> +	if (ret < 0) {
>>> +		puts("Cannot read RAM size from OTP, defaulting to
>>> 512 MiB");
>>> +	} else {
>>> +		if (ram_size == 1024)
>>> +			gd->ram_size = (phys_size_t)0x40000000;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +int dram_init_banksize(void)
>>> +{
>>> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
>>> +	gd->bd->bi_dram[0].size = gd->ram_size;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    #if defined(CONFIG_OF_BOARD_FIXUP)
>>>    int board_fix_fdt(void *blob)
>>>    {
>>>    
>>
>> Viele Grüße,
>> Stefan
>>
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Marek Behun]

Hi, I have found the bug causing this issue.

If I understand the algorithm in get_ram_size correctly, it does
approximately this. Suppose A, B, C, D, E, F are different constatnts.
X(i) is a value at address 1<<i (couting in longs).

save[5] <- X(5)
X(5) <- F
save[4] <- X(4)
X(4) <- E
save[3] <- X(3)
X(3) <- D
save[2] <- X(2)
X(2) <- C
save[1] <- X(1)
X(1) <- B
save[0] <- X(0)
X(0) <- A

So the previous values are stored in array save[]. The algorithm then
checks if the values written (the constants A, B, C, D, E, F) are
present at those addresses. The problem is that the previous value from
save[] is written during checking of address i:

Now suppose the RAM is wrapped similarily as in MOX, so that X(i+3) is
the same as X(i).

After the first part, the values are as follows

X([0,1,2,3,4,5]) = [A,B,C,A,B,C]
save = [D,E,F,_3,_4,_5]

Here _3, _4, _5 are the values at addresses X(3), X(4), X(5) before the
algorithm.

The code that checks the values written does this:

if X(0) != A
    return 0
X(0) <- save[0]       !!! this also writes D to X(3)

if X(1) != B
    return 1
X(1) <- save[1]       !!! this also writes E to X(4)

if X(2) != C
    return 2
X(2) <- save[2]       !!! this also writes F to X(F)

if X(3) != D
    return 3          !!! this should return, but won't
X(3) <- save[3]

...

One solution would be to write the previous values from the array
save[] only immediately before return from the function.

I have to confess that I do not like how this function is written at
all. It does not, for example, solve correctly the case when a device
has 768 MiB of RAM from two chips (512 + 256). Given 1024 MiB as
argument, it would return 1024 MiB, but the system only has 768 MiB.
This maybe is never an issue with devices that run u-boot, but still.

Marek

On Tue, 11 Dec 2018 16:06:42 +0100
Stefan Roese <sr@denx.de> wrote:

> On 11.12.18 15:53, Marek Behún wrote:
> > On Tue, 11 Dec 2018 15:28:11 +0100
> > Stefan Roese <sr@denx.de> wrote:
> >   
> >> Hi Marek,
> >>
> >> On 11.12.18 14:59, Marek Behún wrote:  
> >>> get_ram_size does not work correctly on Mox. On a 512 MiB board it
> >>> detects 1024 MiB of RAM, because on the 512 MiB RAM chip the
> >>> topmost address bit is simply ignored and the RAM wraps - on
> >>> 0x20000000-0x40000000 CPU sees the same data as on
> >>> 0x0-0x20000000.  
> >>
> >> That's what get_ram_size() does: It does detect such aliases when
> >> the same memory is mapped at multiple areas (power of 2). Did you
> >> give it a try with a max value of 1024 MiB? It should return
> >> 512 on such boards.  
> > 
> > I checked it and it returned 1024 MiB.
> > I did
> >    printf("%08x %08x\n",
> >           get_ram_size(0, 512<<20),
> >           get_ram_size(0, 1024<<20));
> > on a 512 MiB board and
> >    0x20000000 0x40000000
> > was printed.  
> 
> Very strange. Could you please debug this issue? get_ram_size()
> should be able to work in such situations.
> 
> Thanks,
> Stefan
>   
> >>      
> >>> ATF does not run RAM size determining code either, it just gets
> >>> RAM size from a register, this register is written before ATF by
> >>> BootROM and we have done it so that there is always 1 GB so that
> >>> we could use same secure firmware image for all Moxes. I tried to
> >>> change this register in secure firmware, but this lead to
> >>> Synchornous Abort events in U-Boot.
> >>>
> >>> Maybe we could move the dram_init funcitons from arm64-common.c to
> >>> specific board files, or maybe we could declare them __weak in
> >>> arm64-common.c and turris_mox can then redefine them.
> >>>
> >>> Would that be OK with you?  
> >>
> >> Please fist check if get_ram_size() can't be used.
> >>
> >> Thanks,
> >> Stefan
> >>      
> >>> Marek
> >>>
> >>> On Thu, 29 Nov 2018 14:07:59 +0100
> >>> Stefan Roese <sr@denx.de> wrote:
> >>>      
> >>>> On 20.11.18 13:04, Marek Behún wrote:  
> >>>>> Depending on the data in the OTP memory, differentiate between
> >>>>> the 512 MiB and 1 GiB versions of Turris Mox and report these
> >>>>> RAM sizes in dram_init and dram_init_banksize.
> >>>>>
> >>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> >>>>> ---
> >>>>>     arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
> >>>>>     board/CZ.NIC/turris_mox/turris_mox.c | 27
> >>>>> +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1
> >>>>> deletion(-)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-mvebu/arm64-common.c
> >>>>> b/arch/arm/mach-mvebu/arm64-common.c index
> >>>>> f47273fde9..5e6ac9fc4a 100644 ---
> >>>>> a/arch/arm/mach-mvebu/arm64-common.c +++
> >>>>> b/arch/arm/mach-mvebu/arm64-common.c @@ -43,8 +43,12 @@ const
> >>>>> struct mbus_dram_target_info *mvebu_mbus_dram_info(void) return
> >>>>> NULL; }
> >>>>>     
> >>>>> -/* DRAM init code ... */
> >>>>> +/*
> >>>>> + * DRAM init code ...
> >>>>> + * Turris Mox defines this itself, depending on data in burned
> >>>>> eFuses
> >>>>> + */
> >>>>>     
> >>>>> +#ifndef CONFIG_TARGET_TURRIS_MOX
> >>>>>     int dram_init_banksize(void)
> >>>>>     {
> >>>>>     	fdtdec_setup_memory_banksize();
> >>>>> @@ -59,6 +63,7 @@ int dram_init(void)
> >>>>>     
> >>>>>     	return 0;
> >>>>>     }
> >>>>> +#endif /* !CONFIG_TARGET_TURRIS_MOX */  
> >>>>
> >>>> 2 Problems with this:
> >>>>
> >>>> a)
> >>>> This does not apply any more with the latest changes in mainline.
> >>>>
> >>>> b)
> >>>> I really don't like #ifdef's here in this common code. Can you
> >>>> not get rid of this somehow? Isn't the turris_mox also using ATF
> >>>> and will read the RAM size from there?
> >>>>
> >>>> U-Boot still has the good old get_ram_size() function, which can
> >>>> easily auto-detect 512MiB vs 1GiB when run with 1GiB as
> >>>> parameter.
> >>>>
> >>>> Thanks,
> >>>> Stefan
> >>>>     
> >>>>>     
> >>>>>     int arch_cpu_init(void)
> >>>>>     {
> >>>>> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
> >>>>> b/board/CZ.NIC/turris_mox/turris_mox.c index
> >>>>> 89b3cd2ce0..9aa2fc004d 100644 ---
> >>>>> a/board/CZ.NIC/turris_mox/turris_mox.c +++
> >>>>> b/board/CZ.NIC/turris_mox/turris_mox.c @@ -14,6 +14,7 @@
> >>>>>     #include <linux/string.h>
> >>>>>     #include <linux/libfdt.h>
> >>>>>     #include <fdt_support.h>
> >>>>> +#include <environment.h>
> >>>>>     
> >>>>>     #ifdef CONFIG_WDT_ARMADA_37XX
> >>>>>     #include <wdt.h>
> >>>>> @@ -40,6 +41,32 @@
> >>>>>     
> >>>>>     DECLARE_GLOBAL_DATA_PTR;
> >>>>>     
> >>>>> +int dram_init(void)
> >>>>> +{
> >>>>> +	int ret, ram_size;
> >>>>> +
> >>>>> +	gd->ram_base = 0;
> >>>>> +	gd->ram_size = (phys_size_t)0x20000000;
> >>>>> +
> >>>>> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
> >>>>> &ram_size);
> >>>>> +	if (ret < 0) {
> >>>>> +		puts("Cannot read RAM size from OTP, defaulting
> >>>>> to 512 MiB");
> >>>>> +	} else {
> >>>>> +		if (ram_size == 1024)
> >>>>> +			gd->ram_size = (phys_size_t)0x40000000;
> >>>>> +	}
> >>>>> +
> >>>>> +	return 0;
> >>>>> +}
> >>>>> +
> >>>>> +int dram_init_banksize(void)
> >>>>> +{
> >>>>> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
> >>>>> +	gd->bd->bi_dram[0].size = gd->ram_size;
> >>>>> +
> >>>>> +	return 0;
> >>>>> +}
> >>>>> +
> >>>>>     #if defined(CONFIG_OF_BOARD_FIXUP)
> >>>>>     int board_fix_fdt(void *blob)
> >>>>>     {
> >>>>>         
> >>>>
> >>>> Viele Grüße,
> >>>> Stefan
> >>>>     
> >>>      
> >>
> >> Viele Grüße,
> >> Stefan
> >>  
> >   
> 
> Viele Grüße,
> Stefan
> 

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Stefan Roese]

Hi Marek,

On 12.12.18 03:23, Marek Behun wrote:
> Hi, I have found the bug causing this issue.

Good.
  
> If I understand the algorithm in get_ram_size correctly, it does
> approximately this. Suppose A, B, C, D, E, F are different constatnts.
> X(i) is a value at address 1<<i (couting in longs).
> 
> save[5] <- X(5)
> X(5) <- F
> save[4] <- X(4)
> X(4) <- E
> save[3] <- X(3)
> X(3) <- D
> save[2] <- X(2)
> X(2) <- C
> save[1] <- X(1)
> X(1) <- B
> save[0] <- X(0)
> X(0) <- A
> 
> So the previous values are stored in array save[]. The algorithm then
> checks if the values written (the constants A, B, C, D, E, F) are
> present at those addresses. The problem is that the previous value from
> save[] is written during checking of address i:
> 
> Now suppose the RAM is wrapped similarily as in MOX, so that X(i+3) is
> the same as X(i).
> 
> After the first part, the values are as follows
> 
> X([0,1,2,3,4,5]) = [A,B,C,A,B,C]
> save = [D,E,F,_3,_4,_5]
> 
> Here _3, _4, _5 are the values at addresses X(3), X(4), X(5) before the
> algorithm.
> 
> The code that checks the values written does this:
> 
> if X(0) != A
>      return 0
> X(0) <- save[0]       !!! this also writes D to X(3)
> 
> if X(1) != B
>      return 1
> X(1) <- save[1]       !!! this also writes E to X(4)
> 
> if X(2) != C
>      return 2
> X(2) <- save[2]       !!! this also writes F to X(F)
> 
> if X(3) != D
>      return 3          !!! this should return, but won't
> X(3) <- save[3]
> 
> ...
> 
> One solution would be to write the previous values from the array
> save[] only immediately before return from the function.

I have to admit that I didn't fully try to understand this issue you
describe above (sorry, lack of time). If you have found a bug and do
have a fix for it, then please submit a patch. Please add all
developers (e.g. Patrick Delaunay etc) who did some work on this code
to Cc, as changes here might be critical.
  
> I have to confess that I do not like how this function is written at
> all. It does not, for example, solve correctly the case when a device
> has 768 MiB of RAM from two chips (512 + 256). Given 1024 MiB as
> argument, it would return 1024 MiB, but the system only has 768 MiB.
> This maybe is never an issue with devices that run u-boot, but still.

If you have a nice and easy implementation to also support such
memory configurations, that would be perfect of course. But I really
think that such non-power-of-2 memory configurations are rather uncommon
for U-Boot and most likely don't need to be supported by this function.
Such configuration usually are a result of using multiple DIMM's (or
SODIMM's) which can be equipped with various sized memories. And here
the memory size can be read from the DIMM itself. So no need to support
this in get_ram_size().

Thanks,
Stefan
  
> Marek
> 
> On Tue, 11 Dec 2018 16:06:42 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
>> On 11.12.18 15:53, Marek Behún wrote:
>>> On Tue, 11 Dec 2018 15:28:11 +0100
>>> Stefan Roese <sr@denx.de> wrote:
>>>    
>>>> Hi Marek,
>>>>
>>>> On 11.12.18 14:59, Marek Behún wrote:
>>>>> get_ram_size does not work correctly on Mox. On a 512 MiB board it
>>>>> detects 1024 MiB of RAM, because on the 512 MiB RAM chip the
>>>>> topmost address bit is simply ignored and the RAM wraps - on
>>>>> 0x20000000-0x40000000 CPU sees the same data as on
>>>>> 0x0-0x20000000.
>>>>
>>>> That's what get_ram_size() does: It does detect such aliases when
>>>> the same memory is mapped at multiple areas (power of 2). Did you
>>>> give it a try with a max value of 1024 MiB? It should return
>>>> 512 on such boards.
>>>
>>> I checked it and it returned 1024 MiB.
>>> I did
>>>     printf("%08x %08x\n",
>>>            get_ram_size(0, 512<<20),
>>>            get_ram_size(0, 1024<<20));
>>> on a 512 MiB board and
>>>     0x20000000 0x40000000
>>> was printed.
>>
>> Very strange. Could you please debug this issue? get_ram_size()
>> should be able to work in such situations.
>>
>> Thanks,
>> Stefan
>>    
>>>>       
>>>>> ATF does not run RAM size determining code either, it just gets
>>>>> RAM size from a register, this register is written before ATF by
>>>>> BootROM and we have done it so that there is always 1 GB so that
>>>>> we could use same secure firmware image for all Moxes. I tried to
>>>>> change this register in secure firmware, but this lead to
>>>>> Synchornous Abort events in U-Boot.
>>>>>
>>>>> Maybe we could move the dram_init funcitons from arm64-common.c to
>>>>> specific board files, or maybe we could declare them __weak in
>>>>> arm64-common.c and turris_mox can then redefine them.
>>>>>
>>>>> Would that be OK with you?
>>>>
>>>> Please fist check if get_ram_size() can't be used.
>>>>
>>>> Thanks,
>>>> Stefan
>>>>       
>>>>> Marek
>>>>>
>>>>> On Thu, 29 Nov 2018 14:07:59 +0100
>>>>> Stefan Roese <sr@denx.de> wrote:
>>>>>       
>>>>>> On 20.11.18 13:04, Marek Behún wrote:
>>>>>>> Depending on the data in the OTP memory, differentiate between
>>>>>>> the 512 MiB and 1 GiB versions of Turris Mox and report these
>>>>>>> RAM sizes in dram_init and dram_init_banksize.
>>>>>>>
>>>>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>>>>>> ---
>>>>>>>      arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
>>>>>>>      board/CZ.NIC/turris_mox/turris_mox.c | 27
>>>>>>> +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1
>>>>>>> deletion(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-mvebu/arm64-common.c
>>>>>>> b/arch/arm/mach-mvebu/arm64-common.c index
>>>>>>> f47273fde9..5e6ac9fc4a 100644 ---
>>>>>>> a/arch/arm/mach-mvebu/arm64-common.c +++
>>>>>>> b/arch/arm/mach-mvebu/arm64-common.c @@ -43,8 +43,12 @@ const
>>>>>>> struct mbus_dram_target_info *mvebu_mbus_dram_info(void) return
>>>>>>> NULL; }
>>>>>>>      
>>>>>>> -/* DRAM init code ... */
>>>>>>> +/*
>>>>>>> + * DRAM init code ...
>>>>>>> + * Turris Mox defines this itself, depending on data in burned
>>>>>>> eFuses
>>>>>>> + */
>>>>>>>      
>>>>>>> +#ifndef CONFIG_TARGET_TURRIS_MOX
>>>>>>>      int dram_init_banksize(void)
>>>>>>>      {
>>>>>>>      	fdtdec_setup_memory_banksize();
>>>>>>> @@ -59,6 +63,7 @@ int dram_init(void)
>>>>>>>      
>>>>>>>      	return 0;
>>>>>>>      }
>>>>>>> +#endif /* !CONFIG_TARGET_TURRIS_MOX */
>>>>>>
>>>>>> 2 Problems with this:
>>>>>>
>>>>>> a)
>>>>>> This does not apply any more with the latest changes in mainline.
>>>>>>
>>>>>> b)
>>>>>> I really don't like #ifdef's here in this common code. Can you
>>>>>> not get rid of this somehow? Isn't the turris_mox also using ATF
>>>>>> and will read the RAM size from there?
>>>>>>
>>>>>> U-Boot still has the good old get_ram_size() function, which can
>>>>>> easily auto-detect 512MiB vs 1GiB when run with 1GiB as
>>>>>> parameter.
>>>>>>
>>>>>> Thanks,
>>>>>> Stefan
>>>>>>      
>>>>>>>      
>>>>>>>      int arch_cpu_init(void)
>>>>>>>      {
>>>>>>> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
>>>>>>> b/board/CZ.NIC/turris_mox/turris_mox.c index
>>>>>>> 89b3cd2ce0..9aa2fc004d 100644 ---
>>>>>>> a/board/CZ.NIC/turris_mox/turris_mox.c +++
>>>>>>> b/board/CZ.NIC/turris_mox/turris_mox.c @@ -14,6 +14,7 @@
>>>>>>>      #include <linux/string.h>
>>>>>>>      #include <linux/libfdt.h>
>>>>>>>      #include <fdt_support.h>
>>>>>>> +#include <environment.h>
>>>>>>>      
>>>>>>>      #ifdef CONFIG_WDT_ARMADA_37XX
>>>>>>>      #include <wdt.h>
>>>>>>> @@ -40,6 +41,32 @@
>>>>>>>      
>>>>>>>      DECLARE_GLOBAL_DATA_PTR;
>>>>>>>      
>>>>>>> +int dram_init(void)
>>>>>>> +{
>>>>>>> +	int ret, ram_size;
>>>>>>> +
>>>>>>> +	gd->ram_base = 0;
>>>>>>> +	gd->ram_size = (phys_size_t)0x20000000;
>>>>>>> +
>>>>>>> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
>>>>>>> &ram_size);
>>>>>>> +	if (ret < 0) {
>>>>>>> +		puts("Cannot read RAM size from OTP, defaulting
>>>>>>> to 512 MiB");
>>>>>>> +	} else {
>>>>>>> +		if (ram_size == 1024)
>>>>>>> +			gd->ram_size = (phys_size_t)0x40000000;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +int dram_init_banksize(void)
>>>>>>> +{
>>>>>>> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
>>>>>>> +	gd->bd->bi_dram[0].size = gd->ram_size;
>>>>>>> +
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>>      #if defined(CONFIG_OF_BOARD_FIXUP)
>>>>>>>      int board_fix_fdt(void *blob)
>>>>>>>      {
>>>>>>>          
>>>>>>
>>>>>> Viele Grüße,
>>>>>> Stefan
>>>>>>      
>>>>>       
>>>>
>>>> Viele Grüße,
>>>> Stefan
>>>>   
>>>    
>>
>> Viele Grüße,
>> Stefan
>>
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Marek Behun]

Hi Stefan,

it turned out that what I found out was not causing the bug.
get_ram_size reported 1 GiB of ram because I tried it when dcache was
already enabled. If I call get_ram_size in dram_init, it returns the
correct size on both 512 MiB and 1 GiB board.

In the next patch I shall define dram_init and dram_init_banksize in
arm64-common.c as __weak, and the definition in turris_mox.c shall call
get_ram_size. Is this acceptable?

Marek

On Wed, 12 Dec 2018 10:44:15 +0100
Stefan Roese <sr@denx.de> wrote:

> Hi Marek,
> 
> On 12.12.18 03:23, Marek Behun wrote:
> > Hi, I have found the bug causing this issue.  
> 
> Good.
>   
> > If I understand the algorithm in get_ram_size correctly, it does
> > approximately this. Suppose A, B, C, D, E, F are different
> > constatnts. X(i) is a value at address 1<<i (couting in longs).
> > 
> > save[5] <- X(5)
> > X(5) <- F
> > save[4] <- X(4)
> > X(4) <- E
> > save[3] <- X(3)
> > X(3) <- D
> > save[2] <- X(2)
> > X(2) <- C
> > save[1] <- X(1)
> > X(1) <- B
> > save[0] <- X(0)
> > X(0) <- A
> > 
> > So the previous values are stored in array save[]. The algorithm
> > then checks if the values written (the constants A, B, C, D, E, F)
> > are present at those addresses. The problem is that the previous
> > value from save[] is written during checking of address i:
> > 
> > Now suppose the RAM is wrapped similarily as in MOX, so that X(i+3)
> > is the same as X(i).
> > 
> > After the first part, the values are as follows
> > 
> > X([0,1,2,3,4,5]) = [A,B,C,A,B,C]
> > save = [D,E,F,_3,_4,_5]
> > 
> > Here _3, _4, _5 are the values at addresses X(3), X(4), X(5) before
> > the algorithm.
> > 
> > The code that checks the values written does this:
> > 
> > if X(0) != A
> >      return 0
> > X(0) <- save[0]       !!! this also writes D to X(3)
> > 
> > if X(1) != B
> >      return 1
> > X(1) <- save[1]       !!! this also writes E to X(4)
> > 
> > if X(2) != C
> >      return 2
> > X(2) <- save[2]       !!! this also writes F to X(F)
> > 
> > if X(3) != D
> >      return 3          !!! this should return, but won't
> > X(3) <- save[3]
> > 
> > ...
> > 
> > One solution would be to write the previous values from the array
> > save[] only immediately before return from the function.  
> 
> I have to admit that I didn't fully try to understand this issue you
> describe above (sorry, lack of time). If you have found a bug and do
> have a fix for it, then please submit a patch. Please add all
> developers (e.g. Patrick Delaunay etc) who did some work on this code
> to Cc, as changes here might be critical.
>   
> > I have to confess that I do not like how this function is written at
> > all. It does not, for example, solve correctly the case when a
> > device has 768 MiB of RAM from two chips (512 + 256). Given 1024
> > MiB as argument, it would return 1024 MiB, but the system only has
> > 768 MiB. This maybe is never an issue with devices that run u-boot,
> > but still.  
> 
> If you have a nice and easy implementation to also support such
> memory configurations, that would be perfect of course. But I really
> think that such non-power-of-2 memory configurations are rather
> uncommon for U-Boot and most likely don't need to be supported by
> this function. Such configuration usually are a result of using
> multiple DIMM's (or SODIMM's) which can be equipped with various
> sized memories. And here the memory size can be read from the DIMM
> itself. So no need to support this in get_ram_size().
> 
> Thanks,
> Stefan
>   
> > Marek
> > 
> > On Tue, 11 Dec 2018 16:06:42 +0100
> > Stefan Roese <sr@denx.de> wrote:
> >   
> >> On 11.12.18 15:53, Marek Behún wrote:  
> >>> On Tue, 11 Dec 2018 15:28:11 +0100
> >>> Stefan Roese <sr@denx.de> wrote:
> >>>      
> >>>> Hi Marek,
> >>>>
> >>>> On 11.12.18 14:59, Marek Behún wrote:  
> >>>>> get_ram_size does not work correctly on Mox. On a 512 MiB board
> >>>>> it detects 1024 MiB of RAM, because on the 512 MiB RAM chip the
> >>>>> topmost address bit is simply ignored and the RAM wraps - on
> >>>>> 0x20000000-0x40000000 CPU sees the same data as on
> >>>>> 0x0-0x20000000.  
> >>>>
> >>>> That's what get_ram_size() does: It does detect such aliases when
> >>>> the same memory is mapped at multiple areas (power of 2). Did you
> >>>> give it a try with a max value of 1024 MiB? It should return
> >>>> 512 on such boards.  
> >>>
> >>> I checked it and it returned 1024 MiB.
> >>> I did
> >>>     printf("%08x %08x\n",
> >>>            get_ram_size(0, 512<<20),
> >>>            get_ram_size(0, 1024<<20));
> >>> on a 512 MiB board and
> >>>     0x20000000 0x40000000
> >>> was printed.  
> >>
> >> Very strange. Could you please debug this issue? get_ram_size()
> >> should be able to work in such situations.
> >>
> >> Thanks,
> >> Stefan
> >>      
> >>>>         
> >>>>> ATF does not run RAM size determining code either, it just gets
> >>>>> RAM size from a register, this register is written before ATF by
> >>>>> BootROM and we have done it so that there is always 1 GB so that
> >>>>> we could use same secure firmware image for all Moxes. I tried
> >>>>> to change this register in secure firmware, but this lead to
> >>>>> Synchornous Abort events in U-Boot.
> >>>>>
> >>>>> Maybe we could move the dram_init funcitons from arm64-common.c
> >>>>> to specific board files, or maybe we could declare them __weak
> >>>>> in arm64-common.c and turris_mox can then redefine them.
> >>>>>
> >>>>> Would that be OK with you?  
> >>>>
> >>>> Please fist check if get_ram_size() can't be used.
> >>>>
> >>>> Thanks,
> >>>> Stefan
> >>>>         
> >>>>> Marek
> >>>>>
> >>>>> On Thu, 29 Nov 2018 14:07:59 +0100
> >>>>> Stefan Roese <sr@denx.de> wrote:
> >>>>>         
> >>>>>> On 20.11.18 13:04, Marek Behún wrote:  
> >>>>>>> Depending on the data in the OTP memory, differentiate between
> >>>>>>> the 512 MiB and 1 GiB versions of Turris Mox and report these
> >>>>>>> RAM sizes in dram_init and dram_init_banksize.
> >>>>>>>
> >>>>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> >>>>>>> ---
> >>>>>>>      arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
> >>>>>>>      board/CZ.NIC/turris_mox/turris_mox.c | 27
> >>>>>>> +++++++++++++++++++++++++++ 2 files changed, 33
> >>>>>>> insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm/mach-mvebu/arm64-common.c
> >>>>>>> b/arch/arm/mach-mvebu/arm64-common.c index
> >>>>>>> f47273fde9..5e6ac9fc4a 100644 ---
> >>>>>>> a/arch/arm/mach-mvebu/arm64-common.c +++
> >>>>>>> b/arch/arm/mach-mvebu/arm64-common.c @@ -43,8 +43,12 @@ const
> >>>>>>> struct mbus_dram_target_info *mvebu_mbus_dram_info(void)
> >>>>>>> return NULL; }
> >>>>>>>      
> >>>>>>> -/* DRAM init code ... */
> >>>>>>> +/*
> >>>>>>> + * DRAM init code ...
> >>>>>>> + * Turris Mox defines this itself, depending on data in
> >>>>>>> burned eFuses
> >>>>>>> + */
> >>>>>>>      
> >>>>>>> +#ifndef CONFIG_TARGET_TURRIS_MOX
> >>>>>>>      int dram_init_banksize(void)
> >>>>>>>      {
> >>>>>>>      	fdtdec_setup_memory_banksize();
> >>>>>>> @@ -59,6 +63,7 @@ int dram_init(void)
> >>>>>>>      
> >>>>>>>      	return 0;
> >>>>>>>      }
> >>>>>>> +#endif /* !CONFIG_TARGET_TURRIS_MOX */  
> >>>>>>
> >>>>>> 2 Problems with this:
> >>>>>>
> >>>>>> a)
> >>>>>> This does not apply any more with the latest changes in
> >>>>>> mainline.
> >>>>>>
> >>>>>> b)
> >>>>>> I really don't like #ifdef's here in this common code. Can you
> >>>>>> not get rid of this somehow? Isn't the turris_mox also using
> >>>>>> ATF and will read the RAM size from there?
> >>>>>>
> >>>>>> U-Boot still has the good old get_ram_size() function, which
> >>>>>> can easily auto-detect 512MiB vs 1GiB when run with 1GiB as
> >>>>>> parameter.
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Stefan
> >>>>>>        
> >>>>>>>      
> >>>>>>>      int arch_cpu_init(void)
> >>>>>>>      {
> >>>>>>> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
> >>>>>>> b/board/CZ.NIC/turris_mox/turris_mox.c index
> >>>>>>> 89b3cd2ce0..9aa2fc004d 100644 ---
> >>>>>>> a/board/CZ.NIC/turris_mox/turris_mox.c +++
> >>>>>>> b/board/CZ.NIC/turris_mox/turris_mox.c @@ -14,6 +14,7 @@
> >>>>>>>      #include <linux/string.h>
> >>>>>>>      #include <linux/libfdt.h>
> >>>>>>>      #include <fdt_support.h>
> >>>>>>> +#include <environment.h>
> >>>>>>>      
> >>>>>>>      #ifdef CONFIG_WDT_ARMADA_37XX
> >>>>>>>      #include <wdt.h>
> >>>>>>> @@ -40,6 +41,32 @@
> >>>>>>>      
> >>>>>>>      DECLARE_GLOBAL_DATA_PTR;
> >>>>>>>      
> >>>>>>> +int dram_init(void)
> >>>>>>> +{
> >>>>>>> +	int ret, ram_size;
> >>>>>>> +
> >>>>>>> +	gd->ram_base = 0;
> >>>>>>> +	gd->ram_size = (phys_size_t)0x20000000;
> >>>>>>> +
> >>>>>>> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
> >>>>>>> &ram_size);
> >>>>>>> +	if (ret < 0) {
> >>>>>>> +		puts("Cannot read RAM size from OTP,
> >>>>>>> defaulting to 512 MiB");
> >>>>>>> +	} else {
> >>>>>>> +		if (ram_size == 1024)
> >>>>>>> +			gd->ram_size =
> >>>>>>> (phys_size_t)0x40000000;
> >>>>>>> +	}
> >>>>>>> +
> >>>>>>> +	return 0;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +int dram_init_banksize(void)
> >>>>>>> +{
> >>>>>>> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
> >>>>>>> +	gd->bd->bi_dram[0].size = gd->ram_size;
> >>>>>>> +
> >>>>>>> +	return 0;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>>      #if defined(CONFIG_OF_BOARD_FIXUP)
> >>>>>>>      int board_fix_fdt(void *blob)
> >>>>>>>      {
> >>>>>>>            
> >>>>>>
> >>>>>> Viele Grüße,
> >>>>>> Stefan
> >>>>>>        
> >>>>>         
> >>>>
> >>>> Viele Grüße,
> >>>> Stefan
> >>>>     
> >>>      
> >>
> >> Viele Grüße,
> >> Stefan
> >>  
> >   
> 
> Viele Grüße,
> Stefan
> 

[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Stefan Roese]

Hi Marek,

On 13.12.18 04:53, Marek Behun wrote:
> it turned out that what I found out was not causing the bug.
> get_ram_size reported 1 GiB of ram because I tried it when dcache was
> already enabled. If I call get_ram_size in dram_init, it returns the
> correct size on both 512 MiB and 1 GiB board.
> 
> In the next patch I shall define dram_init and dram_init_banksize in
> arm64-common.c as __weak, and the definition in turris_mox.c shall call
> get_ram_size. Is this acceptable?

Okay, please prepare the patch and I'll review it then.

Thanks,
Stefan
  
> Marek
> 
> On Wed, 12 Dec 2018 10:44:15 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
>> Hi Marek,
>>
>> On 12.12.18 03:23, Marek Behun wrote:
>>> Hi, I have found the bug causing this issue.
>>
>> Good.
>>    
>>> If I understand the algorithm in get_ram_size correctly, it does
>>> approximately this. Suppose A, B, C, D, E, F are different
>>> constatnts. X(i) is a value at address 1<<i (couting in longs).
>>>
>>> save[5] <- X(5)
>>> X(5) <- F
>>> save[4] <- X(4)
>>> X(4) <- E
>>> save[3] <- X(3)
>>> X(3) <- D
>>> save[2] <- X(2)
>>> X(2) <- C
>>> save[1] <- X(1)
>>> X(1) <- B
>>> save[0] <- X(0)
>>> X(0) <- A
>>>
>>> So the previous values are stored in array save[]. The algorithm
>>> then checks if the values written (the constants A, B, C, D, E, F)
>>> are present at those addresses. The problem is that the previous
>>> value from save[] is written during checking of address i:
>>>
>>> Now suppose the RAM is wrapped similarily as in MOX, so that X(i+3)
>>> is the same as X(i).
>>>
>>> After the first part, the values are as follows
>>>
>>> X([0,1,2,3,4,5]) = [A,B,C,A,B,C]
>>> save = [D,E,F,_3,_4,_5]
>>>
>>> Here _3, _4, _5 are the values at addresses X(3), X(4), X(5) before
>>> the algorithm.
>>>
>>> The code that checks the values written does this:
>>>
>>> if X(0) != A
>>>       return 0
>>> X(0) <- save[0]       !!! this also writes D to X(3)
>>>
>>> if X(1) != B
>>>       return 1
>>> X(1) <- save[1]       !!! this also writes E to X(4)
>>>
>>> if X(2) != C
>>>       return 2
>>> X(2) <- save[2]       !!! this also writes F to X(F)
>>>
>>> if X(3) != D
>>>       return 3          !!! this should return, but won't
>>> X(3) <- save[3]
>>>
>>> ...
>>>
>>> One solution would be to write the previous values from the array
>>> save[] only immediately before return from the function.
>>
>> I have to admit that I didn't fully try to understand this issue you
>> describe above (sorry, lack of time). If you have found a bug and do
>> have a fix for it, then please submit a patch. Please add all
>> developers (e.g. Patrick Delaunay etc) who did some work on this code
>> to Cc, as changes here might be critical.
>>    
>>> I have to confess that I do not like how this function is written at
>>> all. It does not, for example, solve correctly the case when a
>>> device has 768 MiB of RAM from two chips (512 + 256). Given 1024
>>> MiB as argument, it would return 1024 MiB, but the system only has
>>> 768 MiB. This maybe is never an issue with devices that run u-boot,
>>> but still.
>>
>> If you have a nice and easy implementation to also support such
>> memory configurations, that would be perfect of course. But I really
>> think that such non-power-of-2 memory configurations are rather
>> uncommon for U-Boot and most likely don't need to be supported by
>> this function. Such configuration usually are a result of using
>> multiple DIMM's (or SODIMM's) which can be equipped with various
>> sized memories. And here the memory size can be read from the DIMM
>> itself. So no need to support this in get_ram_size().
>>
>> Thanks,
>> Stefan
>>    
>>> Marek
>>>
>>> On Tue, 11 Dec 2018 16:06:42 +0100
>>> Stefan Roese <sr@denx.de> wrote:
>>>    
>>>> On 11.12.18 15:53, Marek Behún wrote:
>>>>> On Tue, 11 Dec 2018 15:28:11 +0100
>>>>> Stefan Roese <sr@denx.de> wrote:
>>>>>       
>>>>>> Hi Marek,
>>>>>>
>>>>>> On 11.12.18 14:59, Marek Behún wrote:
>>>>>>> get_ram_size does not work correctly on Mox. On a 512 MiB board
>>>>>>> it detects 1024 MiB of RAM, because on the 512 MiB RAM chip the
>>>>>>> topmost address bit is simply ignored and the RAM wraps - on
>>>>>>> 0x20000000-0x40000000 CPU sees the same data as on
>>>>>>> 0x0-0x20000000.
>>>>>>
>>>>>> That's what get_ram_size() does: It does detect such aliases when
>>>>>> the same memory is mapped at multiple areas (power of 2). Did you
>>>>>> give it a try with a max value of 1024 MiB? It should return
>>>>>> 512 on such boards.
>>>>>
>>>>> I checked it and it returned 1024 MiB.
>>>>> I did
>>>>>      printf("%08x %08x\n",
>>>>>             get_ram_size(0, 512<<20),
>>>>>             get_ram_size(0, 1024<<20));
>>>>> on a 512 MiB board and
>>>>>      0x20000000 0x40000000
>>>>> was printed.
>>>>
>>>> Very strange. Could you please debug this issue? get_ram_size()
>>>> should be able to work in such situations.
>>>>
>>>> Thanks,
>>>> Stefan
>>>>       
>>>>>>          
>>>>>>> ATF does not run RAM size determining code either, it just gets
>>>>>>> RAM size from a register, this register is written before ATF by
>>>>>>> BootROM and we have done it so that there is always 1 GB so that
>>>>>>> we could use same secure firmware image for all Moxes. I tried
>>>>>>> to change this register in secure firmware, but this lead to
>>>>>>> Synchornous Abort events in U-Boot.
>>>>>>>
>>>>>>> Maybe we could move the dram_init funcitons from arm64-common.c
>>>>>>> to specific board files, or maybe we could declare them __weak
>>>>>>> in arm64-common.c and turris_mox can then redefine them.
>>>>>>>
>>>>>>> Would that be OK with you?
>>>>>>
>>>>>> Please fist check if get_ram_size() can't be used.
>>>>>>
>>>>>> Thanks,
>>>>>> Stefan
>>>>>>          
>>>>>>> Marek
>>>>>>>
>>>>>>> On Thu, 29 Nov 2018 14:07:59 +0100
>>>>>>> Stefan Roese <sr@denx.de> wrote:
>>>>>>>          
>>>>>>>> On 20.11.18 13:04, Marek Behún wrote:
>>>>>>>>> Depending on the data in the OTP memory, differentiate between
>>>>>>>>> the 512 MiB and 1 GiB versions of Turris Mox and report these
>>>>>>>>> RAM sizes in dram_init and dram_init_banksize.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>>>>>>>> ---
>>>>>>>>>       arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
>>>>>>>>>       board/CZ.NIC/turris_mox/turris_mox.c | 27
>>>>>>>>> +++++++++++++++++++++++++++ 2 files changed, 33
>>>>>>>>> insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/mach-mvebu/arm64-common.c
>>>>>>>>> b/arch/arm/mach-mvebu/arm64-common.c index
>>>>>>>>> f47273fde9..5e6ac9fc4a 100644 ---
>>>>>>>>> a/arch/arm/mach-mvebu/arm64-common.c +++
>>>>>>>>> b/arch/arm/mach-mvebu/arm64-common.c @@ -43,8 +43,12 @@ const
>>>>>>>>> struct mbus_dram_target_info *mvebu_mbus_dram_info(void)
>>>>>>>>> return NULL; }
>>>>>>>>>       
>>>>>>>>> -/* DRAM init code ... */
>>>>>>>>> +/*
>>>>>>>>> + * DRAM init code ...
>>>>>>>>> + * Turris Mox defines this itself, depending on data in
>>>>>>>>> burned eFuses
>>>>>>>>> + */
>>>>>>>>>       
>>>>>>>>> +#ifndef CONFIG_TARGET_TURRIS_MOX
>>>>>>>>>       int dram_init_banksize(void)
>>>>>>>>>       {
>>>>>>>>>       	fdtdec_setup_memory_banksize();
>>>>>>>>> @@ -59,6 +63,7 @@ int dram_init(void)
>>>>>>>>>       
>>>>>>>>>       	return 0;
>>>>>>>>>       }
>>>>>>>>> +#endif /* !CONFIG_TARGET_TURRIS_MOX */
>>>>>>>>
>>>>>>>> 2 Problems with this:
>>>>>>>>
>>>>>>>> a)
>>>>>>>> This does not apply any more with the latest changes in
>>>>>>>> mainline.
>>>>>>>>
>>>>>>>> b)
>>>>>>>> I really don't like #ifdef's here in this common code. Can you
>>>>>>>> not get rid of this somehow? Isn't the turris_mox also using
>>>>>>>> ATF and will read the RAM size from there?
>>>>>>>>
>>>>>>>> U-Boot still has the good old get_ram_size() function, which
>>>>>>>> can easily auto-detect 512MiB vs 1GiB when run with 1GiB as
>>>>>>>> parameter.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Stefan
>>>>>>>>         
>>>>>>>>>       
>>>>>>>>>       int arch_cpu_init(void)
>>>>>>>>>       {
>>>>>>>>> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
>>>>>>>>> b/board/CZ.NIC/turris_mox/turris_mox.c index
>>>>>>>>> 89b3cd2ce0..9aa2fc004d 100644 ---
>>>>>>>>> a/board/CZ.NIC/turris_mox/turris_mox.c +++
>>>>>>>>> b/board/CZ.NIC/turris_mox/turris_mox.c @@ -14,6 +14,7 @@
>>>>>>>>>       #include <linux/string.h>
>>>>>>>>>       #include <linux/libfdt.h>
>>>>>>>>>       #include <fdt_support.h>
>>>>>>>>> +#include <environment.h>
>>>>>>>>>       
>>>>>>>>>       #ifdef CONFIG_WDT_ARMADA_37XX
>>>>>>>>>       #include <wdt.h>
>>>>>>>>> @@ -40,6 +41,32 @@
>>>>>>>>>       
>>>>>>>>>       DECLARE_GLOBAL_DATA_PTR;
>>>>>>>>>       
>>>>>>>>> +int dram_init(void)
>>>>>>>>> +{
>>>>>>>>> +	int ret, ram_size;
>>>>>>>>> +
>>>>>>>>> +	gd->ram_base = 0;
>>>>>>>>> +	gd->ram_size = (phys_size_t)0x20000000;
>>>>>>>>> +
>>>>>>>>> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
>>>>>>>>> &ram_size);
>>>>>>>>> +	if (ret < 0) {
>>>>>>>>> +		puts("Cannot read RAM size from OTP,
>>>>>>>>> defaulting to 512 MiB");
>>>>>>>>> +	} else {
>>>>>>>>> +		if (ram_size == 1024)
>>>>>>>>> +			gd->ram_size =
>>>>>>>>> (phys_size_t)0x40000000;
>>>>>>>>> +	}
>>>>>>>>> +
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +int dram_init_banksize(void)
>>>>>>>>> +{
>>>>>>>>> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
>>>>>>>>> +	gd->bd->bi_dram[0].size = gd->ram_size;
>>>>>>>>> +
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>>       #if defined(CONFIG_OF_BOARD_FIXUP)
>>>>>>>>>       int board_fix_fdt(void *blob)
>>>>>>>>>       {
>>>>>>>>>             
>>>>>>>>
>>>>>>>> Viele Grüße,
>>>>>>>> Stefan
>>>>>>>>         
>>>>>>>          
>>>>>>
>>>>>> Viele Grüße,
>>>>>> Stefan
>>>>>>      
>>>>>       
>>>>
>>>> Viele Grüße,
>>>> Stefan
>>>>   
>>>    
>>
>> Viele Grüße,
>> Stefan
>>
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 10/10] configs: turris_mox: Add 64 MiB of boot memory

[Marek Behún]

This is needed for some scenarios, such as booting large FIT image.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 include/configs/turris_mox.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/turris_mox.h b/include/configs/turris_mox.h
index 0aebe2100b..82cdccecc1 100644
--- a/include/configs/turris_mox.h
+++ b/include/configs/turris_mox.h
@@ -8,6 +8,8 @@
 #ifndef _CONFIG_TURRIS_MOX_H
 #define _CONFIG_TURRIS_MOX_H
 
+#define CONFIG_SYS_BOOTM_LEN (64 << 20)
+
 #define CONFIG_LAST_STAGE_INIT
 
 /*
-- 
2.18.1

[U-Boot] [PATCH u-boot-marvell v3 10/10] configs: turris_mox: Add 64 MiB of boot memory

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> This is needed for some scenarios, such as booting large FIT image.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   include/configs/turris_mox.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/include/configs/turris_mox.h b/include/configs/turris_mox.h
> index 0aebe2100b..82cdccecc1 100644
> --- a/include/configs/turris_mox.h
> +++ b/include/configs/turris_mox.h
> @@ -8,6 +8,8 @@
>   #ifndef _CONFIG_TURRIS_MOX_H
>   #define _CONFIG_TURRIS_MOX_H
>   
> +#define CONFIG_SYS_BOOTM_LEN (64 << 20)
> +
>   #define CONFIG_LAST_STAGE_INIT
>   
>   /*
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[U-Boot] [PATCH u-boot-marvell v3 01/10] board: turris_mox: Cosmetic restructurization

[Stefan Roese]

On 20.11.18 13:04, Marek Behún wrote:
> Restructure the board initialization source.
> Remove the module_topology environment variable since it won't be
> needed.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   board/CZ.NIC/turris_mox/turris_mox.c | 136 ++++++++++++++++++---------
>   1 file changed, 89 insertions(+), 47 deletions(-)
> 
> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c
> index c4622a49c2..415c462493 100644
> --- a/board/CZ.NIC/turris_mox/turris_mox.c
> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
> @@ -135,17 +135,15 @@ int board_init(void)
>   	return 0;
>   }
>   
> -int last_stage_init(void)
> +static int mox_do_spi(u8 *in, u8 *out, size_t size)
>   {
>   	struct spi_slave *slave;
>   	struct udevice *dev;
> -	u8 din[10], dout[10];
> -	int ret, i;
> -	size_t len = 0;
> -	char module_topology[128];
> +	int ret;
>   
> -	ret = spi_get_bus_and_cs(0, 1, 20000000, SPI_CPHA, "spi_generic_drv",
> -				 "mox-modules at 1", &dev, &slave);
> +	ret = spi_get_bus_and_cs(0, 1, 1000000, SPI_CPHA | SPI_CPOL,
> +				 "spi_generic_drv", "moxtet at 1", &dev,
> +				 &slave);
>   	if (ret)
>   		goto fail;
>   
> @@ -153,57 +151,101 @@ int last_stage_init(void)
>   	if (ret)
>   		goto fail_free;
>   
> -	memset(din, 0, 10);
> -	memset(dout, 0, 10);
> +	ret = spi_xfer(slave, size * 8, out, in, SPI_XFER_ONCE);
> +
> +	spi_release_bus(slave);
> +fail_free:
> +	spi_free_slave(slave);
> +fail:
> +	return ret;
> +}
> +
> +static int mox_get_topology(const u8 **ptopology, int *psize, int *pis_sd)
> +{
> +	static int is_sd;
> +	static u8 topology[MAX_MOX_MODULES - 1];
> +	static int size;
> +	u8 din[MAX_MOX_MODULES], dout[MAX_MOX_MODULES];
> +	int ret, i;
>   
> -	ret = spi_xfer(slave, 80, dout, din, SPI_XFER_ONCE);
> +	if (size) {
> +		if (ptopology)
> +			*ptopology = topology;
> +		if (psize)
> +			*psize = size;
> +		if (pis_sd)
> +			*pis_sd = is_sd;
> +		return 0;
> +	}
> +
> +	memset(din, 0, MAX_MOX_MODULES);
> +	memset(dout, 0, MAX_MOX_MODULES);
> +
> +	ret = mox_do_spi(din, dout, MAX_MOX_MODULES);
>   	if (ret)
> -		goto fail_release;
> +		return ret;
> +
> +	if (din[0] == 0x10)
> +		is_sd = 1;
> +	else if (din[0] == 0x00)
> +		is_sd = 0;
> +	else
> +		return -ENODEV;
> +
> +	for (i = 1; i < MAX_MOX_MODULES && din[i] != 0xff; ++i)
> +		topology[i - 1] = din[i] & 0xf;
> +	size = i - 1;
> +
> +	if (ptopology)
> +		*ptopology = topology;
> +	if (psize)
> +		*psize = size;
> +	if (pis_sd)
> +		*pis_sd = is_sd;
> +
> +	return 0;
> +}
>   
> -	if (din[0] != 0x00 && din[0] != 0xff)
> -		goto fail_release;
> +int last_stage_init(void)
> +{
> +	int ret, i;
> +	const u8 *topology;
> +	int module_count, is_sd;
> +
> +	ret = mox_get_topology(&topology, &module_count, &is_sd);
> +	if (ret) {
> +		printf("Cannot read module topology!\n");
> +		return 0;
> +	}
>   
> +	printf("Found Turris Mox %s version\n", is_sd ? "SD" : "eMMC");
>   	printf("Module Topology:\n");
> -	for (i = 1; i < 10 && din[i] != 0xff; ++i) {
> -		u8 mid = din[i] & 0xf;
> -		size_t mlen;
> -		const char *mname = "";
> -
> -		switch (mid) {
> -		case 0x1:
> -			mname = "sfp-";
> -			printf("% 4i: SFP Module\n", i);
> +	for (i = 0; i < module_count; ++i) {
> +		switch (topology[i]) {
> +		case MOX_MODULE_SFP:
> +			printf("% 4i: SFP Module\n", i + 1);
> +			break;
> +		case MOX_MODULE_PCI:
> +			printf("% 4i: Mini-PCIe Module\n", i + 1);
> +			break;
> +		case MOX_MODULE_TOPAZ:
> +			printf("% 4i: Topaz Switch Module (4-port)\n", i + 1);
>   			break;
> -		case 0x2:
> -			mname = "pci-";
> -			printf("% 4i: Mini-PCIe Module\n", i);
> +		case MOX_MODULE_PERIDOT:
> +			printf("% 4i: Peridot Switch Module (8-port)\n", i + 1);
>   			break;
> -		case 0x3:
> -			mname = "topaz-";
> -			printf("% 4i: Topaz Switch Module\n", i);
> +		case MOX_MODULE_USB3:
> +			printf("% 4i: USB 3.0 Module (4 ports)\n", i + 1);
> +			break;
> +		case MOX_MODULE_PASSPCI:
> +			printf("% 4i: Passthrough Mini-PCIe Module\n", i + 1);
>   			break;
>   		default:
> -			printf("% 4i: unknown (ID %i)\n", i, mid);
> -		}
> -
> -		mlen = strlen(mname);
> -		if (len + mlen < sizeof(module_topology)) {
> -			strcpy(module_topology + len, mname);
> -			len += mlen;
> +			printf("% 4i: unknown (ID %i)\n", i + 1, topology[i]);
>   		}
>   	}
> -	printf("\n");
> -
> -	module_topology[len > 0 ? len - 1 : 0] = '\0';
>   
> -	env_set("module_topology", module_topology);
> +	printf("\n");
>   
> -fail_release:
> -	spi_release_bus(slave);
> -fail_free:
> -	spi_free_slave(slave);
> -fail:
> -	if (ret)
> -		printf("Cannot read module topology!\n");
> -	return ret;
> +	return 0;
>   }
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de