From daa9bfdb9cff9cc7ff0ad93383bbaf2bd618ab5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:08 +0200 Subject: [PATCH 01/28] pci: pci_mvebu: Fix write_config() with PCI_SIZE_8 or PCI_SIZE_16 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current implementation of write_config() is broken for PCI_SIZE_8 or PCI_SIZE_16 as it always uses writel(), which means that write operation is always 32-bit, so upper 24 bits for PCI_SIZE_8 and upper 16 bits for PCI_SIZE_16 are cleared. Fix this by using writeb() and writew(), respectively. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 0c1d7cd770..8175511514 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -211,8 +211,19 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf, writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF); /* write data */ - data = pci_conv_size_to_32(0, value, offset, size); - writel(data, pcie->base + PCIE_CONF_DATA_OFF); + switch (size) { + case PCI_SIZE_8: + writeb(value, pcie->base + PCIE_CONF_DATA_OFF + (offset & 3)); + break; + case PCI_SIZE_16: + writew(value, pcie->base + PCIE_CONF_DATA_OFF + (offset & 2)); + break; + case PCI_SIZE_32: + writel(value, pcie->base + PCIE_CONF_DATA_OFF); + break; + default: + return -EINVAL; + } return 0; } From 657177ad8e240ea520eba26da5c0ba47bd935787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:09 +0200 Subject: [PATCH 02/28] pci: pci_mvebu: Fix read_config() with PCI_SIZE_8 or PCI_SIZE_16 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When reading 8 or 16 bits from config space, use appropriate readb() or readw() calls. This ensures that PCIe controller does not read more bits from endpoint card as asked by read_config() function. Technically there should not be an issue with reading data from config space which are not later used as there are no clear-by-read registers. But it is better to use correct read operation based on requested size. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 8175511514..3991086e0d 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -184,9 +184,22 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf, writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF); /* read data */ - data = readl(pcie->base + PCIE_CONF_DATA_OFF); + switch (size) { + case PCI_SIZE_8: + data = readb(pcie->base + PCIE_CONF_DATA_OFF + (offset & 3)); + break; + case PCI_SIZE_16: + data = readw(pcie->base + PCIE_CONF_DATA_OFF + (offset & 2)); + break; + case PCI_SIZE_32: + data = readl(pcie->base + PCIE_CONF_DATA_OFF); + break; + default: + return -EINVAL; + } + debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data); - *valuep = pci_conv_32_to_size(data, offset, size); + *valuep = data; return 0; } From a7b61ab58d5d425f24cd369cf9d0bc00de4989a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:10 +0200 Subject: [PATCH 03/28] pci: pci_mvebu: Properly configure and use PCI Bridge (PCIe Root Port) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mysterious "Memory controller" PCI device which is present in PCI config space is improperly configured and crippled PCI Bridge which acts as PCIe Root Port for endpoint PCIe card. This PCI Bridge reports in PCI config space incorrect Class Code (Memory Controller) and incorrect Header Type (Type 0). It looks like HW bug in mvebu PCIe controller but apparently it can be changed via mvebu registers to correct values. The worst thing is that this PCI Bridge is crippled and its PCI config registers in range 0x10-0x34 alias access to internal mvebu registers which have different functionality as PCI Bridge registers. Moreover, configuration of PCI primary and secondary bus numbers (registers 0x18 and 0x19) is done via totally different mvebu registers via totally strange method and cannot be done via PCI Bridge config space. Due to above fact about PCI config range 0x10-0x34, allocate a private cfgcache[] buffer in the driver, to which PCI config access requests to the 0x10-0x34 space will be redirected in mvebu_pcie_read_config() and mvebu_pcie_write_config() functions. Function mvebu_pcie_write_config() will also catch writes to PCI_PRIMARY_BUS (0x18) and PCI_SECONDARY_BUS (0x19) registers and set PCI Bridge primary and secondary bus numbers via mvebu's own method. Also, Expansion ROM Base Address register (0x38) is available, but at different offset 0x30. So recalculate register offset before accessing PCI config space. After these steps U-Boot sees working PCI Bridge and CONFIG_PCI_PNP code can finally start enumerating all PCIe devices correctly, even with more complicated PCI topology. So update also mvebu_pcie_valid_addr() function to reflect state of the real device topology. Each PCIe port is de-facto isolated and every PCI Bridge which is part of PCIe Root Complex is also isolated, so put them on separate PCI buses as (local) device 0. U-Boot already supports enumerating separate PCI buses, real (HW) bus number can be retrieved by "PCI_BUS(bdf) - dev_seq(bus)" code, so update config read/write functions to properly handle more complicated tree topologies (e.g. when a PCIe switch with multiple PCI buses is connected to the PCIe port). Local bus number and local device number on mvebu are used for determining which config request type is used (Type 0 vs Type 1). On normal non-broken PCIe hardware it is done by primary and secondary bus numbers. So correctly translate settings between these numbers to ensure that correct config requests are sent over the PCIe bus. As bus numbers are correctly re-configured, it does not make sense to print some initial bogus configuration during probe, so remove this debug code. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 199 +++++++++++++++++++++++++++++++++------- 1 file changed, 164 insertions(+), 35 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 3991086e0d..be39440283 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -36,6 +36,7 @@ DECLARE_GLOBAL_DATA_PTR; #define PCIE_DEV_REV_OFF 0x0008 #define PCIE_BAR_LO_OFF(n) (0x0010 + ((n) << 3)) #define PCIE_BAR_HI_OFF(n) (0x0014 + ((n) << 3)) +#define PCIE_EXP_ROM_BAR_OFF 0x0030 #define PCIE_CAPAB_OFF 0x0060 #define PCIE_CTRL_STAT_OFF 0x0068 #define PCIE_HEADER_LOG_4_OFF 0x0128 @@ -52,9 +53,9 @@ DECLARE_GLOBAL_DATA_PTR; #define PCIE_CONF_BUS(b) (((b) & 0xff) << 16) #define PCIE_CONF_DEV(d) (((d) & 0x1f) << 11) #define PCIE_CONF_FUNC(f) (((f) & 0x7) << 8) -#define PCIE_CONF_ADDR(dev, reg) \ - (PCIE_CONF_BUS(PCI_BUS(dev)) | PCIE_CONF_DEV(PCI_DEV(dev)) | \ - PCIE_CONF_FUNC(PCI_FUNC(dev)) | PCIE_CONF_REG(reg) | \ +#define PCIE_CONF_ADDR(b, d, f, reg) \ + (PCIE_CONF_BUS(b) | PCIE_CONF_DEV(d) | \ + PCIE_CONF_FUNC(f) | PCIE_CONF_REG(reg) | \ PCIE_CONF_ADDR_EN) #define PCIE_CONF_DATA_OFF 0x18fc #define PCIE_MASK_OFF 0x1910 @@ -80,12 +81,13 @@ struct mvebu_pcie { int devfn; u32 lane_mask; int first_busno; - int local_dev; + int sec_busno; char name[16]; unsigned int mem_target; unsigned int mem_attr; unsigned int io_target; unsigned int io_attr; + u32 cfgcache[0x34 - 0x10]; }; /* @@ -145,23 +147,18 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose) return container_of(hose, struct mvebu_pcie, hose); } -static int mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, pci_dev_t bdf) +static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, + int busno, int dev, int func) { - /* - * There are two devices visible on local bus: - * * on slot configured by function mvebu_pcie_set_local_dev_nr() - * (by default this register is set to 0) there is a - * "Marvell Memory controller", which isn't useful in root complex - * mode, - * * on all other slots the real PCIe card connected to the PCIe slot. - * - * We therefore allow access only to the real PCIe card. - */ - if (PCI_BUS(bdf) == pcie->first_busno && - PCI_DEV(bdf) != !pcie->local_dev) - return 0; + /* On primary bus is only one PCI Bridge */ + if (busno == pcie->first_busno && (dev != 0 || func != 0)) + return false; - return 1; + /* On secondary bus can be only one PCIe device */ + if (busno == pcie->sec_busno && dev != 0) + return false; + + return true; } static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf, @@ -169,19 +166,46 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf, enum pci_size_t size) { struct mvebu_pcie *pcie = dev_get_plat(bus); - u32 data; + int busno = PCI_BUS(bdf) - dev_seq(bus); + u32 addr, data; debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ", PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); - if (!mvebu_pcie_valid_addr(pcie, bdf)) { + if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) { debug("- out of range\n"); *valuep = pci_get_ff(size); return 0; } + /* + * mvebu has different internal registers mapped into PCI config space + * in range 0x10-0x34 for PCI bridge, so do not access PCI config space + * for this range and instead read content from driver virtual cfgcache + */ + if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) { + data = pcie->cfgcache[(offset - 0x10) / 4]; + debug("(addr,size,val)=(0x%04x, %d, 0x%08x) from cfgcache\n", + offset, size, data); + *valuep = pci_conv_32_to_size(data, offset, size); + return 0; + } else if (busno == pcie->first_busno && + (offset & ~3) == PCI_ROM_ADDRESS1) { + /* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */ + offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF; + } + + /* + * PCI bridge is device 0 at primary bus but mvebu has it mapped on + * secondary bus with device number 1. + */ + if (busno == pcie->first_busno) + addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset); + else + addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset); + /* write address */ - writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF); + writel(addr, pcie->base + PCIE_CONF_ADDR_OFF); /* read data */ switch (size) { @@ -198,6 +222,19 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf, return -EINVAL; } + if (busno == pcie->first_busno && + (offset & ~3) == (PCI_HEADER_TYPE & ~3)) { + /* + * Change Header Type of PCI Bridge device to Type 1 + * (0x01, used by PCI Bridges) because mvebu reports + * Type 0 (0x00, used by Upstream and Endpoint devices). + */ + data = pci_conv_size_to_32(data, 0, offset, size); + data &= ~0x007f0000; + data |= PCI_HEADER_TYPE_BRIDGE << 16; + data = pci_conv_32_to_size(data, offset, size); + } + debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data); *valuep = data; @@ -209,19 +246,64 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf, enum pci_size_t size) { struct mvebu_pcie *pcie = dev_get_plat(bus); - u32 data; + int busno = PCI_BUS(bdf) - dev_seq(bus); + u32 addr, data; debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ", PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value); - if (!mvebu_pcie_valid_addr(pcie, bdf)) { + if (!mvebu_pcie_valid_addr(pcie, busno, PCI_DEV(bdf), PCI_FUNC(bdf))) { debug("- out of range\n"); return 0; } + /* + * mvebu has different internal registers mapped into PCI config space + * in range 0x10-0x34 for PCI bridge, so do not access PCI config space + * for this range and instead write content to driver virtual cfgcache + */ + if (busno == pcie->first_busno && offset >= 0x10 && offset < 0x34) { + debug("Writing to cfgcache only\n"); + data = pcie->cfgcache[(offset - 0x10) / 4]; + data = pci_conv_size_to_32(data, value, offset, size); + /* mvebu PCI bridge does not have configurable bars */ + if ((offset & ~3) == PCI_BASE_ADDRESS_0 || + (offset & ~3) == PCI_BASE_ADDRESS_1) + data = 0x0; + pcie->cfgcache[(offset - 0x10) / 4] = data; + /* mvebu has its own way how to set PCI primary bus number */ + if (offset == PCI_PRIMARY_BUS) { + pcie->first_busno = data & 0xff; + debug("Primary bus number was changed to %d\n", + pcie->first_busno); + } + /* mvebu has its own way how to set PCI secondary bus number */ + if (offset == PCI_SECONDARY_BUS || + (offset == PCI_PRIMARY_BUS && size != PCI_SIZE_8)) { + pcie->sec_busno = (data >> 8) & 0xff; + mvebu_pcie_set_local_bus_nr(pcie, pcie->sec_busno); + debug("Secondary bus number was changed to %d\n", + pcie->sec_busno); + } + return 0; + } else if (busno == pcie->first_busno && + (offset & ~3) == PCI_ROM_ADDRESS1) { + /* mvebu has Expansion ROM Base Address (0x38) at offset 0x30 */ + offset -= PCI_ROM_ADDRESS1 - PCIE_EXP_ROM_BAR_OFF; + } + + /* + * PCI bridge is device 0 at primary bus but mvebu has it mapped on + * secondary bus with device number 1. + */ + if (busno == pcie->first_busno) + addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset); + else + addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset); + /* write address */ - writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF); + writel(addr, pcie->base + PCIE_CONF_ADDR_OFF); /* write data */ switch (size) { @@ -301,22 +383,63 @@ static int mvebu_pcie_probe(struct udevice *dev) struct mvebu_pcie *pcie = dev_get_plat(dev); struct udevice *ctlr = pci_get_controller(dev); struct pci_controller *hose = dev_get_uclass_priv(ctlr); - int bus = dev_seq(dev); u32 reg; debug("%s: PCIe %d.%d - up, base %08x\n", __func__, pcie->port, pcie->lane, (u32)pcie->base); - /* Read Id info and local bus/dev */ - debug("direct conf read %08x, local bus %d, local dev %d\n", - readl(pcie->base), mvebu_pcie_get_local_bus_nr(pcie), - mvebu_pcie_get_local_dev_nr(pcie)); + /* + * Change Class Code of PCI Bridge device to PCI Bridge (0x600400) + * because default value is Memory controller (0x508000) which + * U-Boot cannot recognize as P2P Bridge. + * + * Note that this mvebu PCI Bridge does not have compliant Type 1 + * Configuration Space. Header Type is reported as Type 0 and in + * range 0x10-0x34 it has aliased internal mvebu registers 0x10-0x34 + * (e.g. PCIE_BAR_LO_OFF) and register 0x38 is reserved. + * + * Driver for this range redirects access to virtual cfgcache[] buffer + * which avoids changing internal mvebu registers. And changes Header + * Type response value to Type 1. + */ + reg = readl(pcie->base + PCIE_DEV_REV_OFF); + reg &= ~0xffffff00; + reg |= (PCI_CLASS_BRIDGE_PCI << 8) << 8; + writel(reg, pcie->base + PCIE_DEV_REV_OFF); - pcie->first_busno = bus; - pcie->local_dev = 1; - - mvebu_pcie_set_local_bus_nr(pcie, bus); - mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev); + /* + * mvebu uses local bus number and local device number to determinate + * type of config request. Type 0 is used if target bus number equals + * local bus number and target device number differs from local device + * number. Type 1 is used if target bus number differs from local bus + * number. And when target bus number equals local bus number and + * target device equals local device number then request is routed to + * PCI Bridge which represent local PCIe Root Port. + * + * It means that PCI primary and secondary buses shares one bus number + * which is configured via local bus number. Determination if config + * request should go to primary or secondary bus is done based on local + * device number. + * + * PCIe is point-to-point bus, so at secondary bus is always exactly one + * device with number 0. So set local device number to 1, it would not + * conflict with any device on secondary bus number and will ensure that + * accessing secondary bus and all buses behind secondary would work + * automatically and correctly. Therefore this configuration of local + * device number implies that setting of local bus number configures + * secondary bus number. Set it to 0 as U-Boot CONFIG_PCI_PNP code will + * later configure it via config write requests to the correct value. + * mvebu_pcie_write_config() catches config write requests which tries + * to change primary/secondary bus number and correctly updates local + * bus number based on new secondary bus number. + * + * With this configuration is PCI Bridge available at secondary bus as + * device number 1. But it must be available at primary bus as device + * number 0. So in mvebu_pcie_read_config() and mvebu_pcie_write_config() + * functions rewrite address to the real one when accessing primary bus. + */ + mvebu_pcie_set_local_bus_nr(pcie, 0); + mvebu_pcie_set_local_dev_nr(pcie, 1); pcie->mem.start = (u32)mvebu_pcie_membase; pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1; @@ -366,6 +489,12 @@ static int mvebu_pcie_probe(struct udevice *dev) writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0)); writel(0, pcie->base + PCIE_BAR_HI_OFF(0)); + /* PCI Bridge support 32-bit I/O and 64-bit prefetch mem addressing */ + pcie->cfgcache[(PCI_IO_BASE - 0x10) / 4] = + PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8); + pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] = + PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16); + return 0; } From 452f2e73c6e9a9f9c2cd928e5f2fde819b9c91f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:11 +0200 Subject: [PATCH 04/28] pci: pci_mvebu: Remove unused functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Functions mvebu_pcie_get_local_bus_nr() and mvebu_pcie_get_local_dev_nr() are not used, so remove them. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index be39440283..173755e299 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -126,22 +126,6 @@ static void mvebu_pcie_set_local_dev_nr(struct mvebu_pcie *pcie, int devno) writel(stat, pcie->base + PCIE_STAT_OFF); } -static int mvebu_pcie_get_local_bus_nr(struct mvebu_pcie *pcie) -{ - u32 stat; - - stat = readl(pcie->base + PCIE_STAT_OFF); - return (stat & PCIE_STAT_BUS) >> 8; -} - -static int mvebu_pcie_get_local_dev_nr(struct mvebu_pcie *pcie) -{ - u32 stat; - - stat = readl(pcie->base + PCIE_STAT_OFF); - return (stat & PCIE_STAT_DEV) >> 16; -} - static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose) { return container_of(hose, struct mvebu_pcie, hose); From 79b4eb21b486e30b29c5ba3038ea6ba0e1968458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:12 +0200 Subject: [PATCH 05/28] pci: pci_mvebu: Fix place of link up detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PCI Bridge is always accessible also when link is down. So move detection of link up from mvebu_pcie_of_to_plat() function to mvebu_pcie_valid_addr() function which is used when accessing PCI config space. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 173755e299..4cb237d2c4 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -138,6 +138,10 @@ static bool mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, if (busno == pcie->first_busno && (dev != 0 || func != 0)) return false; + /* Access to other buses is possible when link is up */ + if (busno != pcie->first_busno && !mvebu_pcie_link_up(pcie)) + return false; + /* On secondary bus can be only one PCIe device */ if (busno == pcie->sec_busno && dev != 0) return false; @@ -369,9 +373,6 @@ static int mvebu_pcie_probe(struct udevice *dev) struct pci_controller *hose = dev_get_uclass_priv(ctlr); u32 reg; - debug("%s: PCIe %d.%d - up, base %08x\n", __func__, - pcie->port, pcie->lane, (u32)pcie->base); - /* * Change Class Code of PCI Bridge device to PCI Bridge (0x600400) * because default value is Memory controller (0x508000) which @@ -603,13 +604,6 @@ static int mvebu_pcie_of_to_plat(struct udevice *dev) if (ret < 0) goto err; - /* Check link and skip ports that have no link */ - if (!mvebu_pcie_link_up(pcie)) { - debug("%s: %s - down\n", __func__, pcie->name); - ret = -ENODEV; - goto err; - } - return 0; err: From 42ab3b30046007f000adfd67427a72593f5b3d81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:13 +0200 Subject: [PATCH 06/28] pci: pci_mvebu: Do not automatically enable bus mastering on PCI Bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that PCI Bridge is working, U-Boot's CONFIG_PCI_PNP code automatically enables memory access and bus mastering when it is needed. So do not prematurely enable memory access and bus mastering. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 4cb237d2c4..4c7fd8d5a9 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -451,14 +451,6 @@ static int mvebu_pcie_probe(struct udevice *dev) /* Setup windows and configure host bridge */ mvebu_pcie_setup_wins(pcie); - /* Master + slave enable. */ - reg = readl(pcie->base + PCIE_CMD_OFF); - reg |= PCI_COMMAND_MEMORY; - reg |= PCI_COMMAND_IO; - reg |= PCI_COMMAND_MASTER; - reg |= BIT(10); /* disable interrupts */ - writel(reg, pcie->base + PCIE_CMD_OFF); - /* PCI memory space */ pci_set_region(hose->regions + 0, pcie->mem.start, pcie->mem.start, PCIE_MEM_SIZE, PCI_REGION_MEM); From 2344a76f29a54d6521af0a4d3b5b9e6c407a7bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:14 +0200 Subject: [PATCH 07/28] pci: pci_mvebu: Setup PCI controller to Root Complex mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root Complex should be the default mode, let's set it explicitly. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index 4c7fd8d5a9..a327a83411 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -62,6 +62,7 @@ DECLARE_GLOBAL_DATA_PTR; #define PCIE_MASK_ENABLE_INTS (0xf << 24) #define PCIE_CTRL_OFF 0x1a00 #define PCIE_CTRL_X1_MODE BIT(0) +#define PCIE_CTRL_RC_MODE BIT(1) #define PCIE_STAT_OFF 0x1a04 #define PCIE_STAT_BUS (0xff << 8) #define PCIE_STAT_DEV (0x1f << 16) @@ -373,6 +374,11 @@ static int mvebu_pcie_probe(struct udevice *dev) struct pci_controller *hose = dev_get_uclass_priv(ctlr); u32 reg; + /* Setup PCIe controller to Root Complex mode */ + reg = readl(pcie->base + PCIE_CTRL_OFF); + reg |= PCIE_CTRL_RC_MODE; + writel(reg, pcie->base + PCIE_CTRL_OFF); + /* * Change Class Code of PCI Bridge device to PCI Bridge (0x600400) * because default value is Memory controller (0x508000) which From 03a8a5e26a32d842ce2c92be9dc9b9d1b10c9057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Fri, 22 Oct 2021 16:22:15 +0200 Subject: [PATCH 08/28] pci: pci_mvebu: Fix comment about driver class name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a pci driver, not an eth driver. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- drivers/pci/pci_mvebu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c index a327a83411..c575e9412b 100644 --- a/drivers/pci/pci_mvebu.c +++ b/drivers/pci/pci_mvebu.c @@ -633,7 +633,7 @@ static int mvebu_pcie_bind(struct udevice *parent) struct udevice *dev; ofnode subnode; - /* Lookup eth driver */ + /* Lookup pci driver */ drv = lists_uclass_lookup(UCLASS_PCI); if (!drv) { puts("Cannot find PCI driver\n"); From 0a14341cf8f60e6a9ba2edb0802507cbb073e806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:52 +0200 Subject: [PATCH 09/28] tools: kwboot: Initialize rfds to zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicitly zero out the rfds fd_set with FD_ZERO() before using it. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/kwboot.c b/tools/kwboot.c index 7e1be29623..695d433b96 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1151,6 +1151,7 @@ kwboot_terminal(int tty) fd_set rfds; int nfds = 0; + FD_ZERO(&rfds); FD_SET(tty, &rfds); nfds = nfds < tty ? tty : nfds; From 2ecca3d0d76023f219ac4350b0112cd19fbdda2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:53 +0200 Subject: [PATCH 10/28] tools: kwboot: Fix initialization of tty device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicitly disable 2 stop bits by clearing CSTOPB flag, disable modem control flow by clearing CRTSCTS flag and do not send hangup after closing device by clearing HUPCL flag. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/kwboot.c b/tools/kwboot.c index 695d433b96..c55b41025b 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -657,6 +657,7 @@ kwboot_open_tty(const char *path, int baudrate) cfmakeraw(&tio); tio.c_cflag |= CREAD | CLOCAL; + tio.c_cflag &= ~(CSTOPB | HUPCL | CRTSCTS); tio.c_cc[VMIN] = 1; tio.c_cc[VTIME] = 0; From 5923ef686a61c7ac15ed990487e4a4fd312ddeec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:54 +0200 Subject: [PATCH 11/28] tools: kwboot: Reserve enough space for patching kwbimage in memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SPI image header and data parts do not have to be aligned to 128 byte xmodem block size. So reserve additional memory for aligning header part and additional memory for aligning data part. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index c55b41025b..4e29317f10 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1672,8 +1672,10 @@ main(int argc, char **argv) else /* ensure we have enough space for baudrate change code */ after_img_rsv += KWBOOT_BAUDRATE_BIN_HEADER_SZ + + KWBOOT_XM_BLKSZ + sizeof(kwboot_pre_baud_code) + - sizeof(kwboot_baud_code); + sizeof(kwboot_baud_code) + + KWBOOT_XM_BLKSZ; if (imgpath) { img = kwboot_read_image(imgpath, &size, after_img_rsv); From ad9a3ac5005e7b70a50b621a5340cead6fcc673f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:55 +0200 Subject: [PATCH 12/28] tools: kwboot: Validate 4-byte image data checksum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data part of the image contains 4-byte checksum. Validate it when processing the image. Signed-off-by: Pali Rohár [ refactored ] Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tools/kwboot.c b/tools/kwboot.c index 4e29317f10..bc44301535 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1251,6 +1251,37 @@ kwboot_hdr_csum8(const void *hdr) return csum; } +static uint32_t * +kwboot_img_csum32_ptr(void *img) +{ + struct main_hdr_v1 *hdr = img; + uint32_t datasz; + + datasz = le32_to_cpu(hdr->blocksize) - sizeof(uint32_t); + + return img + le32_to_cpu(hdr->srcaddr) + datasz; +} + +static uint32_t +kwboot_img_csum32(const void *img) +{ + const struct main_hdr_v1 *hdr = img; + uint32_t datasz, csum = 0; + const uint32_t *data; + + datasz = le32_to_cpu(hdr->blocksize) - sizeof(csum); + if (datasz % sizeof(uint32_t)) + return 0; + + data = img + le32_to_cpu(hdr->srcaddr); + while (datasz > 0) { + csum += le32_to_cpu(*data++); + datasz -= 4; + } + + return cpu_to_le32(csum); +} + static int kwboot_img_is_secure(void *img) { @@ -1462,6 +1493,9 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize)) goto err; + if (kwboot_img_csum32(img) != *kwboot_img_csum32_ptr(img)) + goto err; + is_secure = kwboot_img_is_secure(img); if (hdr->blockid != IBR_HDR_UART_ID) { From 063cb352814b5363aebb4c733a41b951787b1f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:56 +0200 Subject: [PATCH 13/28] tools: kwboot: Inject baudrate change back code after data part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some vendor U-Boot kwbimage binaries (e.g. those for A375) have load address set to zero. Therefore it is not possible to inject code which changes baudrate back to 115200 Bd before the data part. So instead inject it after the data part and change kwbimage execution address to that offset. Also store original execution address into baudrate change code, so after it changes baudrate back to 115200 Bd, it can jump to orignal address. Signed-off-by: Pali Rohár [ refactored ] Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 72 ++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index bc44301535..bf26a667b7 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1295,34 +1295,22 @@ kwboot_img_is_secure(void *img) } static void * -kwboot_img_grow_data_left(void *img, size_t *size, size_t grow) +kwboot_img_grow_data_right(void *img, size_t *size, size_t grow) { - uint32_t hdrsz, datasz, srcaddr; struct main_hdr_v1 *hdr = img; - uint8_t *data; + void *result; - srcaddr = le32_to_cpu(hdr->srcaddr); - - hdrsz = kwbheader_size(hdr); - data = (uint8_t *)img + srcaddr; - datasz = *size - srcaddr; - - /* only move data if there is not enough space */ - if (hdrsz + grow > srcaddr) { - size_t need = hdrsz + grow - srcaddr; - - /* move data by enough bytes */ - memmove(data + need, data, datasz); - *size += need; - srcaddr += need; - } - - srcaddr -= grow; - hdr->srcaddr = cpu_to_le32(srcaddr); - hdr->destaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) - grow); + /* + * 32-bit checksum comes after end of image code, so we will be putting + * new code there. So we get this pointer and then increase data size + * (since increasing data size changes kwboot_img_csum32_ptr() return + * value). + */ + result = kwboot_img_csum32_ptr(img); hdr->blocksize = cpu_to_le32(le32_to_cpu(hdr->blocksize) + grow); + *size += grow; - return (uint8_t *)img + srcaddr; + return result; } static void @@ -1400,14 +1388,20 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz) } static void -_copy_baudrate_change_code(struct main_hdr_v1 *hdr, void *dst, int pre, - int old_baud, int new_baud) +_inject_baudrate_change_code(void *img, size_t *size, int pre, + int old_baud, int new_baud) { - size_t codesz = sizeof(kwboot_baud_code); - uint8_t *code = dst; + uint32_t codesz = sizeof(kwboot_baud_code); + struct main_hdr_v1 *hdr = img; + uint8_t *code; if (pre) { - size_t presz = sizeof(kwboot_pre_baud_code); + uint32_t presz = sizeof(kwboot_pre_baud_code); + uint32_t orig_datasz; + + orig_datasz = le32_to_cpu(hdr->blocksize) - sizeof(uint32_t); + + code = kwboot_img_grow_data_right(img, size, presz + codesz); /* * We need to prepend code that loads lr register with original @@ -1421,9 +1415,12 @@ _copy_baudrate_change_code(struct main_hdr_v1 *hdr, void *dst, int pre, memcpy(code, kwboot_pre_baud_code, presz); *(uint32_t *)code = hdr->execaddr; - hdr->execaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) + 4); + hdr->execaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) + + orig_datasz + 4); code += presz; + } else { + code = kwboot_add_bin_ohdr_v1(img, size, codesz); } memcpy(code, kwboot_baud_code, codesz - 8); @@ -1516,9 +1513,6 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) } if (baudrate) { - uint32_t codesz = sizeof(kwboot_baud_code); - void *code; - if (image_ver == 0) { fprintf(stderr, "Cannot inject code for changing baudrate into v0 image header\n"); @@ -1539,20 +1533,16 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) */ kwboot_printv("Injecting binary header code for changing baudrate to %d Bd\n", baudrate); - - code = kwboot_add_bin_ohdr_v1(img, size, codesz); - _copy_baudrate_change_code(hdr, code, 0, 115200, baudrate); + _inject_baudrate_change_code(img, size, 0, 115200, baudrate); /* * Now inject code that changes the baudrate back to 115200 Bd. - * This code is prepended to the data part of the image, so it - * is executed before U-Boot proper. + * This code is appended after the data part of the image, and + * execaddr is changed so that it is executed before U-Boot + * proper. */ kwboot_printv("Injecting code for changing baudrate back\n"); - - codesz += sizeof(kwboot_pre_baud_code); - code = kwboot_img_grow_data_left(img, size, codesz); - _copy_baudrate_change_code(hdr, code, 1, baudrate, 115200); + _inject_baudrate_change_code(img, size, 1, baudrate, 115200); /* recompute header size */ hdrsz = kwbheader_size(hdr); From 82c5a0ac713500f4da9d8562cd4764c733b65e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:57 +0200 Subject: [PATCH 14/28] tools: kwboot: Recalculate 4-byte data checksum after injecting baudrate code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If data part of image is modified, update 4-byte data checksum. It looks like A385 BootROM does not verify this checksum for image loaded via UART, but we do not know if other BootROMs are also ignoring it. It is always better to provide correct checksum. Signed-off-by: Pali Rohár [ refactored ] Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/kwboot.c b/tools/kwboot.c index bf26a667b7..1131c2eb1c 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1544,6 +1544,9 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) kwboot_printv("Injecting code for changing baudrate back\n"); _inject_baudrate_change_code(img, size, 1, baudrate, 115200); + /* Update the 32-bit data checksum */ + *kwboot_img_csum32_ptr(img) = kwboot_img_csum32(img); + /* recompute header size */ hdrsz = kwbheader_size(hdr); } From 4bebab69a9483fd6ae85b06a088790a6c77f90ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:58 +0200 Subject: [PATCH 15/28] tools: kwboot: Correctly set configuration of UART for BootROM messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For kwbimage v1, tell BootROM to send BootROM messages to UART port number 0 (used also for UART booting) with default baudrate (which should be 115200) and do not touch UART MPP configuration. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/kwboot.c b/tools/kwboot.c index 1131c2eb1c..6228838228 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1507,6 +1507,17 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) } if (!is_secure) { + if (image_ver == 1) { + /* + * Tell BootROM to send BootROM messages to UART port + * number 0 (used also for UART booting) with default + * baudrate (which should be 115200) and do not touch + * UART MPP configuration. + */ + hdr->options &= ~0x1F; + hdr->options |= MAIN_HDR_V1_OPT_BAUD_DEFAULT; + hdr->options |= 0 << 3; + } if (image_ver == 0) ((struct main_hdr_v0 *)img)->nandeccmode = IBR_HDR_ECC_DISABLED; hdr->nandpagesize = 0; From 8e2e7ca1fe3a822aec31b4672b4615c07afa89c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:12:59 +0200 Subject: [PATCH 16/28] tools: kwboot: Show verbose message when waiting for baudrate change magic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is hard to debug why kwboot is failing when the last message is 'Finishing transfer' and no additional output. So show verbose message when kwboot finished transfer and is waiting for baudrate change magic sequence. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 6228838228..7fd28aa754 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1065,7 +1065,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate) if (baudrate) { char buf[sizeof(kwb_baud_magic)]; - /* Wait 1s for baudrate change magic */ + kwboot_printv("Waiting 1s for baudrate change magic\n"); rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000); if (rc) return rc; From ed792c2938331a68f4b271a6d4d0250057449608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:13:00 +0200 Subject: [PATCH 17/28] tools: kwboot: Simplify code for aligning image header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expression (hdrsz % KWBOOT_XM_BLKSZ) is non-zero therefore expression (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) is always less than value KWBOOT_XM_BLKSZ. So there is no need to add another modulo. Also rename variable `offset` to `grow` which better describes what is stored in this variable. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 7fd28aa754..adec4ec97d 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1563,8 +1563,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) } if (hdrsz % KWBOOT_XM_BLKSZ) { - size_t offset = (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % - KWBOOT_XM_BLKSZ; + size_t grow = KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ; if (is_secure) { fprintf(stderr, "Cannot align image with secure header\n"); @@ -1572,7 +1571,7 @@ kwboot_img_patch(void *img, size_t *size, int baudrate) } kwboot_printv("Aligning image header to Xmodem block size\n"); - kwboot_img_grow_hdr(img, size, offset); + kwboot_img_grow_hdr(img, size, grow); } hdr->checksum = kwboot_hdr_csum8(hdr) - csum; From e511cc3b1ad8ced8e18464edf22f5b8a83ec1cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:13:01 +0200 Subject: [PATCH 18/28] tools: kwboot: Do not modify kwbimage header before increasing its size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures that kwboot_img_grow_hdr() function still sees valid kwbimage header. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index adec4ec97d..bb7555369c 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1352,17 +1352,18 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz) uint32_t num_args; uint32_t offset; uint32_t ohdrsz; + uint8_t *prev_ext; if (hdr->ext & 0x1) { for_each_opt_hdr_v1 (ohdr, img) if (opt_hdr_v1_next(ohdr) == NULL) break; - *opt_hdr_v1_ext(ohdr) |= 1; - ohdr = opt_hdr_v1_next(ohdr); + prev_ext = opt_hdr_v1_ext(ohdr); + ohdr = _opt_hdr_v1_next(ohdr); } else { - hdr->ext |= 1; ohdr = (void *)(hdr + 1); + prev_ext = &hdr->ext; } /* @@ -1377,6 +1378,8 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz) ohdrsz = sizeof(*ohdr) + 4 + 4 * num_args + binsz + 4; kwboot_img_grow_hdr(hdr, size, ohdrsz); + *prev_ext |= 1; + ohdr->headertype = OPT_HDR_V1_BINARY_TYPE; ohdr->headersz_msb = ohdrsz >> 16; ohdr->headersz_lsb = cpu_to_le16(ohdrsz & 0xffff); From d656f5a0ee224f29253573a6641ccb8fc1a3afad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:13:02 +0200 Subject: [PATCH 19/28] tools: kwboot: Calculate real used space in kwbimage header when calling kwboot_img_grow_hdr() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Size of the header stored in kwbimage may be larger than real used size in the kwbimage header. If there is unused space in kwbimage header then use it for growing it. So update code to calculate used space of kwbimage header. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index bb7555369c..5d7cb7a774 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -1318,11 +1318,20 @@ kwboot_img_grow_hdr(void *img, size_t *size, size_t grow) { uint32_t hdrsz, datasz, srcaddr; struct main_hdr_v1 *hdr = img; + struct opt_hdr_v1 *ohdr; uint8_t *data; srcaddr = le32_to_cpu(hdr->srcaddr); - hdrsz = kwbheader_size(img); + /* calculate real used space in kwbimage header */ + if (kwbimage_version(img) == 0) { + hdrsz = kwbheader_size(img); + } else { + hdrsz = sizeof(*hdr); + for_each_opt_hdr_v1 (ohdr, hdr) + hdrsz += opt_hdr_v1_size(ohdr); + } + data = (uint8_t *)img + srcaddr; datasz = *size - srcaddr; @@ -1339,8 +1348,10 @@ kwboot_img_grow_hdr(void *img, size_t *size, size_t grow) if (kwbimage_version(img) == 1) { hdrsz += grow; - hdr->headersz_msb = hdrsz >> 16; - hdr->headersz_lsb = cpu_to_le16(hdrsz & 0xffff); + if (hdrsz > kwbheader_size(img)) { + hdr->headersz_msb = hdrsz >> 16; + hdr->headersz_lsb = cpu_to_le16(hdrsz & 0xffff); + } } } From d14a342073ca7ed108ca1c2477e102da431a527c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:13:03 +0200 Subject: [PATCH 20/28] tools: kwboot: Change retry loop from decreasing to increasing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch does not change behavior of the code, just allows to implement new changes more easily. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 5d7cb7a774..16c5a84825 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -925,7 +925,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, *done_print = 0; - retries = 16; + retries = 0; do { rc = kwboot_tty_send(fd, block, sizeof(*block)); if (rc) @@ -944,7 +944,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, if (!allow_non_xm && c != ACK) kwboot_progress(-1, '+'); - } while (c == NAK && retries-- > 0); + } while (c == NAK && retries++ < 16); if (non_xm_print) kwboot_printv("\n"); @@ -973,7 +973,7 @@ kwboot_xm_finish(int fd) kwboot_printv("Finishing transfer\n"); - retries = 16; + retries = 0; do { rc = kwboot_tty_send_char(fd, EOT); if (rc) @@ -982,7 +982,7 @@ kwboot_xm_finish(int fd) rc = kwboot_xm_recv_reply(fd, &c, 0, NULL, 0, NULL); if (rc) return rc; - } while (c == NAK && retries-- > 0); + } while (c == NAK && retries++ < 16); return _xm_reply_to_error(c); } From a6fcac274a99c9675f4b6841321e085c79326429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 25 Oct 2021 15:13:04 +0200 Subject: [PATCH 21/28] tools: kwboot: Resend first 3 xmodem retry packets immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently when kwboot receive some garbage reply which does not understand, it waits 1s before it tries to resend packet again. The most common error on UART is that receiver sees some bit flipped which results in invalid reply. This behavior slows down xmodem transfer over UART as basically on every error kwboot is waiting one second. To fix this, try to resend xmodem packet for first 3 attempts immediately without any delay. If broken reply is received also after the 3 attempts, continue retrying with 1s delay like it was before. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 16c5a84825..bb7cae9f05 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -851,7 +851,8 @@ kwboot_baud_magic_handle(int fd, char c, int baudrate) } static int -kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print, +kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm, + int allow_non_xm, int *non_xm_print, int baudrate, int *baud_changed) { int timeout = allow_non_xm ? KWBOOT_HDR_RSP_TIMEO : blk_rsp_timeo; @@ -904,6 +905,10 @@ kwboot_xm_recv_reply(int fd, char *c, int allow_non_xm, int *non_xm_print, *non_xm_print = 1; } } else { + if (nak_on_non_xm) { + *c = NAK; + break; + } timeout = recv_until - _now(); if (timeout < 0) { errno = ETIMEDOUT; @@ -937,7 +942,8 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, *done_print = 1; } - rc = kwboot_xm_recv_reply(fd, &c, allow_non_xm, &non_xm_print, + rc = kwboot_xm_recv_reply(fd, &c, retries < 3, + allow_non_xm, &non_xm_print, baudrate, &baud_changed); if (rc) goto can; @@ -979,7 +985,8 @@ kwboot_xm_finish(int fd) if (rc) return rc; - rc = kwboot_xm_recv_reply(fd, &c, 0, NULL, 0, NULL); + rc = kwboot_xm_recv_reply(fd, &c, retries < 3, + 0, NULL, 0, NULL); if (rc) return rc; } while (c == NAK && retries++ < 16); From 455c0d22fb084550ecc6aaf810a9c7b684b2dee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 27 Oct 2021 20:56:58 +0200 Subject: [PATCH 22/28] tools: kwboot: Fix sending retry of last header packet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the trasfer of last header packet, it is possible that baudrate change pattern is received, and also that NAK byte is received so that the packet should be sent again. Thus we should not clear the baudrate change state when sending retry of that packet. Move code for initializing state variables from kwboot_xm_recv_reply() to kwboot_xm_sendblock(). Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index bb7cae9f05..b2c48812c3 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -859,11 +859,6 @@ kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm, uint64_t recv_until = _now() + timeout; int rc; - if (non_xm_print) - *non_xm_print = 0; - if (baud_changed) - *baud_changed = 0; - while (1) { rc = kwboot_tty_recv(fd, c, 1, timeout); if (rc) { @@ -929,6 +924,8 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, char c; *done_print = 0; + non_xm_print = 0; + baud_changed = 0; retries = 0; do { From cab817d260341244f011f7a0c490df51b33c05aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 27 Oct 2021 20:56:59 +0200 Subject: [PATCH 23/28] tools: kwboot: Do not call tcdrain() after each sent packet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kwboot puts each xmodem packet to kernel queue, then waits until all bytes of that packet are transmitted over UART and then waits for xmodem reply until it is received into kernel queue. If some reply is received during the time we are waiting until all bytes are transmitted, then kernel puts them into the queue and returns it to kwboot in next read() call. So there is no need to wait (with tcdrain() function) until all bytes from xmodem packet are transmitted over UART, since any reply received either during that time or after is returned to kwboot with the next read(). Therefore do not call tcdrain() after each xmodem packet sent. Instead directly wait for any reply after putting xmodem packet into write kernel queue. This change could speed up xmodem transfer in case tcdrain() function waits for a longer time. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index b2c48812c3..a6bfd3d4ce 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -404,7 +404,7 @@ out: } static int -kwboot_tty_send(int fd, const void *buf, size_t len) +kwboot_tty_send(int fd, const void *buf, size_t len, int nodrain) { if (!buf) return 0; @@ -412,13 +412,16 @@ kwboot_tty_send(int fd, const void *buf, size_t len) if (kwboot_write(fd, buf, len) < 0) return -1; + if (nodrain) + return 0; + return tcdrain(fd); } static int kwboot_tty_send_char(int fd, unsigned char c) { - return kwboot_tty_send(fd, &c, 1); + return kwboot_tty_send(fd, &c, 1, 0); } static speed_t @@ -705,7 +708,7 @@ kwboot_bootmsg(int tty, void *msg) break; for (count = 0; count < 128; count++) { - rc = kwboot_tty_send(tty, msg, 8); + rc = kwboot_tty_send(tty, msg, 8, 0); if (rc) { usleep(msg_req_delay * 1000); continue; @@ -737,7 +740,7 @@ kwboot_debugmsg(int tty, void *msg) if (rc) break; - rc = kwboot_tty_send(tty, msg, 8); + rc = kwboot_tty_send(tty, msg, 8, 0); if (rc) { usleep(msg_req_delay * 1000); continue; @@ -929,7 +932,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, retries = 0; do { - rc = kwboot_tty_send(fd, block, sizeof(*block)); + rc = kwboot_tty_send(fd, block, sizeof(*block), 1); if (rc) return rc; From 56452295c315397255d05e91940976f68c247d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 27 Oct 2021 20:57:00 +0200 Subject: [PATCH 24/28] tools: kwboot: Increase delay after changing baudrate in ARM code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Increase loop cycles from 600000 to 2998272, which should increase delay from 1ms to about 5ms on 1200 MHz CPU. The Number 2998272 was chosen as the nearest value around 3000000 which can be encoded into one ARM mov instruction. It avoids usage of movt instruction which is not supported by ARMv5te cores. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index a6bfd3d4ce..84294cadfe 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -119,7 +119,7 @@ static unsigned char kwboot_baud_code[] = { /* ; writel(UART_BASE + DLL, new_dll); */ /* ; writel(UART_BASE + DLH, new_dlh); */ /* ; writel(UART_BASE + LCR, lcr & ~DLAB); */ - /* ; msleep(1); */ + /* ; msleep(5); */ /* ; return 0; */ /* ; } */ @@ -130,7 +130,7 @@ static unsigned char kwboot_baud_code[] = { 0x01, 0x00, 0x4d, 0xe3, /* movt r0, #0xd001 */ /* ; r2 = address of preamble string */ - 0xd0, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ + 0xcc, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ /* ; Send preamble string over UART */ /* .Lloop_preamble: */ @@ -177,15 +177,15 @@ static unsigned char kwboot_baud_code[] = { /* ; Read old baudrate value */ /* ; r2 = old_baudrate */ - 0x8c, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ + 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ /* ; Calculate base clock */ /* ; r1 = r2 * r1 */ 0x92, 0x01, 0x01, 0xe0, /* mul r1, r2, r1 */ /* ; Read new baudrate value */ - /* ; r2 = baudrate */ - 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, baudrate */ + /* ; r2 = new_baudrate */ + 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */ /* ; Calculate new Divisor Latch */ /* ; r1 = DIV_ROUND(r1, r2) = */ @@ -225,10 +225,10 @@ static unsigned char kwboot_baud_code[] = { 0x80, 0x10, 0xc1, 0xe3, /* bic r1, r1, #0x80 */ 0x0c, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0c] */ - /* ; Sleep 1ms ~~ 600000 cycles at 1200 MHz */ - /* ; r1 = 600000 */ - 0x9f, 0x1d, 0xa0, 0xe3, /* mov r1, #0x27c0 */ - 0x09, 0x10, 0x40, 0xe3, /* movt r1, #0x0009 */ + /* ; Loop 0x2dc000 (2998272) cycles */ + /* ; which is about 5ms on 1200 MHz CPU */ + /* ; r1 = 0x2dc000 */ + 0xb7, 0x19, 0xa0, 0xe3, /* mov r1, #0x2dc000 */ /* .Lloop_sleep: */ 0x01, 0x10, 0x41, 0xe2, /* sub r1, r1, #1 */ 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ From 558176dcb14d589b5854bea0988301e27b60091d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 27 Oct 2021 20:57:01 +0200 Subject: [PATCH 25/28] tools: kwboot: Replace ARM mov + movt instruction pair by mov + orr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Older Armada SoCs have custom ARMv5te compatible core which does not support movt instruction. So replace mov + movt instruction pair used for immediate move construction by mov + orr instructions which are supported also by ARMv5te. After this change kwboot ARM code should be compatible with any 32-bit ARM core compatible by ARMv2 or new. At least GNU AS does not throw any error or warning. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 84294cadfe..62c218ef64 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -126,8 +126,8 @@ static unsigned char kwboot_baud_code[] = { 0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */ /* ; r0 = UART_BASE */ - 0x02, 0x0a, 0xa0, 0xe3, /* mov r0, #0x2000 */ - 0x01, 0x00, 0x4d, 0xe3, /* movt r0, #0xd001 */ + 0x0d, 0x02, 0xa0, 0xe3, /* mov r0, #0xd0000000 */ + 0x12, 0x0a, 0x80, 0xe3, /* orr r0, r0, #0x12000 */ /* ; r2 = address of preamble string */ 0xcc, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ From 8dbe027fc7d371d17a17a8bb5af8e99b5f20802b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Wed, 27 Oct 2021 20:57:02 +0200 Subject: [PATCH 26/28] tools: kwboot: Do not use stack when setting baudrate back to default value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ARM code we inject into the image to change baudrate back to the default value of 115200 Baud, which is run after successful UART transfer of the whole image, cannot use stack as at this stage stack pointer is not initialized yet. Stack can only be used when BootROM is executing binary header, to preserve state of registers, since BootROM expects that. Change the ARM baudrate code to not use stack at all and put binary header specific pre + post code (which stores and restores registers) into separate arrays. The baudrate change code now jumps at it's end and expects that there is either code which returns to the BootROM or jumps to the original exec address. Signed-off-by: Pali Rohár Reviewed-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 114 ++++++++++++++++++++++++++++--------------------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 62c218ef64..359b43c0d8 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -78,14 +78,7 @@ struct kwboot_block { #define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */ #define KWBOOT_HDR_RSP_TIMEO 10000 /* ms */ -/* ARM code making baudrate changing function return to original exec address */ -static unsigned char kwboot_pre_baud_code[] = { - /* exec_addr: */ - 0x00, 0x00, 0x00, 0x00, /* .word 0 */ - 0x0c, 0xe0, 0x1f, 0xe5, /* ldr lr, exec_addr */ -}; - -/* ARM code for binary header injection to change baudrate */ +/* ARM code to change baudrate */ static unsigned char kwboot_baud_code[] = { /* ; #define UART_BASE 0xd0012000 */ /* ; #define THR 0x00 */ @@ -123,14 +116,12 @@ static unsigned char kwboot_baud_code[] = { /* ; return 0; */ /* ; } */ - 0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */ - /* ; r0 = UART_BASE */ 0x0d, 0x02, 0xa0, 0xe3, /* mov r0, #0xd0000000 */ 0x12, 0x0a, 0x80, 0xe3, /* orr r0, r0, #0x12000 */ /* ; r2 = address of preamble string */ - 0xcc, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ + 0xc8, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ /* ; Send preamble string over UART */ /* .Lloop_preamble: */ @@ -177,7 +168,7 @@ static unsigned char kwboot_baud_code[] = { /* ; Read old baudrate value */ /* ; r2 = old_baudrate */ - 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ + 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ /* ; Calculate base clock */ /* ; r1 = r2 * r1 */ @@ -185,7 +176,7 @@ static unsigned char kwboot_baud_code[] = { /* ; Read new baudrate value */ /* ; r2 = new_baudrate */ - 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */ + 0x80, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */ /* ; Calculate new Divisor Latch */ /* ; r1 = DIV_ROUND(r1, r2) = */ @@ -234,9 +225,7 @@ static unsigned char kwboot_baud_code[] = { 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ 0xfc, 0xff, 0xff, 0x1a, /* bne .Lloop_sleep */ - /* ; Return 0 - no error */ - 0x00, 0x00, 0xa0, 0xe3, /* mov r0, #0 */ - 0xfe, 0x9f, 0xbd, 0xe8, /* pop { r1 - r12, pc } */ + 0x05, 0x00, 0x00, 0xea, /* b end */ /* ; Preamble string */ /* preamble: */ @@ -252,10 +241,29 @@ static unsigned char kwboot_baud_code[] = { /* ; Placeholder for new baudrate value */ /* new_baudrate: */ 0x00, 0x00, 0x00, 0x00, /* .word 0 */ + + /* end: */ }; -#define KWBOOT_BAUDRATE_BIN_HEADER_SZ (sizeof(kwboot_baud_code) + \ - sizeof(struct opt_hdr_v1) + 8 + 16) +/* ARM code for storing registers for future returning back to the bootrom */ +static unsigned char kwboot_baud_code_binhdr_pre[] = { + 0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */ +}; + +/* ARM code for returning back to the bootrom */ +static unsigned char kwboot_baud_code_binhdr_post[] = { + /* ; Return 0 - no error */ + 0x00, 0x00, 0xa0, 0xe3, /* mov r0, #0 */ + 0xfe, 0x9f, 0xbd, 0xe8, /* pop { r1 - r12, pc } */ +}; + +/* ARM code for jumping to the original image exec_addr */ +static unsigned char kwboot_baud_code_data_jump[] = { + 0x04, 0xf0, 0x1f, 0xe5, /* ldr pc, exec_addr */ + /* ; Placeholder for exec_addr */ + /* exec_addr: */ + 0x00, 0x00, 0x00, 0x00, /* .word 0 */ +}; static const char kwb_baud_magic[16] = "$baudratechange"; @@ -1409,44 +1417,51 @@ kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz) } static void -_inject_baudrate_change_code(void *img, size_t *size, int pre, +_inject_baudrate_change_code(void *img, size_t *size, int for_data, int old_baud, int new_baud) { - uint32_t codesz = sizeof(kwboot_baud_code); struct main_hdr_v1 *hdr = img; + uint32_t orig_datasz; + uint32_t codesz; uint8_t *code; - if (pre) { - uint32_t presz = sizeof(kwboot_pre_baud_code); - uint32_t orig_datasz; - + if (for_data) { orig_datasz = le32_to_cpu(hdr->blocksize) - sizeof(uint32_t); - code = kwboot_img_grow_data_right(img, size, presz + codesz); - - /* - * We need to prepend code that loads lr register with original - * value of hdr->execaddr. We do this by putting the original - * exec address before the code that loads it relatively from - * it's beginning. - * Afterwards we change the exec address to this code (which is - * at offset 4, because the first 4 bytes contain the original - * exec address). - */ - memcpy(code, kwboot_pre_baud_code, presz); - *(uint32_t *)code = hdr->execaddr; - - hdr->execaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) + - orig_datasz + 4); - - code += presz; + codesz = sizeof(kwboot_baud_code) + + sizeof(kwboot_baud_code_data_jump); + code = kwboot_img_grow_data_right(img, size, codesz); } else { + codesz = sizeof(kwboot_baud_code_binhdr_pre) + + sizeof(kwboot_baud_code) + + sizeof(kwboot_baud_code_binhdr_post); code = kwboot_add_bin_ohdr_v1(img, size, codesz); + + codesz = sizeof(kwboot_baud_code_binhdr_pre); + memcpy(code, kwboot_baud_code_binhdr_pre, codesz); + code += codesz; } - memcpy(code, kwboot_baud_code, codesz - 8); - *(uint32_t *)(code + codesz - 8) = cpu_to_le32(old_baud); - *(uint32_t *)(code + codesz - 4) = cpu_to_le32(new_baud); + codesz = sizeof(kwboot_baud_code) - 2 * sizeof(uint32_t); + memcpy(code, kwboot_baud_code, codesz); + code += codesz; + *(uint32_t *)code = cpu_to_le32(old_baud); + code += sizeof(uint32_t); + *(uint32_t *)code = cpu_to_le32(new_baud); + code += sizeof(uint32_t); + + if (for_data) { + codesz = sizeof(kwboot_baud_code_data_jump) - sizeof(uint32_t); + memcpy(code, kwboot_baud_code_data_jump, codesz); + code += codesz; + *(uint32_t *)code = hdr->execaddr; + code += sizeof(uint32_t); + hdr->execaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) + orig_datasz); + } else { + codesz = sizeof(kwboot_baud_code_binhdr_post); + memcpy(code, kwboot_baud_code_binhdr_post, codesz); + code += codesz; + } } static int @@ -1729,10 +1744,13 @@ main(int argc, char **argv) baudrate = 0; else /* ensure we have enough space for baudrate change code */ - after_img_rsv += KWBOOT_BAUDRATE_BIN_HEADER_SZ + - KWBOOT_XM_BLKSZ + - sizeof(kwboot_pre_baud_code) + + after_img_rsv += sizeof(struct opt_hdr_v1) + 8 + 16 + + sizeof(kwboot_baud_code_binhdr_pre) + sizeof(kwboot_baud_code) + + sizeof(kwboot_baud_code_binhdr_post) + + KWBOOT_XM_BLKSZ + + sizeof(kwboot_baud_code) + + sizeof(kwboot_baud_code_data_jump) + KWBOOT_XM_BLKSZ; if (imgpath) { From 62a98f496a0e8b0ffd67320e24834d400e45d841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 1 Nov 2021 14:00:02 +0100 Subject: [PATCH 27/28] tools: kwboot: Do not send magic seq when changing baudrate back to 115200 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After successful transfer of whole image only two things can happen: - BootROM starts execution of data block, which changes UART baudrate back to 115200 Bd, - board crashes and causes CPU reset In both cases UART baudrate is reset to the default speed. So there is no need to send special magic sequence to inform kwboot that baudrate is going to be reset and kwboot does not need to wait for this event and can do it immediately after BootROM acknowledges end of xmodem transfer. Move ARM code for sending magic sequence from main baudrate change section to binhdr_pre section which is executed only before changing baudrate from the default value of 115200 Bd to some new value. Remove kwboot code waiting for magic sequence after successful xmodem transfer. Rationale: sometimes when using very high UART speeds, magic sequence is damaged and kwboot fails at this last stage. Removal of this magic sequence makes booting more stable. Data transfer protocol (xmodem) is using checksums and retransmit, so it already deals with possible errors on transfer line. Signed-off-by: Pali Rohár Signed-off-by: Marek Behún Reviewed-by: Stefan Roese --- tools/kwboot.c | 115 ++++++++++++++++++++++++++----------------------- 1 file changed, 60 insertions(+), 55 deletions(-) diff --git a/tools/kwboot.c b/tools/kwboot.c index 359b43c0d8..bacca15301 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -81,23 +81,15 @@ struct kwboot_block { /* ARM code to change baudrate */ static unsigned char kwboot_baud_code[] = { /* ; #define UART_BASE 0xd0012000 */ - /* ; #define THR 0x00 */ /* ; #define DLL 0x00 */ /* ; #define DLH 0x04 */ /* ; #define LCR 0x0c */ /* ; #define DLAB 0x80 */ /* ; #define LSR 0x14 */ - /* ; #define THRE 0x20 */ /* ; #define TEMT 0x40 */ /* ; #define DIV_ROUND(a, b) ((a + b/2) / b) */ /* ; */ /* ; u32 set_baudrate(u32 old_b, u32 new_b) { */ - /* ; const u8 *str = "$baudratechange"; */ - /* ; u8 c; */ - /* ; do { */ - /* ; c = *str++; */ - /* ; writel(UART_BASE + THR, c); */ - /* ; } while (c); */ /* ; while */ /* ; (!(readl(UART_BASE + LSR) & TEMT)); */ /* ; u32 lcr = readl(UART_BASE + LCR); */ @@ -120,29 +112,6 @@ static unsigned char kwboot_baud_code[] = { 0x0d, 0x02, 0xa0, 0xe3, /* mov r0, #0xd0000000 */ 0x12, 0x0a, 0x80, 0xe3, /* orr r0, r0, #0x12000 */ - /* ; r2 = address of preamble string */ - 0xc8, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ - - /* ; Send preamble string over UART */ - /* .Lloop_preamble: */ - /* */ - /* ; Wait until Transmitter Holding is Empty */ - /* .Lloop_thre: */ - /* ; r1 = UART_BASE[LSR] & THRE */ - 0x14, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x14] */ - 0x20, 0x00, 0x11, 0xe3, /* tst r1, #0x20 */ - 0xfc, 0xff, 0xff, 0x0a, /* beq .Lloop_thre */ - - /* ; Put character into Transmitter FIFO */ - /* ; r1 = *r2++ */ - 0x01, 0x10, 0xd2, 0xe4, /* ldrb r1, [r2], #1 */ - /* ; UART_BASE[THR] = r1 */ - 0x00, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0] */ - - /* ; Loop until end of preamble string */ - 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ - 0xf8, 0xff, 0xff, 0x1a, /* bne .Lloop_preamble */ - /* ; Wait until Transmitter FIFO is Empty */ /* .Lloop_txempty: */ /* ; r1 = UART_BASE[LSR] & TEMT */ @@ -168,7 +137,7 @@ static unsigned char kwboot_baud_code[] = { /* ; Read old baudrate value */ /* ; r2 = old_baudrate */ - 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ + 0x74, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ /* ; Calculate base clock */ /* ; r1 = r2 * r1 */ @@ -176,7 +145,7 @@ static unsigned char kwboot_baud_code[] = { /* ; Read new baudrate value */ /* ; r2 = new_baudrate */ - 0x80, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */ + 0x70, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */ /* ; Calculate new Divisor Latch */ /* ; r1 = DIV_ROUND(r1, r2) = */ @@ -225,14 +194,8 @@ static unsigned char kwboot_baud_code[] = { 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ 0xfc, 0xff, 0xff, 0x1a, /* bne .Lloop_sleep */ - 0x05, 0x00, 0x00, 0xea, /* b end */ - - /* ; Preamble string */ - /* preamble: */ - 0x24, 0x62, 0x61, 0x75, /* .asciz "$baudratechange" */ - 0x64, 0x72, 0x61, 0x74, - 0x65, 0x63, 0x68, 0x61, - 0x6e, 0x67, 0x65, 0x00, + /* ; Jump to the end of execution */ + 0x01, 0x00, 0x00, 0xea, /* b end */ /* ; Placeholder for old baudrate value */ /* old_baudrate: */ @@ -245,12 +208,66 @@ static unsigned char kwboot_baud_code[] = { /* end: */ }; -/* ARM code for storing registers for future returning back to the bootrom */ +/* ARM code from binary header executed by BootROM before changing baudrate */ static unsigned char kwboot_baud_code_binhdr_pre[] = { + /* ; #define UART_BASE 0xd0012000 */ + /* ; #define THR 0x00 */ + /* ; #define LSR 0x14 */ + /* ; #define THRE 0x20 */ + /* ; */ + /* ; void send_preamble(void) { */ + /* ; const u8 *str = "$baudratechange"; */ + /* ; u8 c; */ + /* ; do { */ + /* ; while */ + /* ; ((readl(UART_BASE + LSR) & THRE)); */ + /* ; c = *str++; */ + /* ; writel(UART_BASE + THR, c); */ + /* ; } while (c); */ + /* ; } */ + + /* ; Preserve registers for BootROM */ 0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */ + + /* ; r0 = UART_BASE */ + 0x0d, 0x02, 0xa0, 0xe3, /* mov r0, #0xd0000000 */ + 0x12, 0x0a, 0x80, 0xe3, /* orr r0, r0, #0x12000 */ + + /* ; r2 = address of preamble string */ + 0x00, 0x20, 0x8f, 0xe2, /* adr r2, .Lstr_preamble */ + + /* ; Skip preamble data section */ + 0x03, 0x00, 0x00, 0xea, /* b .Lloop_preamble */ + + /* ; Preamble string */ + /* .Lstr_preamble: */ + 0x24, 0x62, 0x61, 0x75, /* .asciz "$baudratechange" */ + 0x64, 0x72, 0x61, 0x74, + 0x65, 0x63, 0x68, 0x61, + 0x6e, 0x67, 0x65, 0x00, + + /* ; Send preamble string over UART */ + /* .Lloop_preamble: */ + /* */ + /* ; Wait until Transmitter Holding is Empty */ + /* .Lloop_thre: */ + /* ; r1 = UART_BASE[LSR] & THRE */ + 0x14, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x14] */ + 0x20, 0x00, 0x11, 0xe3, /* tst r1, #0x20 */ + 0xfc, 0xff, 0xff, 0x0a, /* beq .Lloop_thre */ + + /* ; Put character into Transmitter FIFO */ + /* ; r1 = *r2++ */ + 0x01, 0x10, 0xd2, 0xe4, /* ldrb r1, [r2], #1 */ + /* ; UART_BASE[THR] = r1 */ + 0x00, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0] */ + + /* ; Loop until end of preamble string */ + 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ + 0xf8, 0xff, 0xff, 0x1a, /* bne .Lloop_preamble */ }; -/* ARM code for returning back to the bootrom */ +/* ARM code for returning from binary header back to BootROM */ static unsigned char kwboot_baud_code_binhdr_post[] = { /* ; Return 0 - no error */ 0x00, 0x00, 0xa0, 0xe3, /* mov r0, #0 */ @@ -1078,18 +1095,6 @@ kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate) return rc; if (baudrate) { - char buf[sizeof(kwb_baud_magic)]; - - kwboot_printv("Waiting 1s for baudrate change magic\n"); - rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000); - if (rc) - return rc; - - if (memcmp(buf, kwb_baud_magic, sizeof(buf))) { - errno = EPROTO; - return -1; - } - kwboot_printv("\nChanging baudrate back to 115200 Bd\n\n"); rc = kwboot_tty_change_baudrate(tty, 115200); if (rc) From 57fa6fb93291cdcabc1c38d874d12257a879f8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pali=20Roh=C3=A1r?= Date: Mon, 1 Nov 2021 10:12:51 +0100 Subject: [PATCH 28/28] arm: a37xx: pci: Program the data strobe for config read requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to the Armada 3720 Functional Specification Data Strobe applies for both read and write config requests. Data strobe bits configure which bytes from the start address should be returned for read request. Set value 0xf (all 4 bits) into Data Strobe register to read all four bytes from specified 32-bit config space register. Same value for Data Strobe register is programmed by Linux pci-aardvark.c driver for config read requests. Without this patch pci-aardvark driver sets data strobe register only during config write operations. So any followup config read operations could result with just partial datai returned (if previous write operation was not 32-bit wide). This patch fixes it and ensures that config read operations always read all bytes from requested register. Signed-off-by: Pali Rohár Reviewed-by: Stefan Roese --- drivers/pci/pci-aardvark.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index 9e623b6e61..4e94b776c5 100644 --- a/drivers/pci/pci-aardvark.c +++ b/drivers/pci/pci-aardvark.c @@ -472,6 +472,9 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, advk_writel(pcie, reg, PIO_ADDR_LS); advk_writel(pcie, 0, PIO_ADDR_MS); + /* Program the data strobe */ + advk_writel(pcie, 0xf, PIO_WR_DATA_STRB); + retry_count = 0; retry: