[PATCH u-boot-marvell 1/4] arm: mvebu: turris_mox: Fix early SPI communication

The SPI clock signal changes value when the SPI configuration register is configured. This can sometimes lead to the device misinterpreting the enablement of the SPI controller as actual clock tick. This can be solved by first setting the SPI CS1 pin from GPIO to SPI mode, and only after that writing the SPI configuration register.
Signed-off-by: Marek Behún marek.behun@nic.cz --- board/CZ.NIC/turris_mox/turris_mox.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 377191baef..7118096f32 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -67,9 +67,9 @@ int board_fix_fdt(void *blob) * to read SPI by reading/writing SPI registers directly */
- writel(0x563fa, ARMADA_37XX_NB_GPIO_SEL); writel(0x10df, ARMADA_37XX_SPI_CFG); - writel(0x2005b, ARMADA_37XX_SPI_CTRL); + clrbits_le32(ARMADA_37XX_NB_GPIO_SEL, BIT(12)); + setbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17));
while (!(readl(ARMADA_37XX_SPI_CTRL) & 0x2)) udelay(1); @@ -89,7 +89,7 @@ int board_fix_fdt(void *blob)
size = i;
- writel(0x5b, ARMADA_37XX_SPI_CTRL); + clrbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17));
if (size > 1 && (topology[1] == MOX_MODULE_PCI || topology[1] == MOX_MODULE_USB3 ||

With recent changes to the mmc subsystem (chip detect code etc) update the sdhci node of the Turris Mox device tree.
Signed-off-by: Marek Behún marek.behun@nic.cz --- arch/arm/dts/armada-3720-turris-mox.dts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts index c36a5b8895..4c65c3e32c 100644 --- a/arch/arm/dts/armada-3720-turris-mox.dts +++ b/arch/arm/dts/armada-3720-turris-mox.dts @@ -45,6 +45,20 @@ regulator-boot-on; };
+ vsdc_reg: vsdc-reg { + compatible = "regulator-gpio"; + regulator-name = "vsdc"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + regulator-boot-on; + + gpios = <&gpiosb 23 GPIO_ACTIVE_HIGH>; + gpios-states = <0>; + states = <1800000 0x1 + 3300000 0x0>; + enable-active-high; + }; + mdio { #address-cells = <1>; #size-cells = <0>; @@ -93,7 +107,11 @@ };
&sdhci1 { + wp-inverted; bus-width = <4>; + cd-gpios = <&gpionb 10 GPIO_ACTIVE_HIGH>; + vqmmc-supply = <&vsdc_reg>; + marvell,pad-type = "sd"; status = "okay"; };

On 30.03.20 19:19, Marek Behún wrote:
With recent changes to the mmc subsystem (chip detect code etc) update the sdhci node of the Turris Mox device tree.
Signed-off-by: Marek Behún marek.behun@nic.cz
arch/arm/dts/armada-3720-turris-mox.dts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts index c36a5b8895..4c65c3e32c 100644 --- a/arch/arm/dts/armada-3720-turris-mox.dts +++ b/arch/arm/dts/armada-3720-turris-mox.dts @@ -45,6 +45,20 @@ regulator-boot-on; };
- vsdc_reg: vsdc-reg {
compatible = "regulator-gpio";
regulator-name = "vsdc";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <3300000>;
regulator-boot-on;
gpios = <&gpiosb 23 GPIO_ACTIVE_HIGH>;
gpios-states = <0>;
states = <1800000 0x1
3300000 0x0>;
enable-active-high;
- };
- mdio { #address-cells = <1>; #size-cells = <0>;
@@ -93,7 +107,11 @@ };
&sdhci1 {
- wp-inverted; bus-width = <4>;
- cd-gpios = <&gpionb 10 GPIO_ACTIVE_HIGH>;
- vqmmc-supply = <&vsdc_reg>;
- marvell,pad-type = "sd"; status = "okay"; };
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan

Patch Linux's device tree according to which Mox modules are connected. Linux's device tree is supposed to have some nodes already preprogrammed. If user wants to use different device tree, they should disable CONFIG_OF_BOARD_SETUP in U-Boot's config, so that the boot command does not fail.
Signed-off-by: Marek Behún marek.behun@nic.cz --- board/CZ.NIC/turris_mox/turris_mox.c | 247 +++++++++++++++++++++++++++ configs/turris_mox_defconfig | 1 + 2 files changed, 248 insertions(+)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 7118096f32..5137ed9d00 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -3,6 +3,7 @@ * Copyright (C) 2018 Marek Behun marek.behun@nic.cz */
+#include <stdarg.h> #include <common.h> #include <init.h> #include <asm/gpio.h> @@ -34,7 +35,11 @@ #define ARMADA_37XX_SPI_DOUT 0xd0010608 #define ARMADA_37XX_SPI_DIN 0xd001060c
+#define ETH1_PATH "/soc/internal-regs@d0000000/ethernet@40000" +#define MDIO_PATH "/soc/internal-regs@d0000000/mdio@32004" +#define SFP_GPIO_PATH "/soc/internal-regs@d0000000/spi@10600/moxtet@1/gpio@0" #define PCIE_PATH "/soc/pcie@d0070000" +#define SFP_PATH "/sfp"
DECLARE_GLOBAL_DATA_PTR;
@@ -548,3 +553,245 @@ int last_stage_init(void)
return 0; } + +#if defined(CONFIG_OF_BOARD_SETUP) + +static int vnode_by_path(void *blob, const char *fmt, va_list ap) +{ + char path[128]; + + vsprintf(path, fmt, ap); + return fdt_path_offset(blob, path); +} + +static int node_by_path(void *blob, const char *fmt, ...) +{ + va_list ap; + int res; + + va_start(ap, fmt); + res = vnode_by_path(blob, fmt, ap); + va_end(ap); + + return res; +} + +static int phandle_by_path(void *blob, const char *fmt, ...) +{ + va_list ap; + int node, phandle, res; + + va_start(ap, fmt); + node = vnode_by_path(blob, fmt, ap); + va_end(ap); + + if (node < 0) + return node; + + phandle = fdt_get_phandle(blob, node); + if (phandle > 0) + return phandle; + + phandle = fdt_get_max_phandle(blob); + if (phandle < 0) + return phandle; + + phandle += 1; + + res = fdt_setprop_u32(blob, node, "linux,phandle", phandle); + if (res < 0) + return res; + + res = fdt_setprop_u32(blob, node, "phandle", phandle); + if (res < 0) + return res; + + return phandle; +} + +static int enable_by_path(void *blob, const char *fmt, ...) +{ + va_list ap; + int node; + + va_start(ap, fmt); + node = vnode_by_path(blob, fmt, ap); + va_end(ap); + + if (node < 0) + return node; + + return fdt_setprop_string(blob, node, "status", "okay"); +} + +static bool is_topaz(int id) +{ + return topaz && id == peridot + topaz - 1; +} + +static int switch_addr(int id) +{ + return is_topaz(id) ? 0x2 : 0x10 + id; +} + +static int setup_switch(void *blob, int id) +{ + int res, addr, i, node, phandle; + + addr = switch_addr(id); + + /* first enable the switch by setting status = "okay" */ + res = enable_by_path(blob, MDIO_PATH "/switch%i@%x", id, addr); + if (res < 0) + return res; + + /* + * now if there are more switches or a SFP module coming after, + * enable corresponding ports + */ + if (id < peridot + topaz - 1) + res = enable_by_path(blob, + MDIO_PATH "/switch%i@%x/ports/port@a", + id, addr); + else if (id == peridot - 1 && !topaz && sfp) + res = enable_by_path(blob, + MDIO_PATH "/switch%i@%x/ports/port-sfp@a", + id, addr); + else + res = 0; + if (res < 0) + return res; + + if (id >= peridot + topaz - 1) + return 0; + + /* finally change link property if needed */ + node = node_by_path(blob, MDIO_PATH "/switch%i@%x/ports/port@a", id, + addr); + if (node < 0) + return node; + + for (i = id + 1; i < peridot + topaz; ++i) { + phandle = phandle_by_path(blob, + MDIO_PATH "/switch%i@%x/ports/port@%x", + i, switch_addr(i), + is_topaz(i) ? 5 : 9); + if (phandle < 0) + return phandle; + + if (i == id + 1) + res = fdt_setprop_u32(blob, node, "link", phandle); + else + res = fdt_appendprop_u32(blob, node, "link", phandle); + if (res < 0) + return res; + } + + return 0; +} + +static int remove_disabled_nodes(void *blob) +{ + while (1) { + int res, offset; + + offset = fdt_node_offset_by_prop_value(blob, -1, "status", + "disabled", 9); + if (offset < 0) + break; + + res = fdt_del_node(blob, offset); + if (res < 0) + return res; + } + + return 0; +} + +int ft_board_setup(void *blob, bd_t *bd) +{ + int node, phandle, res; + + if (pci || usb || passpci) { + node = fdt_path_offset(blob, PCIE_PATH); + if (node < 0) + return node; + + res = fdt_setprop_string(blob, node, "status", "okay"); + if (res < 0) + return res; + } + + if (peridot || topaz) { + int i; + + res = enable_by_path(blob, ETH1_PATH); + if (res < 0) + return res; + + for (i = 0; i < peridot + topaz; ++i) { + res = setup_switch(blob, i); + if (res < 0) + return res; + } + } + + if (sfp) { + res = enable_by_path(blob, SFP_PATH); + if (res < 0) + return res; + + res = enable_by_path(blob, ETH1_PATH); + if (res < 0) + return res; + + if (!peridot) { + phandle = phandle_by_path(blob, SFP_PATH); + if (phandle < 0) + return res; + + node = node_by_path(blob, ETH1_PATH); + if (node < 0) + return node; + + res = fdt_setprop_u32(blob, node, "sfp", phandle); + if (res < 0) + return res; + + res = fdt_setprop_string(blob, node, "phy-mode", + "sgmii"); + if (res < 0) + return res; + } + + res = enable_by_path(blob, SFP_GPIO_PATH); + if (res < 0) + return res; + + if (sfp_pos) { + char newname[16]; + + /* moxtet-sfp is on non-zero position, change default */ + node = node_by_path(blob, SFP_GPIO_PATH); + if (node < 0) + return node; + + res = fdt_setprop_u32(blob, node, "reg", sfp_pos); + if (res < 0) + return res; + + sprintf(newname, "gpio@%x", sfp_pos); + + res = fdt_set_name(blob, node, newname); + if (res < 0) + return res; + } + } + + fdt_fixup_ethernet(blob); + remove_disabled_nodes(blob); + + return 0; +} + +#endif diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig index 2e637044c1..3bc69cda4d 100644 --- a/configs/turris_mox_defconfig +++ b/configs/turris_mox_defconfig @@ -37,6 +37,7 @@ CONFIG_CMD_BTRFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_MAC_PARTITION=y CONFIG_OF_BOARD_FIXUP=y +CONFIG_OF_BOARD_SETUP=y CONFIG_DEFAULT_DEVICE_TREE="armada-3720-turris-mox" CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y

