Re: [PATCH] watchdog: use time_after_eq() in watchdog_reset()
On 15.04.21 08:54, Rasmus Villemoes wrote: On 15/04/2021 07.38, Stefan Roese wrote: On 13.04.21 16:43, Rasmus Villemoes wrote: Some boards don't work with the rate-limiting done in the generic watchdog_reset() provided by wdt-uclass. For example, on powerpc, get_timer() ceases working during bootm since interrupts are disabled before the kernel image gets decompressed, and when the decompression takes longer than the watchdog device allows (or enough of the budget that the kernel doesn't get far enough to assume responsibility for petting the watchdog), the result is a non-booting board. As a somewhat hacky workaround (because DT is supposed to describe hardware), allow specifying hw_margin_ms=0 in device tree to effectively disable the ratelimiting and actually ping the watchdog every time watchdog_reset() is called. For that to work, the "has enough time passed" check just needs to be tweaked a little to allow the now==next_reset case as well. Suggested-by: Christophe Leroy Signed-off-by: Rasmus Villemoes --- It's the option I dislike the most (because of the DT abuse), but I also do accept that it's the one with the minimal code impact, and apparently the path of least resistance. So here it is. Right. An alternative way would have been to add a new Kconfig symbol to define the default value of "reset_period" so that it can be configured to different values via Kconfig as well. No, I don't think we should not go in that direction. Another thing I have on my todo-list is to rewrite the watchdog_reset() in wdt-uclass to handle _all_ DM watchdogs, not just the first one. Great. This is a current flaw in the U-Boot WDT implementation. Some boards make use of both the one in the CPU/SOC as well as some gpio-triggered one. Both the hw_margin_ms/reset_period and the last_reset data would then have to live with the device, not be static variables. Last I looked, it does seem that the DM code supports having a piece of class-owned, per-device data, so it shouldn't be too hard to do. Thanks for looking into this. Thanks, Stefan
Re: [PATCH v2 2/2] powerpc: introduce CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD
On 15.04.21 08:46, Rasmus Villemoes wrote: On 15/04/2021 07.34, Stefan Roese wrote: On 13.04.21 21:38, Rasmus Villemoes wrote: I have checked with objdump that the generated code doesn't change when this option is left at its default value of 0: gcc is smart enough to see that wd_limit is constant 0, the ">=" comparisons are tautologically true, hence all assignments to "flushed" are eliminated Nitpicking: s/tautologically/autologically. No. https://www.merriam-webster.com/dictionary/tautological https://en.wikipedia.org/wiki/Tautology_(logic) "In Mathematical logic, a tautology (from Greek: ταυτολογία) is a formula or assertion that is true in every possible interpretation." It even exists in gcc docs in the form of -Wtautological-compare. [But I do admit that "tautologically true" is a pleonasm; "tautologies" or "automatically true" could have been used instead. And going full meta, now that I look up pleonasm in wikipedia to check that I'm using it right, that article has a reference to "tautology (language)" in its second sentence.] Interesting. Thanks for the detailed info. /* wait for all dcbst to complete on bus */ asm volatile("sync" : : : "memory"); @@ -27,7 +33,11 @@ void flush_cache(ulong start_addr, ulong size) for (addr = start; (addr <= end) && (addr >= start); addr += CONFIG_SYS_CACHELINE_SIZE) { asm volatile("icbi 0,%0" : : "r" (addr) : "memory"); - WATCHDOG_RESET(); + flushed += CONFIG_SYS_CACHELINE_SIZE; + if (flushed >= wd_limit) { + WATCHDOG_RESET(); + flushed = 0; + } It might make sense to pull these 2 identical code snippets into a function to not duplicate this code. Something like static ulong maybe_reset(ulong flushed) { const ulong wd_limit = ...; flushed += CONFIG_SYS_CACHELINE_SIZE; if (flushed >= wd_limit) { WATCHDOG_RESET(); flushed = 0; } return flushed; } and "flushed = maybe_reset(flushed);"? I don't think that improves readability. Yes, something like this. I agree that it's not really beautiful but for my personal taste it's a bit better. But I don't have hard feeling here. Let's see what other comment on this. Thanks, Stefan
Re: [PATCH] watchdog: use time_after_eq() in watchdog_reset()
On 15/04/2021 07.38, Stefan Roese wrote: > On 13.04.21 16:43, Rasmus Villemoes wrote: >> Some boards don't work with the rate-limiting done in the generic >> watchdog_reset() provided by wdt-uclass. >> >> For example, on powerpc, get_timer() ceases working during bootm since >> interrupts are disabled before the kernel image gets decompressed, and >> when the decompression takes longer than the watchdog device >> allows (or enough of the budget that the kernel doesn't get far enough >> to assume responsibility for petting the watchdog), the result is a >> non-booting board. >> >> As a somewhat hacky workaround (because DT is supposed to describe >> hardware), allow specifying hw_margin_ms=0 in device tree to >> effectively disable the ratelimiting and actually ping the watchdog >> every time watchdog_reset() is called. For that to work, the "has >> enough time passed" check just needs to be tweaked a little to allow >> the now==next_reset case as well. >> >> Suggested-by: Christophe Leroy >> Signed-off-by: Rasmus Villemoes >> --- >> >> It's the option I dislike the most (because of the DT abuse), but I >> also do accept that it's the one with the minimal code impact, and >> apparently the path of least resistance. So here it is. > > Right. An alternative way would have been to add a new Kconfig symbol > to define the default value of "reset_period" so that it can be > configured to different values via Kconfig as well. No, I don't think we should not go in that direction. Another thing I have on my todo-list is to rewrite the watchdog_reset() in wdt-uclass to handle _all_ DM watchdogs, not just the first one. Some boards make use of both the one in the CPU/SOC as well as some gpio-triggered one. Both the hw_margin_ms/reset_period and the last_reset data would then have to live with the device, not be static variables. Last I looked, it does seem that the DM code supports having a piece of class-owned, per-device data, so it shouldn't be too hard to do. Rasmus
Re: [PATCH v2 2/2] powerpc: introduce CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD
On 15/04/2021 07.34, Stefan Roese wrote: > On 13.04.21 21:38, Rasmus Villemoes wrote: >> >> I have checked with objdump that the generated code doesn't change >> when this option is left at its default value of 0: gcc is smart >> enough to see that wd_limit is constant 0, the ">=" comparisons are >> tautologically true, hence all assignments to "flushed" are eliminated > > Nitpicking: > s/tautologically/autologically. No. https://www.merriam-webster.com/dictionary/tautological https://en.wikipedia.org/wiki/Tautology_(logic) "In Mathematical logic, a tautology (from Greek: ταυτολογία) is a formula or assertion that is true in every possible interpretation." It even exists in gcc docs in the form of -Wtautological-compare. [But I do admit that "tautologically true" is a pleonasm; "tautologies" or "automatically true" could have been used instead. And going full meta, now that I look up pleonasm in wikipedia to check that I'm using it right, that article has a reference to "tautology (language)" in its second sentence.] >> /* wait for all dcbst to complete on bus */ >> asm volatile("sync" : : : "memory"); >> @@ -27,7 +33,11 @@ void flush_cache(ulong start_addr, ulong size) >> for (addr = start; (addr <= end) && (addr >= start); >> addr += CONFIG_SYS_CACHELINE_SIZE) { >> asm volatile("icbi 0,%0" : : "r" (addr) : "memory"); >> - WATCHDOG_RESET(); >> + flushed += CONFIG_SYS_CACHELINE_SIZE; >> + if (flushed >= wd_limit) { >> + WATCHDOG_RESET(); >> + flushed = 0; >> + } > > It might make sense to pull these 2 identical code snippets into a > function to not duplicate this code. Something like static ulong maybe_reset(ulong flushed) { const ulong wd_limit = ...; flushed += CONFIG_SYS_CACHELINE_SIZE; if (flushed >= wd_limit) { WATCHDOG_RESET(); flushed = 0; } return flushed; } and "flushed = maybe_reset(flushed);"? I don't think that improves readability. Thanks, Rasmus
Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay
On Thu, Apr 15, 2021 at 4:41 AM Ramon Fried wrote: > > On Wed, Apr 14, 2021 at 5:36 PM Marek Vasut wrote: > > > > On 4/14/21 4:07 PM, Patrick DELAUNAY wrote: > > > Hi, > > > > Hi, > > > > > On 4/9/21 2:22 PM, Marek Vasut wrote: > > >> On 4/9/21 10:00 AM, Patrick Delaunay wrote: > > >>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver > > >>> but the value should be read from DT. > > >>> > > >>> STM32 use the generic binding defined in linux: > > >>> Documentation/devicetree/bindings/net/ethernet-phy.yaml > > >>> > > >>>reset-gpios: > > >>> maxItems: 1 > > >>> description: > > >>>The GPIO phandle and specifier for the PHY reset signal. > > >>> > > >>>reset-assert-us: > > >>> description: > > >>>Delay after the reset was asserted in microseconds. If this > > >>>property is missing the delay will be skipped. > > >>> > > >>>reset-deassert-us: > > >>> description: > > >>>Delay after the reset was deasserted in microseconds. If > > >>>this property is missing the delay will be skipped. > > >>> > > >>> See also U-Boot: doc/device-tree-bindings/net/phy.txt > > >> > > >> Since this is a PHY property, shouldn't that be handled in > > >> drivers/net/phy/ instead of MAC driver ? > > > > > > > > > I was my first idea but I don't found found the correct location in phy > > > (driver or uclass) > > > > > > to manage these generic property and the generic property "reset-gpios" > > > was already > > > > > > managed in eth driver, so I continue to patch the driver. > > > > > > > > > But I come back to this idea after your remark > > > > > > => in linux these property are managed in > > > > > > drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register > > > > > > parse DT node and add info in mdio > > > > > > drivers/net/phy/mdio_device.c::mdio_device_reset > > > > > > called in mdio_probe / mdio_remove > > > > > > > > > In my first search, I don't found the same level in the U-Boot drivers > > > in drivers/net/phy/ > > > > Note that this is MDIO-wide PHY reset (e.g. you can have single reset > > line connected to multiple PHYs on single MDIO bus), this is not > > PHY-specific reset. > > > > > but I miss the uclass defined in drivers/net/eth-phy-uclass.c > > > > > > > > > Finally I think I need to manage the generic binding property > > > > > > (reset-gpios, reset-assert-us, reset-deassert-us) directly in > > > > > > => drivers/net/mdio-uclass > > > > > > > > > The GPIO RESET will be managed in mdio ops: pre_probe/ post_remove > > > > > > as it is done in linux > > > > > > warning: today post_remove ops don't exit in u-class. > > > > > > > > > Do you think it is the correct location ? > > > > For single-PHY reset, the correct location is in drivers/net/phy/ somewhere. > > > > > Do you think it should be a new serie (migrate the eqos property in mdio) > > > > > > after this eqos is accepted > > > > > > or I shoudl sent a new serie to replace this serie. > > > > I'll leave that decision to Ramon/Joe. > Joe, I'll leave this to you. You know what, let's go with the new series, please migrate it to the net/phy. Thanks, Ramon
Re: [PATCH] watchdog: use time_after_eq() in watchdog_reset()
On 13.04.21 16:43, Rasmus Villemoes wrote: Some boards don't work with the rate-limiting done in the generic watchdog_reset() provided by wdt-uclass. For example, on powerpc, get_timer() ceases working during bootm since interrupts are disabled before the kernel image gets decompressed, and when the decompression takes longer than the watchdog device allows (or enough of the budget that the kernel doesn't get far enough to assume responsibility for petting the watchdog), the result is a non-booting board. As a somewhat hacky workaround (because DT is supposed to describe hardware), allow specifying hw_margin_ms=0 in device tree to effectively disable the ratelimiting and actually ping the watchdog every time watchdog_reset() is called. For that to work, the "has enough time passed" check just needs to be tweaked a little to allow the now==next_reset case as well. Suggested-by: Christophe Leroy Signed-off-by: Rasmus Villemoes --- It's the option I dislike the most (because of the DT abuse), but I also do accept that it's the one with the minimal code impact, and apparently the path of least resistance. So here it is. Right. An alternative way would have been to add a new Kconfig symbol to define the default value of "reset_period" so that it can be configured to different values via Kconfig as well. drivers/watchdog/wdt-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 0603ffbd36..2687135296 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -148,7 +148,7 @@ void watchdog_reset(void) /* Do not reset the watchdog too often */ now = get_timer(0); - if (time_after(now, next_reset)) { + if (time_after_eq(now, next_reset)) { next_reset = now + reset_period; wdt_reset(gd->watchdog_dev); } Reviewed-by: Stefan Roese Thanks, Stefan
Re: [PATCH v2 2/2] powerpc: introduce CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD
On 13.04.21 21:38, Rasmus Villemoes wrote: When flush_cache() is called during boot on our ~7M kernel image, the hundreds of thousands of WATCHDOG_RESET calls end up adding significantly to boottime. Flushing a single cache line doesn't take many microseconds, so doing these calls for every cache line is complete overkill. The generic watchdog_reset() provided by wdt-uclass.c actually contains some rate-limiting logic that should in theory mitigate this, but alas, that rate-limiting must be disabled on powerpc because of its get_timer() implementation - get_timer() works just fine until interrupts are disabled, but it just so happens that the "big" flush_cache() call happens in the part of bootm where interrupts are indeed disabled. [1] [2] [3] I have checked with objdump that the generated code doesn't change when this option is left at its default value of 0: gcc is smart enough to see that wd_limit is constant 0, the ">=" comparisons are tautologically true, hence all assignments to "flushed" are eliminated Nitpicking: s/tautologically/autologically. BTW, I've never heard or read this word before. as dead stores. On our board, setting the option to something like 65536 ends up reducing total boottime by about 0.8 seconds. [1] https://patchwork.ozlabs.org/project/uboot/patch/20200605111657.28773-1-rasmus.villem...@prevas.dk/ [2] https://lists.denx.de/pipermail/u-boot/2021-April/446906.html [3] https://lists.denx.de/pipermail/u-boot/2021-April/447280.html Signed-off-by: Rasmus Villemoes --- arch/powerpc/Kconfig | 1 + arch/powerpc/lib/Kconfig | 9 + arch/powerpc/lib/cache.c | 14 -- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/lib/Kconfig diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6a2e88fed2..133447648c 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -49,5 +49,6 @@ source "arch/powerpc/cpu/mpc83xx/Kconfig" source "arch/powerpc/cpu/mpc85xx/Kconfig" source "arch/powerpc/cpu/mpc86xx/Kconfig" source "arch/powerpc/cpu/mpc8xx/Kconfig" +source "arch/powerpc/lib/Kconfig" endmenu diff --git a/arch/powerpc/lib/Kconfig b/arch/powerpc/lib/Kconfig new file mode 100644 index 00..b30b5edf7c --- /dev/null +++ b/arch/powerpc/lib/Kconfig @@ -0,0 +1,9 @@ +config CACHE_FLUSH_WATCHDOG_THRESHOLD + int "Bytes to flush between WATCHDOG_RESET calls" + default 0 + help + The flush_cache() function periodically, and by default for + every cache line, calls WATCHDOG_RESET(). When flushing a + large area, that may add a significant amount of + overhead. This option allows you to set a threshold for how + many bytes to flush between each WATCHDOG_RESET call. diff --git a/arch/powerpc/lib/cache.c b/arch/powerpc/lib/cache.c index 3e487f50fe..115ae8e160 100644 --- a/arch/powerpc/lib/cache.c +++ b/arch/powerpc/lib/cache.c @@ -12,6 +12,8 @@ void flush_cache(ulong start_addr, ulong size) { ulong addr, start, end; + ulong wd_limit = CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD; + ulong flushed = 0; start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1); end = start_addr + size - 1; @@ -19,7 +21,11 @@ void flush_cache(ulong start_addr, ulong size) for (addr = start; (addr <= end) && (addr >= start); addr += CONFIG_SYS_CACHELINE_SIZE) { asm volatile("dcbst 0,%0" : : "r" (addr) : "memory"); - WATCHDOG_RESET(); + flushed += CONFIG_SYS_CACHELINE_SIZE; + if (flushed >= wd_limit) { + WATCHDOG_RESET(); + flushed = 0; + } } /* wait for all dcbst to complete on bus */ asm volatile("sync" : : : "memory"); @@ -27,7 +33,11 @@ void flush_cache(ulong start_addr, ulong size) for (addr = start; (addr <= end) && (addr >= start); addr += CONFIG_SYS_CACHELINE_SIZE) { asm volatile("icbi 0,%0" : : "r" (addr) : "memory"); - WATCHDOG_RESET(); + flushed += CONFIG_SYS_CACHELINE_SIZE; + if (flushed >= wd_limit) { + WATCHDOG_RESET(); + flushed = 0; + } It might make sense to pull these 2 identical code snippets into a function to not duplicate this code. Other than this: Reviewed-by: Stefan Roese Thanks, Stefan
Re: [PATCH v2 1/2] powerpc: lib: remove leftover CONFIG_5xx
On 13.04.21 21:38, Rasmus Villemoes wrote: CONFIG_5xx hasn't existed since commit 502589777416 (powerpc, 5xx: remove support for 5xx). Remove this last mention of it. Signed-off-by: Rasmus Villemoes --- arch/powerpc/lib/cache.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/powerpc/lib/cache.c b/arch/powerpc/lib/cache.c index 3c3c470bbb..3e487f50fe 100644 --- a/arch/powerpc/lib/cache.c +++ b/arch/powerpc/lib/cache.c @@ -11,7 +11,6 @@ void flush_cache(ulong start_addr, ulong size) { -#ifndef CONFIG_5xx ulong addr, start, end; start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1); @@ -33,5 +32,4 @@ void flush_cache(ulong start_addr, ulong size) asm volatile("sync" : : : "memory"); /* flush prefetch queue */ asm volatile("isync" : : : "memory"); -#endif } Reviewed-by: Stefan Roese Thanks, Stefan
Re: [PATCH v6 1/7] riscv: dts: add fu740 support
Hi Green, > From: Green Wan [mailto:green@sifive.com] > Sent: Thursday, April 08, 2021 9:40 PM > Cc: bmeng...@gmail.com; Green Wan; Greentime Hu; Rick Jian-Zhi Chen(陳建志); > Paul Walmsley; Palmer Dabbelt; Anup Patel; Atish Patra; Pragnesh Patel; > Lukasz Majewski; Joe Hershberger; Ramon Fried; u-boot@lists.denx.de > Subject: [PATCH v6 1/7] riscv: dts: add fu740 support > > Add dts support for fu740. The HiFive Unmatched support is based on > fu740 cpu and drivers in following patch set. > > Signed-off-by: Green Wan > [greentime.hu: set fu740 speed to 1.2GHz] > Signed-off-by: Greentime Hu > Reviewed-by: Bin Meng > --- > arch/riscv/dts/Makefile | 1 + > arch/riscv/dts/fu740-c000-u-boot.dtsi | 105 ++ > arch/riscv/dts/fu740-c000.dtsi| 329 ++ > include/dt-bindings/clock/sifive-fu740-prci.h | 25 ++ > include/dt-bindings/reset/sifive-fu740-prci.h | 19 + > 5 files changed, 479 insertions(+) > create mode 100644 arch/riscv/dts/fu740-c000-u-boot.dtsi > create mode 100644 arch/riscv/dts/fu740-c000.dtsi > create mode 100644 include/dt-bindings/clock/sifive-fu740-prci.h > create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h > > diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile > index 8138d89d84..ed32f56c9e 100644 > --- a/arch/riscv/dts/Makefile > +++ b/arch/riscv/dts/Makefile > @@ -2,6 +2,7 @@ > > dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb > dtb-$(CONFIG_TARGET_SIFIVE_UNLEASHED) += hifive-unleashed-a00.dtb > +dtb-$(CONFIG_TARGET_SIFIVE_UNMATCHED) += hifive-unmatched-a00.dtb Makefile need the dts file, but it is not exist in this patch. It doesn't make sense. Maybe you can combine with the dts relative files in [PATCH v6 6/7] into one patch and name as : riscv: dts: ... LGTM. Other than that, Reviewed-by: Rick Chen > dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb > dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb > > diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi > b/arch/riscv/dts/fu740-c000-u-boot.dtsi > new file mode 100644 > index 00..a5d0688b06 > --- /dev/null > +++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi > @@ -0,0 +1,105 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +/* > + * (C) Copyright 2020-2021 SiFive, Inc > + */ > + > +#include > + > +/ { > + cpus { > + assigned-clocks = <&prci PRCI_CLK_COREPLL>; > + assigned-clock-rates = <12>; > + u-boot,dm-spl; > + cpu0: cpu@0 { > + clocks = <&prci PRCI_CLK_COREPLL>; > + u-boot,dm-spl; > + status = "okay"; > + cpu0_intc: interrupt-controller { > + u-boot,dm-spl; > + }; > + }; > + cpu1: cpu@1 { > + clocks = <&prci PRCI_CLK_COREPLL>; > + u-boot,dm-spl; > + cpu1_intc: interrupt-controller { > + u-boot,dm-spl; > + }; > + }; > + cpu2: cpu@2 { > + clocks = <&prci PRCI_CLK_COREPLL>; > + u-boot,dm-spl; > + cpu2_intc: interrupt-controller { > +u-boot,dm-spl; > + }; > + }; > + cpu3: cpu@3 { > + clocks = <&prci PRCI_CLK_COREPLL>; > + u-boot,dm-spl; > + cpu3_intc: interrupt-controller { > + u-boot,dm-spl; > + }; > + }; > + cpu4: cpu@4 { > + clocks = <&prci PRCI_CLK_COREPLL>; > + u-boot,dm-spl; > + cpu4_intc: interrupt-controller { > + u-boot,dm-spl; > + }; > + }; > + }; > + > + soc { > + u-boot,dm-spl; > + clint: clint@200 { > + compatible = "riscv,clint0"; > + interrupts-extended = <&cpu0_intc 3 &cpu0_intc 7 > + &cpu1_intc 3 &cpu1_intc 7 > + &cpu2_intc 3 &cpu2_intc 7 > + &cpu3_intc 3 &cpu3_intc 7 > + &cpu4_intc 3 &cpu4_intc 7>; > + reg = <0x0 0x200 0x0 0x1>; > + u-boot,dm-spl; > + }; > + prci: clock-controller@1000 { > + #reset-cells = <1>; > + resets = <&prci PRCI_RST_DDR_CTRL_N>, > +<&prci PRCI_RST_DDR_AXI_N>, > +<&prci PRCI_RST_DDR_AHB_N>, > +
Re: [PATCH] mtd: cfi: Fix PPB lock status readout
On 11.04.21 20:47, Marek Vasut wrote: According to S26KL512S datasheet [1] and S29GL01GS datasheet [2], the procedure to read out PPB lock bits is to send the PPB Entry, PPB Read, Reset/ASO Exit. Currently, the code does send incorrect PPB Entry, PPB Read and Reset/ASO Exit is completely missing. The PPB Entry sent is implemented by sending flash_unlock_seq() and flash_write_cmd(..., FLASH_CMD_READ_ID). This translates to sequence 0x555:0xaa, 0x2aa:0x55, 0x555:0x90=FLASH_CMD_READ_ID. However, both [1] and [2] specify the last byte of PPB Entry as 0xc0=AMD_CMD_SET_PPB_ENTRY instead of 0x90=FLASH_CMD_READ_ID, that is 0x555:0xaa, 0x2aa:0x55, 0x555:0xc0=AMD_CMD_SET_PPB_ENTRY. Since this does make sense, this patch fixes it and thus also aligns the code in flash_get_size() with flash_real_protect(). The PPB Read returns 00h in case of Protected state and 01h in case of Unprotected state, according to [1] Note 83 and [2] Note 17, so invert the result. Moreover, align the arguments with similar code in flash_real_protect(). Finally, Reset/ASO Exit command should be executed to exit the PPB mode, so add the missing reset. [1] https://www.cypress.com/file/213346/download Document Number: 001-99198 Rev. *M Table 40. Command Definitions, Nonvolatile Sector Protection Command Set Definitions [2] https://www.cypress.com/file/177976/download Document Number: 001-98285 Rev. *R Table 7.1 Command Definitions, Nonvolatile Sector Protection Command Set Definitions Fixes: 03deff433e ("cfi_flash: Read PPB sector protection from device for AMD/Spansion chips") Signed-off-by: Marek Vasut Cc: Stefan Roese Reviewed-by: Stefan Roese Thanks, Stefan --- drivers/mtd/cfi_flash.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 9642d7c7dc..9c27fea5d8 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -2276,12 +2276,12 @@ ulong flash_get_size(phys_addr_t base, int banknum) flash_unlock_seq(info, 0); flash_write_cmd(info, 0, info->addr_unlock1, - FLASH_CMD_READ_ID); + AMD_CMD_SET_PPB_ENTRY); info->protect[sect_cnt] = - flash_isset( - info, sect_cnt, - FLASH_OFFSET_PROTECT, - FLASH_STATUS_PROTECT); + !flash_isset(info, sect_cnt, +0, 0x01); + flash_write_cmd(info, 0, 0, + info->cmd_reset); break; default: /* default: not protected */ 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: s...@denx.de
Re: [PATCH] arm: at91: gardena-smart-gateway-at91sam: Adjust to production values
Hi Reto, On 14.04.21 16:49, Reto Schneider wrote: From: Reto Schneider This commit updates the default config with the values that are actually used "in the wild" and which are close to what is used on the MediaTek MT7688 based, 2nd generation of the GARDENA smart gateway: - Reduce startup time by setting bootdelay to 0 (still allows accessing the shell, one just has to send a key press quicker) - Adjusting U-Boot environment volume names and MTD partitions to the actual layout Signed-off-by: Reto Schneider --- configs/gardena-smart-gateway-at91sam_defconfig | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/configs/gardena-smart-gateway-at91sam_defconfig b/configs/gardena-smart-gateway-at91sam_defconfig index e3d5bc47d6..76a1c42ce0 100644 --- a/configs/gardena-smart-gateway-at91sam_defconfig +++ b/configs/gardena-smart-gateway-at91sam_defconfig @@ -24,6 +24,7 @@ CONFIG_NAND_BOOT=y CONFIG_BOOTDELAY=3 Please see below... CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk mtdparts=atmel_nand:256k(bootstrap)ro,768k(uboot)ro,256k(env_redundant),256k(env),512k(dtb),6M(kernel)ro,-(rootfs) rootfstype=ubifs ubi.mtd=6 root=ubi0:rootfs rw" +CONFIG_BOOTDELAY=0 I'm wondering if this defconfig was generated via make savedefconfig? As there are multiple configurations for BOOTDELAY. Could you please double-check again? Other than that: Reviewed-by: Stefan Roese Thanks, Stefan CONFIG_SYS_CONSOLE_IS_IN_ENV=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_SPL_SYS_MALLOC_SIMPLE=y @@ -46,8 +47,8 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y CONFIG_CMD_FAT=y CONFIG_CMD_MTDPARTS=y -CONFIG_MTDIDS_DEFAULT="nand0=nand0" -CONFIG_MTDPARTS_DEFAULT="nand0:1536k(uboot),1024k(unused),512k(dtb_old),4608k(kernel_old),86528k(ubi),-(rootfs_old)" +CONFIG_MTDIDS_DEFAULT="nand0=atmel_nand" +CONFIG_MTDPARTS_DEFAULT="atmel_nand:1536k(uboot),10752k(unused),-(ubi)" CONFIG_CMD_UBI=y CONFIG_OF_CONTROL=y CONFIG_SPL_OF_CONTROL=y @@ -55,8 +56,8 @@ CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clocks clock-names interrupt CONFIG_ENV_IS_IN_UBI=y CONFIG_SYS_REDUNDAND_ENVIRONMENT=y CONFIG_ENV_UBI_PART="ubi" -CONFIG_ENV_UBI_VOLUME="env" -CONFIG_ENV_UBI_VOLUME_REDUND="env_r" +CONFIG_ENV_UBI_VOLUME="uboot_env0" +CONFIG_ENV_UBI_VOLUME_REDUND="uboot_env1" CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_DM=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: s...@denx.de
Please pull u-boot-x86
Hi Tom, This PR includes the following changes for v2021.07: - Minor fix to Apollo Lake devicetree bindings for FSP - Refactor Designware PCIe drivers to core and SoC parts - Add Amlogic Meson Designware PCIe controller driver Azure results: PASS https://dev.azure.com/bmeng/GitHub/_build/results?buildId=346&view=results The following changes since commit a94ab561e2f49a80d8579930e840b810ab1a1330: Merge branch '2021-04-13-assorted-improvements' (2021-04-13 09:50:45 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-x86 for you to fetch changes up to 2c32c701ea6cc04881589c09cc229ee77acae723: pci: add Amlogic Meson Designware PCIe controller (2021-04-15 10:43:17 +0800) Neil Armstrong (4): pci: add common Designware PCIe functions pci: pcie_dw_ti: migrate to common Designware PCIe functions pci: pcie_dw_rockchip: migrate to common Designware PCIe functions pci: add Amlogic Meson Designware PCIe controller Wolfgang Wallner (2): dt-bindings: fsp: Fix Apollo Lake FSP-S devicetree bindings x86: mtrr: Fix function descriptions arch/x86/include/asm/mtrr.h| 4 +- doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt | 19 ++--- drivers/pci/Kconfig| 16 +++- drivers/pci/Makefile | 2 + drivers/pci/pcie_dw_common.c | 365 +++ drivers/pci/pcie_dw_common.h | 155 drivers/pci/pcie_dw_meson.c| 459 + drivers/pci/pcie_dw_rockchip.c | 472 +++- drivers/pci/pcie_dw_ti.c | 444 + 9 files changed, 1073 insertions(+), 863 deletions(-) create mode 100644 drivers/pci/pcie_dw_common.c create mode 100644 drivers/pci/pcie_dw_common.h create mode 100644 drivers/pci/pcie_dw_meson.c Regards, Bin
Re: [PATCH RFT v2 0/4] pci: add common Designware PCIe functions and support Amlogic Meson PCIe controller
On Thu, Apr 15, 2021 at 6:24 AM Bin Meng wrote: > > Hi, > > On Thu, Apr 8, 2021 at 3:41 PM Neil Armstrong wrote: > > > > On 08/04/2021 07:29, Bin Meng wrote: > > > Hi Neil, > > > > > > On Tue, Apr 6, 2021 at 8:22 PM Neil Armstrong > > > wrote: > > >> > > >> Hi Bin, > > >> > > >> On 25/03/2021 15:49, Neil Armstrong wrote: > > >>> With the introduction of pcie_dw_rockchip, and need to support the DW > > >>> PCIe in the > > >>> Amlogic AXG & G12 SoCs, most of the DW PCIe helpers would be duplicated. > > >>> > > >>> This introduce a "common" DW PCIe helpers file with common code merged > > >>> from the > > >>> dw_ti and dw_rockchip drivers and adapted to fit with the upcoming > > >>> dw_meson. > > >>> > > >>> The following changes will switch the dw_ti and dw_rockchip, and > > >>> introduce a new > > >>> driver to support the Amlogic AXG & G12 SoCs using these new common > > >>> helpers. > > >>> > > >>> The dw_meson has been validated, but the dw_ti and dw_rockchip would > > >>> need testing > > >>> on hardware to validate nothing has been broken. > > >> > > >> How should be proceed ? I can't possibly test patches 2 & 3, but Green > > >> has tested it > > >> on Sifive, should patch 1 & 4 be merged then we should wait feedback > > >> from the Ti & Rockchip > > >> owners ? > > > > > > This series is not assigned to me in patchwork so I cannot decide > > > anything :) > > > > I can switch them to you, but I never know who to assign for these > > multi-vendor series... > > > > > > > > But my opinion was that let's wait for some more time, and if nobody > > > jumps out let's merge this series. > > > > Fine fore me ! > > I am going to apply this series via the x86 tree if there are no > further comments. series applied to u-boot-x86, thanks!
Re: [PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus
Hi Bin, On Thu, 15 Apr 2021 06:30:27 +0800 Bin Meng wrote: > Hi, > > On Thu, Apr 15, 2021 at 3:39 AM Simon Glass wrote: > > > > On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu > > wrote: > > > > > > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges") > > > sets the VGA bridge bits by checking pplat->class, but if the parent > > > device is the pci host bus device, it can be skipped. Moreover, it > > > shouldn't access the pplat because the parent has different plat data. > > > > > > Without this fix, "pci enum" command cause a synchronous abort. > > > > > > pci_auto_config_devices: start > > > PCI Autoconfig: Bus Memory region: [7800-7fff], > > > Physical Memory [7800-7fffx] > > > PCI Autoconfig: Bus I/O region: [0-], > > > Physical Memory [77f0-77f0x] > > > pci_auto_config_devices: device pci_6:0.0 > > > PCI Autoconfig: BAR 0, Mem, size=0x100, address=0x7800 > > > bus_lower=0x7900 > > > > > > PCI Autoconfig: BAR 1, Mem, size=0x800, No room in resource, avail > > > start=7900 / size=800, need=800 > > > PCI: Failed autoconfig bar 14 > > > > > > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004 > > > > > > PCI Autoconfig: BAR 3, Mem, size=0x200, address=0x7a00 > > > bus_lower=0x7c00 > > > > > > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100 > > > > > > PCI Autoconfig: ROM, size=0x8, address=0x7c00 bus_lower=0x7c08 > > > > > > "Synchronous Abort" handler, esr 0x9606 > > > elr: e002bd28 lr : e002bce8 (reloc) > > > elr: fff6fd28 lr : fff6fce8 > > > x0 : 1041 x1 : 003e > > > x2 : ffb0f8c8 x3 : 0001 > > > x4 : 0080 x5 : > > > x6 : fff718fc x7 : 000f > > > x8 : ffb0f238 x9 : 0008 > > > x10: x11: 0010 > > > x12: 0006 x13: 0001869f > > > x14: ffb0fcd0 x15: 0020 > > > x16: fff71cc4 x17: > > > x18: ffb13d90 x19: ffb14320 > > > x20: x21: ffb14090 > > > x22: ffb0f8c8 x23: 0001 > > > x24: ffb14c10 x25: > > > x26: x27: > > > x28: ffb14c70 x29: ffb0f830 > > > > > > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280) > > > Resetting CPU ... > > > > > > Signed-off-by: Masami Hiramatsu > > > --- > > > drivers/pci/pci-uclass.c |3 +++ > > > 1 file changed, 3 insertions(+) > > > > Reviewed-by: Simon Glass > > I can't find this patch in my inbox, nor in the patchwork. Hmm, it is strange... I set up gitconfig but it seems not working well. Let me send patch via my MUA next time. > Could you please resend? OK, I attached it to this mail. (maybe it is safer in this case) Thank you, -- Linaro dm-pci-skip-setting-vga-bridge Description: Binary data
Re: [PATCH 1/8] ARM: dragonboard410c: Enable DM_ETH
On Sat, Apr 3, 2021 at 12:48 AM Peter Robinson wrote: > > The dragonboard410c only uses USB ethernet interfaces so just > enable DM_ETH. > > Signed-off-by: Peter Robinson > Cc: Ramon Fried > --- > configs/dragonboard410c_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/dragonboard410c_defconfig > b/configs/dragonboard410c_defconfig > index f5db4720b7..9c4311aa45 100644 > --- a/configs/dragonboard410c_defconfig > +++ b/configs/dragonboard410c_defconfig > @@ -33,6 +33,7 @@ CONFIG_FASTBOOT_FLASH=y > CONFIG_FASTBOOT_FLASH_MMC_DEV=0 > CONFIG_MSM_GPIO=y > CONFIG_PM8916_GPIO=y > +CONFIG_DM_ETH=y > CONFIG_LED=y > CONFIG_LED_GPIO=y > CONFIG_DM_MMC=y > -- > 2.31.1 > Acked-by: Ramon Fried
[PATCH v2] checkpatch: Ignore ENOSYS warnings
There are no system calls in U-Boot, but ENOSYS is still allowed (and preferred since 42a2668743 ("dm: core: Document the common error codes")). Silence this warning. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- Changes in v2: - Removed accidentally-included tag .checkpatch.conf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.checkpatch.conf b/.checkpatch.conf index ed0c2150ba..9e40ea060b 100644 --- a/.checkpatch.conf +++ b/.checkpatch.conf @@ -26,6 +26,9 @@ # addresses are __aligned(2)". --ignore PREFER_ETHER_ADDR_COPY +# ENOSYS is a conventionally used error, even though U-Boot lacks system calls. +--ignore ENOSYS + # A bit shorter of a description is OK with us. --min-conf-desc-length=2 -- 2.31.0
Re: [PATCH 3/6] net: calxedagmac: Convert to DM_ETH
On Mon, Apr 12, 2021 at 3:05 AM Andre Przywara wrote: > > To squash that nasty warning message and make better use of the newly > gained OF_CONTROL feature, let's convert the calxedagmac driver to the > "new" driver model. > The conversion is pretty straight forward, mostly just adjusting the > use of the involved data structures. > The only actual change is the required split of the receive routine into > a receive and free_pkt part. > Also this allows us to get rid of the hardcoded platform information and > explicit init calls. > > This also uses the opportunity to wrap the code decoding the MMIO > register base address, to make it safe for using PHYS_64BIT later. > > Signed-off-by: Andre Przywara > --- > arch/arm/Kconfig | 1 + > board/highbank/highbank.c| 13 --- > configs/highbank_defconfig | 1 + > drivers/net/Kconfig | 7 ++ > drivers/net/calxedaxgmac.c | 192 +++ > include/configs/highbank.h | 2 - > include/netdev.h | 1 - > scripts/config_whitelist.txt | 1 - > 8 files changed, 137 insertions(+), 81 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index bd6064923fe..0082d06182a 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -756,6 +756,7 @@ config ARCH_HIGHBANK > select CLK > select CLK_CCF > select AHCI > + select DM_ETH > > config ARCH_INTEGRATOR > bool "ARM Ltd. Integrator family" > diff --git a/board/highbank/highbank.c b/board/highbank/highbank.c > index 2e2300a307f..0667a48965c 100644 > --- a/board/highbank/highbank.c > +++ b/board/highbank/highbank.c > @@ -10,7 +10,6 @@ > #include > #include > #include > -#include > #include > #include > > @@ -52,18 +51,6 @@ int board_init(void) > return 0; > } > > -/* We know all the init functions have been run now */ > -int board_eth_init(struct bd_info *bis) > -{ > - int rc = 0; > - > -#ifdef CONFIG_CALXEDA_XGMAC > - rc += calxedaxgmac_initialize(0, 0xfff5); > - rc += calxedaxgmac_initialize(1, 0xfff51000); > -#endif > - return rc; > -} > - > #ifdef CONFIG_SCSI_AHCI_PLAT > void scsi_init(void) > { > diff --git a/configs/highbank_defconfig b/configs/highbank_defconfig > index 773ed7a00bf..c3352b827d7 100644 > --- a/configs/highbank_defconfig > +++ b/configs/highbank_defconfig > @@ -27,3 +27,4 @@ CONFIG_SCSI=y > CONFIG_CONS_INDEX=0 > CONFIG_OF_LIBFDT=y > CONFIG_OF_BOARD=y > +CONFIG_CALXEDA_XGMAC=y > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index cf062fad4da..cebd84035c8 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -171,6 +171,13 @@ config CORTINA_NI_ENET > This driver supports the Cortina-Access Ethernet MAC for > all supported CA SoCs. > > +config CALXEDA_XGMAC > + bool "Calxeda XGMAC support" > + depends on DM_ETH > + help > + This driver supports the XGMAC in Calxeda Highbank and Midway > + machines. > + > config DWC_ETH_QOS > bool "Synopsys DWC Ethernet QOS device support" > depends on DM_ETH > diff --git a/drivers/net/calxedaxgmac.c b/drivers/net/calxedaxgmac.c > index 8b2ee49b441..b98d709117a 100644 > --- a/drivers/net/calxedaxgmac.c > +++ b/drivers/net/calxedaxgmac.c > @@ -10,6 +10,8 @@ > #include > #include > #include > +#include > +#include /* for dev_set_priv() */ > > #define TX_NUM_DESC1 > #define RX_NUM_DESC32 > @@ -212,6 +214,18 @@ struct xgmac_dma_desc { > __le32 res[3]; > }; > > +static struct xgmac_regs *xgmac_get_regs(struct eth_pdata *pdata) > +{ > + /* > +* We use PHYS_64BIT on Highbank, so phys_addr_t is bigger than > +* a pointer. U-Boot doesn't use LPAE (not even the MMU on highbank), > +* so we can't access anything above 4GB. > +* We have a check in the probe function below the ensure this, > +* so casting to a 32-bit pointer type is fine here. > +*/ > + return (struct xgmac_regs *)(uintptr_t)pdata->iobase; > +} > + > /* XGMAC Descriptor Access Helpers */ > static inline void desc_set_buf_len(struct xgmac_dma_desc *p, u32 buf_sz) > { > @@ -304,8 +318,6 @@ struct calxeda_eth_dev { > > u32 tx_currdesc; > u32 rx_currdesc; > - > - struct eth_device *dev; > } __aligned(32); > > /* > @@ -313,10 +325,10 @@ struct calxeda_eth_dev { > * advanced descriptors. > */ > > -static void init_rx_desc(struct calxeda_eth_dev *priv) > +static void init_rx_desc(struct eth_pdata *pdata, struct calxeda_eth_dev > *priv) > { > struct xgmac_dma_desc *rxdesc = priv->rx_chain; > - struct xgmac_regs *regs = (struct xgmac_regs *)priv->dev->iobase; > + struct xgmac_regs *regs = xgmac_get_regs(pdata); > void *rxbuffer = priv->rxbuffer; > int i; > > @@ -330,17 +342,16 @@ static void init_rx_desc(struct calxeda_eth_dev *priv) > } > } > >
Re: [PATCH 0/2] net: jr2: Fix for jr2 switch
On Mon, Apr 12, 2021 at 12:26 AM Horatiu Vultur wrote: > > Hi, > > A gentle ping. Thanks. > > The 03/10/2021 09:31, Horatiu Vultur wrote: > > This patch series contains two patches. The first patch resets the > > switch at probe time while the second one fixes an issue with the > > serdes6g configuration which is used on jr2_pcb111 board > > > > Horatiu Vultur (2): > > net: jr2: Reset switch > > net: jr2: Fix Serdes6G configuration > > > > arch/mips/dts/mscc,jr2.dtsi | 6 ++-- > > drivers/net/mscc_eswitch/jr2_switch.c | 43 +++ > > 2 files changed, 42 insertions(+), 7 deletions(-) > > > > -- > > 2.30.1 > > > > -- > /Horatiu It was reviewed. it will be picked up by Tom eventually.
Re: [PATCH v3] cmd: net: Add the "arp" command
On Sat, Apr 10, 2021 at 5:17 PM wrote: > > From: Joe Xue > > The command is to query and show mac address of a specific ipAddress. > > Signed-off-by: Joe Xue > --- > > cmd/Kconfig | 6 ++ > cmd/net.c | 37 + > doc/usage/arp.rst | 31 +++ > include/net.h | 5 + > net/arp.c | 24 > net/arp.h | 4 > net/net.c | 8 > 7 files changed, 115 insertions(+) > create mode 100644 doc/usage/arp.rst > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index 9bf5e863e4..1da4cb67f6 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -1587,6 +1587,12 @@ config CMD_PING > help > Send ICMP ECHO_REQUEST to network host > > +config CMD_ARP > + bool "arp" > + help > + Sends ARP_REQUEST to network host and shows the result if there is > arp > + response. > + > config CMD_CDP > bool "cdp" > help > diff --git a/cmd/net.c b/cmd/net.c > index beb2877dfd..56703e9641 100644 > --- a/cmd/net.c > +++ b/cmd/net.c > @@ -480,3 +480,40 @@ U_BOOT_CMD( > ); > > #endif /* CONFIG_CMD_LINK_LOCAL */ > + > +#ifdef CONFIG_CMD_ARP > +static int do_arp(struct cmd_tbl *cmdtp, int flag, int argc, char *const > argv[]) > +{ > + u8 *ethaddr = arp_query_ethaddr; > + > + if (argc < 2) > + return CMD_RET_USAGE; > + > + arp_query_ip = string_to_ip(argv[1]); > + if (arp_query_ip.s_addr == 0) > + return CMD_RET_USAGE; > + > + if ((arp_query_ip.s_addr & net_netmask.s_addr) != > + (net_ip.s_addr & net_netmask.s_addr)) { > + printf("The host %s is not in the same network\n", argv[1]); > + return CMD_RET_SUCCESS; > + } Why do we care about that ? > + > + if (net_loop(ARP) < 0) { > + printf("arp failed; host %s is not alive\n", argv[1]); > + return CMD_RET_FAILURE; > + } > + > + printf("%s\t%02x:%02x:%02x:%02x:%02x:%02x\n", argv[1], ethaddr[0], > + ethaddr[1], ethaddr[2], ethaddr[3], ethaddr[4], ethaddr[5]); > + > + return CMD_RET_SUCCESS; > +} > + > +U_BOOT_CMD( > + arp,2, 1, do_arp, > + "send ARP ARP_REQUEST to network host", > + "ipAddress" > +); > + > +#endif /* CONFIG_CMD_ARP */ > diff --git a/doc/usage/arp.rst b/doc/usage/arp.rst > new file mode 100644 > index 00..b1f08a2ae9 > --- /dev/null > +++ b/doc/usage/arp.rst > @@ -0,0 +1,31 @@ > +arp command > +=== > + > +Synopis Typo, Synopsis > +--- > + > +:: > + > +arp ipAddress > + > +Description > +--- > + > +The arp command is used to send ARP_REQUEST to network host and show the > result. > + > +ipAddress > +the host ip address > + > +Example > +--- > + > +:: > + > +=> arp 192.168.0.1 > +Using host_ens33 device > +192.168.0.1 84:94:8c:5f:e1:62 > + > +Return value > + > + > +The return value $? is 0 if the comand running was successful else 1 Typo, command > diff --git a/include/net.h b/include/net.h > index b95d6a6f60..60b4bf610e 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -580,6 +580,11 @@ extern char *net_dns_env_var; /* the env > var to put the ip into */ > extern struct in_addr net_ping_ip; /* the ip address to ping */ > #endif > > +#if defined(CONFIG_CMD_ARP) > +extern u8 arp_query_ethaddr[6];/* the arp query result */ > +extern struct in_addr arp_query_ip; /* the ip address to arp query */ > +#endif > + > #if defined(CONFIG_CMD_CDP) > /* when CDP completes these hold the return values */ > extern ushort cdp_native_vlan; /* CDP returned native VLAN */ > diff --git a/net/arp.c b/net/arp.c > index 1d06ed2572..83c24072d7 100644 > --- a/net/arp.c > +++ b/net/arp.c > @@ -220,11 +220,20 @@ void arp_receive(struct ethernet_hdr *et, struct > ip_udp_hdr *ip, int len) > net_get_arp_handler()((uchar *)arp, 0, reply_ip_addr, > 0, len); > > +#ifdef CONFIG_CMD_ARP > + if (arp_query_ip.s_addr != 0) { > + arp_query_ip.s_addr = 0; > + net_set_state(NETLOOP_SUCCESS); > + } else { > +#endif > /* set the mac address in the waiting packet's header >and transmit it */ > memcpy(((struct ethernet_hdr > *)net_tx_packet)->et_dest, >&arp->ar_sha, ARP_HLEN); > net_send_packet(net_tx_packet, > arp_wait_tx_packet_size); > +#ifdef CONFIG_CMD_ARP > + } > +#endif > > /* no arp request pending now */ > net_arp_wait_packet_ip.s_addr = 0; > @@ -243,3 +252,18 @@ bool arp_is_waiting(void) > { > return !!net_arp_wait_packet_ip.s_a
Re: [PATCH 13/37] net: fec_mxc: support i.MX8ULP
On Mon, Apr 12, 2021 at 2:53 PM Peng Fan (OSS) wrote: > > From: Peng Fan > > Support i.MX8ULP in fec_mxc > > Signed-off-by: Peng Fan > --- > drivers/net/Kconfig | 2 +- > drivers/net/fec_mxc.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index cf062fad4d..a443b499ba 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -314,7 +314,7 @@ config FEC_MXC_MDIO_BASE > > config FEC_MXC > bool "FEC Ethernet controller" > - depends on MX28 || MX5 || MX6 || MX7 || IMX8 || IMX8M || VF610 > + depends on MX28 || MX5 || MX6 || MX7 || IMX8 || IMX8M || IMX8ULP || > VF610 > help > This driver supports the 10/100 Fast Ethernet controller for > NXP i.MX processors. > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c > index ec21157d71..57ba856915 100644 > --- a/drivers/net/fec_mxc.c > +++ b/drivers/net/fec_mxc.c > @@ -628,7 +628,7 @@ static int fec_init(struct eth_device *dev, struct > bd_info *bd) > writel(0x, &fec->eth->gaddr2); > > /* Do not access reserved register */ > - if (!is_mx6ul() && !is_mx6ull() && !is_imx8() && !is_imx8m()) { > + if (!is_mx6ul() && !is_mx6ull() && !is_imx8() && !is_imx8m() && > !is_imx8ulp()) { > /* clear MIB RAM */ > for (i = mib_ptr; i <= mib_ptr + 0xfc; i += 4) > writel(0, i); > -- > 2.30.0 > Reviewed-by: Ramon Fried
Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay
On Wed, Apr 14, 2021 at 5:36 PM Marek Vasut wrote: > > On 4/14/21 4:07 PM, Patrick DELAUNAY wrote: > > Hi, > > Hi, > > > On 4/9/21 2:22 PM, Marek Vasut wrote: > >> On 4/9/21 10:00 AM, Patrick Delaunay wrote: > >>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver > >>> but the value should be read from DT. > >>> > >>> STM32 use the generic binding defined in linux: > >>> Documentation/devicetree/bindings/net/ethernet-phy.yaml > >>> > >>>reset-gpios: > >>> maxItems: 1 > >>> description: > >>>The GPIO phandle and specifier for the PHY reset signal. > >>> > >>>reset-assert-us: > >>> description: > >>>Delay after the reset was asserted in microseconds. If this > >>>property is missing the delay will be skipped. > >>> > >>>reset-deassert-us: > >>> description: > >>>Delay after the reset was deasserted in microseconds. If > >>>this property is missing the delay will be skipped. > >>> > >>> See also U-Boot: doc/device-tree-bindings/net/phy.txt > >> > >> Since this is a PHY property, shouldn't that be handled in > >> drivers/net/phy/ instead of MAC driver ? > > > > > > I was my first idea but I don't found found the correct location in phy > > (driver or uclass) > > > > to manage these generic property and the generic property "reset-gpios" > > was already > > > > managed in eth driver, so I continue to patch the driver. > > > > > > But I come back to this idea after your remark > > > > => in linux these property are managed in > > > > drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register > > > > parse DT node and add info in mdio > > > > drivers/net/phy/mdio_device.c::mdio_device_reset > > > > called in mdio_probe / mdio_remove > > > > > > In my first search, I don't found the same level in the U-Boot drivers > > in drivers/net/phy/ > > Note that this is MDIO-wide PHY reset (e.g. you can have single reset > line connected to multiple PHYs on single MDIO bus), this is not > PHY-specific reset. > > > but I miss the uclass defined in drivers/net/eth-phy-uclass.c > > > > > > Finally I think I need to manage the generic binding property > > > > (reset-gpios, reset-assert-us, reset-deassert-us) directly in > > > > => drivers/net/mdio-uclass > > > > > > The GPIO RESET will be managed in mdio ops: pre_probe/ post_remove > > > > as it is done in linux > > > > warning: today post_remove ops don't exit in u-class. > > > > > > Do you think it is the correct location ? > > For single-PHY reset, the correct location is in drivers/net/phy/ somewhere. > > > Do you think it should be a new serie (migrate the eqos property in mdio) > > > > after this eqos is accepted > > > > or I shoudl sent a new serie to replace this serie. > > I'll leave that decision to Ramon/Joe. Joe, I'll leave this to you.
Re: [PATCH] ls1012a: net: pfe: remove pfe stop from bootcmd
On Wed, Apr 14, 2021 at 1:34 PM Mian Yousaf Kaukab wrote: > > When using bootefi to boot a EFI binary, u-boot is supposed to > provide networking service for EFI application. Currently, 'pfe stop' > command is called from bootcmd before running bootefi. As a result > network stops working for EFI applications and console is flooded with > "Rx pkt not on expected port" messages. > > Implement board_quiesce_devices() for ls1012a boards and call > pfe_command_stop() from it instead of calling 'pfe stop' from > *_bootcmd and bootcmd. > > Tested-by: Anji Jagarlmudi > Signed-off-by: Mian Yousaf Kaukab > --- > board/freescale/ls1012afrdm/ls1012afrdm.c | 8 > board/freescale/ls1012aqds/ls1012aqds.c | 8 > board/freescale/ls1012ardb/ls1012ardb.c | 8 > drivers/net/pfe_eth/pfe_cmd.c | 2 +- > include/configs/ls1012a2g5rdb.h | 6 +++--- > include/configs/ls1012a_common.h | 4 ++-- > include/configs/ls1012afrdm.h | 6 +++--- > include/configs/ls1012afrwy.h | 6 +++--- > include/configs/ls1012aqds.h | 6 +++--- > include/configs/ls1012ardb.h | 6 +++--- > include/net/pfe_eth/pfe/pfe_hw.h | 6 ++ > 11 files changed, 48 insertions(+), 18 deletions(-) > > diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c > b/board/freescale/ls1012afrdm/ls1012afrdm.c > index 2cd651b943fb..7e87e5a9846f 100644 > --- a/board/freescale/ls1012afrdm/ls1012afrdm.c > +++ b/board/freescale/ls1012afrdm/ls1012afrdm.c > @@ -23,6 +23,7 @@ > #include > #include > #include > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -185,6 +186,13 @@ int board_init(void) > return 0; > } > > +#ifdef CONFIG_FSL_PFE > +void board_quiesce_devices(void) > +{ > +pfe_command_stop(0, NULL); > +} > +#endif > + > int ft_board_setup(void *blob, struct bd_info *bd) > { > arch_fixup_fdt(blob); > diff --git a/board/freescale/ls1012aqds/ls1012aqds.c > b/board/freescale/ls1012aqds/ls1012aqds.c > index cfe3f3360cd9..4f98a8652f21 100644 > --- a/board/freescale/ls1012aqds/ls1012aqds.c > +++ b/board/freescale/ls1012aqds/ls1012aqds.c > @@ -32,6 +32,7 @@ > #include "../common/qixis.h" > #include "ls1012aqds_qixis.h" > #include "ls1012aqds_pfe.h" > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -163,6 +164,13 @@ int board_init(void) > return 0; > } > > +#ifdef CONFIG_FSL_PFE > +void board_quiesce_devices(void) > +{ > +pfe_command_stop(0, NULL); > +} > +#endif > + > int esdhc_status_fixup(void *blob, const char *compat) > { > char esdhc0_path[] = "/soc/esdhc@156"; > diff --git a/board/freescale/ls1012ardb/ls1012ardb.c > b/board/freescale/ls1012ardb/ls1012ardb.c > index 41bcf6f935e9..62e8af48cf14 100644 > --- a/board/freescale/ls1012ardb/ls1012ardb.c > +++ b/board/freescale/ls1012ardb/ls1012ardb.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -186,6 +187,13 @@ int board_init(void) > return 0; > } > > +#ifdef CONFIG_FSL_PFE > +void board_quiesce_devices(void) > +{ > + pfe_command_stop(0, NULL); > +} > +#endif > + > #ifdef CONFIG_TARGET_LS1012ARDB > int esdhc_status_fixup(void *blob, const char *compat) > { > diff --git a/drivers/net/pfe_eth/pfe_cmd.c b/drivers/net/pfe_eth/pfe_cmd.c > index 1e69525cb71f..364750f65c75 100644 > --- a/drivers/net/pfe_eth/pfe_cmd.c > +++ b/drivers/net/pfe_eth/pfe_cmd.c > @@ -418,7 +418,7 @@ static void send_dummy_pkt_to_hif(void) > writel(buf, TMU_PHY_INQ_PKTINFO); > } > > -static void pfe_command_stop(int argc, char *const argv[]) > +void pfe_command_stop(int argc, char *const argv[]) > { > int pfe_pe_id, hif_stop_loop = 10; > u32 rx_status; > diff --git a/include/configs/ls1012a2g5rdb.h b/include/configs/ls1012a2g5rdb.h > index bbc3ffd7f0d3..c55fc6487756 100644 > --- a/include/configs/ls1012a2g5rdb.h > +++ b/include/configs/ls1012a2g5rdb.h > @@ -79,7 +79,7 @@ > "installer=load mmc 0:2 $load_addr "\ >"/flex_installer_arm64.itb; "\ >"bootm $load_addr#$board\0" \ > - "qspi_bootcmd=pfe stop; echo Trying load from qspi..;" \ > + "qspi_bootcmd=echo Trying load from qspi..;"\ > "sf probe && sf read $load_addr " \ > "$kernel_addr $kernel_size; env exists secureboot " \ > "&& sf read $kernelheader_addr_r $kernelheader_addr " \ > @@ -89,11 +89,11 @@ > #undef CONFIG_BOOTCOMMAND > #ifdef CONFIG_TFABOOT > #undef QSPI_NOR_BOOTCOMMAND > -#define QSPI_NOR_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; > " \ > +#define QSPI_NOR_BOOTCOMMAND "run distro_bootcmd; run qspi_bootcmd; " \ > "env exists secureboot && esbc_halt;" > #else > #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_SD_BOOT_QSPI) > -#define CONFIG_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; " > \ > +#define CO
Re: [PATCH v3 13/28] pci: Update to use new sequence numbers
Hi Tim, On Thu, 15 Apr 2021, 12:29 Tim Harvey, wrote: > On Wed, Apr 14, 2021 at 12:37 PM Simon Glass wrote: > > > > Hi Tim, > > > > On Wed, 14 Apr 2021 at 06:32, Tim Harvey wrote: > > > > > > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass wrote: > > > > > > > > Now that we know the sequence number at bind time, there is no need > for > > > > special-case code in dm_pci_hose_probe_bus(). > > > > > > > > Note: the PCI_CAP_ID_EA code may need a look, but there are no test > > > > failures so I have left it as is. > > > > > > > > Signed-off-by: Simon Glass > > > > --- > > > > > > > > Changes in v3: > > > > - Update PCI to use manual sequence numbering > > > > > > > > Changes in v2: > > > > - Use the sequence number directly instead of max bus > > > > > > > > drivers/pci/pci-uclass.c | 45 > > > > > drivers/pci/pci_auto.c | 10 - > > > > 2 files changed, 32 insertions(+), 23 deletions(-) > > > > > > > > Applied to u-boot-dm/next, thanks! > > > > > > Hi Simon, > > > > > > I have a (not yet submitted) pending patch to migrate the gwventana > > > board to DM_PCI / DM_ETH that this particular patch broke and I'm > > > trying to understand why. > > > > > > The Gateworks Ventana boards have a PCIe switch where the downstream > > > PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on > > > one of those downstream ports. For non-dm PCI I have a > > > 'board_pci_fixup_dev' function that allows board support to configure > > > the switch's GPIO and toggle downstream PERST# during enumeration. > > > When I add this function to dm_pci I end up getting a data abort when > > > the e1000 driver tries to initialize (after PCI enumeration). > > > > Firstly, I think I did the PCI conversion about 6 years ago so it is > > not fresh in my memory. > > > > > > > > Any idea what is causing this and what I need to do to work around it? > > > Nothing in my patch deals with device sequence numbers. > > > > An abort presumably indicates that the memory is not there, so perhaps > > the e1000 is still in reset, or has been set up to appear at a > > different address? > > > > Simon, > > It does appear that everything downstream from the switch is > inaccessible 'after' enumeration and I've verified that PERST# both > upstream and downstream is in the correct state and never gets > asserted again. > > The issue appears to be related to the PCI_CAP_ID_EA code and per your > commit log it sounds like you were not sure if that was correct. > Adding back in the following resolves the issue for me: > > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > index 9abdbc54b0..fcfe520e5e 100644 > --- a/drivers/pci/pci-uclass.c > +++ b/drivers/pci/pci-uclass.c > @@ -646,6 +646,15 @@ int dm_pci_hose_probe_bus(struct udevice *bus) > return log_msg_ret("probe", ret); > } > > + if (!ea_pos) { > + if (sub_bus != dev_seq(bus)) { > + debug("%s: Internal error, bus '%s' got seq > %d, expected %d\n", > + __func__, bus->name, > dev_seq(bus), sub_bus); > + return -EPIPE; > + } > + sub_bus = pci_get_bus_max(); > + } > + > dm_pciauto_postscan_setup_bridge(bus, sub_bus); > > return sub_bus; > > Adding some debugging it appears that the devices I have downstream > from the bridge have 'ea_os = 0'. > > Does the above let you know what's going on? > Maybe,but I think someone just sent a patch for it.can you check the mailing list? Regards, Simon > Tim > > > > > > > You should be abe to use 'pci hdr 0.4.0' or whatever to look at the > > BARs and make sure they are value with 'md', etc. > > > > > Additionally I feel like the best way to add support for the custom > > > downstream PCI switch reset requirements is to add a UCLASS_PCI driver > > > for the switch in my board support but when I attempted that solution > > > I run into an issue where pci read/write's cause a prefetch abort > > > because I need to use imx_pcie_dm_{read,write}_config instead of the > > > default ops. I'm not quite sure how to get hold of the ops for the imx > > > > If you write a PCI driver then you can provide the access operations > > yourselfsee drivers/pci for some examples. I suggest you add > > subnodes to the devicetree and specify the compatible string of your > > PCI switch so it picks up the right driver. > > > > > controller to set this up properly. Furthermore if I do end up with > > > that solution I later need to be able to walk the PCI bus to perform > > > various dt fixups on pci device nodes - do you have an example of how > > > I could walk the bus using DM_PCI? > > > > Do you mean a breadth-first search? It is something like: > > > > struct udevice *dev; > > > > static void do_something(struct udevice *parent) > > { > >device_foreach_child_probe(dev, parent) { > >do_something((dev); > >} > > > >
Re: [PATCH v3 13/28] pci: Update to use new sequence numbers
On Wed, Apr 14, 2021 at 12:37 PM Simon Glass wrote: > > Hi Tim, > > On Wed, 14 Apr 2021 at 06:32, Tim Harvey wrote: > > > > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass wrote: > > > > > > Now that we know the sequence number at bind time, there is no need for > > > special-case code in dm_pci_hose_probe_bus(). > > > > > > Note: the PCI_CAP_ID_EA code may need a look, but there are no test > > > failures so I have left it as is. > > > > > > Signed-off-by: Simon Glass > > > --- > > > > > > Changes in v3: > > > - Update PCI to use manual sequence numbering > > > > > > Changes in v2: > > > - Use the sequence number directly instead of max bus > > > > > > drivers/pci/pci-uclass.c | 45 > > > drivers/pci/pci_auto.c | 10 - > > > 2 files changed, 32 insertions(+), 23 deletions(-) > > > > > > Applied to u-boot-dm/next, thanks! > > > > Hi Simon, > > > > I have a (not yet submitted) pending patch to migrate the gwventana > > board to DM_PCI / DM_ETH that this particular patch broke and I'm > > trying to understand why. > > > > The Gateworks Ventana boards have a PCIe switch where the downstream > > PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on > > one of those downstream ports. For non-dm PCI I have a > > 'board_pci_fixup_dev' function that allows board support to configure > > the switch's GPIO and toggle downstream PERST# during enumeration. > > When I add this function to dm_pci I end up getting a data abort when > > the e1000 driver tries to initialize (after PCI enumeration). > > Firstly, I think I did the PCI conversion about 6 years ago so it is > not fresh in my memory. > > > > > Any idea what is causing this and what I need to do to work around it? > > Nothing in my patch deals with device sequence numbers. > > An abort presumably indicates that the memory is not there, so perhaps > the e1000 is still in reset, or has been set up to appear at a > different address? > Simon, It does appear that everything downstream from the switch is inaccessible 'after' enumeration and I've verified that PERST# both upstream and downstream is in the correct state and never gets asserted again. The issue appears to be related to the PCI_CAP_ID_EA code and per your commit log it sounds like you were not sure if that was correct. Adding back in the following resolves the issue for me: diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 9abdbc54b0..fcfe520e5e 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -646,6 +646,15 @@ int dm_pci_hose_probe_bus(struct udevice *bus) return log_msg_ret("probe", ret); } + if (!ea_pos) { + if (sub_bus != dev_seq(bus)) { + debug("%s: Internal error, bus '%s' got seq %d, expected %d\n", + __func__, bus->name, dev_seq(bus), sub_bus); + return -EPIPE; + } + sub_bus = pci_get_bus_max(); + } + dm_pciauto_postscan_setup_bridge(bus, sub_bus); return sub_bus; Adding some debugging it appears that the devices I have downstream from the bridge have 'ea_os = 0'. Does the above let you know what's going on? Tim > You should be abe to use 'pci hdr 0.4.0' or whatever to look at the > BARs and make sure they are value with 'md', etc. > > > Additionally I feel like the best way to add support for the custom > > downstream PCI switch reset requirements is to add a UCLASS_PCI driver > > for the switch in my board support but when I attempted that solution > > I run into an issue where pci read/write's cause a prefetch abort > > because I need to use imx_pcie_dm_{read,write}_config instead of the > > default ops. I'm not quite sure how to get hold of the ops for the imx > > If you write a PCI driver then you can provide the access operations > yourselfsee drivers/pci for some examples. I suggest you add > subnodes to the devicetree and specify the compatible string of your > PCI switch so it picks up the right driver. > > > controller to set this up properly. Furthermore if I do end up with > > that solution I later need to be able to walk the PCI bus to perform > > various dt fixups on pci device nodes - do you have an example of how > > I could walk the bus using DM_PCI? > > Do you mean a breadth-first search? It is something like: > > struct udevice *dev; > > static void do_something(struct udevice *parent) > { >device_foreach_child_probe(dev, parent) { >do_something((dev); >} > > ... > > struct udevice *bus; > int ret; > > ret = uclass_first_device_err(UCLASS_PCI, &bus); > if (ret) >return log_msg_ret("pci", ret); > do_something(bus); > > > > > Best regards, > > > > Tim > > Regards, > Simon
[PATCH] arm: at91: gardena-smart-gateway-at91sam: Adjust to production values
From: Reto Schneider This commit updates the default config with the values that are actually used "in the wild" and which are close to what is used on the MediaTek MT7688 based, 2nd generation of the GARDENA smart gateway: - Reduce startup time by setting bootdelay to 0 (still allows accessing the shell, one just has to send a key press quicker) - Adjusting U-Boot environment volume names and MTD partitions to the actual layout Signed-off-by: Reto Schneider --- configs/gardena-smart-gateway-at91sam_defconfig | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/configs/gardena-smart-gateway-at91sam_defconfig b/configs/gardena-smart-gateway-at91sam_defconfig index e3d5bc47d6..76a1c42ce0 100644 --- a/configs/gardena-smart-gateway-at91sam_defconfig +++ b/configs/gardena-smart-gateway-at91sam_defconfig @@ -24,6 +24,7 @@ CONFIG_NAND_BOOT=y CONFIG_BOOTDELAY=3 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk mtdparts=atmel_nand:256k(bootstrap)ro,768k(uboot)ro,256k(env_redundant),256k(env),512k(dtb),6M(kernel)ro,-(rootfs) rootfstype=ubifs ubi.mtd=6 root=ubi0:rootfs rw" +CONFIG_BOOTDELAY=0 CONFIG_SYS_CONSOLE_IS_IN_ENV=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_SPL_SYS_MALLOC_SIMPLE=y @@ -46,8 +47,8 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y CONFIG_CMD_FAT=y CONFIG_CMD_MTDPARTS=y -CONFIG_MTDIDS_DEFAULT="nand0=nand0" -CONFIG_MTDPARTS_DEFAULT="nand0:1536k(uboot),1024k(unused),512k(dtb_old),4608k(kernel_old),86528k(ubi),-(rootfs_old)" +CONFIG_MTDIDS_DEFAULT="nand0=atmel_nand" +CONFIG_MTDPARTS_DEFAULT="atmel_nand:1536k(uboot),10752k(unused),-(ubi)" CONFIG_CMD_UBI=y CONFIG_OF_CONTROL=y CONFIG_SPL_OF_CONTROL=y @@ -55,8 +56,8 @@ CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clocks clock-names interrupt CONFIG_ENV_IS_IN_UBI=y CONFIG_SYS_REDUNDAND_ENVIRONMENT=y CONFIG_ENV_UBI_PART="ubi" -CONFIG_ENV_UBI_VOLUME="env" -CONFIG_ENV_UBI_VOLUME_REDUND="env_r" +CONFIG_ENV_UBI_VOLUME="uboot_env0" +CONFIG_ENV_UBI_VOLUME_REDUND="uboot_env1" CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_DM=y -- 2.29.2
Re: [PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus
Hi, On Thu, Apr 15, 2021 at 3:39 AM Simon Glass wrote: > > On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu > wrote: > > > > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges") > > sets the VGA bridge bits by checking pplat->class, but if the parent > > device is the pci host bus device, it can be skipped. Moreover, it > > shouldn't access the pplat because the parent has different plat data. > > > > Without this fix, "pci enum" command cause a synchronous abort. > > > > pci_auto_config_devices: start > > PCI Autoconfig: Bus Memory region: [7800-7fff], > > Physical Memory [7800-7fffx] > > PCI Autoconfig: Bus I/O region: [0-], > > Physical Memory [77f0-77f0x] > > pci_auto_config_devices: device pci_6:0.0 > > PCI Autoconfig: BAR 0, Mem, size=0x100, address=0x7800 > > bus_lower=0x7900 > > > > PCI Autoconfig: BAR 1, Mem, size=0x800, No room in resource, avail > > start=7900 / size=800, need=800 > > PCI: Failed autoconfig bar 14 > > > > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004 > > > > PCI Autoconfig: BAR 3, Mem, size=0x200, address=0x7a00 > > bus_lower=0x7c00 > > > > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100 > > > > PCI Autoconfig: ROM, size=0x8, address=0x7c00 bus_lower=0x7c08 > > > > "Synchronous Abort" handler, esr 0x9606 > > elr: e002bd28 lr : e002bce8 (reloc) > > elr: fff6fd28 lr : fff6fce8 > > x0 : 1041 x1 : 003e > > x2 : ffb0f8c8 x3 : 0001 > > x4 : 0080 x5 : > > x6 : fff718fc x7 : 000f > > x8 : ffb0f238 x9 : 0008 > > x10: x11: 0010 > > x12: 0006 x13: 0001869f > > x14: ffb0fcd0 x15: 0020 > > x16: fff71cc4 x17: > > x18: ffb13d90 x19: ffb14320 > > x20: x21: ffb14090 > > x22: ffb0f8c8 x23: 0001 > > x24: ffb14c10 x25: > > x26: x27: > > x28: ffb14c70 x29: ffb0f830 > > > > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280) > > Resetting CPU ... > > > > Signed-off-by: Masami Hiramatsu > > --- > > drivers/pci/pci-uclass.c |3 +++ > > 1 file changed, 3 insertions(+) > > Reviewed-by: Simon Glass I can't find this patch in my inbox, nor in the patchwork. Could you please resend? Regards, Bin
Re: [PATCH RFT v2 0/4] pci: add common Designware PCIe functions and support Amlogic Meson PCIe controller
Hi, On Thu, Apr 8, 2021 at 3:41 PM Neil Armstrong wrote: > > On 08/04/2021 07:29, Bin Meng wrote: > > Hi Neil, > > > > On Tue, Apr 6, 2021 at 8:22 PM Neil Armstrong > > wrote: > >> > >> Hi Bin, > >> > >> On 25/03/2021 15:49, Neil Armstrong wrote: > >>> With the introduction of pcie_dw_rockchip, and need to support the DW > >>> PCIe in the > >>> Amlogic AXG & G12 SoCs, most of the DW PCIe helpers would be duplicated. > >>> > >>> This introduce a "common" DW PCIe helpers file with common code merged > >>> from the > >>> dw_ti and dw_rockchip drivers and adapted to fit with the upcoming > >>> dw_meson. > >>> > >>> The following changes will switch the dw_ti and dw_rockchip, and > >>> introduce a new > >>> driver to support the Amlogic AXG & G12 SoCs using these new common > >>> helpers. > >>> > >>> The dw_meson has been validated, but the dw_ti and dw_rockchip would need > >>> testing > >>> on hardware to validate nothing has been broken. > >> > >> How should be proceed ? I can't possibly test patches 2 & 3, but Green has > >> tested it > >> on Sifive, should patch 1 & 4 be merged then we should wait feedback from > >> the Ti & Rockchip > >> owners ? > > > > This series is not assigned to me in patchwork so I cannot decide anything > > :) > > I can switch them to you, but I never know who to assign for these > multi-vendor series... > > > > > But my opinion was that let's wait for some more time, and if nobody > > jumps out let's merge this series. > > Fine fore me ! I am going to apply this series via the x86 tree if there are no further comments. Regards, Bin
Re: Getting rid of falcon mode
On Wed, Apr 14, 2021 at 08:37:07PM +0100, Simon Glass wrote: > Hi, > > On Tue, 13 Apr 2021 at 09:32, Alex G. wrote: > > > > ## Introduction > > > > Today we use "falcon mode" to mean "boot linux straight from SPL". This > > designation makes sense, since falcons "fly at high speed and change > > direction rapidly" according to Wikipedia. > > > > The way we implement falcon mode is to reserve two areas of storage: > >* kernel area/partition > >* dtb area/partition > > By using some "special cases", and "spl export", SPL can more or less > > figure out how to skip u-boot. > > > > > > ## The plot twist > > > > People familiar with FIT, will have recognized that the above is > > achievable with a very basic FIT image. With some advantages: > > > > - No "special cases" in SPL code > > - Signed kernel images > > - Signed kernel devicetree > > - Devicetree overlays > > - Automatic selection of correct devicetree > > > > > > ## The problems > > > > The advantages of FIT are not obvious by looking at SPL code. A > > noticeable amount of SPL code is hidden under #ifdef CONFIG_SPL_OS_BOOT, > > leading one to believe that SPL_OS_BOOT is very important. It must be > > since it takes up so much code. > > > > Enabling falcon mode is not well documented, and requires a lot of trial > > and error. I've had to define 7 macros, and one new function to get it > > working on my board -- and vividly remember the grief. This is an > > antiquated way of doing things, and completely ignores the u-boot > > devicetree -- we could just as well have defined those seven values in > > the dtb. > > > > SPL assumes that it must load u-boot, unless in instances under > > CONFIG_SPL_OS_BOOT. This has cause me a huge amount of grief and > > confusion over the past several months. I have no less than three patch > > series trying to address shortfalls there. It's awful. > > > > > > ## The proposal > > > > I propose we drop falcon mode support for legacy images. > > > >- Drop CONFIG_SPL_OS_BOOT. Support for this is implied by SPL_FIT > >- Drop the "dtb area/partition". The dtb is derived from the FIT > >- Drop "spl export" > > > > > > How do we deal with devicetrees in the FIT then? The options are to use > > a modified devicetree which has the desired "/chosen" node, or use DTB > > overlays. > > > > What are the reasons why we shouldn't go this route? > > I think this makes sense, on the basis that it is a legacy image and > people can always use the U-Boot path if needed. > > I believe Falcon boot made a lot more sense when the cache was off. > But at least in my experience, we were able to get through two SPLs > and two U-Boots and boot a kernel about 800ms on a Cortex-A15, so > Falcon mode might not be so relevant anymore, and supporting a legacy > image seems like unnecessary complexity. I'm curious where the end of that 800ms is, and I think we might need to post grabserial logs so everyone is on the same page about where / when we're at in booting. -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/5] common: Rename macro appropriately
On Mon, 12 Apr 2021 at 23:16, Steffen Jaeckel wrote: > > While doing code-review internally this got nitpicked by 2 reviewers, so > I decided to include this here. > > Signed-off-by: Steffen Jaeckel > --- > > common/autoboot.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 11/11] test: Add K210 PLL tests to sandbox defconfigs
On Mon, 12 Apr 2021 at 04:58, Sean Anderson wrote: > > This adds the unit test for the K210 PLL to the sandbox defconfigs. > > Signed-off-by: Sean Anderson > --- > > configs/sandbox64_defconfig| 2 ++ > configs/sandbox_defconfig | 2 ++ > configs/sandbox_flattree_defconfig | 2 ++ > 3 files changed, 6 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH] dm: core: Fix uninitialized return value from dm_scan_fdt_node
On Thu, 8 Apr 2021 at 22:15, Sean Anderson wrote: > > If there are no nodes or if all nodes are disabled, this function would > return err without setting it first. Fix this by initializing err to > zero. > > Fixes: 94f7afdf7e ("dm: core: Ignore disabled devices when binding") > > Signed-off-by: Sean Anderson > --- > > drivers/core/root.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH v2 03/17] drivers: reset: Handle gracefully NULL pointers
Hi Kishon, On Mon, 5 Apr 2021 at 11:28, Kishon Vijay Abraham I wrote: > > From: Jean-Jacques Hiblot > > The reset framework provides devm_reset_control_get_optional() > which can return NULL (not an error case). So all the other reset_ops > should handle NULL gracefully. Prepare the way for a managed reset > API by handling NULL pointers without crashing nor failing. > > Signed-off-by: Jean-Jacques Hiblot > Signed-off-by: Vignesh Raghavendra > Signed-off-by: Kishon Vijay Abraham I > --- > drivers/reset/reset-uclass.c | 30 +- > 1 file changed, 17 insertions(+), 13 deletions(-) > > diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c > index 071c389ca0..98304bc0ee 100644 > --- a/drivers/reset/reset-uclass.c > +++ b/drivers/reset/reset-uclass.c > @@ -13,9 +13,12 @@ > #include > #include > > -static inline struct reset_ops *reset_dev_ops(struct udevice *dev) > +struct reset_ops nop_reset_ops = { > +}; > + > +static inline struct reset_ops *reset_dev_ops(struct reset_ctl *r) > { > - return (struct reset_ops *)dev->driver->ops; > + return r ? (struct reset_ops *)r->dev->driver->ops : &nop_reset_ops; This behaviour still seems odd to me. Why do you have a reset driver with no ops? That is not allowed. > } > > static int reset_of_xlate_default(struct reset_ctl *reset_ctl, > @@ -54,9 +57,10 @@ static int reset_get_by_index_tail(int ret, ofnode node, > debug("%s %d\n", ofnode_get_name(args->node), args->args[0]); > return ret; > } > - ops = reset_dev_ops(dev_reset); > > reset_ctl->dev = dev_reset; > + ops = reset_dev_ops(reset_ctl); > + > if (ops->of_xlate) > ret = ops->of_xlate(reset_ctl, args); > else > @@ -162,29 +166,29 @@ int reset_get_by_name(struct udevice *dev, const char > *name, > > int reset_request(struct reset_ctl *reset_ctl) > { > - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > + struct reset_ops *ops = reset_dev_ops(reset_ctl); > > debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); > > - return ops->request(reset_ctl); > + return ops->request ? ops->request(reset_ctl) : 0; Can you check this first and return -ENOSYS ? E.g. if (!ops->request) return -ENOSYS; return ops->request(reset_ctl); Same below > } > > int reset_free(struct reset_ctl *reset_ctl) > { > - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > + struct reset_ops *ops = reset_dev_ops(reset_ctl); > > debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); > > - return ops->rfree(reset_ctl); > + return ops->rfree ? ops->rfree(reset_ctl) : 0; > } > > int reset_assert(struct reset_ctl *reset_ctl) > { > - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > + struct reset_ops *ops = reset_dev_ops(reset_ctl); > > debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); > > - return ops->rst_assert(reset_ctl); > + return ops->rst_assert ? ops->rst_assert(reset_ctl) : 0; > } > > int reset_assert_bulk(struct reset_ctl_bulk *bulk) > @@ -202,11 +206,11 @@ int reset_assert_bulk(struct reset_ctl_bulk *bulk) > > int reset_deassert(struct reset_ctl *reset_ctl) > { > - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > + struct reset_ops *ops = reset_dev_ops(reset_ctl); > > debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); > > - return ops->rst_deassert(reset_ctl); > + return ops->rst_deassert ? ops->rst_deassert(reset_ctl) : 0; > } > > int reset_deassert_bulk(struct reset_ctl_bulk *bulk) > @@ -224,11 +228,11 @@ int reset_deassert_bulk(struct reset_ctl_bulk *bulk) > > int reset_status(struct reset_ctl *reset_ctl) > { > - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > + struct reset_ops *ops = reset_dev_ops(reset_ctl); > > debug("%s(reset_ctl=%p)\n", __func__, reset_ctl); > > - return ops->rst_status(reset_ctl); > + return ops->rst_status ? ops->rst_status(reset_ctl) : 0; > } > > int reset_release_all(struct reset_ctl *reset_ctl, int count) > -- > 2.17.1 > Regards, Simon
Re: [PATCH v10 2/3] x86: correct usage of CFLAGS_NON_EFI
Hi Heinrich, On Sun, 11 Apr 2021 at 10:23, Heinrich Schuchardt wrote: > > The current usage of the variable CFLAGS_NON_EFI on the x86 architecture > deviates from other architectures. > > Variable CFLAGS_NON_EFI is the list of compiler flags to be removed when > building UEFI applications. It is not a list of flags to be added anywhere. > > Signed-off-by: Heinrich Schuchardt > --- > v10: > new patch > --- > arch/x86/config.mk | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) Well it used to be the flags to add when not building EFI apps... I suppose it doesn't matter so long as it works. Regards, Simon
Re: [PATCH v10 1/3] test: fix test/dm/regmap.c
On Sun, 11 Apr 2021 at 10:23, Heinrich Schuchardt wrote: > > regmap_read() only fills the first two bytes of val. The last two bytes are > random data from the stack. This means the test will fail randomly. > > For low endian systems we could simply initialize val to 0 and get correct > results. But tests should not depend on endianness. So let's use a pointer > conversion instead. > > Signed-off-by: Heinrich Schuchardt > --- > v10: > new patch > --- > test/dm/regmap.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 2/6] spl: LOAD_FIT_FULL: Do not hard-code os to IH_OS_U_BOOT
On Tue, 30 Mar 2021 at 18:54, Alex G. wrote: > > > > On 3/29/21 2:43 AM, Simon Glass wrote: > > On Fri, 12 Mar 2021 at 10:32, Alexandru Gagniuc > > wrote: > >> > >> The information on the OS should be contained in the FIT, as the > >> self-explanatory "os" property of a node under /images. Hard-coding > >> this to U_BOOT might send us down the wrong path later in the boot > >> process. > >> > >> Signed-off-by: Alexandru Gagniuc > >> --- > >> common/spl/spl.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> Reviewed-by: Simon Glass
Re: [PATCH v1 1/2] cmd: bind: Fix driver binding on a device
On Fri, 9 Apr 2021 at 08:36, Patrice Chotard wrote: > > Fix a regression brings by commit 84f8e36f03fa ("cmd: bind: allow to > bind driver with driver data") > > As example, the following bind command doesn't work: > >bind /soc/usb-otg@4900 usb_ether > > As usb_ether driver has no compatible string, it can't be find by > lists_bind_fdt(). In bind_by_node_path(), which called lists_bind_fdt(), > the driver entry is known, pass it to lists_bind_fdt() to force the driver > entry selection. > > For this, add a new parameter struct *driver to lists_bind_fdt(). > Fix also all lists_bind_fdt() callers. > > Fixes: 84f8e36f03fa ("cmd: bind: allow to bind driver with driver data") > > Signed-off-by: Patrice Chotard > Reported-by: Herbert Poetzl > Cc: Marek Vasut > Cc: Herbert Poetzl > --- > > cmd/bind.c | 2 +- > drivers/core/device.c | 2 +- > drivers/core/lists.c | 11 --- > drivers/core/root.c| 2 +- > drivers/misc/imx8/scu.c| 2 +- > drivers/serial/serial-uclass.c | 2 +- > drivers/timer/timer-uclass.c | 2 +- > include/dm/lists.h | 3 ++- > test/dm/nop.c | 2 +- > test/dm/test-fdt.c | 2 +- > 10 files changed, 18 insertions(+), 12 deletions(-) > Reviewed-by: Simon Glass Really this command needs a test. Regards, Simon
Re: MXC I2C recover/idle_bus does not work if CONFIG_DM_I2C is configured
Hi Kees, On Fri, 9 Apr 2021 at 08:00, Trommel, Kees (Contractor) wrote: > > Simon, > > I found this issue in 2020.10. I just checked the master version and found > that the "flags |= desc->flags" statement in dm_gpio_set_dir_flags has been > removed. So it looks like that this issue is already solved. Yes I believe so. - Simon > > Kees. > > -Original Message- > From: Heiko Schocher > Sent: 09 April 2021 06:40 > To: Trommel, Kees (Contractor) > Cc: u-boot@lists.denx.de; Simon Glass > Subject: Re: MXC I2C recover/idle_bus does not work if CONFIG_DM_I2C is > configured > > Hello Kees, > > added Simon to cc... > > On 08.04.21 14:20, Trommel, Kees (Contractor) wrote: > > Heiko, > > > > When an I2C transaction fails because a previous transaction (by the > > kernel) was aborted halfway the MXC I2C driver tries to recover from > > this by calling i2c_idle_bus (if CONFIG_DM_I2C is configured). > > i2c_idle_bus is defined in drivers/i2c/mxc_i2c.c > > Ok, so imx based hardware. > > > As part of the recover procedure i2c_idle_bus (tries) to change the > > direction of I2C GPIO pins from output to input using the function > > dm_gpio_set_dir_flags. However dm_gpio_set_dir_flags only sets flags > > without clearing any previously set flags, see statement "flags |= > > desc->flags" in dm_gpio_set_dir_flags. Because it is not allowed to set > > both GPIOD_IS_IN and GPIOD_IS_OUT (see function check_dir_flags) only the > > dm_gpio_set_dir_flags(GPIO_D_IS_OUT) calls are successful, the > > dm_gpio_set_dir_flags(GPIO_D_IS_IN) calls fail and the GPIO pin is never > > set as an input. This causes that i2c_idle_bus finds that clearing the bus > > is unsuccessful and returns an error. > > Ok, sounds like a bug to me. > > > Although i2c_idle_bus returns an error the i2c recover procedure itself is > > successful and the next I2C transfer will be successful. This requires that > > the I2C application to do a retry. This will work but is not the intention > > of the I2C driver. > > > > I want to fix this problem. However it looks like that no dm_gpio API > > exists to change the direction of an GPIO pin while this is required to > > successful recover the i2c bus and detect that the recovery is successful. > > To fix this issue I see 2 possibilities: > > > > 1. Use the old fashioned APIs gpio_direction_input and > > gpio_direction_output 2. Add a new dm_gpio API > > dm_gpio_clear_and_set_dir_flags that allows to clear all existing set > > flags before setting the new flag(s) > > Hmm.. I see, we only "or" the flags in dm_gpio_set_dir_flags() ... and than > check if there are conflicts ... May we rework dm_gpio_set_dir_flags() so we > prevent the conflicts we check in check_dir_flags() ? > > Assumption is, the caller of dm_gpio_set_dir_flags() knows what he do... > > @Simon: What do you think? > > > I prefer option 2 but I like to hear the opinion of the developer that > > designed the dm_gpio interface. > > I did not designed the gpio interface! > > bye, > Heiko > > > > Kind regards, > > > > Kees Trommel. > > > > --- > > This communication contains confidential information. If you are not the > > intended recipient please return this email to the sender and delete it > > from your records. > > > > Diese Nachricht enthaelt vertrauliche Informationen. Sollten Sie nicht der > > beabsichtigte Empfaenger dieser E-mail sein, senden Sie bitte diese an den > > Absender zurueck und loeschen Sie die E-mail aus Ihrem System. > > > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH 1/4] command: Use a constant pointer for the help
Hi Sean, On Fri, 9 Apr 2021 at 04:32, Sean Anderson wrote: > > On 4/6/21 12:30 AM, Simon Glass wrote: > > This text should never change during execution, so it makes sense to > > use a const char * so that it can be declared as const in the code. > > Update struct cmd_tbl with a const char * pointer for 'help'. > > > > Signed-off-by: Simon Glass > > --- > > > > include/command.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/command.h b/include/command.h > > index 747f8f80958..675c16a1249 100644 > > --- a/include/command.h > > +++ b/include/command.h > > @@ -45,7 +45,7 @@ struct cmd_tbl { > > char *const argv[]); > > char*usage; /* Usage message(short) */ > > Should usage also be const? I think so, but would need to check it builds, etc. Regards, Simon
Re: [PATCH 01/11] clk: Allow force setting clock defaults before relocation
Hi Sean, On Mon, 12 Apr 2021 at 04:58, Sean Anderson wrote: > > Since 291da96b8e ("clk: Allow clock defaults to be set during re-reloc > state for SPL only") it has been impossible to set clock defaults before > relocation. This is annoying on boards without SPL, since there is no way > to set clock defaults before U-Boot proper. In particular, the aisram rate > must be changed before relocation on the K210, since U-Boot will hang if we > try and change the rate while we are using aisram. > > To get around this, (ab)use the stage parameter to force setting defaults, > even if they would be otherwise posponed for later. A device tree property > was decided against because of the concerns in the original commit thread > about the overhead of repeatedly parsing the device tree. > > Signed-off-by: Sean Anderson > --- > > drivers/clk/clk-uclass.c | 9 +++-- > include/clk.h| 4 +++- > 2 files changed, 10 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass But I think this should be an enum.
Re: [PATCH v2 3/4] efi_capsule: Add a function to get the public key needed for capsule authentication
On Mon, 12 Apr 2021 at 16:06, Sughosh Ganu wrote: > > Define a function which would be used in the scenario where the > public key is stored on the platform's dtb. This dtb is concatenated > with the u-boot binary during the build process. Platforms which have > a different mechanism for getting the public key would define their > own platform specific function under a different Kconfig symbol. > > Signed-off-by: Sughosh Ganu > --- > > Changes since V1: > * Remove the weak function, and add the functionality to retrieve the > public key under the config symbol CONFIG_EFI_PKEY_DTB_EMBED. > > lib/efi_loader/efi_capsule.c | 43 +++- > 1 file changed, 38 insertions(+), 5 deletions(-) Is this defined in a header file somewhere? > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 2cc8f2dee0..d95e9377fe 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -14,10 +14,13 @@ > #include > #include > > +#include > #include > #include > #include > > +DECLARE_GLOBAL_DATA_PTR; > + > const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID; > static const efi_guid_t efi_guid_firmware_management_capsule_id = > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > @@ -208,15 +211,45 @@ skip: > const efi_guid_t efi_guid_capsule_root_cert_guid = > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; > > -__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) > +#if defined(CONFIG_EFI_PKEY_DTB_EMBED) Can you drop the #ifdef ? > +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len) pkey should really have a time...what is it? > { > - /* The platform is supposed to provide > -* a method for getting the public key > -* stored in the form of efi signature > -* list > + /* > +* This is a function for retrieving the public key from the > +* platform's device tree. The platform's device tree has been > +* concatenated with the u-boot binary. > +* If a platform has a different mechanism to get the public > +* key, it can define it's own kconfig symbol and define a > +* function to retrieve the public key > */ > + const void *fdt_blob = gd->fdt_blob; > + const void *blob; prop? val? It is not a DT blob > + const char *cnode_name = "capsule-key"; > + const char *snode_name = "signature"; I believe these FIT things are defined in image.h > + int sig_node; > + int len; > + > + sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name); > + if (sig_node < 0) { > + EFI_PRINT("Unable to get signature node offset\n"); > + return -FDT_ERR_NOTFOUND; > + } Can you use the livetree API? > + > + blob = fdt_getprop(fdt_blob, sig_node, cnode_name, &len); > + > + if (!blob || len < 0) { > + EFI_PRINT("Unable to get capsule-key value\n"); > + *pkey = NULL; > + *pkey_len = 0; > + return -FDT_ERR_NOTFOUND; > + } > + > + *pkey = (void *)blob; > + *pkey_len = len; > + > return 0; > } > +#endif /* CONFIG_EFI_PKEY_DTB_EMBED */ > > efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t > capsule_size, > void **image, efi_uintn_t *image_size) > -- > 2.17.1 > Regards, Simon
Re: [PATCH v2 3/3] test: Add test for partitions
Hi Sean, On Mon, 12 Apr 2021 at 23:53, Sean Anderson wrote: > > This is technically a library function, but we use MMCs for testing, so > it is easier to do it with DM. At the moment, the only block devices in > sandbox are MMCs (AFAIK) so we just test with those. > > Signed-off-by: Sean Anderson > --- > > Changes in v2: > - New > > test/dm/Makefile | 1 + > test/dm/part.c | 76 > 2 files changed, 77 insertions(+) > create mode 100644 test/dm/part.c > > diff --git a/test/dm/Makefile b/test/dm/Makefile > index f5cc5540e8..7d017f8750 100644 > --- a/test/dm/Makefile > +++ b/test/dm/Makefile > @@ -98,5 +98,6 @@ endif > ifneq ($(CONFIG_EFI_PARTITION),) > obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o > endif > +obj-$(CONFIG_EFI_PARTITION) += part.o > endif > endif # !SPL > diff --git a/test/dm/part.c b/test/dm/part.c > new file mode 100644 > index 00..051e9010b6 > --- /dev/null > +++ b/test/dm/part.c > @@ -0,0 +1,76 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2020 Sean Anderson > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +static int dm_test_part(struct unit_test_state *uts) > +{ > + char str_disk_guid[UUID_STR_LEN + 1]; > + struct blk_desc *mmc_dev_desc; > + struct disk_partition part_info; > + struct disk_partition parts[2] = { > + { > + .start = 48, /* GPT data takes up the first 34 blocks > or so */ > + .size = 1, > + .name = "test1", > + }, > + { > + .start = 49, > + .size = 1, > + .name = "test2", > + }, > + }; > + > + ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc)); > + if (CONFIG_IS_ENABLED(RANDOM_UUID)) { > + gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD); > + gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD); > + gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD); > + } > + ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts, > + ARRAY_SIZE(parts))); > + > +#define test(expected, part_str, whole) \ Can this be a function instead of a macro? > + ut_asserteq(expected, \ > + part_get_info_by_dev_and_name_or_num("mmc", part_str, \ > +&mmc_dev_desc, \ > +&part_info, whole)) > + > + test(-ENODEV, "", true); > + env_set("bootdevice", "0"); > + test(0, "", true); > + env_set("bootdevice", "1"); > + test(1, "", false); > + test(1, "-", false); > + env_set("bootdevice", ""); > + test(-EPROTONOSUPPORT, "0", false); > + test(0, "0", true); > + test(0, ":0", true); > + test(0, ".0", true); > + test(0, ".0:0", true); > + test(-EINVAL, "#test1", true); > + test(1, "1", false); > + test(1, "1", true); > + test(-ENOENT, "1:0", false); > + test(0, "1:0", true); > + test(1, "1:1", false); > + test(2, "1:2", false); > + test(1, "1.0", false); > + test(0, "1.0:0", true); > + test(1, "1.0:1", false); > + test(2, "1.0:2", false); > + test(-EINVAL, "1#bogus", false); > + test(1, "1#test1", false); > + test(2, "1#test2", false); > + > + return 0; > +} > +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); > -- > 2.25.1 > Regards, Simon
Re: [PATCH v2 2/3] part: Fix bogus return from part_get_info_by_dev_and_name
On Mon, 12 Apr 2021 at 23:53, Sean Anderson wrote: > > blk_get_device_by_str returns the device number on success. So we must > check if the return was negative to determine an error. > > Signed-off-by: Sean Anderson > --- > > Changes in v2: > - New > > disk/part.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus
On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu wrote: > > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges") > sets the VGA bridge bits by checking pplat->class, but if the parent > device is the pci host bus device, it can be skipped. Moreover, it > shouldn't access the pplat because the parent has different plat data. > > Without this fix, "pci enum" command cause a synchronous abort. > > pci_auto_config_devices: start > PCI Autoconfig: Bus Memory region: [7800-7fff], > Physical Memory [7800-7fffx] > PCI Autoconfig: Bus I/O region: [0-], > Physical Memory [77f0-77f0x] > pci_auto_config_devices: device pci_6:0.0 > PCI Autoconfig: BAR 0, Mem, size=0x100, address=0x7800 > bus_lower=0x7900 > > PCI Autoconfig: BAR 1, Mem, size=0x800, No room in resource, avail > start=7900 / size=800, need=800 > PCI: Failed autoconfig bar 14 > > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004 > > PCI Autoconfig: BAR 3, Mem, size=0x200, address=0x7a00 > bus_lower=0x7c00 > > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100 > > PCI Autoconfig: ROM, size=0x8, address=0x7c00 bus_lower=0x7c08 > > "Synchronous Abort" handler, esr 0x9606 > elr: e002bd28 lr : e002bce8 (reloc) > elr: fff6fd28 lr : fff6fce8 > x0 : 1041 x1 : 003e > x2 : ffb0f8c8 x3 : 0001 > x4 : 0080 x5 : > x6 : fff718fc x7 : 000f > x8 : ffb0f238 x9 : 0008 > x10: x11: 0010 > x12: 0006 x13: 0001869f > x14: ffb0fcd0 x15: 0020 > x16: fff71cc4 x17: > x18: ffb13d90 x19: ffb14320 > x20: x21: ffb14090 > x22: ffb0f8c8 x23: 0001 > x24: ffb14c10 x25: > x26: x27: > x28: ffb14c70 x29: ffb0f830 > > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280) > Resetting CPU ... > > Signed-off-by: Masami Hiramatsu > --- > drivers/pci/pci-uclass.c |3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 02/13] ata: ahci-pci: Use scsi_ops to initialize ops
On Tue, 13 Apr 2021 at 16:22, Masami Hiramatsu wrote: > > Without this fix, scsi-scan will cause a synchronous abort > when accessing ops->scan. > > Signed-off-by: Masami Hiramatsu > --- > drivers/ata/ahci-pci.c |2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup
On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu wrote: > > Update the highest subordinate bus number after probing the devices > under the bus for setting up the bridge correctly. > The commit 42f3663a3f67 ("pci: Update to use new sequence numbers") > removed this but it is required if a PCIe bridge is under the bus. > > Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers") > Signed-off-by: Masami Hiramatsu > --- > drivers/pci/pci-uclass.c |3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass Can we add a test for this?
Re: [PATCH] checkpatch: Ignore ENOSYS warnings
Hi Sean, On Tue, 13 Apr 2021 at 00:31, Sean Anderson wrote: > > On 4/12/21 2:04 PM, Simon Glass wrote: > > Hi Sean, > > > > On Mon, 12 Apr 2021 at 16:22, Sean Anderson wrote: > >> > >> There are no system calls in U-Boot, but ENOSYS is still allowed (and > >> preferred since 42a2668743 ("dm: core: Document the common error codes")). > >> Silence this warning. > >> > >> Signed-off-by: Sean Anderson > >> Seriies-to: sjg > > > > This looks OK, except for that tag. > > Should I resend the patch? Yes I think that would be easiest. Reviewed-by: Simon Glass
Re: [PATCH v2 01/17] dm: core: Add helper to compare node names
On Mon, 5 Apr 2021 at 22:28, Kishon Vijay Abraham I wrote: > > Add helper to compare node names. > > Signed-off-by: Kishon Vijay Abraham I > --- > drivers/core/ofnode.c | 13 + > include/dm/ofnode.h | 10 ++ > 2 files changed, 23 insertions(+) > > diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c > index fa0bd2a9c4..adc96fcaf3 100644 > --- a/drivers/core/ofnode.c > +++ b/drivers/core/ofnode.c > @@ -18,6 +18,19 @@ > #include > #include > > +bool ofnode_name_eq(ofnode node, const char *name) > +{ > + const char *node_name; > + size_t len; > + > + assert(ofnode_valid(node)); > + > + node_name = ofnode_get_name(node); > + len = strchrnul(node_name, '@') - node_name; > + > + return (strlen(name) == len) && (!strncmp(node_name, name, len)); drop extra brackets > +} > + > int ofnode_read_u32(ofnode node, const char *propname, u32 *outp) > { > return ofnode_read_u32_index(node, propname, 0, outp); > diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h > index 2c0597c407..a866b9b9b8 100644 > --- a/include/dm/ofnode.h > +++ b/include/dm/ofnode.h > @@ -231,6 +231,16 @@ static inline ofnode ofnode_root(void) > return node; > } > > +/** > + * ofnode_name_eq() - Check if the node name is equivalent to a given name > + *ignoring the unit address > + * > + * @node: valid node reference that has to be compared > + * @name: name that has to be compared with the node name > + * @return 1 if matches, 0 if it doesn't match true / false > + */ > +bool ofnode_name_eq(ofnode node, const char *name); > + > /** > * ofnode_read_u32() - Read a 32-bit integer from a property > * > -- > 2.17.1 >
Re: [PATCH v2 02/17] dm: test: Add test case to check node name ignoring unit address
On Mon, 5 Apr 2021 at 11:28, Kishon Vijay Abraham I wrote: > > Add test to check node name ignoring unit address. > > Signed-off-by: Kishon Vijay Abraham I > --- > test/dm/core.c | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Simon Glass > > diff --git a/test/dm/core.c b/test/dm/core.c > index 35ca689d64..b88b1966fd 100644 > --- a/test/dm/core.c > +++ b/test/dm/core.c > @@ -178,6 +178,20 @@ static int dm_test_autobind_uclass_pdata_alloc(struct > unit_test_state *uts) > } > DM_TEST(dm_test_autobind_uclass_pdata_alloc, UT_TESTF_SCAN_PDATA); > > +/* compare node names ignoring the unit address */ > +static int dm_test_compare_node_name(struct unit_test_state *uts) > +{ > + ofnode node; > + > + node = ofnode_path("/mmio-bus@0"); > + ut_assert(ofnode_valid(node)); > + ut_assert(ofnode_name_eq(node, "mmio-bus")); > + > + return 0; > +} > + > +DM_TEST(dm_test_compare_node_name, UT_TESTF_SCAN_PDATA); > + > /* Test that binding with uclass plat setting occurs correctly */ > static int dm_test_autobind_uclass_pdata_valid(struct unit_test_state *uts) > { > -- > 2.17.1 >
Re: Getting rid of falcon mode
Hi, On Tue, 13 Apr 2021 at 09:32, Alex G. wrote: > > ## Introduction > > Today we use "falcon mode" to mean "boot linux straight from SPL". This > designation makes sense, since falcons "fly at high speed and change > direction rapidly" according to Wikipedia. > > The way we implement falcon mode is to reserve two areas of storage: >* kernel area/partition >* dtb area/partition > By using some "special cases", and "spl export", SPL can more or less > figure out how to skip u-boot. > > > ## The plot twist > > People familiar with FIT, will have recognized that the above is > achievable with a very basic FIT image. With some advantages: > > - No "special cases" in SPL code > - Signed kernel images > - Signed kernel devicetree > - Devicetree overlays > - Automatic selection of correct devicetree > > > ## The problems > > The advantages of FIT are not obvious by looking at SPL code. A > noticeable amount of SPL code is hidden under #ifdef CONFIG_SPL_OS_BOOT, > leading one to believe that SPL_OS_BOOT is very important. It must be > since it takes up so much code. > > Enabling falcon mode is not well documented, and requires a lot of trial > and error. I've had to define 7 macros, and one new function to get it > working on my board -- and vividly remember the grief. This is an > antiquated way of doing things, and completely ignores the u-boot > devicetree -- we could just as well have defined those seven values in > the dtb. > > SPL assumes that it must load u-boot, unless in instances under > CONFIG_SPL_OS_BOOT. This has cause me a huge amount of grief and > confusion over the past several months. I have no less than three patch > series trying to address shortfalls there. It's awful. > > > ## The proposal > > I propose we drop falcon mode support for legacy images. > >- Drop CONFIG_SPL_OS_BOOT. Support for this is implied by SPL_FIT >- Drop the "dtb area/partition". The dtb is derived from the FIT >- Drop "spl export" > > > How do we deal with devicetrees in the FIT then? The options are to use > a modified devicetree which has the desired "/chosen" node, or use DTB > overlays. > > What are the reasons why we shouldn't go this route? I think this makes sense, on the basis that it is a legacy image and people can always use the U-Boot path if needed. I believe Falcon boot made a lot more sense when the cache was off. But at least in my experience, we were able to get through two SPLs and two U-Boots and boot a kernel about 800ms on a Cortex-A15, so Falcon mode might not be so relevant anymore, and supporting a legacy image seems like unnecessary complexity. Regards, Simon
Re: [PATCH v3 13/28] pci: Update to use new sequence numbers
Hi Tim, On Wed, 14 Apr 2021 at 06:32, Tim Harvey wrote: > > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass wrote: > > > > Now that we know the sequence number at bind time, there is no need for > > special-case code in dm_pci_hose_probe_bus(). > > > > Note: the PCI_CAP_ID_EA code may need a look, but there are no test > > failures so I have left it as is. > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v3: > > - Update PCI to use manual sequence numbering > > > > Changes in v2: > > - Use the sequence number directly instead of max bus > > > > drivers/pci/pci-uclass.c | 45 > > drivers/pci/pci_auto.c | 10 - > > 2 files changed, 32 insertions(+), 23 deletions(-) > > > > Applied to u-boot-dm/next, thanks! > > Hi Simon, > > I have a (not yet submitted) pending patch to migrate the gwventana > board to DM_PCI / DM_ETH that this particular patch broke and I'm > trying to understand why. > > The Gateworks Ventana boards have a PCIe switch where the downstream > PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on > one of those downstream ports. For non-dm PCI I have a > 'board_pci_fixup_dev' function that allows board support to configure > the switch's GPIO and toggle downstream PERST# during enumeration. > When I add this function to dm_pci I end up getting a data abort when > the e1000 driver tries to initialize (after PCI enumeration). Firstly, I think I did the PCI conversion about 6 years ago so it is not fresh in my memory. > > Any idea what is causing this and what I need to do to work around it? > Nothing in my patch deals with device sequence numbers. An abort presumably indicates that the memory is not there, so perhaps the e1000 is still in reset, or has been set up to appear at a different address? You should be abe to use 'pci hdr 0.4.0' or whatever to look at the BARs and make sure they are value with 'md', etc. > Additionally I feel like the best way to add support for the custom > downstream PCI switch reset requirements is to add a UCLASS_PCI driver > for the switch in my board support but when I attempted that solution > I run into an issue where pci read/write's cause a prefetch abort > because I need to use imx_pcie_dm_{read,write}_config instead of the > default ops. I'm not quite sure how to get hold of the ops for the imx If you write a PCI driver then you can provide the access operations yourselfsee drivers/pci for some examples. I suggest you add subnodes to the devicetree and specify the compatible string of your PCI switch so it picks up the right driver. > controller to set this up properly. Furthermore if I do end up with > that solution I later need to be able to walk the PCI bus to perform > various dt fixups on pci device nodes - do you have an example of how > I could walk the bus using DM_PCI? Do you mean a breadth-first search? It is something like: struct udevice *dev; static void do_something(struct udevice *parent) { device_foreach_child_probe(dev, parent) { do_something((dev); } ... struct udevice *bus; int ret; ret = uclass_first_device_err(UCLASS_PCI, &bus); if (ret) return log_msg_ret("pci", ret); do_something(bus); > > Best regards, > > Tim Regards, Simon
[PATCH resend 2/2] env: add CONFIG_ENV_SECT_SIZE_AUTO
This is roughly the U-Boot side equivalent to commit e282c422e0 (tools: fw_env: use erasesize from MEMGETINFO ioctl). The motivation is the case where one has a board with several revisions, where the SPI flashes have different erase sizes. In our case, we have an 8K environment, and the flashes have erase sizes of 4K (newer boards) and 64K (older boards). Currently, we must set CONFIG_ENV_SECT_SIZE to 64K to make the code work on the older boards, but for the newer ones, that ends up wasting quite a bit of time reading/erasing/restoring the last 56K. At first, I wanted to allow setting CONFIG_ENV_SECT_SIZE to 0 to mean "use the erase size the chip reports", but that config option is used in a number of preprocessor conditionals, and shared between ENV_IS_IN_FLASH and ENV_IS_IN_SPI_FLASH. So instead, introduce a new boolean config option, which for now can only be used with ENV_IS_IN_SPI_FLASH. If left off, there's no change in behaviour. The only slightly annoying detail is that, when selected, the compiler is apparently not smart enough to see that the the saved_size and saved_offset variables are only used under the same "if (sect_size > CONFIG_ENV_SIZE)" condition as where they are computed, so we need to initialize them to 0 to avoid "may be used uninitialized" warnings. On our newer boards with the 4K erase size, saving the environment now takes 0.080 seconds instead of 0.53 seconds, which directly translates to that much faster boot time since our logic always causes the environment to be written during boot. Signed-off-by: Rasmus Villemoes --- env/Kconfig | 14 ++ env/sf.c| 10 -- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/env/Kconfig b/env/Kconfig index b473d7cfe1..844c312870 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -324,6 +324,20 @@ config ENV_IS_IN_SPI_FLASH during a "saveenv" operation. CONFIG_ENV_OFFSET_REDUND must be aligned to an erase sector boundary. +config ENV_SECT_SIZE_AUTO + bool "Use automatically detected sector size" + depends on ENV_IS_IN_SPI_FLASH + help + Some boards exist in multiple variants, with different + flashes having different sector sizes. In such cases, you + can select this option to make U-Boot use the actual sector + size when figuring out how much to erase, which can thus be + more efficient on the flashes with smaller erase size. Since + the environment must always be aligned on a sector boundary, + CONFIG_ENV_OFFSET must be aligned to the largest of the + different sector sizes, and CONFIG_ENV_SECT_SIZE should be + set to that value. + config USE_ENV_SPI_BUS bool "SPI flash bus for environment" depends on ENV_IS_IN_SPI_FLASH diff --git a/env/sf.c b/env/sf.c index d9ed08a78e..06cc62e005 100644 --- a/env/sf.c +++ b/env/sf.c @@ -72,7 +72,7 @@ static int env_sf_save(void) { env_t env_new; char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE; - u32 saved_size, saved_offset, sector; + u32 saved_size = 0, saved_offset = 0, sector; u32 sect_size = CONFIG_ENV_SECT_SIZE; int ret; @@ -80,6 +80,9 @@ static int env_sf_save(void) if (ret) return ret; + if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO)) + sect_size = env_flash->mtd.erasesize; + ret = env_export(&env_new); if (ret) return -EIO; @@ -187,7 +190,7 @@ out: #else static int env_sf_save(void) { - u32 saved_size, saved_offset, sector; + u32 saved_size = 0, saved_offset = 0, sector; u32 sect_size = CONFIG_ENV_SECT_SIZE; char*saved_buffer = NULL; int ret = 1; @@ -197,6 +200,9 @@ static int env_sf_save(void) if (ret) return ret; + if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO)) + sect_size = env_flash->mtd.erasesize; + /* Is the sector larger than the env (i.e. embedded) */ if (sect_size > CONFIG_ENV_SIZE) { saved_size = sect_size - CONFIG_ENV_SIZE; -- 2.29.2
[PATCH resend 0/2] add CONFIG_ENV_SECT_SIZE_AUTO
This is a resend, just rebased to current master, of patches I sent way back in May. This is roughly the U-Boot side equivalent to commit e282c422e0 (tools: fw_env: use erasesize from MEMGETINFO ioctl), at least for SPI_FLASH backend. When CONFIG_ENV_SECT_SIZE_AUTO is not selected (and it is of course default n), there's no functional change, and the compiler even seems to generate identical binary code. The motivation is to cut about half a second off boottime on our newer revisions, while still having the same U-Boot binary work on both. Rasmus Villemoes (2): env/sf.c: use a variable to hold the sector size env: add CONFIG_ENV_SECT_SIZE_AUTO env/Kconfig | 14 ++ env/sf.c| 32 2 files changed, 34 insertions(+), 12 deletions(-) -- 2.29.2
[PATCH resend 1/2] env/sf.c: use a variable to hold the sector size
As preparation for the next patch, use a local variable to represent the sector size. No functional change. Signed-off-by: Rasmus Villemoes --- env/sf.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/env/sf.c b/env/sf.c index 88ec1108b6..d9ed08a78e 100644 --- a/env/sf.c +++ b/env/sf.c @@ -73,6 +73,7 @@ static int env_sf_save(void) env_t env_new; char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE; u32 saved_size, saved_offset, sector; + u32 sect_size = CONFIG_ENV_SECT_SIZE; int ret; ret = setup_flash_device(); @@ -93,8 +94,8 @@ static int env_sf_save(void) } /* Is the sector larger than the env (i.e. embedded) */ - if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { - saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE; + if (sect_size > CONFIG_ENV_SIZE) { + saved_size = sect_size - CONFIG_ENV_SIZE; saved_offset = env_new_offset + CONFIG_ENV_SIZE; saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size); if (!saved_buffer) { @@ -107,11 +108,11 @@ static int env_sf_save(void) goto done; } - sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE); + sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, sect_size); puts("Erasing SPI flash..."); ret = spi_flash_erase(env_flash, env_new_offset, - sector * CONFIG_ENV_SECT_SIZE); + sector * sect_size); if (ret) goto done; @@ -122,7 +123,7 @@ static int env_sf_save(void) if (ret) goto done; - if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { + if (sect_size > CONFIG_ENV_SIZE) { ret = spi_flash_write(env_flash, saved_offset, saved_size, saved_buffer); if (ret) @@ -187,6 +188,7 @@ out: static int env_sf_save(void) { u32 saved_size, saved_offset, sector; + u32 sect_size = CONFIG_ENV_SECT_SIZE; char*saved_buffer = NULL; int ret = 1; env_t env_new; @@ -196,8 +198,8 @@ static int env_sf_save(void) return ret; /* Is the sector larger than the env (i.e. embedded) */ - if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { - saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE; + if (sect_size > CONFIG_ENV_SIZE) { + saved_size = sect_size - CONFIG_ENV_SIZE; saved_offset = CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE; saved_buffer = malloc(saved_size); if (!saved_buffer) @@ -213,11 +215,11 @@ static int env_sf_save(void) if (ret) goto done; - sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE); + sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, sect_size); puts("Erasing SPI flash..."); ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET, - sector * CONFIG_ENV_SECT_SIZE); + sector * sect_size); if (ret) goto done; @@ -227,7 +229,7 @@ static int env_sf_save(void) if (ret) goto done; - if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { + if (sect_size > CONFIG_ENV_SIZE) { ret = spi_flash_write(env_flash, saved_offset, saved_size, saved_buffer); if (ret) -- 2.29.2
Re: [PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support
On Wed, Apr 14, 2021 at 10:12:42AM +0900, Masami Hiramatsu wrote: > Hello Tom, > > 2021年4月14日(水) 2:47 Tom Rini : > > > > On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote: > > > > > Add the DeveloperBox 96boards EE support. This board is also > > > known as Socionext SynQuacer E-Series. It contians one "SC2A11" > > > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots, > > > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via > > > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA > > > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard > > > MicroATX Form Factor. > > > > > > For more information, see this page; > > > https://www.96boards.org/product/developerbox/ > > > > > > Signed-off-by: Masami Hiramatsu > > [snip] > > > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h > > > b/arch/arm/include/asm/arch-sc2a11/gpio.h > > > new file mode 100644 > > > index 00..6779803080 > > > --- /dev/null > > > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h > > > @@ -0,0 +1,9 @@ > > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > > +/* > > > + * Copyright 2021 (C) Linaro Ltd. > > > + */ > > > + > > > +#ifndef __ASM_ARCH_SC2A11_GPIO_H > > > +#define __ASM_ARCH_SC2A11_GPIO_H > > > + > > > +#endif > > > > Please update the list in arch/arm/include/asm/gpio.h to not look for > > asm/arch/gpio.h on this SoC, thanks. > > Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h. > > BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO > instead of updating the header? Such a clean-up would be appreciated. -- Tom signature.asc Description: PGP signature
Re: [PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox
On Wed, Apr 14, 2021 at 10:06:04AM +0900, Masami Hiramatsu wrote: > Hello Tom, > > Thank you for your comment! > > 2021年4月14日(水) 2:47 Tom Rini : > > > > On Wed, Apr 14, 2021 at 12:30:15AM +0900, Masami Hiramatsu wrote: > > > > > Add device trees for 96boards EE DeveloperBox and basement SynQuacer > > > SoC dtsi. These files are imported from EDK2 with cosmetic change > > > and modified for accessing SPI NOR flash. > > > > > > Signed-off-by: Masami Hiramatsu > > > --- > > > arch/arm/dts/DeveloperBox.dts | 146 + > > > arch/arm/dts/Makefile |2 > > > arch/arm/dts/SynQuacer.dtsi | 590 > > > + > > > arch/arm/dts/SynQuacerCaches.dtsi | 72 + > > > > This poses a bit of a naming challenge. I assume, but please correct me > > if I'm wrong, that you don't intend to push these dts files to the Linux > > kernel. So that means EDK2 is the primary source of the files, yes? > > Yes, those are originally written for the EDK2 and I ported it. OK. > > We > > want to keep them as-is from the upstream project, and note that > > relevant git hash/tag/etc that matches where we pulled from, to make > > future syncs easier, in the commit message. > > Let me confirm what you mean, is the git hash/tag/etc what the commit > I copied from? and note it in commit message or in the devicetree file? Correct. > BTW, I made some changes on it for U-Boot drivers. > - Enabled OP-TEE node by default (EDK2 checks OP-TEE existance > and enables it) > - Add SPI node information for accessing SPI-NOR from U-Boot (EDK2 > embedded such information inside) > Thus the DeveloperBox.dts may not be kept as-is. These kinds of changes can be done in something such as DeveloperBox-u-boot.dtsi. Check out the logic in scripts/Makefile.lib around automatic inclusion of a "-u-boot.dtsi" file. > > I assume this is not part of the uniphier family, so we should start by > > naming these as arch/arm/dts/synquacer-... to fit with the general > > scheme. Perhaps -developerbox, -core and -caches ? > > OK. BTW, I'm not sure what is the best way to use SYS_CPU and SYS_SOC. > This SoC family name is SynQuacer and the SoC itself is SC2A11. > In this case, > CONFIG_SYS_CPU=synquacer > CONFIG_SYS_SOC=sc2a11 > or > CONFIG_SYS_SOC=synquacer-sc2a11 I think the first example you list is likely the best. > > And for any changes > > we do need, they go in a -u-boot.dtsi file and then potentially > > automatically included (if it fits in the logic we have today for that) > > or specifically #included otherwise. > > It seems that (CONFIG_SYS_SOC)-u-boot.dtsi is automatically included, > am I correct? For the full list and how it works, see scripts/Makefile.lib -- Tom signature.asc Description: PGP signature
[PATCH] imx: Add support for Ronetix's iMX8MM-CM board
Supported peripherals: ETH, SD, eMMC, PMIC, QSPI NOR Flash U-Boot SPL 2021.04-00759-g59701858a1-dirty (Apr 14 2021 - 17:23:43 +0200) PMIC: BD71847 ID=0xa1 Normal Boot WDT: Not starting Trying to boot from MMC1 U-Boot 2021.04-00759-g59701858a1-dirty (Apr 14 2021 - 17:23:43 +0200) CPU: Freescale i.MX8MMQL rev1.0 at 1200 MHz Reset cause: POR Model: Ronetix i.MX8MM-CM board DRAM: 1 GiB WDT: Started with servicing (60s timeout) MMC: FSL_SDHC: 1, FSL_SDHC: 2 Loading Environment from MMC... OK In:serial Out: serial Err: serial Net: Warning: ethernet@30be (eth0) using random MAC address - 4a:f6:bf:8d:d8:a9 eth0: ethernet@30be Hit any key to stop autoboot: 0 Signed-off-by: Ilko Iliev --- arch/arm/dts/Makefile |1 + arch/arm/dts/imx8mm-cm-u-boot.dtsi| 253 +++ arch/arm/dts/imx8mm-cm.dts| 558 + arch/arm/mach-imx/imx8m/Kconfig |8 + board/ronetix/common |1 + board/ronetix/imx8mm-cm/Kconfig | 14 + board/ronetix/imx8mm-cm/MAINTAINERS |6 + board/ronetix/imx8mm-cm/Makefile | 12 + board/ronetix/imx8mm-cm/README| 43 + board/ronetix/imx8mm-cm/imx8mm_cm.c | 68 + .../ronetix/imx8mm-cm/imximage-8mm-lpddr4.cfg |9 + board/ronetix/imx8mm-cm/lpddr4_timing.c | 1862 + board/ronetix/imx8mm-cm/spl.c | 168 ++ configs/imx8mm_cm_defconfig | 110 + include/configs/imx8mm_cm.h | 107 + 15 files changed, 3220 insertions(+) create mode 100644 arch/arm/dts/imx8mm-cm-u-boot.dtsi create mode 100644 arch/arm/dts/imx8mm-cm.dts create mode 12 board/ronetix/common create mode 100644 board/ronetix/imx8mm-cm/Kconfig create mode 100644 board/ronetix/imx8mm-cm/MAINTAINERS create mode 100644 board/ronetix/imx8mm-cm/Makefile create mode 100644 board/ronetix/imx8mm-cm/README create mode 100644 board/ronetix/imx8mm-cm/imx8mm_cm.c create mode 100644 board/ronetix/imx8mm-cm/imximage-8mm-lpddr4.cfg create mode 100644 board/ronetix/imx8mm-cm/lpddr4_timing.c create mode 100644 board/ronetix/imx8mm-cm/spl.c create mode 100644 configs/imx8mm_cm_defconfig create mode 100644 include/configs/imx8mm_cm.h diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index b58f841472..b52c65fd09 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -827,6 +827,7 @@ dtb-$(CONFIG_ARCH_IMX8) += \ imx8-giedi.dtb dtb-$(CONFIG_ARCH_IMX8M) += \ + imx8mm-cm.dtb \ imx8mm-evk.dtb \ imx8mm-venice.dtb \ imx8mm-venice-gw71xx-0x.dtb \ diff --git a/arch/arm/dts/imx8mm-cm-u-boot.dtsi b/arch/arm/dts/imx8mm-cm-u-boot.dtsi new file mode 100644 index 00..c22b14a6e2 --- /dev/null +++ b/arch/arm/dts/imx8mm-cm-u-boot.dtsi @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2019 NXP + */ + +/ { + binman: binman { + multiple-images; + }; + + wdt-reboot { + compatible = "wdt-reboot"; + wdt = <&wdog1>; + u-boot,dm-spl; + }; + + firmware { + optee { + compatible = "linaro,optee-tz"; + method = "smc"; + }; + }; +}; + +&{/soc@0} { + u-boot,dm-pre-reloc; + u-boot,dm-spl; +}; + +&clk { + u-boot,dm-spl; + u-boot,dm-pre-reloc; + /delete-property/ assigned-clocks; + /delete-property/ assigned-clock-parents; + /delete-property/ assigned-clock-rates; +}; + +&osc_24m { + u-boot,dm-spl; + u-boot,dm-pre-reloc; +}; + +&aips1 { + u-boot,dm-spl; + u-boot,dm-pre-reloc; +}; + +&aips2 { + u-boot,dm-spl; +}; + +&aips3 { + u-boot,dm-spl; +}; + +&iomuxc { + u-boot,dm-spl; +}; + +®_usdhc2_vmmc { + u-boot,off-on-delay-us = <2>; +}; + +&pinctrl_reg_usdhc2_vmmc { + u-boot,dm-spl; +}; + +&pinctrl_uart2 { + u-boot,dm-spl; +}; + +&pinctrl_usdhc2_gpio { + u-boot,dm-spl; +}; + +&pinctrl_usdhc2 { + u-boot,dm-spl; +}; + +&pinctrl_usdhc3 { + u-boot,dm-spl; +}; + +&gpio1 { + u-boot,dm-spl; +}; + +&gpio2 { + u-boot,dm-spl; +}; + +&gpio3 { + u-boot,dm-spl; +}; + +&gpio4 { + u-boot,dm-spl; +}; + +&gpio5 { + u-boot,dm-spl; +}; + +&uart2 { + u-boot,dm-spl; +}; + +&usdhc1 { + u-boot,dm-spl; +}; + +&usdhc2 { + u-boot,dm-spl; + sd-uhs-sdr104; + sd-uhs-ddr50; + fsl,signal-voltage-switch-extra-delay-ms = <8>; +}; + +&usdhc3 { + u-boot,dm-spl; + mmc-hs400-1_8v; + mmc-hs400-enhanced-strobe; +}; + +&i2c2 { + u-boot,dm-spl; +}; + +&{/soc@0/bus@3080/i2c@30a3/pmic@4b} { + u-boot,dm-spl; +}; + +&{/soc@0/bus@3080/i2c@30a3/pmic@4b/regulators} { + u-boot,dm-spl; +}; + +&pinctrl_i2c2 { + u-boot,dm-spl; +}; + +&pinctrl_pmic { + u-boot,dm-s
Re: [PATCH v2 7/7] arm: cache: cp15: don't map the reserved region with no-map property
Hi, On 2/8/21 2:26 PM, Patrick Delaunay wrote: No more map the reserved region with "no-map" property by marking the corresponding TLB entries with invalid entry (=0) to avoid speculative access. This patch fixes potential issue when predictive access is done by ARM core. Signed-off-by: Patrick Delaunay Signed-off-by: Patrick Delaunay --- (no changes since v1) arch/arm/include/asm/system.h | 3 +++ arch/arm/lib/cache-cp15.c | 19 +-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 11fceec4d2..c63ed07f2c 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -444,6 +444,7 @@ static inline void set_cr(unsigned int val) /* options available for data cache on each page */ enum dcache_option { + INVALID_ENTRY = 0, DCACHE_OFF = TTB_SECT | TTB_SECT_MAIR(0) | TTB_SECT_XN_MASK, DCACHE_WRITETHROUGH = TTB_SECT | TTB_SECT_MAIR(1), DCACHE_WRITEBACK = TTB_SECT | TTB_SECT_MAIR(2), @@ -474,6 +475,7 @@ enum dcache_option { * 11 1 Outer/Inner Write-Back, Read-Allocate Write-Allocate */ enum dcache_option { + INVALID_ENTRY = 0, DCACHE_OFF = TTB_SECT_DOMAIN(0) | TTB_SECT_XN_MASK | TTB_SECT, DCACHE_WRITETHROUGH = TTB_SECT_DOMAIN(0) | TTB_SECT | TTB_SECT_C_MASK, DCACHE_WRITEBACK = DCACHE_WRITETHROUGH | TTB_SECT_B_MASK, @@ -483,6 +485,7 @@ enum dcache_option { #define TTB_SECT_AP (3 << 10) /* options available for data cache on each page */ enum dcache_option { + INVALID_ENTRY = 0, DCACHE_OFF = 0x12, DCACHE_WRITETHROUGH = 0x1a, DCACHE_WRITEBACK = 0x1e, diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c index 8a49e5217c..8a354d364d 100644 --- a/arch/arm/lib/cache-cp15.c +++ b/arch/arm/lib/cache-cp15.c @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -101,18 +102,32 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, __weak void dram_bank_mmu_setup(int bank) { struct bd_info *bd = gd->bd; + struct lmb lmb; int i; /* bd->bi_dram is available only after relocation */ if ((gd->flags & GD_FLG_RELOC) == 0) return; + /* +* don't allow cache on reserved memory tagged 'no-map' in DT +* => avoid speculative access to "secure" data +*/ + if (IS_ENABLED(CONFIG_LMB)) + lmb_init_and_reserve(&lmb, bd, (void *)gd->fdt_blob); + debug("%s: bank: %d\n", __func__, bank); for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT; i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) + (bd->bi_dram[bank].size >> MMU_SECTION_SHIFT); -i++) - set_section_dcache(i, DCACHE_DEFAULT_OPTION); +i++) { + if (IS_ENABLED(CONFIG_LMB) && + lmb_is_reserved_flags(&lmb, i << MMU_SECTION_SHIFT, + LMB_NOMAP)) + set_section_dcache(i, INVALID_ENTRY); + else + set_section_dcache(i, DCACHE_DEFAULT_OPTION); + } } /* to activate the MMU we need to set up virtual memory: use 1M areas */ Hi, After more test on stm32mp15x platform, the patch [6/7] introduced performance issue for the boot time. The device tree parsing done in lmb_init_and_reserve() to found the the reserved memory region with "no-map" tag is done with data cache deactivated (as it is called before MMU and data cache activation). This parsing increase the device boot time (several second lost in stm32mp15). So I need to sent a update version for this patch [6/7] But I will also drop this patch patch [7/7] to avoid to increase the boot time on other arm platform using cp15. Regards Patrick
Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay
On 4/14/21 4:07 PM, Patrick DELAUNAY wrote: Hi, Hi, On 4/9/21 2:22 PM, Marek Vasut wrote: On 4/9/21 10:00 AM, Patrick Delaunay wrote: The gpio reset assert/deassert delay are today harcoded in U-Boot driver but the value should be read from DT. STM32 use the generic binding defined in linux: Documentation/devicetree/bindings/net/ethernet-phy.yaml reset-gpios: maxItems: 1 description: The GPIO phandle and specifier for the PHY reset signal. reset-assert-us: description: Delay after the reset was asserted in microseconds. If this property is missing the delay will be skipped. reset-deassert-us: description: Delay after the reset was deasserted in microseconds. If this property is missing the delay will be skipped. See also U-Boot: doc/device-tree-bindings/net/phy.txt Since this is a PHY property, shouldn't that be handled in drivers/net/phy/ instead of MAC driver ? I was my first idea but I don't found found the correct location in phy (driver or uclass) to manage these generic property and the generic property "reset-gpios" was already managed in eth driver, so I continue to patch the driver. But I come back to this idea after your remark => in linux these property are managed in drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register parse DT node and add info in mdio drivers/net/phy/mdio_device.c::mdio_device_reset called in mdio_probe / mdio_remove In my first search, I don't found the same level in the U-Boot drivers in drivers/net/phy/ Note that this is MDIO-wide PHY reset (e.g. you can have single reset line connected to multiple PHYs on single MDIO bus), this is not PHY-specific reset. but I miss the uclass defined in drivers/net/eth-phy-uclass.c Finally I think I need to manage the generic binding property (reset-gpios, reset-assert-us, reset-deassert-us) directly in => drivers/net/mdio-uclass The GPIO RESET will be managed in mdio ops: pre_probe/ post_remove as it is done in linux warning: today post_remove ops don't exit in u-class. Do you think it is the correct location ? For single-PHY reset, the correct location is in drivers/net/phy/ somewhere. Do you think it should be a new serie (migrate the eqos property in mdio) after this eqos is accepted or I shoudl sent a new serie to replace this serie. I'll leave that decision to Ramon/Joe.
Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay
Hi, On 4/9/21 2:22 PM, Marek Vasut wrote: On 4/9/21 10:00 AM, Patrick Delaunay wrote: The gpio reset assert/deassert delay are today harcoded in U-Boot driver but the value should be read from DT. STM32 use the generic binding defined in linux: Documentation/devicetree/bindings/net/ethernet-phy.yaml reset-gpios: maxItems: 1 description: The GPIO phandle and specifier for the PHY reset signal. reset-assert-us: description: Delay after the reset was asserted in microseconds. If this property is missing the delay will be skipped. reset-deassert-us: description: Delay after the reset was deasserted in microseconds. If this property is missing the delay will be skipped. See also U-Boot: doc/device-tree-bindings/net/phy.txt Since this is a PHY property, shouldn't that be handled in drivers/net/phy/ instead of MAC driver ? I was my first idea but I don't found found the correct location in phy (driver or uclass) to manage these generic property and the generic property "reset-gpios" was already managed in eth driver, so I continue to patch the driver. But I come back to this idea after your remark => in linux these property are managed in drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register parse DT node and add info in mdio drivers/net/phy/mdio_device.c::mdio_device_reset called in mdio_probe / mdio_remove In my first search, I don't found the same level in the U-Boot drivers in drivers/net/phy/ but I miss the uclass defined in drivers/net/eth-phy-uclass.c Finally I think I need to manage the generic binding property (reset-gpios, reset-assert-us, reset-deassert-us) directly in => drivers/net/mdio-uclass The GPIO RESET will be managed in mdio ops: pre_probe/ post_remove as it is done in linux warning: today post_remove ops don't exit in u-class. Do you think it is the correct location ? Do you think it should be a new serie (migrate the eqos property in mdio) after this eqos is accepted or I shoudl sent a new serie to replace this serie. Regards Patrick
Re: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support
Hi Priyanka, On Wed, Apr 14, 2021 at 7:54 PM Priyanka Jain wrote: > > > > >-Original Message- > >From: Bin Meng > >Sent: Sunday, March 14, 2021 5:45 PM > >To: Priyanka Jain ; Ramon Fried > >; Simon Glass ; u- > >b...@lists.denx.de > >Cc: Tom Rini ; Vladimir Oltean ; > >Bin Meng > >Subject: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support > > > >QEMU ppce500 target can dynamically instantiate an eTSEC device if "-device > >eTSEC" is given to QEMU. This commit enables eTSEC driver and the required > >fixed PHY driver to create a usable network configuration using eTSEC. > > > >Unlike a real world 85xx board that usually stores the eTSEC MAC address in > >an > >EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for QEMU otherwise U- > >Boot ethernet initialization complains no valid ethernet address is set. > > > >Signed-off-by: Bin Meng > >Reviewed-by: Vladimir Oltean > >--- > > > >(no changes since v1) > > > > configs/qemu-ppce500_defconfig | 4 > > 1 file changed, 4 insertions(+) > > > >diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig > >index 151834b4cf..a1b9ea56ca 100644 > >--- a/configs/qemu-ppce500_defconfig > >+++ b/configs/qemu-ppce500_defconfig > >@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y > > CONFIG_OF_BOARD=y > > CONFIG_ENV_OVERWRITE=y > > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > >+CONFIG_NET_RANDOM_ETHADDR=y > > CONFIG_DM=y > > CONFIG_SIMPLE_BUS_CORRECT_RANGE=y > > CONFIG_BLK=y > >@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y CONFIG_DM_I2C=y > >CONFIG_SYS_I2C_FSL=y # CONFIG_MMC is not set > >+CONFIG_PHY_FIXED=y > > CONFIG_DM_ETH=y > >+CONFIG_DM_MDIO=y > > CONFIG_E1000=y > >+CONFIG_TSEC_ENET=y > > CONFIG_DM_PCI=y > > CONFIG_PCI_MPC85XX=y > > CONFIG_DM_RTC=y > >-- > >2.25.1 > > I tried integrating the series and was getting below error: > 2021-04-12T09:39:56.7536565Z FAILED > test/py/tests/test_efi_selftest.py::test_efi_selftest - u_boot_spawn.T... > 2021-04-12T09:39:56.7537048Z = 1 failed, 108 passed, 227 skipped, 1 > deselected, 3 warnings in 65.61s (0:01:05) = > > Details at > https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/2112/logs/251 > > I reverted this patch and it then build fine . > https://github.com/u-boot/u-boot/pull/65 As I mentioned in this series cover letter, Azure results were all PASS. Please see: https://dev.azure.com/bmeng/GitHub/_build/results?buildId=343&view=results Regards, Bin
Re: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support
On Wed, Apr 14, 2021 at 11:54:39AM +, Priyanka Jain wrote: > > > >-Original Message- > >From: Bin Meng > >Sent: Sunday, March 14, 2021 5:45 PM > >To: Priyanka Jain ; Ramon Fried > >; Simon Glass ; u- > >b...@lists.denx.de > >Cc: Tom Rini ; Vladimir Oltean ; > >Bin Meng > >Subject: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support > > > >QEMU ppce500 target can dynamically instantiate an eTSEC device if "-device > >eTSEC" is given to QEMU. This commit enables eTSEC driver and the required > >fixed PHY driver to create a usable network configuration using eTSEC. > > > >Unlike a real world 85xx board that usually stores the eTSEC MAC address in > >an > >EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for QEMU otherwise U- > >Boot ethernet initialization complains no valid ethernet address is set. > > > >Signed-off-by: Bin Meng > >Reviewed-by: Vladimir Oltean > >--- > > > >(no changes since v1) > > > > configs/qemu-ppce500_defconfig | 4 > > 1 file changed, 4 insertions(+) > > > >diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig > >index 151834b4cf..a1b9ea56ca 100644 > >--- a/configs/qemu-ppce500_defconfig > >+++ b/configs/qemu-ppce500_defconfig > >@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y > > CONFIG_OF_BOARD=y > > CONFIG_ENV_OVERWRITE=y > > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > >+CONFIG_NET_RANDOM_ETHADDR=y > > CONFIG_DM=y > > CONFIG_SIMPLE_BUS_CORRECT_RANGE=y > > CONFIG_BLK=y > >@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y CONFIG_DM_I2C=y > >CONFIG_SYS_I2C_FSL=y # CONFIG_MMC is not set > >+CONFIG_PHY_FIXED=y > > CONFIG_DM_ETH=y > >+CONFIG_DM_MDIO=y > > CONFIG_E1000=y > >+CONFIG_TSEC_ENET=y > > CONFIG_DM_PCI=y > > CONFIG_PCI_MPC85XX=y > > CONFIG_DM_RTC=y > >-- > >2.25.1 > > I tried integrating the series and was getting below error: > 2021-04-12T09:39:56.7536565Z FAILED > test/py/tests/test_efi_selftest.py::test_efi_selftest - u_boot_spawn.T... > 2021-04-12T09:39:56.7537048Z = 1 failed, 108 passed, 227 skipped, 1 > deselected, 3 warnings in 65.61s (0:01:05) = > > Details at > https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/2112/logs/251 > > I reverted this patch and it then build fine . > https://github.com/u-boot/u-boot/pull/65 Did this fail more than once in that job? Sometimes due to I assume some race/etc, that test will fail from time to time. -- Tom signature.asc Description: PGP signature
RE: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support
>-Original Message- >From: Bin Meng >Sent: Sunday, March 14, 2021 5:45 PM >To: Priyanka Jain ; Ramon Fried >; Simon Glass ; u- >b...@lists.denx.de >Cc: Tom Rini ; Vladimir Oltean ; >Bin Meng >Subject: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support > >QEMU ppce500 target can dynamically instantiate an eTSEC device if "-device >eTSEC" is given to QEMU. This commit enables eTSEC driver and the required >fixed PHY driver to create a usable network configuration using eTSEC. > >Unlike a real world 85xx board that usually stores the eTSEC MAC address in an >EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for QEMU otherwise U- >Boot ethernet initialization complains no valid ethernet address is set. > >Signed-off-by: Bin Meng >Reviewed-by: Vladimir Oltean >--- > >(no changes since v1) > > configs/qemu-ppce500_defconfig | 4 > 1 file changed, 4 insertions(+) > >diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig >index 151834b4cf..a1b9ea56ca 100644 >--- a/configs/qemu-ppce500_defconfig >+++ b/configs/qemu-ppce500_defconfig >@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y > CONFIG_OF_BOARD=y > CONFIG_ENV_OVERWRITE=y > CONFIG_SYS_RELOC_GD_ENV_ADDR=y >+CONFIG_NET_RANDOM_ETHADDR=y > CONFIG_DM=y > CONFIG_SIMPLE_BUS_CORRECT_RANGE=y > CONFIG_BLK=y >@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y CONFIG_DM_I2C=y >CONFIG_SYS_I2C_FSL=y # CONFIG_MMC is not set >+CONFIG_PHY_FIXED=y > CONFIG_DM_ETH=y >+CONFIG_DM_MDIO=y > CONFIG_E1000=y >+CONFIG_TSEC_ENET=y > CONFIG_DM_PCI=y > CONFIG_PCI_MPC85XX=y > CONFIG_DM_RTC=y >-- >2.25.1 I tried integrating the series and was getting below error: 2021-04-12T09:39:56.7536565Z FAILED test/py/tests/test_efi_selftest.py::test_efi_selftest - u_boot_spawn.T... 2021-04-12T09:39:56.7537048Z = 1 failed, 108 passed, 227 skipped, 1 deselected, 3 warnings in 65.61s (0:01:05) = Details at https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/2112/logs/251 I reverted this patch and it then build fine . https://github.com/u-boot/u-boot/pull/65 Kindly check. Regards Priyanka
[PATCH v4 1/2] timer: mpc83xx_timer: fix build with CONFIG_{HW_, }WATCHDOG
The code, which is likely copied from arch/powerpc/lib/interrupts.c, lacks a fallback definition of CONFIG_SYS_WATCHDOG_FREQ and refers to a non-existing timestamp variable - obviously priv->timestamp is meant. Signed-off-by: Rasmus Villemoes --- drivers/timer/mpc83xx_timer.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c index f4f6e90387..9adb4b4cab 100644 --- a/drivers/timer/mpc83xx_timer.c +++ b/drivers/timer/mpc83xx_timer.c @@ -20,6 +20,10 @@ DECLARE_GLOBAL_DATA_PTR; +#ifndef CONFIG_SYS_WATCHDOG_FREQ +#define CONFIG_SYS_WATCHDOG_FREQ (CONFIG_SYS_HZ / 2) +#endif + /** * struct mpc83xx_timer_priv - Private data structure for MPC83xx timer driver * @decrementer_count: Value to which the decrementer register should be re-set @@ -171,7 +175,7 @@ void timer_interrupt(struct pt_regs *regs) priv->timestamp++; #if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG) - if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) + if ((priv->timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) WATCHDOG_RESET(); #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */ -- 2.29.2
Re: [PATCH] efi_loader: fix possible buffer overflow
On 4/14/21 7:43 AM, Ilias Apalodimas wrote: On Wed, Apr 14, 2021 at 11:55:49AM +0900, Masahisa Kojima wrote: Variable "final" will have SHA512 digest, but currently the array size is not sufficient. Let's fix it. Signed-off-by: Masahisa Kojima --- lib/efi_loader/efi_tcg2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index ed86a220fb..d5eca68769 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -515,7 +515,7 @@ static efi_status_t tcg2_create_digest(const u8 *input, u32 length, sha1_context ctx; sha256_context ctx_256; sha512_context ctx_512; - u8 final[TPM2_ALG_SHA512]; + u8 final[TPM2_SHA512_DIGEST_SIZE]; efi_status_t ret; u32 active; int i; -- 2.17.1 Thanks! Reviewed-by: Ilias Apalodimas I have queued the patch for my next pull request. Reviewed-by: Heinrich Schuchardt
Re: [PATCH] efi_loader: esrt: Remove incorrect invocations of EFI_CALL macro
On 4/14/21 9:08 AM, Sughosh Ganu wrote: Remove function invocations using the EFI_CALL macro for those functions that do not have an EFI_ENTRY call in their definition. Such functions can use u-boot api's which rely on u-boot global data(gd) pointer. The Arm and RiscV architectures maintain a separate gd pointer, one for u-boot, and a separate gd for the efi application. Calling a function through the EFI_CALL macro changes the gd pointer to that used for the efi application, with u-boot gd being unavailable. Any function then trying to dereference u-boot's gd will result in an abort. Fix this issue by removing the EFI_CALL macro for all of such functions which do not begin by an EFI_ENTRY function call. Signed-off-by: Sughosh Ganu Thanks for reviewing the ESRT code. Reviewed-by: Heinrich Schuchardt
[PATCH] ls1012a: net: pfe: remove pfe stop from bootcmd
When using bootefi to boot a EFI binary, u-boot is supposed to provide networking service for EFI application. Currently, 'pfe stop' command is called from bootcmd before running bootefi. As a result network stops working for EFI applications and console is flooded with "Rx pkt not on expected port" messages. Implement board_quiesce_devices() for ls1012a boards and call pfe_command_stop() from it instead of calling 'pfe stop' from *_bootcmd and bootcmd. Tested-by: Anji Jagarlmudi Signed-off-by: Mian Yousaf Kaukab --- board/freescale/ls1012afrdm/ls1012afrdm.c | 8 board/freescale/ls1012aqds/ls1012aqds.c | 8 board/freescale/ls1012ardb/ls1012ardb.c | 8 drivers/net/pfe_eth/pfe_cmd.c | 2 +- include/configs/ls1012a2g5rdb.h | 6 +++--- include/configs/ls1012a_common.h | 4 ++-- include/configs/ls1012afrdm.h | 6 +++--- include/configs/ls1012afrwy.h | 6 +++--- include/configs/ls1012aqds.h | 6 +++--- include/configs/ls1012ardb.h | 6 +++--- include/net/pfe_eth/pfe/pfe_hw.h | 6 ++ 11 files changed, 48 insertions(+), 18 deletions(-) diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c b/board/freescale/ls1012afrdm/ls1012afrdm.c index 2cd651b943fb..7e87e5a9846f 100644 --- a/board/freescale/ls1012afrdm/ls1012afrdm.c +++ b/board/freescale/ls1012afrdm/ls1012afrdm.c @@ -23,6 +23,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -185,6 +186,13 @@ int board_init(void) return 0; } +#ifdef CONFIG_FSL_PFE +void board_quiesce_devices(void) +{ +pfe_command_stop(0, NULL); +} +#endif + int ft_board_setup(void *blob, struct bd_info *bd) { arch_fixup_fdt(blob); diff --git a/board/freescale/ls1012aqds/ls1012aqds.c b/board/freescale/ls1012aqds/ls1012aqds.c index cfe3f3360cd9..4f98a8652f21 100644 --- a/board/freescale/ls1012aqds/ls1012aqds.c +++ b/board/freescale/ls1012aqds/ls1012aqds.c @@ -32,6 +32,7 @@ #include "../common/qixis.h" #include "ls1012aqds_qixis.h" #include "ls1012aqds_pfe.h" +#include DECLARE_GLOBAL_DATA_PTR; @@ -163,6 +164,13 @@ int board_init(void) return 0; } +#ifdef CONFIG_FSL_PFE +void board_quiesce_devices(void) +{ +pfe_command_stop(0, NULL); +} +#endif + int esdhc_status_fixup(void *blob, const char *compat) { char esdhc0_path[] = "/soc/esdhc@156"; diff --git a/board/freescale/ls1012ardb/ls1012ardb.c b/board/freescale/ls1012ardb/ls1012ardb.c index 41bcf6f935e9..62e8af48cf14 100644 --- a/board/freescale/ls1012ardb/ls1012ardb.c +++ b/board/freescale/ls1012ardb/ls1012ardb.c @@ -28,6 +28,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -186,6 +187,13 @@ int board_init(void) return 0; } +#ifdef CONFIG_FSL_PFE +void board_quiesce_devices(void) +{ + pfe_command_stop(0, NULL); +} +#endif + #ifdef CONFIG_TARGET_LS1012ARDB int esdhc_status_fixup(void *blob, const char *compat) { diff --git a/drivers/net/pfe_eth/pfe_cmd.c b/drivers/net/pfe_eth/pfe_cmd.c index 1e69525cb71f..364750f65c75 100644 --- a/drivers/net/pfe_eth/pfe_cmd.c +++ b/drivers/net/pfe_eth/pfe_cmd.c @@ -418,7 +418,7 @@ static void send_dummy_pkt_to_hif(void) writel(buf, TMU_PHY_INQ_PKTINFO); } -static void pfe_command_stop(int argc, char *const argv[]) +void pfe_command_stop(int argc, char *const argv[]) { int pfe_pe_id, hif_stop_loop = 10; u32 rx_status; diff --git a/include/configs/ls1012a2g5rdb.h b/include/configs/ls1012a2g5rdb.h index bbc3ffd7f0d3..c55fc6487756 100644 --- a/include/configs/ls1012a2g5rdb.h +++ b/include/configs/ls1012a2g5rdb.h @@ -79,7 +79,7 @@ "installer=load mmc 0:2 $load_addr "\ "/flex_installer_arm64.itb; "\ "bootm $load_addr#$board\0" \ - "qspi_bootcmd=pfe stop; echo Trying load from qspi..;" \ + "qspi_bootcmd=echo Trying load from qspi..;"\ "sf probe && sf read $load_addr " \ "$kernel_addr $kernel_size; env exists secureboot " \ "&& sf read $kernelheader_addr_r $kernelheader_addr " \ @@ -89,11 +89,11 @@ #undef CONFIG_BOOTCOMMAND #ifdef CONFIG_TFABOOT #undef QSPI_NOR_BOOTCOMMAND -#define QSPI_NOR_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; " \ +#define QSPI_NOR_BOOTCOMMAND "run distro_bootcmd; run qspi_bootcmd; " \ "env exists secureboot && esbc_halt;" #else #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_SD_BOOT_QSPI) -#define CONFIG_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; " \ +#define CONFIG_BOOTCOMMAND "run distro_bootcmd; run qspi_bootcmd; " \ "env exists secureboot && esbc_halt;" #endif #endif diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h index a908b0acb098..6f55acc7db11 100644 --- a/include/configs/ls1012a_common.h +++ b/include/configs/ls1012a_
RE: [v1 12/17] ddr: altera: Add SDRAM driver for Intel N5X device
> -Original Message- > From: Lim, Elly Siew Chin > Sent: Wednesday, March 31, 2021 10:39 PM > To: u-boot@lists.denx.de > Cc: Marek Vasut ; Tan, Ley Foon > ; See, Chin Liang ; > Simon Goldschmidt ; Chee, Tien Fong > ; Westergreen, Dalon > ; Simon Glass ; Gan, > Yau Wai ; Lim, Elly Siew Chin > > Subject: [v1 12/17] ddr: altera: Add SDRAM driver for Intel N5X device > > The DDR subsystem in Diamond Mesa is consisted of controller, PHY, > memory reset manager and memory clock manager. > > Configuration settings of controller, PHY and memory reset manager > is come from DDR handoff data in bitstream, which contain the register > base addresses and user settings from Quartus. > > Configuration settings of memory clock manager is come from the HPS > handoff data in bitstream, however the register base address is defined > in device tree. > > The calibration is fully done in HPS, which requires IMEM and DMEM > binaries loading to PHY SRAM for running this calibration, both > IMEM and DMEM binaries are also part of bitstream, this bitstream > would be loaded to OCRAM by SDM, and configured by DDR driver. > > Signed-off-by: Siew Chin Lim > Signed-off-by: Tien Fong Chee > --- > arch/arm/mach-socfpga/include/mach/firewall.h |6 + > .../include/mach/system_manager_soc64.h| 10 +- > drivers/ddr/altera/Makefile|3 +- > drivers/ddr/altera/sdram_n5x.c | 2316 > > drivers/ddr/altera/sdram_soc64.c | 10 +- > 5 files changed, 2342 insertions(+), 3 deletions(-) > create mode 100644 drivers/ddr/altera/sdram_n5x.c [...] > --- /dev/null > +++ b/drivers/ddr/altera/sdram_n5x.c > @@ -0,0 +1,2316 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020-2021 Intel Corporation > + * > + */ > + > +#include Sorting this. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "sdram_soc64.h" > +#include > + > +DECLARE_GLOBAL_DATA_PTR; > + > +/* MPFE NOC registers */ > +#define FPGA2SDRAM_MGR_MAIN_SIDEBANDMGR_FLAGOUTSET0 > 0xF8024050 > + > +/* Memory reset manager */ > +#define MEM_RST_MGR_STATUS 0x8 > + > +/* Register and bit in memory reset manager */ > +#define MEM_RST_MGR_STATUS_RESET_COMPLETEBIT(0) > +#define MEM_RST_MGR_STATUS_PWROKIN_STATUSBIT(1) > +#define MEM_RST_MGR_STATUS_CONTROLLER_RSTBIT(2) > +#define MEM_RST_MGR_STATUS_AXI_RST BIT(3) > + > +#define TIMEOUT_200MS 200 > +#define TIMEOUT_5000MS5000 > + > +/* DDR4 umctl2 */ > +#define DDR4_MSTR_OFFSET 0x0 > +#define DDR4_FREQ_RATIO BIT(22) > + > +#define DDR4_STAT_OFFSET 0x4 > +#define DDR4_STAT_SELFREF_TYPE (BIT(5) | BIT(4)) > +#define DDR4_STAT_SELFREF_TYPE_SHIFT 4 > +#define DDR4_STAT_OPERATING_MODE (BIT(2) | BIT(1) | BIT(0)) > + > +#define DDR4_MRCTRL0_OFFSET 0x10 > +#define DDR4_MRCTRL0_MR_TYPE BIT(0) > +#define DDR4_MRCTRL0_MPR_EN BIT(1) > +#define DDR4_MRCTRL0_MR_RANK (BIT(5) | BIT(4)) > +#define DDR4_MRCTRL0_MR_RANK_SHIFT 4 > +#define DDR4_MRCTRL0_MR_ADDR (BIT(15) | BIT(14) | BIT(13) | > BIT(12)) This is mask value? If yes, can use GENMASK() macro. Same for the defines below. > +#define DDR4_MRCTRL0_MR_ADDR_SHIFT 12 > +#define DDR4_MRCTRL0_MR_WR BIT(31) > + > +#define DDR4_MRCTRL1_OFFSET 0x14 > +#define DDR4_MRCTRL1_MR_DATA 0x3 > + > +#define DDR4_MRSTAT_OFFSET 0x18 > +#define DDR4_MRSTAT_MR_WR_BUSY BIT(0) > + > +#define DDR4_MRCTRL2_OFFSET 0x1C > + > +#define DDR4_PWRCTL_OFFSET 0x30 > +#define DDR4_PWRCTL_SELFREF_EN BIT(0) > +#define DDR4_PWRCTL_POWERDOWN_EN BIT(1) > +#define DDR4_PWRCTL_EN_DFI_DRAM_CLK_DISABLE BIT(3) > +#define DDR4_PWRCTL_SELFREF_SW BIT(5) > + > +#define DDR4_PWRTMG_OFFSET 0x34 > +#define DDR4_HWLPCTL_OFFSET 0x38 > +#define DDR4_RFSHCTL0_OFFSET 0x50 > +#define DDR4_RFSHCTL1_OFFSET 0x54 > + > +#define DDR4_RFSHCTL3_OFFSET 0x60 > +#define DDR4_RFSHCTL3_DIS_AUTO_REFRESH BIT(0) > +#define DDR4_RFSHCTL3_REFRESH_MODE (BIT(6) | BIT(5) | > BIT(4)) > +#define DDR4_RFSHCTL3_REFRESH_MODE_SHIFT 4 > + > +#define DDR4_ECCCFG0_OFFSET 0x70 > +#define DDR4_ECC_MODE(BIT(2) | BIT(1) | BIT(0)) > +#define DDR4_DIS_SCRUB BIT(4) > +#define LPDDR4_ECCCFG0_ECC_REGION_MAP_GRANU_SHIFT30 > +#define LPDDR4_ECCCFG0_ECC_REGION_MAP_SHIFT 8 > + > +#define DDR4_ECCCFG1_OFFSET 0x74 > +#define LPDDR4_ECCCFG1_ECC_REGIONS_PARITY_LOCK BIT(4) > + > +#define DDR4_CRCPARCTL0_OFFSET 0xC0 > +#define DDR4_CRCPARCT
Re: [RFC PATCH v5 2/2] board: sifive: unmatched: clear feature disable CSR
On Wed, Apr 14, 2021 at 11:22 AM Rick Chen wrote: > > Hi Green, > > > From: Green Wan [mailto:green@sifive.com] > > Sent: Tuesday, April 13, 2021 5:32 PM > > Cc: Green Wan; Sean Anderson; Bin Meng; Rick Jian-Zhi Chen(陳建志); Paul > > Walmsley; Pragnesh Patel; Bin Meng; Simon Glass; Atish Patra; Leo Yu-Chi > > Liang(梁育齊); Brad Kim; open list > > Subject: [RFC PATCH v5 2/2] board: sifive: unmatched: clear feature disable > > CSR > > > > Clear feature disable CSR to turn on all features of hart. The detail > > is specified at section, 'SiFive Feature Disable CSR', in user manual > > > > https://sifive.cdn.prismic.io/sifive/aee0dd4c-d156-496e-a6c4-db0cf54bbe68_sifive_U74MC_rtl_full_20G1.03.00_manual.pdf > > > > Signed-off-by: Green Wan > > Reviewed-by: Sean Anderson > > Reviewed-by: Bin Meng > > --- > > board/sifive/unmatched/spl.c | 15 +++ > > Is this CSR depends on unmatched board ? > I just wonder why not add in /arch/riscv/cpu/ and create fu74, just > like /arch/riscv/cpu/fu540/spl.c ? You're right. This CSR doesn't depend on the Unmatched. Moving to arch/riscv/cpu/fu740/spl.c makes more sense. will do it. > > Thanks, > Rick > > > 1 file changed, 15 insertions(+) > > > > diff --git a/board/sifive/unmatched/spl.c b/board/sifive/unmatched/spl.c > > index 5e1333b09a..ed48dac511 100644 > > --- a/board/sifive/unmatched/spl.c > > +++ b/board/sifive/unmatched/spl.c > > @@ -12,6 +12,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -22,6 +23,20 @@ > > #define MODE_SELECT_SD 0xb > > #define MODE_SELECT_MASK GENMASK(3, 0) > > > > +#define CSR_U74_FEATURE_DISABLE0x7c1 > > + > > +void harts_early_init(void) > > +{ > > + /* > > +* Feature Disable CSR > > +* > > +* Clear feature disable CSR to '0' to turn on all features for > > +* each core. This operation must be in M-mode. > > +*/ > > + if (CONFIG_IS_ENABLED(RISCV_MMODE)) > > + csr_write(CSR_U74_FEATURE_DISABLE, 0); > > +} > > + > > int spl_board_init_f(void) > > { > > int ret; > > -- > > 2.31.0
[PATCH v4 2/2] allow opting out of WATCHDOG_RESET() from timer interrupt
Having WATCHDOG_RESET() called automatically from the timer interrupt runs counter to the idea of a watchdog device - if the board runs into an infinite loops with interrupts still enabled, the watchdog will never fire. When using CONFIG_(SPL_)WDT, the watchdog_reset function is a lot more complicated than just poking a few SOC-specific registers - it involves accessing all kinds of global data, and if the interrupt happens at the wrong time (say, in the middle of an WATCHDOG_RESET() call from ordinary code), that can end up corrupting said global data. Allow the board to opt out of calling WATCHDOG_RESET() from the timer interrupt handler by setting CONFIG_SYS_WATCHDOG_FREQ to 0 - as that setting is currently nonsensical (it would be compile-time divide-by-zero), it cannot affect any existing boards. Add documentation for both the existing and extended meaning of CONFIG_SYS_WATCHDOG_FREQ. Signed-off-by: Rasmus Villemoes --- README| 9 + arch/m68k/lib/time.c | 2 +- arch/powerpc/lib/interrupts.c | 2 +- drivers/timer/mpc83xx_timer.c | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/README b/README index a565748e43..ad13092bbb 100644 --- a/README +++ b/README @@ -747,6 +747,15 @@ The following options need to be configured: SoC, then define this variable and provide board specific code for the "hw_watchdog_reset" function. + CONFIG_SYS_WATCHDOG_FREQ + Some platforms automatically call WATCHDOG_RESET() + from the timer interrupt handler every + CONFIG_SYS_WATCHDOG_FREQ interrupts. If not set by the + board configuration file, a default of CONFIG_SYS_HZ/2 + (i.e. 500) is used. Setting CONFIG_SYS_WATCHDOG_FREQ + to 0 disables calling WATCHDOG_RESET() from the timer + interrupt. + - Real-Time Clock: When CONFIG_CMD_DATE is selected, the type of the RTC diff --git a/arch/m68k/lib/time.c b/arch/m68k/lib/time.c index cbe29e72a8..ebb2ac54db 100644 --- a/arch/m68k/lib/time.c +++ b/arch/m68k/lib/time.c @@ -71,7 +71,7 @@ void dtimer_interrupt(void *not_used) timestamp++; #if defined(CONFIG_WATCHDOG) || defined (CONFIG_HW_WATCHDOG) - if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) { + if (CONFIG_SYS_WATCHDOG_FREQ && (timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) { WATCHDOG_RESET (); } #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */ diff --git a/arch/powerpc/lib/interrupts.c b/arch/powerpc/lib/interrupts.c index 73f270002c..5ba4cd0c13 100644 --- a/arch/powerpc/lib/interrupts.c +++ b/arch/powerpc/lib/interrupts.c @@ -80,7 +80,7 @@ void timer_interrupt(struct pt_regs *regs) timestamp++; #if defined(CONFIG_WATCHDOG) || defined (CONFIG_HW_WATCHDOG) - if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) + if (CONFIG_SYS_WATCHDOG_FREQ && (timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) WATCHDOG_RESET (); #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */ diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c index 9adb4b4cab..952293195f 100644 --- a/drivers/timer/mpc83xx_timer.c +++ b/drivers/timer/mpc83xx_timer.c @@ -175,7 +175,7 @@ void timer_interrupt(struct pt_regs *regs) priv->timestamp++; #if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG) - if ((priv->timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) + if (CONFIG_SYS_WATCHDOG_FREQ && (priv->timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) WATCHDOG_RESET(); #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */ -- 2.29.2
[PATCH v4 0/2] allow opting out of WATCHDOG_RESET() from timer interrupt
This is a resend of v3 from a year ago. Please consider applying. v4: rebase to current master. v3: add fixup patch for mpc83xx_timer, add documentation hunk to README, also update m68k instead of only ppc. v2: add documentation comment Rasmus Villemoes (2): timer: mpc83xx_timer: fix build with CONFIG_{HW_,}WATCHDOG allow opting out of WATCHDOG_RESET() from timer interrupt README| 9 + arch/m68k/lib/time.c | 2 +- arch/powerpc/lib/interrupts.c | 2 +- drivers/timer/mpc83xx_timer.c | 6 +- 4 files changed, 16 insertions(+), 3 deletions(-) -- 2.29.2
Re: [PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support
On Wed, Apr 14, 2021 at 03:29:23PM +0900, Masami Hiramatsu wrote: > Hi Takahiro, > > 2021年4月14日(水) 13:48 Takahiro Akashi : > > > > > > So why not define UEFI load options (BOOT) and use UEFI boot manager > > > > ("bootefi bootmgr")? > > > > That is the way how UEFI (at least boot manager) boots the kernel. > > > > > > Good point! Actually, I'm not sure how to define the BOOT in > > > config.h (I only > > > know how to include efivars file when build). Could you tell me how to do > > > it? > > > I would like to rewrite the default boot commands. > > > > For example, > > => efidebug boot add 1 USBBOOT usb 0:1 /EFI/BOOT/bootaa64.efi > > => efidebug boot add 2 MMCBOOT mmc 0:1 /EFI/BOOT/bootaa64.efi > > => efidebug boot order 1 2 > > => bootefi bootmgr > > Hmm, but this can not be embedded in the build process, can this? Probably there are a couple of ways; You may put them in "pre_boot_environment_command" if you don't mind :) But it would be best to run them as part of OS installation process. Or you may want to provide a default efivars file(?). > > > > Since "BOOTxxx" are non-volatile variables, we don't have to > > set them again once those commands are run. > > What is the default behavior of "bootefi bootmgr" if there is no > BOOT is set? Nothing will happen. > If it just do nothing and exit, I think I can add it to the top of > CONFIG_BOOTCOMMAND > so that U-Boot can try it first. > (BOOT will be set by user after boot) > > > But distro_bootcmd can also detect and try to boot "bootaa64.efi" anyway. > > (I'm not sure about the order of devices to detect though.) > > Hmm, interesting. OK, I'll try to enable distro_bootcmd. I'm pretty sure it will work. -Takahiro Akashi > Thank you, > > > > > -Takahiro Akashi > > > > > Thank you, > > > > > > -- > > > Masami Hiramatsu > > > > -- > Masami Hiramatsu
[PATCH] efi_loader: esrt: Remove incorrect invocations of EFI_CALL macro
Remove function invocations using the EFI_CALL macro for those functions that do not have an EFI_ENTRY call in their definition. Such functions can use u-boot api's which rely on u-boot global data(gd) pointer. The Arm and RiscV architectures maintain a separate gd pointer, one for u-boot, and a separate gd for the efi application. Calling a function through the EFI_CALL macro changes the gd pointer to that used for the efi application, with u-boot gd being unavailable. Any function then trying to dereference u-boot's gd will result in an abort. Fix this issue by removing the EFI_CALL macro for all of such functions which do not begin by an EFI_ENTRY function call. Signed-off-by: Sughosh Ganu --- I have squashed the earlier patch[1] into this one. This patch should supersede the earlier patch. [1] - https://patchwork.ozlabs.org/project/uboot/patch/20210410150948.24240-1-sughosh.g...@linaro.org/ lib/efi_loader/efi_esrt.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c index 40f53260e4..3ca55ce23a 100644 --- a/lib/efi_loader/efi_esrt.c +++ b/lib/efi_loader/efi_esrt.c @@ -139,7 +139,7 @@ efi_status_t efi_esrt_allocate_install(u32 num_entries) /* If there was a previous ESRT, deallocate its memory now. */ if (esrt) - ret = EFI_CALL(efi_free_pool(esrt)); + ret = efi_free_pool(esrt); esrt = new_esrt; @@ -253,8 +253,8 @@ efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp) return EFI_INVALID_PARAMETER; } - ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, -(void **)&img_info)); + ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, + (void **)&img_info); if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT failed to allocate memory for image info.\n"); return ret; @@ -298,7 +298,7 @@ efi_status_t efi_esrt_add_from_fmp(struct efi_firmware_management_protocol *fmp) } out: - EFI_CALL(efi_free_pool(img_info)); + efi_free_pool(img_info); return EFI_SUCCESS; } @@ -384,8 +384,8 @@ efi_status_t efi_esrt_populate(void) goto out; } - ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, -(void **)&img_info)); + ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size, + (void **)&img_info); if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT failed to allocate memory for image info\n"); goto out; @@ -405,13 +405,13 @@ efi_status_t efi_esrt_populate(void) if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT failed to obtain image info from FMP\n"); - EFI_CALL(efi_free_pool(img_info)); + efi_free_pool(img_info); goto out; } num_entries += desc_count; - EFI_CALL(efi_free_pool(img_info)); + efi_free_pool(img_info); } EFI_PRINT("ESRT create table with %u entries\n", num_entries); @@ -430,9 +430,9 @@ efi_status_t efi_esrt_populate(void) */ it_handle = base_handle; for (u32 idx = 0; idx < no_handles; idx++, it_handle++) { - ret = EFI_CALL(efi_search_protocol(*it_handle, - &efi_guid_firmware_management_protocol, - &handler)); + ret = efi_search_protocol(*it_handle, + &efi_guid_firmware_management_protocol, + &handler); if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT unable to find FMP handle (%u)\n", @@ -448,7 +448,7 @@ efi_status_t efi_esrt_populate(void) out: - EFI_CALL(efi_free_pool(base_handle)); + efi_free_pool(base_handle); return ret; } @@ -490,8 +490,8 @@ efi_status_t efi_esrt_register(void) return ret; } - ret = EFI_CALL(efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK, - efi_esrt_new_fmp_notify, NULL, NULL, &ev)); + ret = efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK, + efi_esrt_new_fmp_notify, NULL, NULL, &ev); if (ret != EFI_SUCCESS) { EFI_PRINT("ESRT failed to create event\n"); return ret; -- 2.17.1