Hi Marek,
On 30.03.20 19:19, Marek Behún wrote:
Patch Linux's device tree according to which Mox modules are connected. Linux's device tree is supposed to have some nodes already preprogrammed. If user wants to use different device tree, they should disable CONFIG_OF_BOARD_SETUP in U-Boot's config, so that the boot command does not fail.
There is a lot that you dynamically change in the DT here. Could you please summarize a bit more of the details in the commit text and the code of what exactly is being done / changed in the DT?
Thanks, Stefan
Signed-off-by: Marek Behún marek.behun@nic.cz
board/CZ.NIC/turris_mox/turris_mox.c | 247 +++++++++++++++++++++++++++ configs/turris_mox_defconfig | 1 + 2 files changed, 248 insertions(+)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 7118096f32..5137ed9d00 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -3,6 +3,7 @@
- Copyright (C) 2018 Marek Behun marek.behun@nic.cz
*/
+#include <stdarg.h> #include <common.h> #include <init.h> #include <asm/gpio.h> @@ -34,7 +35,11 @@ #define ARMADA_37XX_SPI_DOUT 0xd0010608 #define ARMADA_37XX_SPI_DIN 0xd001060c
+#define ETH1_PATH "/soc/internal-regs@d0000000/ethernet@40000" +#define MDIO_PATH "/soc/internal-regs@d0000000/mdio@32004" +#define SFP_GPIO_PATH "/soc/internal-regs@d0000000/spi@10600/moxtet@1/gpio@0" #define PCIE_PATH "/soc/pcie@d0070000" +#define SFP_PATH "/sfp"
DECLARE_GLOBAL_DATA_PTR;
@@ -548,3 +553,245 @@ int last_stage_init(void)
return 0; }
+#if defined(CONFIG_OF_BOARD_SETUP)
+static int vnode_by_path(void *blob, const char *fmt, va_list ap) +{
- char path[128];
- vsprintf(path, fmt, ap);
- return fdt_path_offset(blob, path);
+}
+static int node_by_path(void *blob, const char *fmt, ...) +{
- va_list ap;
- int res;
- va_start(ap, fmt);
- res = vnode_by_path(blob, fmt, ap);
- va_end(ap);
- return res;
+}
+static int phandle_by_path(void *blob, const char *fmt, ...) +{
- va_list ap;
- int node, phandle, res;
- va_start(ap, fmt);
- node = vnode_by_path(blob, fmt, ap);
- va_end(ap);
- if (node < 0)
return node;
- phandle = fdt_get_phandle(blob, node);
- if (phandle > 0)
return phandle;
- phandle = fdt_get_max_phandle(blob);
- if (phandle < 0)
return phandle;
- phandle += 1;
- res = fdt_setprop_u32(blob, node, "linux,phandle", phandle);
- if (res < 0)
return res;
- res = fdt_setprop_u32(blob, node, "phandle", phandle);
- if (res < 0)
return res;
- return phandle;
+}
+static int enable_by_path(void *blob, const char *fmt, ...) +{
- va_list ap;
- int node;
- va_start(ap, fmt);
- node = vnode_by_path(blob, fmt, ap);
- va_end(ap);
- if (node < 0)
return node;
- return fdt_setprop_string(blob, node, "status", "okay");
+}
+static bool is_topaz(int id) +{
- return topaz && id == peridot + topaz - 1;
+}
+static int switch_addr(int id) +{
- return is_topaz(id) ? 0x2 : 0x10 + id;
+}
+static int setup_switch(void *blob, int id) +{
- int res, addr, i, node, phandle;
- addr = switch_addr(id);
- /* first enable the switch by setting status = "okay" */
- res = enable_by_path(blob, MDIO_PATH "/switch%i@%x", id, addr);
- if (res < 0)
return res;
- /*
* now if there are more switches or a SFP module coming after,
* enable corresponding ports
*/
- if (id < peridot + topaz - 1)
res = enable_by_path(blob,
MDIO_PATH "/switch%i@%x/ports/port@a",
id, addr);
- else if (id == peridot - 1 && !topaz && sfp)
res = enable_by_path(blob,
MDIO_PATH "/switch%i@%x/ports/port-sfp@a",
id, addr);
- else
res = 0;
- if (res < 0)
return res;
- if (id >= peridot + topaz - 1)
return 0;
- /* finally change link property if needed */
- node = node_by_path(blob, MDIO_PATH "/switch%i@%x/ports/port@a", id,
addr);
- if (node < 0)
return node;
- for (i = id + 1; i < peridot + topaz; ++i) {
phandle = phandle_by_path(blob,
MDIO_PATH "/switch%i@%x/ports/port@%x",
i, switch_addr(i),
is_topaz(i) ? 5 : 9);
if (phandle < 0)
return phandle;
if (i == id + 1)
res = fdt_setprop_u32(blob, node, "link", phandle);
else
res = fdt_appendprop_u32(blob, node, "link", phandle);
if (res < 0)
return res;
- }
- return 0;
+}
+static int remove_disabled_nodes(void *blob) +{
- while (1) {
int res, offset;
offset = fdt_node_offset_by_prop_value(blob, -1, "status",
"disabled", 9);
if (offset < 0)
break;
res = fdt_del_node(blob, offset);
if (res < 0)
return res;
- }
- return 0;
+}
+int ft_board_setup(void *blob, bd_t *bd) +{
- int node, phandle, res;
- if (pci || usb || passpci) {
node = fdt_path_offset(blob, PCIE_PATH);
if (node < 0)
return node;
res = fdt_setprop_string(blob, node, "status", "okay");
if (res < 0)
return res;
- }
- if (peridot || topaz) {
int i;
res = enable_by_path(blob, ETH1_PATH);
if (res < 0)
return res;
for (i = 0; i < peridot + topaz; ++i) {
res = setup_switch(blob, i);
if (res < 0)
return res;
}
- }
- if (sfp) {
res = enable_by_path(blob, SFP_PATH);
if (res < 0)
return res;
res = enable_by_path(blob, ETH1_PATH);
if (res < 0)
return res;
if (!peridot) {
phandle = phandle_by_path(blob, SFP_PATH);
if (phandle < 0)
return res;
node = node_by_path(blob, ETH1_PATH);
if (node < 0)
return node;
res = fdt_setprop_u32(blob, node, "sfp", phandle);
if (res < 0)
return res;
res = fdt_setprop_string(blob, node, "phy-mode",
"sgmii");
if (res < 0)
return res;
}
res = enable_by_path(blob, SFP_GPIO_PATH);
if (res < 0)
return res;
if (sfp_pos) {
char newname[16];
/* moxtet-sfp is on non-zero position, change default */
node = node_by_path(blob, SFP_GPIO_PATH);
if (node < 0)
return node;
res = fdt_setprop_u32(blob, node, "reg", sfp_pos);
if (res < 0)
return res;
sprintf(newname, "gpio@%x", sfp_pos);
res = fdt_set_name(blob, node, newname);
if (res < 0)
return res;
}
- }
- fdt_fixup_ethernet(blob);
- remove_disabled_nodes(blob);
- return 0;
+}
+#endif diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig index 2e637044c1..3bc69cda4d 100644 --- a/configs/turris_mox_defconfig +++ b/configs/turris_mox_defconfig @@ -37,6 +37,7 @@ CONFIG_CMD_BTRFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_MAC_PARTITION=y CONFIG_OF_BOARD_FIXUP=y +CONFIG_OF_BOARD_SETUP=y CONFIG_DEFAULT_DEVICE_TREE="armada-3720-turris-mox" CONFIG_ENV_IS_IN_SPI_FLASH=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y
Viele Grüße, Stefan

Commit e8e9715df2d4 requires the USB3 regulator node to have the enable-active-high property for the regulator to work properly. The GPIO_ACTIVE_HIGH constant is not enough anymore.
Signed-off-by: Marek Behún marek.behun@nic.cz Fixes: e8e9715df2d4 ("regulator: fixed: Modify enable-active-high...") --- arch/arm/dts/armada-3720-turris-mox.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts index 4c65c3e32c..a1e0ad5020 100644 --- a/arch/arm/dts/armada-3720-turris-mox.dts +++ b/arch/arm/dts/armada-3720-turris-mox.dts @@ -42,6 +42,7 @@ startup-delay-us = <2000000>; shutdown-delay-us = <1000000>; gpio = <&gpiosb 0 GPIO_ACTIVE_HIGH>; + enable-active-high; regulator-boot-on; };

On 30.03.20 19:19, Marek Behún wrote:
Commit e8e9715df2d4 requires the USB3 regulator node to have the enable-active-high property for the regulator to work properly. The GPIO_ACTIVE_HIGH constant is not enough anymore.
Signed-off-by: Marek Behún marek.behun@nic.cz Fixes: e8e9715df2d4 ("regulator: fixed: Modify enable-active-high...")
arch/arm/dts/armada-3720-turris-mox.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts index 4c65c3e32c..a1e0ad5020 100644 --- a/arch/arm/dts/armada-3720-turris-mox.dts +++ b/arch/arm/dts/armada-3720-turris-mox.dts @@ -42,6 +42,7 @@ startup-delay-us = <2000000>; shutdown-delay-us = <1000000>; gpio = <&gpiosb 0 GPIO_ACTIVE_HIGH>;
regulator-boot-on; };enable-active-high;
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan

Hi Marek,
On 30.03.20 19:19, Marek Behún wrote:
The SPI clock signal changes value when the SPI configuration register is configured. This can sometimes lead to the device misinterpreting the enablement of the SPI controller as actual clock tick. This can be solved by first setting the SPI CS1 pin from GPIO to SPI mode, and only after that writing the SPI configuration register.
Signed-off-by: Marek Behún marek.behun@nic.cz
board/CZ.NIC/turris_mox/turris_mox.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 377191baef..7118096f32 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -67,9 +67,9 @@ int board_fix_fdt(void *blob) * to read SPI by reading/writing SPI registers directly */
- writel(0x563fa, ARMADA_37XX_NB_GPIO_SEL); writel(0x10df, ARMADA_37XX_SPI_CFG);
- writel(0x2005b, ARMADA_37XX_SPI_CTRL);
clrbits_le32(ARMADA_37XX_NB_GPIO_SEL, BIT(12));
setbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17));
while (!(readl(ARMADA_37XX_SPI_CTRL) & 0x2)) udelay(1);
@@ -89,7 +89,7 @@ int board_fix_fdt(void *blob)
size = i;
- writel(0x5b, ARMADA_37XX_SPI_CTRL);
- clrbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17));
Could you please add some comments to those bit manipulations above? I know it was not there before. But please add something like the commit text here, explaining what this code is doing and why.
Thanks, Stefan
participants (2)
-
Marek Behún
-
Stefan Roese