Re: [RFC PATCH 0/1] Add boot hartid to a Device tree
On Thu, 5 Mar 2020 at 04:28, Schaefer, Daniel (DualStudy) wrote: > > Hi, > > I have started to implement the corresponding changes in EDK2: > https://github.com/changab/edk2-staging-riscv/compare/5f63e9249751ccb9302514455b9a1a7038f34547...RISC-V-DT-fixup > What happens is: The DTB is embedded in the FW image and passed to sbi_init > in SEC phase. We initialize OpenSBI as early as possible and because it also > wants to modify the device tree, I have to pass it the DTB as next_arg1. > Then it's passed to PEI in the firmware context and further to DXE via a HOB. > In DXE the boot-hartid is inserted and it's installed into the EFI system > config table from where the EFISTUB reads it. > > Unfortunately I don't get any console output after the EFISTUB calls > ExitBootServices, so my patch is still WIP. > Atish, which platform file in OpenSBI did you use to test your changes? > platform/qemu/virt/platform.c or platform/sifive/fu540/platform.c ? > Maybe the failure is unrelated to the new patches - we've never booted Linux > from EDKII yet. > > > I'm a bit skeptical whether DT is the best way to pass the boothartid to the > kernel. Ard has argued that it's good because it's independent from UEFI, but > the proper kernel doesn't read the hartid from there - it gets it from a0. > Only the EFISTUB reads the hartid from the device tree. Therefore the > solution we need is EFI specific anyways, right? > One of the design goals is that we don't want to force bootloaders, which > load the EFISTUB (e.g. UEFI Shell, grub chainloading, systemd-boot), to > change. > > If we let the firmware embed the hartid in the DT, the user cannot swap out > the DT later for their custom one. They need to use the one given by the > firmware. > Of course we could add commands and config to bootloaders to load and fixup > (embed hartid) the device tree... but, as mentioned earlier, we want to avoid > that. > Additionally from EDKII side we're also planning to run later stages, > including the bootloader, in S-Mode, where they wouldn't even have access to > mhartid and thus couldn't fixup the DT. > > If instead the firmware writes the hartid into the EFI system config table, > the EFISTUB can read it from there, just like it does the device tree > already. Then bootloaders can put a user supplied DT in the config table, > too, like they always have. > > What do you all think - does that make sense? > It doesn't really matter from the EFI stub pov if the information comes from the DT or from another config table. But the idea that RISC-V is 'special', and that you have to pass *two* pieces of information around instead of just one like on every other platform is something that you should really think about carefully - there may be other places where it will bite you down the road. Also, the idea that a DT is a file that you ship with the kernel is giving a *lot* of grief in the ARM world. The DT describes the hardware, not the software, and so it is the responsibility of the platform to provide it. If the platform gives you some way to drop in another version of the DT in one way or the other, that is absolutely fine, but in this case, the platform should know how to update /chosen/boot-hartid (which it already can, in any case). But don't make it part of the boot protocol, it really doesn't belong there.
Re: [PATCH] net: phy: Fix overlong PHY timeout
On Fri, Feb 21, 2020 at 5:33 PM Matthias Brugger wrote:
>
> Hi Joe,
>
> On 30/01/2020 12:00, Matthias Brugger wrote:
> >
> >
> > On 03/01/2020 23:08, Andre Przywara wrote:
> >> Commit 27c3f70f3b50 ("net: phy: Increase link up delay in
> >> genphy_update_link()") increased the per-iteration waiting time from
> >> 1ms to 50ms, without adjusting the timeout counter. This lead to the
> >> timeout increasing from the typical 4 seconds to over three minutes.
> >>
> >> Adjust the timeout counter evaluation by that factor of 50 to bring the
> >> timeout back to the intended value.
> >>
> >> Signed-off-by: Andre Przywara
A "Fixes:" tag would have been nice...
Anyway:
Tested-by: Simon Goldschmidt
> >
> > I tested this on RPi4 with the genet patches on top. Now the timeout is
> > reasonable :)
> >
> > Tested-by: Matthias Brugger
> >
>
> Friedly reminder, are you planning to take this fix for v2020.04?
Yes, please! This timeout is really annoying right now!
Regards,
Simon
>
> Regards,
> Matthias
>
> >> ---
> >> drivers/net/phy/phy.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> >> index 80a7664e49..5cf9c165b6 100644
> >> --- a/drivers/net/phy/phy.c
> >> +++ b/drivers/net/phy/phy.c
> >> @@ -244,7 +244,7 @@ int genphy_update_link(struct phy_device *phydev)
> >> /*
> >> * Timeout reached ?
> >> */
> >> -if (i > PHY_ANEG_TIMEOUT) {
> >> +if (i > (PHY_ANEG_TIMEOUT / 50)) {
> >> printf(" TIMEOUT !\n");
> >> phydev->link = 0;
> >> return -ETIMEDOUT;
> >>
>
Re: [PATCH] net: phy: fix autoneg timeout
On Thu, Mar 5, 2020 at 5:40 AM Stefan Roese wrote:
>
> Hi Simon,
>
> On 04.03.20 21:12, Simon Goldschmidt wrote:
> > Recently, genphy_update_link() has been changed to use a 50ms polling
> > interval instead of the previous 1ms. However, the timeout to give up
> > waiting for a link remained unchanged, calculating the iterations.
> >
> > As a result, PHY_ANEG_TIMEOUT now specifies "multiples of 50ms" instead
> > of just to be a number of milliseconds.
> >
> > Fix this by dividing PHY_ANEG_TIMEOUT by 50 in this loop. This gets us
> > back to a 4 seconds timeout for the default value (instead of 200s).
> >
> > Fixes: net: phy: Increase link up delay in genphy_update_link()
> > ("27c3f70f3b50")
> > Signed-off-by: Simon Goldschmidt
> > ---
> >
> > This should be applied before the next release as it fixes a regression
> > of v2020.01!
> >
> > drivers/net/phy/phy.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 80a7664e49..5cf9c165b6 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -244,7 +244,7 @@ int genphy_update_link(struct phy_device *phydev)
> > /*
> >* Timeout reached ?
> >*/
> > - if (i > PHY_ANEG_TIMEOUT) {
> > + if (i > (PHY_ANEG_TIMEOUT / 50)) {
> > printf(" TIMEOUT !\n");
> > phydev->link = 0;
> > return -ETIMEDOUT;
> >
>
> A similar fix from Andre for this is queued for this for quite some
> time now:
>
> https://patchwork.ozlabs.org/patch/1217524/
Right. I thought I remembered a patch but hadn't found it...
I'll drop this one then.
Regards,
Simon
>
> Joe or Tom, could you please take this one?
>
> Thanks,
> Stefan
[PATCH 3/4] cmd: mem: Use IS_ENABLED instead of alt_test variable
This patch uses the IS_ENABLED() macro to check, which mtest variant
is enabled.
Signed-off-by: Stefan Roese
---
cmd/mem.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/cmd/mem.c b/cmd/mem.c
index f519adaee2..2ccc7032ad 100644
--- a/cmd/mem.c
+++ b/cmd/mem.c
@@ -875,11 +875,6 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
ulong errs = 0; /* number of errors, or -1 if interrupted */
ulong pattern = 0;
int iteration;
-#if defined(CONFIG_SYS_ALT_MEMTEST)
- const int alt_test = 1;
-#else
- const int alt_test = 0;
-#endif
start = CONFIG_SYS_MEMTEST_START;
end = CONFIG_SYS_MEMTEST_END;
@@ -921,7 +916,7 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
printf("Iteration: %6d\r", iteration + 1);
debug("\n");
- if (alt_test) {
+ if (IS_ENABLED(CONFIG_SYS_ALT_MEMTEST)) {
errs = mem_test_alt(buf, start, end, dummy);
} else {
errs = mem_test_quick(buf, start, end, pattern,
--
2.25.1
[PATCH 1/4] cmd: mem: Correctly count the errors in mtest
This patch changes mtest to correctly count the overall errors and
print them even in the abort (Ctrl-C) case.
Signed-off-by: Stefan Roese
---
cmd/mem.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/cmd/mem.c b/cmd/mem.c
index 6d54f19527..9367278aa8 100644
--- a/cmd/mem.c
+++ b/cmd/mem.c
@@ -871,7 +871,7 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
ulong start, end;
vu_long *buf, *dummy;
ulong iteration_limit = 0;
- int ret;
+ ulong count = 0;
ulong errs = 0; /* number of errors, or -1 if interrupted */
ulong pattern = 0;
int iteration;
@@ -929,6 +929,7 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
}
if (errs == -1UL)
break;
+ count += errs;
}
/*
@@ -947,14 +948,10 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
if (errs == -1UL) {
/* Memory test was aborted - write a newline to finish off */
putc('\n');
- ret = 1;
- } else {
- printf("Tested %d iteration(s) with %lu errors.\n",
- iteration, errs);
- ret = errs != 0;
}
+ printf("Tested %d iteration(s) with %lu errors.\n", iteration, count);
- return ret;
+ return errs != 0;
}
#endif /* CONFIG_CMD_MEMTEST */
--
2.25.1
[PATCH 4/4] cmd: mem: Add bitflip memory test to alternate mtest
This additional bitflip memory test is inspired by the bitflip test
in memtester v4.3.0. It show some errors on some problematic GARDENA
MT7688 based boards. The other memory tests usually don't show any
errors here.
Signed-off-by: Stefan Roese
---
cmd/mem.c | 60 +++
1 file changed, 60 insertions(+)
diff --git a/cmd/mem.c b/cmd/mem.c
index 2ccc7032ad..0bfb6081e7 100644
--- a/cmd/mem.c
+++ b/cmd/mem.c
@@ -801,6 +801,59 @@ static ulong mem_test_alt(vu_long *buf, ulong start_addr,
ulong end_addr,
return errs;
}
+static int compare_regions(volatile unsigned long *bufa,
+ volatile unsigned long *bufb, size_t count)
+{
+ volatile unsigned long *p1 = bufa;
+ volatile unsigned long *p2 = bufb;
+ int errs = 0;
+ size_t i;
+
+ for (i = 0; i < count; i++, p1++, p2++) {
+ if (*p1 != *p2) {
+ printf("FAILURE: 0x%08lx != 0x%08lx (delta=0x%08lx ->
bit %ld) at offset 0x%08lx\n",
+ (unsigned long)*p1, (unsigned long)*p2,
+ *p1 ^ *p2, __ffs(*p1 ^ *p2),
+ (unsigned long)(i * sizeof(unsigned long)));
+ errs++;
+ }
+ }
+
+ return errs;
+}
+
+static ulong test_bitflip_comparison(volatile unsigned long *bufa,
+volatile unsigned long *bufb, size_t count)
+{
+ volatile unsigned long *p1 = bufa;
+ volatile unsigned long *p2 = bufb;
+ unsigned int j, k;
+ unsigned long q;
+ size_t i;
+ int max;
+ int errs = 0;
+
+ max = sizeof(unsigned long) * 8;
+ for (k = 0; k < max; k++) {
+ q = 0x0001L << k;
+ for (j = 0; j < 8; j++) {
+ WATCHDOG_RESET();
+ q = ~q;
+ p1 = (volatile unsigned long *)bufa;
+ p2 = (volatile unsigned long *)bufb;
+ for (i = 0; i < count; i++)
+ *p1++ = *p2++ = (i % 2) == 0 ? q : ~q;
+
+ errs += compare_regions(bufa, bufb, count);
+ }
+
+ if (ctrlc())
+ return -1UL;
+ }
+
+ return errs;
+}
+
static ulong mem_test_quick(vu_long *buf, ulong start_addr, ulong end_addr,
vu_long pattern, int iteration)
{
@@ -918,6 +971,13 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
debug("\n");
if (IS_ENABLED(CONFIG_SYS_ALT_MEMTEST)) {
errs = mem_test_alt(buf, start, end, dummy);
+ if (errs == -1UL)
+ break;
+ count += errs;
+ errs = test_bitflip_comparison(buf,
+ buf + (end - start) / 2,
+ (end - start) /
+ sizeof(unsigned long));
} else {
errs = mem_test_quick(buf, start, end, pattern,
iteration);
--
2.25.1
[PATCH 2/4] cmd: mem: Drop eldk-4.2 workaround and use cast in unmap_sysmem()
Use a cast instead of the "eldk-4.2" workaround for unmap_sysmem().
Signed-off-by: Stefan Roese
---
cmd/mem.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/cmd/mem.c b/cmd/mem.c
index 9367278aa8..f519adaee2 100644
--- a/cmd/mem.c
+++ b/cmd/mem.c
@@ -932,18 +932,8 @@ static int do_mem_mtest(cmd_tbl_t *cmdtp, int flag, int
argc,
count += errs;
}
- /*
-* Work-around for eldk-4.2 which gives this warning if we try to
-* case in the unmap_sysmem() call:
-* warning: initialization discards qualifiers from pointer target type
-*/
- {
- void *vbuf = (void *)buf;
- void *vdummy = (void *)dummy;
-
- unmap_sysmem(vbuf);
- unmap_sysmem(vdummy);
- }
+ unmap_sysmem((void *)buf);
+ unmap_sysmem((void *)dummy);
if (errs == -1UL) {
/* Memory test was aborted - write a newline to finish off */
--
2.25.1
Re: [PATCH] net: phy: fix autoneg timeout
Hi Simon,
On 04.03.20 21:12, Simon Goldschmidt wrote:
Recently, genphy_update_link() has been changed to use a 50ms polling
interval instead of the previous 1ms. However, the timeout to give up
waiting for a link remained unchanged, calculating the iterations.
As a result, PHY_ANEG_TIMEOUT now specifies "multiples of 50ms" instead
of just to be a number of milliseconds.
Fix this by dividing PHY_ANEG_TIMEOUT by 50 in this loop. This gets us
back to a 4 seconds timeout for the default value (instead of 200s).
Fixes: net: phy: Increase link up delay in genphy_update_link() ("27c3f70f3b50")
Signed-off-by: Simon Goldschmidt
---
This should be applied before the next release as it fixes a regression
of v2020.01!
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 80a7664e49..5cf9c165b6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -244,7 +244,7 @@ int genphy_update_link(struct phy_device *phydev)
/*
* Timeout reached ?
*/
- if (i > PHY_ANEG_TIMEOUT) {
+ if (i > (PHY_ANEG_TIMEOUT / 50)) {
printf(" TIMEOUT !\n");
phydev->link = 0;
return -ETIMEDOUT;
A similar fix from Andre for this is queued for this for quite some
time now:
https://patchwork.ozlabs.org/patch/1217524/
Joe or Tom, could you please take this one?
Thanks,
Stefan
Re: [PULL] u-boot-sh/master
On Thu, Mar 05, 2020 at 01:19:13AM +0100, Marek Vasut wrote: > The following changes since commit 9e1d65f36b83c5422ece3c0ea28d07a2246cb07f: > > configs: Resync with savedefconfig (2020-02-28 13:28:38 -0500) > > are available in the Git repository at: > > git://git.denx.de/u-boot-sh.git master > > for you to fetch changes up to 7ca08e335b1274c5eb88c58aacaa389c63907d40: > > ARM: rmobile: Limit bootloader size to 1 MiB on R-Car Gen3 (2020-03-01 > 21:58:32 +0100) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 33/33] riscv: Add Sipeed Maix support
Hi Rick, On Wed, Mar 4, 2020 at 11:11 PM Sean Anderson wrote: > > On 3/4/20 2:47 AM, Rick Chen wrote: > > Hi Sean > > > > This patch series become larger and larger from v1 with 11 patches to > > v5 with 33 patches. > > I didn't intend for it to balloon so large. My original goal for this > revision was just to get the SPI and MMC slots working. However, I > discovered that I would need to implement a pinmux slot and GPIOs for > the MMC slot to work (since I think that is the primary motivating > factor for using U-Boot on this board, as opposed to the native > bootloader). > > > You shall just fix the suggestions from the previous version in the > > next version. > > I will try to do that :) > > > Additional extra features and subsystem drivers that you want to > > support, you shall send them individually instead of mixing them > > together. > > In the past, I have been told to keep this sort of thing in one series > (e.g. by Bin Meng). I'm really not sure what things I should split off > and which I should keep together. > If separate patches are sent via different tree, we only get a complete new RISC-V board support after all trees are merged. Also there might be inter-dependencies between patches. My practice is to wait for sub-domain maintainers to give Ack or RB tags, then pull all series via one tree. > > > > If you can separate them into different series(spi, gpio,led , > > pinctrl, watchdog, those drivers were not present in v1) and they will > > not block the reviewing schedule each other. > > Hm, ok. Perhaps split those off into series which depend on this one? > > > Narrow down the dependency, it can help to speed up the patch work. > > And I am happy to help pulling the RISC-V relative patchs via riscv > > tree ASAP. > > Ok, I will try and get another version out which fixes the feedback I > have gotten on those patches. Regards, Bin
bitbake u-boot-imx: undefined reference to `eth_setenv_enetaddr'
Hi, I am building uboot through yocto and getting the below errors. | /nobackup/sikumar3/inode_sumo/build/tmp/work/imx6uloib-poky-linux-gnueabi/u-boot-imx/2018.03-r0/git/board/cisco/mx6ul_oib/mx6ul_oib.c:403: undefined reference to `eth_setenv_enetaddr' I see that my .config CONFIG_CMD_NET is enabled. But, I am not sure why I am getting this error. /nobackup/sikumar3/inode_sumo/build/tmp/work/imx6uloib-poky-linux-gnueabi/u-boot-imx/2018.03-r0$ find . -iname .config ./build/mx6ul_oib_config/.config /nobackup/sikumar3/inode_sumo/build/tmp/work/imx6uloib-poky-linux-gnueabi/u-boot-imx/2018.03-r0$ grep -rn CONFIG_CMD_NET ./build/mx6ul_oib_config/.config 399:CONFIG_CMD_NET=y /nobackup/sikumar3/inode_sumo/build/tmp/work/imx6uloib-poky-linux-gnueabi/u-boot-imx/2018.03-r0$ Regards, Siva
Re: [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI
Hi Sean > Hi Sean > > > On Mon, 2020-03-02 at 10:43 -0500, Sean Anderson wrote: > > > > > On 3/2/20 4:08 AM, Rick Chen wrote: > > > > Hi Sean > > > > > > > > > The IPI code could have race conditions in several places. > > > > > * Several harts could race on the value of gd->arch->clint/plic > > > > > * Non-boot harts could race with the main hart on the DM subsystem In > > > > > addition, if an IPI was pending when U-Boot started, it would cause > > > > > the > > > > > IPI handler to jump to address 0. > > > > > > > > > > To address these problems, a new function riscv_init_ipi is > > > > > introduced. It > > > > > is called once during arch_cpu_init_dm. Before this point, no > > > > > riscv_*_ipi > > > > > functions may be called. Access is synchronized by > > > > > gd->arch->ipi_ready. > > > > > > > > > > Signed-off-by: Sean Anderson > > > > > --- > > > > > > > > > > Changes in v5: > > > > > - New > > > > > > > > > > arch/riscv/cpu/cpu.c | 9 > > > > > arch/riscv/include/asm/global_data.h | 1 + > > > > > arch/riscv/include/asm/smp.h | 43 ++ > > > > > arch/riscv/lib/andes_plic.c | 34 +- > > > > > arch/riscv/lib/sbi_ipi.c | 5 ++ > > > > > arch/riscv/lib/sifive_clint.c| 33 +- > > > > > arch/riscv/lib/smp.c | 68 > > > > > > > > > > 7 files changed, 101 insertions(+), 92 deletions(-) > > > > > > > > > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > > > > > index e457f6acbf..a971ec8694 100644 > > > > > --- a/arch/riscv/cpu/cpu.c > > > > > +++ b/arch/riscv/cpu/cpu.c > > > > > @@ -96,6 +96,15 @@ int arch_cpu_init_dm(void) > > > > > csr_write(CSR_SATP, 0); > > > > > } > > > > > > > > > > +#ifdef CONFIG_SMP > > > > > + ret = riscv_init_ipi(); > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + /* Atomically set a flag enabling IPI handling */ > > > > > + WRITE_ONCE(gd->arch.ipi_ready, 1); > > > > > > > > I think it shall not have race condition here. > > > > Can you explain more detail why there will occur race condition ? > > > > > > > > Hi Lukas > > > > > > > > Do you have any comments ? > > > > > > > > Thanks > > > > Rick > > > > > > On the K210, there may already be an IPI pending when U-Boot starts. > > > (Perhaps the prior stage sends an IPI but does not clear it). As soon as > > > interrupts are enabled, the hart then tries to call riscv_clear_ipi(). > > > Because the clint/plic has not yet been enabled, the clear_ipi function > > > will try and bind/probe the device. This can have really nasty effects, > > > since > > > the boot hart is *also* trying to bind/probe devices. > > > > > > In addition, a hart could end up trying to probe the clint/plic because > > > it could receive the IPI before (from its perspective) gd->arch.clint > > > (or plic) gets initialized. > > > > > > > We did not have a problem with pending IPIs on other platforms. It > > should suffice to clear SSIP / MSIP before enabling the interrupts. > > > > Can you try to clear mip/sip in startup flow before secondary_hart_loop: > Maybe it can help to overcome the problem of calling riscv_clear_ipi() > before riscv_init_ipi() that you added. How about the verified result ? Is this can help to fix your problem without any modification (the original flow) Avoid unexpected condition can increase the robustness indeed. But clarify the root cause more precisely is still necessary here. Thanks, Rick > > Thanks, > Rick > > > > Aside from the above, I think the macro approach is a bit confusing, > > > since it's unclear at first glance what function will be initializing > > > the clint/plic. Given U-Boot's otherwise completely SMP-unsafe design, I > > > think it's better to be explicit and conservative in these areas. > > > > > > > I agree, the patch makes this more clear and helps make the code more > > robust. > > > > Thanks, > > Lukas
RE: [U-Boot] [PATCH V3 15/27] imx8m: Fix MMU table issue for OPTEE memory
> Subject: Re: [U-Boot] [PATCH V3 15/27] imx8m: Fix MMU table issue for
> OPTEE memory
>
> Hi Peng,
>
> On Tue, Aug 27, 2019 at 9:38 AM Peng Fan wrote:
> >
> > When running with OPTEE, the MMU table in u-boot does not remove the
> > OPTEE memory from its settings. So ARM speculative prefetch in u-boot
> > may access that OPTEE memory. Due to trust zone is enabled by OPTEE
> > and that memory is set to secure access, then the speculative prefetch
> > will fail and cause various memory issue in u-boot.
> > The fail address register and int_status register in trustzone has
> > logged that speculative access from u-boot.
> >
> > Signed-off-by: Peng Fan
> > ---
> > arch/arm/mach-imx/imx8m/soc.c | 20 ++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c
> > b/arch/arm/mach-imx/imx8m/soc.c index 5115471eff..dd393b581b
> 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -112,16 +112,18 @@ static struct mm_region imx8m_mem_map[] = {
> > /* DRAM1 */
> > .virt = 0x4000UL,
> > .phys = 0x4000UL,
> > - .size = 0xC000UL,
> > + .size = PHYS_SDRAM_SIZE,
> > .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > PTE_BLOCK_OUTER_SHARE
> > +#ifdef PHYS_SDRAM_2_SIZE
> > }, {
> > /* DRAM2 */
> > .virt = 0x1UL,
> > .phys = 0x1UL,
> > - .size = 0x04000UL,
> > + .size = PHYS_SDRAM_2_SIZE,
> > .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > PTE_BLOCK_OUTER_SHARE
> > +#endif
> > }, {
> > /* List terminator */
> > 0,
> > @@ -130,6 +132,20 @@ static struct mm_region imx8m_mem_map[] = {
> >
> > struct mm_region *mem_map = imx8m_mem_map;
> >
> > +void enable_caches(void)
> > +{
> > + /*
> > +* If OPTEE runs, remove OPTEE memory from MMU table to
> > +* avoid speculative prefetch. OPTEE runs at the top of
> > +* the first memory bank
> > +*/
> > + if (rom_pointer[1])
> > + imx8m_mem_map[5].size -= rom_pointer[1];
> > +
> > + icache_enable();
> > + dcache_enable();
> > +}
> > +
> > static u32 get_cpu_variant_type(u32 type) {
> > struct ocotp_regs *ocotp = (struct ocotp_regs
> > *)OCOTP_BASE_ADDR;
> > --
> > 2.16.4
> >
> > ___
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> >
> s.denx.de%2Flistinfo%2Fu-bootdata=02%7C01%7Cpeng.fan%40nxp.co
> m%7C
> >
> abfc9387a622474e599508d7c049395c%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C
> >
> 0%7C0%7C637189293044493841sdata=fMrSP%2BMAzI20VMGhXm0U
> 6kejjaClNOo
> > bFjB4HVkTt%2Fs%3Dreserved=0
>
> Unfortunately, I'm still facing a similar issue when OP-TEE is loaded on
> iMX8MM-based module, despite BL32_BASE(0xbe00) +
> BL32_SIZE(0x200) region is excluded.
> Based on debug ouput the latest one is:
>
> idx=2 PTE bfff7010 at level 1: bfffd003 Checking if pte fits for
> virt=bde0 size=20 blocksize=4000
> addr=bde0 level=2
> idx=2 PTE bfff7010 at level 1: bfffd003 idx=1ef PTE
> bfffdf78 at level 2: 0 Checking if pte fits for virt=bde0
> size=20 blocksize=20 Setting PTE bfffdf78 to block
> virt=bde0
Wired. If this is true, this is bug in u-boot MMU part.
Regards,
Peng.
>
>
> U-Boot hangs just after writing to System Control Register in
> arch/arm/cpu/armv8/cache_v8.c:419:
>
> /* enable the mmu */
> set_sctlr(get_sctlr() | CR_M);
>
> In my setup mainline U-Boot/TF-A/OP-TEE are used:
> u-boot: 133276f14a ("Merge branch '2020-02-25-master-imports'")
> tf-a: 6e46981f84 ("Merge "Update pathnames in maintainers.rst file"
> into integration")
> op-tee: a67dc424ff ("ta: pkcs11: API for slot/token information")
>
> OP-TEE is compiled with these flags:
>CFG_ARM64_core=y
>CFG_NXP_CAAM=n
>CFG_NXPCRYPT=n
>PLATFORM=imx-mx8mmevk
>CFG_TEE_CORE_LOG_LEVEL=4
>DEBUG=y
>CFG_TEE_CORE_DEBUG=y
>
> With DCACHE disabled obviously everything works. Also I don't face any
> issues if TF-A is compiled without SPD=opteed.
>
> Maybe it rings the bell and you already saw similar issues before?
>
> Thanks
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opan...@gmail.com
> skype: igor.opanyuk
> +380 (93) 836 40 67
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fua.linke
> din.com%2Fin%2Fiopaniukdata=02%7C01%7Cpeng.fan%40nxp.com%7
> Cabfc9387a622474e599508d7c049395c%7C686ea1d3bc2b4c6fa92cd99c5c3
> 01635%7C0%7C0%7C637189293044493841sdata=CBnHEQfZu%2FtQv
> fbL%2FGn2H%2FBilRgKeQe5CHaUt0BjKP4%3Dreserved=0
Re: [PATCH] Makefile: doesn't need check stack size when dtb is not built
On Wed, Mar 04, 2020 at 05:22:25PM -0700, Stephen Warren wrote:
> On 3/4/20 5:15 PM, AKASHI Takahiro wrote:
> > On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
> > > On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
> > > > The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into
> > > > initial stack") adds an extra check for stack size in BSS if
> > > > CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled.
> > > > This check, however, doesn't make sense under the configuration where
> > > > control dtb won't be built in and it should be void in such cases.
> > >
> > > Don't you want to edit the following hunk from the original patch instead
> > > or
> > > as well?
> > >
> > > +ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
> > > +ALL-y += init_sp_bss_offset_check
> > > +endif
> > >
> > > That's why there's no rule for init_sp_bss_offset_check in the else case.
> >
> > I intentionally left it as it is because someone may in the future
> > want to add other *sanity checks* whether dtb is used or not.
>
> I'd probably expect the following in that case:
>
> ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
> ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)
> ALL-y += init_sp_bss_offset_check
> endif
> ALL-y += some_other_check
> endif
>
> > Rather, my concern is:
> > Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)"
> > sufficient and appropriate to guard the check?
>
> I think OF_SEPARATE is the only case this check makes sense. OF_EMBED sounds
> like the build process puts the DTB into .data or similar, so the issue
> doesn't apply. Since OF_HOSTFILE is a sandbox thing where the DT is read
> from a file, the check definitely doesn't apply.
Okay, sounds reasonable.
(and the target should be dts/dt.dtb rather than u-boot.dtb for clarity?)
Thanks,
-Takahiro Akashi
Re: [PATCH] Makefile: doesn't need check stack size when dtb is not built
On 3/4/20 5:15 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into
initial stack") adds an extra check for stack size in BSS if
CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled.
This check, however, doesn't make sense under the configuration where
control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or
as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
+ALL-y += init_sp_bss_offset_check
+endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future
want to add other *sanity checks* whether dtb is used or not.
I'd probably expect the following in that case:
ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)
ALL-y += init_sp_bss_offset_check
endif
ALL-y += some_other_check
endif
Rather, my concern is:
Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)"
sufficient and appropriate to guard the check?
I think OF_SEPARATE is the only case this check makes sense. OF_EMBED
sounds like the build process puts the DTB into .data or similar, so the
issue doesn't apply. Since OF_HOSTFILE is a sandbox thing where the DT
is read from a file, the check definitely doesn't apply.
[PULL] u-boot-sh/master
The following changes since commit 9e1d65f36b83c5422ece3c0ea28d07a2246cb07f: configs: Resync with savedefconfig (2020-02-28 13:28:38 -0500) are available in the Git repository at: git://git.denx.de/u-boot-sh.git master for you to fetch changes up to 7ca08e335b1274c5eb88c58aacaa389c63907d40: ARM: rmobile: Limit bootloader size to 1 MiB on R-Car Gen3 (2020-03-01 21:58:32 +0100) Marek Vasut (1): ARM: rmobile: Limit bootloader size to 1 MiB on R-Car Gen3 include/configs/rcar-gen3-common.h | 5 - 1 file changed, 4 insertions(+), 1 deletion(-)
Re: [PATCH] Makefile: doesn't need check stack size when dtb is not built
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
> On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
> > The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into
> > initial stack") adds an extra check for stack size in BSS if
> > CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled.
> > This check, however, doesn't make sense under the configuration where
> > control dtb won't be built in and it should be void in such cases.
>
> Don't you want to edit the following hunk from the original patch instead or
> as well?
>
> +ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
> +ALL-y += init_sp_bss_offset_check
> +endif
>
> That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future
want to add other *sanity checks* whether dtb is used or not.
Rather, my concern is:
Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)"
sufficient and appropriate to guard the check?
Thanks,
-Takahiro Akashi
Re: [PULL] u-boot-usb/master
On Wed, Mar 04, 2020 at 01:22:47PM +0100, Marek Vasut wrote: > The following changes since commit 9e1d65f36b83c5422ece3c0ea28d07a2246cb07f: > > configs: Resync with savedefconfig (2020-02-28 13:28:38 -0500) > > are available in the Git repository at: > > git://git.denx.de/u-boot-usb.git master > > for you to fetch changes up to 491cabf067c6a8bb4a5859ab9f7e0ddc5b5347c8: > > gadget: f_thor: add missing line breaks for pr_err() (2020-03-01 > 21:58:54 +0100) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PULL] u-boot-socfpga/master
On Wed, Mar 04, 2020 at 01:23:09PM +0100, Marek Vasut wrote: > The following changes since commit 9e1d65f36b83c5422ece3c0ea28d07a2246cb07f: > > configs: Resync with savedefconfig (2020-02-28 13:28:38 -0500) > > are available in the Git repository at: > > git://git.denx.de/u-boot-socfpga.git master > > for you to fetch changes up to 468ba8d00b5af7828302e297736ae23d4873cfb0: > > ARM: socfpga: Add initial support for the ABB SECU board (2020-03-03 > 22:11:36 +0100) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: caching BLOBLISTT_SPL_HANDOFF
Hi Rasmus, On Mon, 2 Mar 2020 at 13:01, Rasmus Villemoes wrote: > > On 02/03/2020 20.47, Simon Glass wrote: > > Hi Rasmus, > > > > On Fri, 28 Feb 2020 at 16:09, Rasmus Villemoes > > wrote: > >> > > >> Now that particular one seems a bit fishy: Why is it ok to cache the > >> location of the BLOBLISTT_SPL_HANDOFF blob in gd->spl_handoff? Later in > >> the init sequence there's a call to reserve_bloblist, and later again > >> reloc_bloblist. Doesn't that leave gd->spl_handoff stale? > > > > Yes it does. It is only supposed to be used in the early stages of > > U-Boot (proper) init. > > Yes, that's what I thought - and if it's only actually used once or > twice during the early stages, there's not much point in caching it. > > > Actually I think that member could be dropped and we could search for > > it each time: > > > > ./arch/x86/cpu/broadwell/cpu_from_spl.c > > Yes, there didn't seem to be many users, so it should not be that hard > to get rid of. I also think that sets a better precedent for future > bloblist users. Sounds good, thanks. I wonder if one day we will want to support multiple bloblists in different memory locations. Regards, Simon
Re: [RFC] dm: uclass: add functions to get device by platdata
Hi Walter,
On Wed, 4 Mar 2020 at 12:40, Walter Lozano wrote:
>
> When OF_PLATDATA is enabled DT information is parsed and platdata
> structures are populated. In this context the links between DT nodes are
> represented as pointers to platdata structures, and there is no clear way
> to access to the device which owns the structure.
>
> This patch implements a set of functions:
>
> - device_find_by_platdata
> - uclass_find_device_by_platdata
>
> to access to the device.
>
> Signed-off-by: Walter Lozano
> ---
> drivers/core/device.c| 19 +++
> drivers/core/uclass.c| 34 ++
> include/dm/device.h | 2 ++
> include/dm/uclass-internal.h | 3 +++
> include/dm/uclass.h | 2 ++
> 5 files changed, 60 insertions(+)
This is interesting. Could you also add the motivation for this? It's
not clear to me who would call this function.
Also it relates to another thing I've been thinking about for a while,
which is to validate that all the structs pointed to are correct.
E.g. if every struct had a magic number like:
struct tpm_platdata {
DM_STRUCT(UCLASS_TPM, DM_STRUCT_PLATDATA, ...)
fields here
};
then we could check the structure pointers are correct.
DM_STRUCT() would define to nothing if we were not building with
CONFIG_DM_DEBUG or similar.
Anyway, I wonder whether you could expand your definition a bit so you
have an enum for the different types of struct you can request:
enum dm_struct_t {
DM_STRUCT_PLATDATA,
...
DM_STRUCT_COUNT,
};
and modify the function so it can request it via the enum?
Regards,
Simon
Re: [PATCH 1/8] malloc: implement USE_DL_PREFIX via inline functions
Hi Simon,
On Wed, 4 Mar 2020 at 13:45, Simon Goldschmidt
wrote:
>
> Am 20.02.2020 um 04:05 schrieb Simon Glass:
> > On Wed, 19 Feb 2020 at 12:39, Simon Goldschmidt
> > wrote:
> >>
> >> Commit cfda60f99ae2 ("sandbox: Use a prefix for all allocation functions")
> >> introduced preprocessor macros for malloc/free etc.
> >>
> >> This is bad practice as it essentially makes 'free' a reserved keyword and
> >> resulted in quite a bit of renaming to avoid that reserved keyword.
> >>
> >> A better solution is to define the allocation functions as 'static inline'.
> >>
> >> As a side effect, exports.h must not export malloc/free for sandbox.
> >>
> >> Signed-off-by: Simon Goldschmidt
> >> ---
> >>
> >> A side-effect is that exports.h may not declare malloc/free. I'm not really
> >> sure if this is correct, but for sandbox, it should probably be ok?
> >
> > Is it possible to fix this? E.g. don't use inline for these two
> > functions on sandbox?
>
> Not using inline for sandbox for these two is *not* an option as these
> two are exactly the ones offending globally known names.
>
> I guess we have to know what we want here: what is exports.h meant for?
> To me it looks like it is meant for "U-Boot API" applications to link
> against. If so, why is it included in U-Boot sources (in over 20 files)?
>
> I guess one solution would be to move (or copy) the DL prefix handling
> into exports.h and _exports.h so that applications linking with
> exports.h implicily call dlmalloc/dlfree, not malloc/free (which would
> be the OS versions for sandbox).
>
> I'll try to prepare v2 in that direction, but I'm still not unsure since
> exports.h is included in internal U-Boot code.
OK sounds good. I do wonder whether we should drop the exports.h
inclusions as part of this? It does seem odd. OR perhaps they are
there just to make sure that the functions match the API?
Regards,
Simon
Re: [PATCH 1/8] malloc: implement USE_DL_PREFIX via inline functions
On Wed, Mar 04, 2020 at 09:44:27PM +0100, Simon Goldschmidt wrote:
> Am 20.02.2020 um 04:05 schrieb Simon Glass:
> > On Wed, 19 Feb 2020 at 12:39, Simon Goldschmidt
> > wrote:
> >>
> >> Commit cfda60f99ae2 ("sandbox: Use a prefix for all allocation functions")
> >> introduced preprocessor macros for malloc/free etc.
> >>
> >> This is bad practice as it essentially makes 'free' a reserved keyword and
> >> resulted in quite a bit of renaming to avoid that reserved keyword.
> >>
> >> A better solution is to define the allocation functions as 'static inline'.
> >>
> >> As a side effect, exports.h must not export malloc/free for sandbox.
> >>
> >> Signed-off-by: Simon Goldschmidt
> >> ---
> >>
> >> A side-effect is that exports.h may not declare malloc/free. I'm not really
> >> sure if this is correct, but for sandbox, it should probably be ok?
> >
> > Is it possible to fix this? E.g. don't use inline for these two
> > functions on sandbox?
>
> Not using inline for sandbox for these two is *not* an option as these
> two are exactly the ones offending globally known names.
>
> I guess we have to know what we want here: what is exports.h meant for?
> To me it looks like it is meant for "U-Boot API" applications to link
> against. If so, why is it included in U-Boot sources (in over 20 files)?
Yes, it's for API users. But, we also need to be sure our exported
functions have the right calling interface. So I suspect a whole lot
less files should be including it, and we could have a single place to
confirm our exported functions have the right ABI, but not also use this
file in so many places?
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 1/8] malloc: implement USE_DL_PREFIX via inline functions
Am 20.02.2020 um 04:05 schrieb Simon Glass:
> On Wed, 19 Feb 2020 at 12:39, Simon Goldschmidt
> wrote:
>>
>> Commit cfda60f99ae2 ("sandbox: Use a prefix for all allocation functions")
>> introduced preprocessor macros for malloc/free etc.
>>
>> This is bad practice as it essentially makes 'free' a reserved keyword and
>> resulted in quite a bit of renaming to avoid that reserved keyword.
>>
>> A better solution is to define the allocation functions as 'static inline'.
>>
>> As a side effect, exports.h must not export malloc/free for sandbox.
>>
>> Signed-off-by: Simon Goldschmidt
>> ---
>>
>> A side-effect is that exports.h may not declare malloc/free. I'm not really
>> sure if this is correct, but for sandbox, it should probably be ok?
>
> Is it possible to fix this? E.g. don't use inline for these two
> functions on sandbox?
Not using inline for sandbox for these two is *not* an option as these
two are exactly the ones offending globally known names.
I guess we have to know what we want here: what is exports.h meant for?
To me it looks like it is meant for "U-Boot API" applications to link
against. If so, why is it included in U-Boot sources (in over 20 files)?
I guess one solution would be to move (or copy) the DL prefix handling
into exports.h and _exports.h so that applications linking with
exports.h implicily call dlmalloc/dlfree, not malloc/free (which would
be the OS versions for sandbox).
I'll try to prepare v2 in that direction, but I'm still not unsure since
exports.h is included in internal U-Boot code.
Regards,
Simon
>
>>
>> include/_exports.h | 2 ++
>> include/exports.h | 2 ++
>> include/malloc.h | 44 +---
>> 3 files changed, 33 insertions(+), 15 deletions(-)
>
> Reviewed-by: Simon Glass
>
Re: [PATCH 2/8] Revert "mtd: Rename free() to rfree()"
Am 19.02.2020 um 21:04 schrieb Fabio Estevam: > On Wed, Feb 19, 2020 at 4:40 PM Simon Goldschmidt > wrote: >> >> This reverts commit 8d38a8459b0de45f5ff41f3e11c278a5cf395fd0. > > You missed to explain the reason for the revert. Yes, I'll fix that in v2. Regards, Simon
[PATCH] net: phy: fix autoneg timeout
Recently, genphy_update_link() has been changed to use a 50ms polling
interval instead of the previous 1ms. However, the timeout to give up
waiting for a link remained unchanged, calculating the iterations.
As a result, PHY_ANEG_TIMEOUT now specifies "multiples of 50ms" instead
of just to be a number of milliseconds.
Fix this by dividing PHY_ANEG_TIMEOUT by 50 in this loop. This gets us
back to a 4 seconds timeout for the default value (instead of 200s).
Fixes: net: phy: Increase link up delay in genphy_update_link() ("27c3f70f3b50")
Signed-off-by: Simon Goldschmidt
---
This should be applied before the next release as it fixes a regression
of v2020.01!
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 80a7664e49..5cf9c165b6 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -244,7 +244,7 @@ int genphy_update_link(struct phy_device *phydev)
/*
* Timeout reached ?
*/
- if (i > PHY_ANEG_TIMEOUT) {
+ if (i > (PHY_ANEG_TIMEOUT / 50)) {
printf(" TIMEOUT !\n");
phydev->link = 0;
return -ETIMEDOUT;
--
2.20.1
Re: [PATCH v4 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
Am 19.02.2020 um 08:27 schrieb Simon Goldschmidt: > On Tue, Feb 18, 2020 at 6:53 PM Marek Vasut wrote: >> >> On 2/18/20 9:34 AM, Patrick Delaunay wrote: >>> >>> In this serie I update the DWC2 host driver to use the device tree >>> information and the associated PHY and CLOCK drivers when they are >>> availables. >>> >>> The V4 is rebased on latest master (v2020.04-rc2). >>> CI-Tavis build is OK: >>> https://travis-ci.org/patrickdelaunay/u-boot/builds/651479714 >>> >>> NB: CI-Travis build was OK for all target after V3: >>> https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187 >>> As in V2, I cause the warnings for some boards: >>> drivers/usb/host/built-in.o: In function `dwc2_usb_remove': >>> drivers/usb/host/dwc2.c:1441: undefined reference to `clk_disable_bulk' >>> >>> I test this serie on stm32mp157c-ev1 board, with PHY and CLK >>> support >>> >>> The U-CLASS are provided by: >>> - PHY by USBPHYC driver = ./drivers/phy/phy-stm32-usbphyc.c >>> - CLOCK by RCC clock driver = drivers/clk/clk_stm32mp1.c >>> - RESET by RCC reset driver = drivers/reset/stm32-reset.c >>> >>> And I activate the configuration >>> +CONFIG_USB_DWC2=y >> >> Simon, can you test this on SOCFPGA ? > > I can test if it probes, but I don't have anything running on that USB port > the socfpga_socrates board has. Would that be enought to test? Tested the whole series on socfpga_socrates by instantiating the driver. Shows the same behaviour as before (I still have no OTG cable to test attaching a storage device). Regards, Simon > > Regards, > Simon > >> >> [...]
Re: [PATCH v4 3/5] usb: host: dwc2: add clk support
Am 18.02.2020 um 09:35 schrieb Patrick Delaunay:
> Add support for clock with driver model.
>
> This patch don't added dependency because when CONFIG_CLK
> is not activated the clk function are stubbed.
>
> Signed-off-by: Patrick Delaunay
Reviewed-by: Simon Goldschmidt
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> drivers/usb/host/dwc2.c | 30 +-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index 5e7ffaddd9..d56d0e61b5 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -5,14 +5,15 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -39,6 +40,7 @@ struct dwc2_priv {
> struct udevice *vbus_supply;
> #endif
> struct phy phy;
> + struct clk_bulk clks;
> #else
> uint8_t *aligned_buffer;
> uint8_t *status_buffer;
> @@ -1377,6 +1379,26 @@ static int dwc2_shutdown_phy(struct udevice *dev)
> return 0;
> }
>
> +static int dwc2_clk_init(struct udevice *dev)
> +{
> + struct dwc2_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + ret = clk_get_bulk(dev, >clks);
> + if (ret == -ENOSYS || ret == -ENOENT)
> + return 0;
> + if (ret)
> + return ret;
> +
> + ret = clk_enable_bulk(>clks);
> + if (ret) {
> + clk_release_bulk(>clks);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int dwc2_usb_probe(struct udevice *dev)
> {
> struct dwc2_priv *priv = dev_get_priv(dev);
> @@ -1385,6 +1407,10 @@ static int dwc2_usb_probe(struct udevice *dev)
>
> bus_priv->desc_before_addr = true;
>
> + ret = dwc2_clk_init(dev);
> + if (ret)
> + return ret;
> +
> ret = dwc2_setup_phy(dev);
> if (ret)
> return ret;
> @@ -1410,6 +1436,8 @@ static int dwc2_usb_remove(struct udevice *dev)
> dwc2_uninit_common(priv->regs);
>
> reset_release_bulk(>resets);
> + clk_disable_bulk(>clks);
> + clk_release_bulk(>clks);
>
> return 0;
> }
>
Re: [PATCH v4 2/5] usb: host: dwc2: add phy support
Am 18.02.2020 um 09:35 schrieb Patrick Delaunay:
> Use generic phy to initialize the PHY associated to the
> DWC2 device and available in the device tree.
>
> This patch don't added dependency because when CONFIG_PHY
> is not activated, the generic PHY function are stubbed.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - update dev_err
> - update commit message
> - change dev_err to dev_dbg for PHY function call
> - treat dwc2_shutdown_phy error
>
> drivers/usb/host/dwc2.c | 66 +
> 1 file changed, 66 insertions(+)
>
> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
> index e4efaf1e59..5e7ffaddd9 100644
> --- a/drivers/usb/host/dwc2.c
> +++ b/drivers/usb/host/dwc2.c
> @@ -8,6 +8,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -37,6 +38,7 @@ struct dwc2_priv {
> #ifdef CONFIG_DM_REGULATOR
> struct udevice *vbus_supply;
> #endif
> + struct phy phy;
> #else
> uint8_t *aligned_buffer;
> uint8_t *status_buffer;
> @@ -1322,13 +1324,71 @@ static int dwc2_usb_ofdata_to_platdata(struct udevice
> *dev)
> return 0;
> }
>
> +static int dwc2_setup_phy(struct udevice *dev)
> +{
> + struct dwc2_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + ret = generic_phy_get_by_index(dev, 0, >phy);
> + if (ret) {
> + if (ret != -ENOENT) {
Could you invert this logic and add a comment like "no PHY" or something?
> + dev_err(dev, "Failed to get USB PHY: %d.\n", ret);
> + return ret;
> + }
> + return 0;
> + }
> +
> + ret = generic_phy_init(>phy);
> + if (ret) {
> + dev_dbg(dev, "Failed to init USB PHY: %d.\n", ret);
> + return ret;
> + }
> +
> + ret = generic_phy_power_on(>phy);
> + if (ret) {
> + dev_dbg(dev, "Failed to power on USB PHY: %d.\n", ret);
> + generic_phy_exit(>phy);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int dwc2_shutdown_phy(struct udevice *dev)
> +{
> + struct dwc2_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + if (!generic_phy_valid(>phy))
A comment saying that this is for platforms without a phy driver would
be nice.
Other than that:
Reviewed-by: Simon Goldschmidt
> + return 0;
> +
> + ret = generic_phy_power_off(>phy);
> + if (ret) {
> + dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret);
> + return ret;
> + }
> +
> + ret = generic_phy_exit(>phy);
> + if (ret) {
> + dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int dwc2_usb_probe(struct udevice *dev)
> {
> struct dwc2_priv *priv = dev_get_priv(dev);
> struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev);
> + int ret;
>
> bus_priv->desc_before_addr = true;
>
> + ret = dwc2_setup_phy(dev);
> + if (ret)
> + return ret;
> +
> return dwc2_init_common(dev, priv);
> }
>
> @@ -1341,6 +1401,12 @@ static int dwc2_usb_remove(struct udevice *dev)
> if (ret)
> return ret;
>
> + ret = dwc2_shutdown_phy(dev);
> + if (ret) {
> + dev_dbg(dev, "Failed to shutdown USB PHY: %d.\n", ret);
> + return ret;
> + }
> +
> dwc2_uninit_common(priv->regs);
>
> reset_release_bulk(>resets);
>
Re: [PATCH v4 1/5] dm: clk: add stub when CONFIG_CLK is desactivated
Am 18.02.2020 um 09:34 schrieb Patrick Delaunay:
> Add stub for functions clk_...() when CONFIG_CLK is desactivated.
>
> This patch avoids compilation issues for driver using these API
> without protection (#if CONFIG_IS_ENABLED(CLK))
>
> For example, before this patch we have undefined reference to
> `clk_disable_bulk') for code:
> clk_disable_bulk(>clks);
> clk_release_bulk(>clks);
>
> Signed-off-by: Patrick Delaunay
> ---
>
> Changes in v4:
> - Add stub for all functions using 'struct clk' or 'struct clk_bulk'
> after remarks on v3
>
> Changes in v3:
> - Add stub for clk_disable_bulk
>
> Changes in v2: None
>
> include/clk.h | 101 +++---
> 1 file changed, 88 insertions(+), 13 deletions(-)
>
> diff --git a/include/clk.h b/include/clk.h
> index 3336301815..1fb415ddc8 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -312,6 +312,7 @@ static inline int clk_release_bulk(struct clk_bulk *bulk)
> return clk_release_all(bulk->clks, bulk->count);
> }
>
> +#if CONFIG_IS_ENABLED(CLK)
> /**
> * clk_request - Request a clock by provider-specific ID.
> *
> @@ -433,19 +434,6 @@ int clk_disable_bulk(struct clk_bulk *bulk);
> */
> bool clk_is_match(const struct clk *p, const struct clk *q);
>
> -int soc_clk_dump(void);
> -
> -/**
> - * clk_valid() - check if clk is valid
> - *
> - * @clk: the clock to check
> - * @return true if valid, or false
> - */
> -static inline bool clk_valid(struct clk *clk)
> -{
> - return clk && !!clk->dev;
> -}
> -
> /**
> * clk_get_by_id() - Get the clock by its ID
> *
> @@ -465,6 +453,93 @@ int clk_get_by_id(ulong id, struct clk **clkp);
> * @return true on binded, or false on no
> */
> bool clk_dev_binded(struct clk *clk);
> +
> +#else /* CONFIG_IS_ENABLED(CLK) */
> +
> +static inline int clk_request(struct udevice *dev, struct clk *clk)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int clk_free(struct clk *clk)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline ulong clk_get_rate(struct clk *clk)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline struct clk *clk_get_parent(struct clk *clk)
> +{
> + return (struct clk *)-ENOSYS;
This should use ERR_PTR() to care for platforms defining
CONFIG_ERR_PTR_OFFSET.
> +}
> +
> +static inline long long clk_get_parent_rate(struct clk *clk)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline ulong clk_set_rate(struct clk *clk, ulong rate)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int clk_enable(struct clk *clk)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int clk_enable_bulk(struct clk_bulk *bulk)
> +{
> + return bulk && bulk->count == 0 ? 0 : -ENOSYS;
For this test to work, someone would need to set bulk->count to 0. This
is normally done by clk_get_bulk(), but you defined it to only return an
error.
I guess it works for you because all clk_bulk objects you use are from
the heap (which is currently zeroed out in U-Boot) or if they are on the
stack, you have if/else code that doesn't bring you here. Still it seems
wrong.
Regards,
Simon
> +}
> +
> +static inline int clk_disable(struct clk *clk)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int clk_disable_bulk(struct clk_bulk *bulk)
> +{
> + return bulk && bulk->count == 0 ? 0 : -ENOSYS;
> +}
> +
> +static inline bool clk_is_match(const struct clk *p, const struct clk *q)
> +{
> + return false;
> +}
> +
> +static inline int clk_get_by_id(ulong id, struct clk **clkp)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline bool clk_dev_binded(struct clk *clk)
> +{
> + return false;
> +}
> +#endif /* CONFIG_IS_ENABLED(CLK) */
> +
> +/**
> + * clk_valid() - check if clk is valid
> + *
> + * @clk: the clock to check
> + * @return true if valid, or false
> + */
> +static inline bool clk_valid(struct clk *clk)
> +{
> + return clk && !!clk->dev;
> +}
> +
> +int soc_clk_dump(void);
> +
> #endif
>
> #define clk_prepare_enable(clk) clk_enable(clk)
>
[RFC] dm: uclass: add functions to get device by platdata
When OF_PLATDATA is enabled DT information is parsed and platdata
structures are populated. In this context the links between DT nodes are
represented as pointers to platdata structures, and there is no clear way
to access to the device which owns the structure.
This patch implements a set of functions:
- device_find_by_platdata
- uclass_find_device_by_platdata
to access to the device.
Signed-off-by: Walter Lozano
---
drivers/core/device.c| 19 +++
drivers/core/uclass.c| 34 ++
include/dm/device.h | 2 ++
include/dm/uclass-internal.h | 3 +++
include/dm/uclass.h | 2 ++
5 files changed, 60 insertions(+)
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 89ea820d48..54a3a8d870 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -591,6 +591,25 @@ static int device_find_by_ofnode(ofnode node, struct
udevice **devp)
}
#endif
+
+int device_find_by_platdata(void *platdata, struct udevice **devp)
+{
+ struct uclass *uc;
+ struct udevice *dev;
+ int ret;
+
+ list_for_each_entry(uc, >uclass_root, sibling_node) {
+ ret = uclass_find_device_by_platdata(uc->uc_drv->id, platdata,
+ );
+ if (!ret || dev) {
+ *devp = dev;
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
int device_get_child(const struct udevice *parent, int index,
struct udevice **devp)
{
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 58b19a4210..7b0ae5b122 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -271,6 +271,29 @@ int uclass_find_device_by_name(enum uclass_id id, const
char *name,
return -ENODEV;
}
+int uclass_find_device_by_platdata(enum uclass_id id, void * platdata, struct
udevice **devp)
+{
+ struct uclass *uc;
+ struct udevice *dev;
+ int ret;
+
+ *devp = NULL;
+ ret = uclass_get(id, );
+ if (ret)
+ return ret;
+ if (list_empty(>dev_head))
+ return -ENODEV;
+
+ uclass_foreach_dev(dev, uc) {
+ if (dev->platdata == platdata) {
+ *devp = dev;
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
int uclass_find_next_free_req_seq(enum uclass_id id)
{
struct uclass *uc;
@@ -466,6 +489,17 @@ int uclass_get_device_by_name(enum uclass_id id, const
char *name,
return uclass_get_device_tail(dev, ret, devp);
}
+int uclass_get_device_by_platdata(enum uclass_id id, void * platdata,
+ struct udevice **devp)
+{
+ struct udevice *dev;
+ int ret;
+
+ *devp = NULL;
+ ret = uclass_find_device_by_platdata(id, platdata, );
+ return uclass_get_device_tail(dev, ret, devp);
+}
+
int uclass_get_device_by_seq(enum uclass_id id, int seq, struct udevice **devp)
{
struct udevice *dev;
diff --git a/include/dm/device.h b/include/dm/device.h
index ab806d0b7e..3c043a3127 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -396,6 +396,8 @@ enum uclass_id device_get_uclass_id(const struct udevice
*dev);
*/
const char *dev_get_uclass_name(const struct udevice *dev);
+int device_find_by_platdata(void *platdata, struct udevice **devp);
+
/**
* device_get_child() - Get the child of a device by index
*
diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h
index 6e3f15c2b0..f643fc95da 100644
--- a/include/dm/uclass-internal.h
+++ b/include/dm/uclass-internal.h
@@ -100,6 +100,9 @@ int uclass_find_next_device(struct udevice **devp);
int uclass_find_device_by_name(enum uclass_id id, const char *name,
struct udevice **devp);
+int uclass_find_device_by_platdata(enum uclass_id id, void *platdata,
+ struct udevice **devp);
+
/**
* uclass_find_device_by_seq() - Find uclass device based on ID and sequence
*
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 70fca79b44..92c07f8426 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -167,6 +167,8 @@ int uclass_get_device(enum uclass_id id, int index, struct
udevice **devp);
int uclass_get_device_by_name(enum uclass_id id, const char *name,
struct udevice **devp);
+int uclass_get_device_by_platdata(enum uclass_id id, void *platdata,
+ struct udevice **devp);
/**
* uclass_get_device_by_seq() - Get a uclass device based on an ID and sequence
*
--
2.20.1
Re: [v1 PATCH 1/1] riscv: Add boot hartid to Device tree
On Tue, Mar 3, 2020 at 11:59 PM Rick Chen wrote:
>
> Hi Atish
>
> > From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Atish Patra
> > Sent: Friday, February 28, 2020 5:01 AM
> > To: u-boot@lists.denx.de
> > Cc: Atish Patra; Alexander Graf; Anup Patel; Bin Meng; Joe Hershberger;
> > Loic Pallardy; Lukas Auer; Marek Behún; Marek Vasut; Patrick Delaunay; Peng
> > Fan; Philippe Reynes; Simon Glass; Simon Goldschmidt; Stefano Babic;
> > Thierry Reding; tr...@konsulko.com; Heinrich Schuchardt; Ard Biesheuvel;
> > l...@nuviainc.com; abner.ch...@hpe.com; daniel.schae...@hpe.com
> > Subject: [v1 PATCH 1/1] riscv: Add boot hartid to Device tree
> >
> > Linux booting protocol mandates that register "a0" contains the hartid.
> > However, U-boot can not pass the hartid via a0 during via standard UEFI
> > protocol. DT nodes are commonly used to pass such information to the OS.
> >
> > Add a DT node under chosen node to indicate the boot hartid. EFI stub in
> > Linux kernel will parse this node and pass it to the real kernel in "a0"
> > before jumping to it.
> >
> > Signed-off-by: Atish Patra
> > ---
> > arch/riscv/lib/bootm.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c index
> > fad16901c5f2..dd359a45c2e1 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -28,6 +28,19 @@ __weak void board_quiesce_devices(void)
> >
> > int arch_fixup_fdt(void *blob)
> > {
> > + u32 size;
> > + int chosen_offset, err;
> > +
> > + size = fdt_totalsize(blob);
> > + err = fdt_open_into(blob, blob, size + 32);
> > + if (err < 0) {
> > + printf("Device Tree can't be expanded to accommodate new
> > node");
> > + return -1;
> > + }
> > + chosen_offset = fdt_path_offset(blob, "/chosen");
> > + fdt_setprop_u32(blob, chosen_offset, "boot-hartid",
> > + gd->arch.boot_hart);
>
> Is it also OK for RV64 ?
>
Yes. I have tested it. To make it compatible for both 32 bit & 64 bit,
I have chosen a 32bit property
which allows the max hartid to be 2^32-1 which should be sufficient.
> Other than that,
> Reviewed-by: Rick Chen
>
> > +
> > return 0;
> > }
> >
> > --
> > 2.24.0
> >
--
Regards,
Atish
Re: [PATCH v3 0/3] usb: Covert to support Live DT
On 3/4/20 1:59 AM, Kever Yang wrote: > This patch set convert to use APIs leading with dev_ or ofnode_ which > supports live DT instead of fdt_ or fdtdec or devfdt_. > Two functions update there parameter from offset to ofnode: > - usb_get_maximum_speed() > - usb_get_dr_mode() > > For V3 update: > Squesh patch 3rd~9th/9 into one patch because the ofnode_to_offset(node) is > not work with the livetree, so we have to convert the api to support > livetree and its related function at the same time. > > For V2 update: > Drop below patch after include ofnode.h in otg.h: > [PATCH 09/10] usb: Use ofnode as usb_get_dr_mode() parameter > > > Changes in v3: > - Squesh the 3rd~9th into one patch. > > Kever Yang (3): > usb: dwc3-of-simple: Drop redundant inclding header file > usb: ehci-msm: Use dev interface to get device address > usb: Migrate to support live DT for some driver > > drivers/usb/cdns3/core.c | 15 ++- > drivers/usb/cdns3/gadget.c | 2 +- > drivers/usb/common/common.c| 12 +--- > drivers/usb/dwc3/dwc3-generic.c| 16 +++- > drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +- > drivers/usb/gadget/dwc2_udc_otg.c | 5 ++--- > drivers/usb/host/dwc3-of-simple.c | 1 - > drivers/usb/host/dwc3-sti-glue.c | 20 +++- > drivers/usb/host/ehci-msm.c| 4 +--- > drivers/usb/host/ehci-mx6.c| 2 +- > drivers/usb/host/xhci-dwc3.c | 3 +-- > drivers/usb/musb-new/ti-musb.c | 12 +--- > include/linux/usb/otg.h| 10 ++ > 13 files changed, 43 insertions(+), 61 deletions(-) > All patches applied, thanks.
Re: [PATCH v3 2/3] usb: ehci-msm: Use dev interface to get device address
On Tue, 3 Mar 2020 at 18:03, Kever Yang wrote: > > Use dev_read_addr_ptr() instead of devfdt_get_addr() so that we can support > live DT. > > Signed-off-by: Kever Yang > Reviewed-by: Ramon Fried > --- > > Changes in v3: None > > drivers/usb/host/ehci-msm.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v3 3/3] usb: Migrate to support live DT for some driver
On Wed, 4 Mar 2020 at 05:32, Marek Vasut wrote: > > On 3/4/20 1:59 AM, Kever Yang wrote: > > Use ofnode_ instead of fdt_ APIs so that the drivers can support live DT. > > This patch updates usb_get_dr_mode() and usb_get_maximum_speed() to use > > ofnode as parameter instead of fdt offset. And all the drivers who use > > these APIs update to use live dt APIs at the same time. > > > > Signed-off-by: Kever Yang > > I'll wait for RB from Simon on 2/3 and 3/3 , however they look good to me. Reviewed-by: Simon Glass (no need to wait though!)
Re: [PATCH 1/2] Makefile: Add environment variable DEVICE_TREE to header
Hi Michal,
On Tue, 3 Mar 2020 at 23:44, Michal Simek wrote:
>
> On 04. 03. 20 3:47, Simon Glass wrote:
> > Hi Michal,
> >
> > On Mon, 2 Mar 2020 at 23:52, Michal Simek wrote:
> >>
> >> On 02. 03. 20 20:47, Simon Glass wrote:
> >>> Hi Michal,
> >>>
> >>> On Fri, 28 Feb 2020 at 04:03, Michal Simek
> >>> wrote:
>
> On 26. 02. 20 16:33, Simon Glass wrote:
> > Hi Michal,
> >
> > On Tue, 18 Feb 2020 at 09:02, Michal Simek
> > wrote:
> >>
> >> Users have option to overwrite default device tree
> >> (CONFIG_DEFAULT_DEVICE_TREE) via environment variable DEVICE_TREE.
> >>
> >> Feature has been added long time ago by commit 74de8c9a1672
> >> ("dts/Makefile: Build the user specified dts") for a little bit
> >> different
> >> reason.
> >>
> >> But this variable can be also used for different purpose like choosing
> >> proper configuration from FIT image in SPL.
> >> And this is the functionality I would like to use on Xilinx Zynq
> >> devices
> >> that current u-boot.img can be composed in the same way based on
> >> OF_LIST
> >> and different configuration is taken based on platform specific SPL.
> >> SPL requires low level ps7_init_gpl configuration that's why different
> >> boards require different SPL with fixed board_fit_config_name_match().
> >>
> >> Signed-off-by: Michal Simek
> >> ---
> >>
> >> I have done it in this way but maybe there is any smarter way how this
> >> can
> >> be done. Also macro name can change if you want.
> >
> > Can you please add a bit of documentation to doc/README.fdt-control ?
>
> This feature is cover by documentation in this file already. What
> exactly do you think that should be added?
>
> >>>
> >>> Then I suppose I am confused as to what this patch does. I thought it
> >>> was allowing U-Boot to pass the DT to Linux, which wasn't previously
> >>> supported?
> >>
> >> Just a note this is also not supported and would be good to have support
> >> for it.
> >>
> >> The patch is putting device tree name as macro to header and I am using
> >> this value just in zynq SPL to choose proper dtb file from u-boot.img.
> >>
> >> On ZynqMP generic platform I am using mkimage_fit_atf.sh script to
> >> automatically fill default configuration.
> >> https://gitlab.denx.de/u-boot/u-boot/blob/master/arch/arm/mach-zynqmp/mkimage_fit_atf.sh#L94
> >> And I didn't want to create different u-boot.img/itb for Zynq that's why
> >> SPL has device tree built it directly in SPL.
> >>
> >> As I said maybe there is smarter way how to do it but this was the one I
> >> used.
> >> In our case SPL another way could be to put filename from ps7_init_*
> >> files and save it as variable and use it.
> >
> > OK I see. So are you using OF_CONTROL for SPL?
>
> Yes.
OK ta.
Reviewed-by: Simon Glass
Regards,
Simon
Re: [U-Boot] [PATCH V3 15/27] imx8m: Fix MMU table issue for OPTEE memory
Added Clement Faure to the discussion
On Wed, Mar 4, 2020 at 4:34 PM Igor Opaniuk wrote:
>
> Hi Peng,
>
> On Tue, Aug 27, 2019 at 9:38 AM Peng Fan wrote:
> >
> > When running with OPTEE, the MMU table in u-boot does not remove the OPTEE
> > memory from its settings. So ARM speculative prefetch in u-boot may access
> > that OPTEE memory. Due to trust zone is enabled by OPTEE and that memory
> > is set to secure access, then the speculative prefetch will fail and cause
> > various memory issue in u-boot.
> > The fail address register and int_status register in trustzone has logged
> > that speculative access from u-boot.
> >
> > Signed-off-by: Peng Fan
> > ---
> > arch/arm/mach-imx/imx8m/soc.c | 20 ++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> > index 5115471eff..dd393b581b 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -112,16 +112,18 @@ static struct mm_region imx8m_mem_map[] = {
> > /* DRAM1 */
> > .virt = 0x4000UL,
> > .phys = 0x4000UL,
> > - .size = 0xC000UL,
> > + .size = PHYS_SDRAM_SIZE,
> > .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > PTE_BLOCK_OUTER_SHARE
> > +#ifdef PHYS_SDRAM_2_SIZE
> > }, {
> > /* DRAM2 */
> > .virt = 0x1UL,
> > .phys = 0x1UL,
> > - .size = 0x04000UL,
> > + .size = PHYS_SDRAM_2_SIZE,
> > .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > PTE_BLOCK_OUTER_SHARE
> > +#endif
> > }, {
> > /* List terminator */
> > 0,
> > @@ -130,6 +132,20 @@ static struct mm_region imx8m_mem_map[] = {
> >
> > struct mm_region *mem_map = imx8m_mem_map;
> >
> > +void enable_caches(void)
> > +{
> > + /*
> > +* If OPTEE runs, remove OPTEE memory from MMU table to
> > +* avoid speculative prefetch. OPTEE runs at the top of
> > +* the first memory bank
> > +*/
> > + if (rom_pointer[1])
> > + imx8m_mem_map[5].size -= rom_pointer[1];
> > +
> > + icache_enable();
> > + dcache_enable();
> > +}
> > +
> > static u32 get_cpu_variant_type(u32 type)
> > {
> > struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR;
> > --
> > 2.16.4
> >
> > ___
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
>
> Unfortunately, I'm still facing a similar issue when OP-TEE is loaded on
> iMX8MM-based module, despite BL32_BASE(0xbe00) +
> BL32_SIZE(0x200) region is excluded.
> Based on debug ouput the latest one is:
>
> idx=2 PTE bfff7010 at level 1: bfffd003
> Checking if pte fits for virt=bde0 size=20 blocksize=4000
> addr=bde0 level=2
> idx=2 PTE bfff7010 at level 1: bfffd003
> idx=1ef PTE bfffdf78 at level 2: 0
> Checking if pte fits for virt=bde0 size=20 blocksize=20
> Setting PTE bfffdf78 to block virt=bde0
>
>
> U-Boot hangs just after writing to System Control Register in
> arch/arm/cpu/armv8/cache_v8.c:419:
>
> /* enable the mmu */
> set_sctlr(get_sctlr() | CR_M);
>
> In my setup mainline U-Boot/TF-A/OP-TEE are used:
> u-boot: 133276f14a ("Merge branch '2020-02-25-master-imports'")
> tf-a: 6e46981f84 ("Merge "Update pathnames in maintainers.rst file"
> into integration")
> op-tee: a67dc424ff ("ta: pkcs11: API for slot/token information")
>
> OP-TEE is compiled with these flags:
>CFG_ARM64_core=y
>CFG_NXP_CAAM=n
>CFG_NXPCRYPT=n
>PLATFORM=imx-mx8mmevk
>CFG_TEE_CORE_LOG_LEVEL=4
>DEBUG=y
>CFG_TEE_CORE_DEBUG=y
>
> With DCACHE disabled obviously everything works. Also I don't face any issues
> if TF-A is compiled without SPD=opteed.
>
> Maybe it rings the bell and you already saw similar issues before?
>
> Thanks
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opan...@gmail.com
> skype: igor.opanyuk
> +380 (93) 836 40 67
> http://ua.linkedin.com/in/iopaniuk
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk
Re: [PATCH] Makefile: doesn't need check stack size when dtb is not built
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into
initial stack") adds an extra check for stack size in BSS if
CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled.
This check, however, doesn't make sense under the configuration where
control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch
instead or as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
+ALL-y += init_sp_bss_offset_check
+endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
Re: Please pull u-boot-dm
On Tue, Mar 03, 2020 at 04:53:37PM -0700, Simon Glass wrote: > Hi Tom, > > The following changes since commit 8aad16916d04e3db0d1652cb96e840e209e19252: > > Merge tag 'u-boot-stm32-20200203' of > https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-03-02 > 09:20:30 -0500) > > are available in the Git repository at: > > git://git.denx.de/u-boot-dm.git tags/dm-pull-3mar20 > > for you to fetch changes up to 9aa886cc0b4424b49b24486f804fd18aafad00b2: > > video: meson: keep power domain up after booting (2020-03-02 19:47:38 -0700) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI
On Tue, 2020-03-03 at 16:57 -0500, Sean Anderson wrote:
> On 3/3/20 4:53 PM, Lukas Auer wrote:
> > On Mon, 2020-03-02 at 18:43 -0500, Sean Anderson wrote:
> > > On 3/2/20 6:17 PM, Lukas Auer wrote:
> > > > Don't move this. It is intended to be run before the IPI is cleared.
> > >
> > > Hm, ok. I think I moved it to after because of the 'if (!smp_function)'
> > > check, but those two don't really need to be done together.
> > >
> >
> > Thanks! We had problems with code corruption in some situations,
> > because some secondary harts entered OpenSBI after the main hart while
> > OpenSBI expected all harts to be running OpenSBI by that time. Moving
> > this code block was part of the fix for this situation, see [1].
> >
> > [1]:
> > https://gitlab.denx.de/u-boot/u-boot/commit/90ae28143700bae4edd23930a7772899ad259058
>
> Ah, this makes a lot more sense why it was located where it was.
>
> > > > > /*
> > > > >* Clear the IPI to acknowledge the request before jumping to
> > > > > the
> > > > >* requested function.
> > > > >*/
> > > > > ret = riscv_clear_ipi(hart);
> > > > > if (ret) {
> > > > > - pr_err("Cannot clear IPI of hart %ld\n", hart);
> > > > > + pr_err("Cannot clear IPI of hart %ld (error %d)\n",
> > > > > hart, ret);
> > > > > return;
> > > > > }
> > > > >
> > > > > + __smp_mb();
> > > > > +
> > > > > + smp_function = (void (*)(ulong, ulong,
> > > > > ulong))gd->arch.ipi[hart].addr;
> > > > > + /*
> > > > > + * There may be an IPI raised before u-boot begins execution,
> > > > > so check
> > > > > + * to ensure we actually have a function to call.
> > > > > + */
> > > > > + if (!smp_function)
> > > > > + return;
> > > > > + log_debug("hart = %lu func = %p\n", hart, smp_function);
> > > >
> > > > The log messages might be corrupted if multiple harts are calling the
> > > > log function here. I have not looked into the details so this might not
> > > > be an issue. In that case it is fine to keep, otherwise please remove
> > > > it.
> > >
> > > I ran into this problem a lot when debugging. I ended up implementing a
> > > spinlock around puts/putc. I agree it's probably better to remove this,
> > > but I worry that concurrency bugs will become much harder to track down
> > > without some kind of feedback. (This same criticism applies to the log
> > > message above as well).
> > >
> >
> > Especially with your changes, I hope we already have or will soon reach
> > a code robustness level where we won't have too many concurrency bugs
> > in the future. :)
> > Let's remove it for now until the logging backend can handle this
> > cleanly.
>
> Ok. Should the error message above ("Cannot clear IPI of hart...") also
> be removed? I found it tended to corrupt the log output if it was ever
> triggered.
>
Even though it's not ideal, we should keep it for now. Otherwise we
don't have a way to get notified about the error.
Thanks,
Lukas
Re: [PATCH v5 33/33] riscv: Add Sipeed Maix support
On 3/4/20 2:47 AM, Rick Chen wrote: > Hi Sean > > This patch series become larger and larger from v1 with 11 patches to > v5 with 33 patches. I didn't intend for it to balloon so large. My original goal for this revision was just to get the SPI and MMC slots working. However, I discovered that I would need to implement a pinmux slot and GPIOs for the MMC slot to work (since I think that is the primary motivating factor for using U-Boot on this board, as opposed to the native bootloader). > You shall just fix the suggestions from the previous version in the > next version. I will try to do that :) > Additional extra features and subsystem drivers that you want to > support, you shall send them individually instead of mixing them > together. In the past, I have been told to keep this sort of thing in one series (e.g. by Bin Meng). I'm really not sure what things I should split off and which I should keep together. > > If you can separate them into different series(spi, gpio,led , > pinctrl, watchdog, those drivers were not present in v1) and they will > not block the reviewing schedule each other. Hm, ok. Perhaps split those off into series which depend on this one? > Narrow down the dependency, it can help to speed up the patch work. > And I am happy to help pulling the RISC-V relative patchs via riscv > tree ASAP. Ok, I will try and get another version out which fixes the feedback I have gotten on those patches. > > Thanks, > Rick > > > Thanks for the feedback. --Sean
Re: [PATCH v5 19/33] spi: dw: Add device tree properties for fields in CTRL1
On 3/4/20 1:15 AM, Rick Chen wrote:
> Hi Sean
>
>> Some devices have different layouts for the fields in CTRL1 (e.g. the
>
> Still not fix this typo in commit message
> CTRL1 -> CTRL0
>
> Thanks,
> Rick
Ah, whoops. I think I fixed it, the forgot I had done so, and changed it
again. Hopefully I can get this straight.
--Sean
>
>> Kendryte K210). Allow this layout to be configurable from the device tree.
>> The documentation has been taken from Linux.
>>
>> Signed-off-by: Sean Anderson
>> Reviewed-by: Simon Glass
>> ---
>>
>> Changes in v4:
>> - New
>>
>> .../spi/snps,dw-apb-ssi.txt | 43 +++
>> drivers/spi/designware_spi.c | 40 ++---
>> 2 files changed, 68 insertions(+), 15 deletions(-)
>> create mode 100644 doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt
>>
>> diff --git a/doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt
>> b/doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt
>> new file mode 100644
>> index 00..4b6152f6b7
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt
>> @@ -0,0 +1,43 @@
>> +Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
>> +
>> +Required properties:
>> +- compatible : "snps,dw-apb-ssi"
>> +- reg : The register base for the controller. For "mscc,-spi", a second
>> + register set is required (named ICPU_CFG:SPI_MST)
>> +- #address-cells : <1>, as required by generic SPI binding.
>> +- #size-cells : <0>, also as required by generic SPI binding.
>> +- clocks : phandles for the clocks, see the description of clock-names
>> below.
>> + The phandle for the "ssi_clk" is required. The phandle for the "pclk"
>> clock
>> + is optional. If a single clock is specified but no clock-name, it is the
>> + "ssi_clk" clock. If both clocks are listed, the "ssi_clk" must be first.
>> +
>> +Optional properties:
>> +- clock-names : Contains the names of the clocks:
>> +"ssi_clk", for the core clock used to generate the external SPI clock.
>> +"pclk", the interface clock, required for register access.
>> +- cs-gpios : Specifies the gpio pins to be used for chipselects.
>> +- num-cs : The number of chipselects. If omitted, this will default to 4.
>> +- reg-io-width : The I/O register width (in bytes) implemented by this
>> + device. Supported values are 2 or 4 (the default).
>> +- snps,dfs-offset The offset in bits of the DFS field in CTRL0, defaulting
>> to 0
>> +- snps,frf-offset The offset in bits of the FRF field in CTRL0, defaulting
>> to 4
>> +- snps,tmod-offset The offset in bits of the tmode field in CTRL0,
>> defaulting
>> + to 6
>> +- snps,mode-offset The offset in bits of the work mode field in CTRL0,
>> + defaulting to 8
>> +
>> +Child nodes as per the generic SPI binding.
>> +
>> +Example:
>> +
>> + spi@fff0 {
>> + compatible = "snps,dw-apb-ssi";
>> + reg = <0xfff0 0x1000>;
>> + interrupts = <0 154 4>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + clocks = <_m_clk>;
>> + num-cs = <2>;
>> + cs-gpios = < 13 0>,
>> + < 14 0>;
>> + };
>> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
>> index 2dc16736a3..6e1c289297 100644
>> --- a/drivers/spi/designware_spi.c
>> +++ b/drivers/spi/designware_spi.c
>> @@ -3,6 +3,7 @@
>> * Designware master SPI core controller driver
>> *
>> * Copyright (C) 2014 Stefan Roese
>> + * Copyright (C) 2020 Sean Anderson
>> *
>> * Very loosely based on the Linux driver:
>> * drivers/spi/spi-dw.c, which is:
>> @@ -51,20 +52,14 @@
>> #define DW_SPI_DR 0x60
>>
>> /* Bit fields in CTRLR0 */
>> -#define SPI_DFS_OFFSET 0
>> -
>> -#define SPI_FRF_OFFSET 4
>> #define SPI_FRF_SPI0x0
>> #define SPI_FRF_SSP0x1
>> #define SPI_FRF_MICROWIRE 0x2
>> #define SPI_FRF_RESV 0x3
>>
>> -#define SPI_MODE_OFFSET6
>> -#define SPI_SCPH_OFFSET6
>> -#define SPI_SCOL_OFFSET7
>> +#define SPI_MODE_SCPH 0x1
>> +#define SPI_MODE_SCOL 0x2
>>
>> -#define SPI_TMOD_OFFSET8
>> -#define SPI_TMOD_MASK (0x3 << SPI_TMOD_OFFSET)
>> #defineSPI_TMOD_TR 0x0 /* xmit &
>> recv */
>> #define SPI_TMOD_TO0x1 /* xmit only */
>> #define SPI_TMOD_RO0x2 /* recv only */
>> @@ -89,6 +84,12 @@
>> struct dw_spi_platdata {
>> s32 frequency; /* Default clock frequency, -1 for none */
>> void __iomem *regs;
>> +
>> + /* Offsets in CTRL0 */
>> + u8 dfs_off;
>> + u8 frf_off;
>> + u8 tmod_off;
>> + u8 mode_off;
>> };
>>
>> struct dw_spi_priv {
>> @@ -115,6
Re: [PATCH v5 13/33] pinctrl: Add support for Kendryte K210 FPIOA
On 3/4/20 1:47 AM, Rick Chen wrote:
> Hi Sean
>
>> The Fully-Programmable Input/Output Array (FPIOA) device controls pin
>> multiplexing on the K210. The FPIOA can remap any supported function to any
>> multifunctional IO pin. It can also perform basic GPIO functions, such as
>> reading the current value of a pin.
>>
>> Signed-off-by: Sean Anderson
>> ---
>>
>> Changes in v5:
>> - New
>>
>> MAINTAINERS | 2 +
>> .../pinctrl/kendryte,k210-fpioa.txt | 116 +++
>> drivers/pinctrl/Kconfig | 1 +
>> drivers/pinctrl/Makefile | 1 +
>> drivers/pinctrl/kendryte/Kconfig | 7 +
>> drivers/pinctrl/kendryte/Makefile | 1 +
>> drivers/pinctrl/kendryte/pinctrl.c| 663 ++
>> drivers/pinctrl/kendryte/pinctrl.h| 325 +
>> include/dt-bindings/pinctrl/k210-pinctrl.h| 12 +
>> 9 files changed, 1128 insertions(+)
>> create mode 100644 doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
>> create mode 100644 drivers/pinctrl/kendryte/Kconfig
>> create mode 100644 drivers/pinctrl/kendryte/Makefile
>> create mode 100644 drivers/pinctrl/kendryte/pinctrl.c
>> create mode 100644 drivers/pinctrl/kendryte/pinctrl.h
>> create mode 100644 include/dt-bindings/pinctrl/k210-pinctrl.h
>>
>
> Please checkpatch and fix
>
> total: 3 errors, 13 warnings, 5 checks, 1147 lines checked
>
> Thanks
> Rick
Here is the output of checkpatch.
drivers/pinctrl/kendryte/pinctrl.c:25: error: space prohibited before open
square bracket '['
This is from using macros in the style
#define FOO(x) [FOO_##x] = {\
...
}
I think this is more elegant than putting the [] on a separate line,
but it can of course be changed.
drivers/pinctrl/kendryte/pinctrl.c:76: check: Please use a blank line after
function/struct/union/enum declarations
This is from using "local" macros, e.g.
#define FOO(x) FOO_##x
enum foo {
FOO(bar),
FOO(baz)
};
#undef FOO
I think keeping the undefinition of FOO close to where it is last used
aids readability. This can of course be changed (perhaps by moving the
(un)definition inside the enum).
drivers/pinctrl/kendryte/pinctrl.c:83: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:106: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
All these warnings are due to following the function definitions as
defined in pinctrl_ops.
drivers/pinctrl/kendryte/pinctrl.c:122: error: space prohibited before open
square bracket '['
drivers/pinctrl/kendryte/pinctrl.c:133: check: Please use a blank line after
function/struct/union/enum declarations
drivers/pinctrl/kendryte/pinctrl.c:141: error: space prohibited before open
square bracket '['
drivers/pinctrl/kendryte/pinctrl.c:145: check: Please use a blank line after
function/struct/union/enum declarations
drivers/pinctrl/kendryte/pinctrl.c:403: check: Please use a blank line after
function/struct/union/enum declarations
drivers/pinctrl/kendryte/pinctrl.c:411: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:416: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:417: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:442: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:453: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:454: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:454: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:529: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:529: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:530: warning: Prefer 'unsigned int' to bare
use of 'unsigned'
drivers/pinctrl/kendryte/pinctrl.c:612: warning: ENOSYS means 'invalid syscall
nr' and nothing else
drivers/pinctrl/kendryte/pinctrl.h:281: check: Prefer using the BIT macro
This is from the line
#define K210_PC_DRIVE_1 (1 << K210_PC_DRIVE_SHIFT)
Where there are several succeeding and preceding lines which have
different values instead of 1.
--Sean
Re: [PATCH v5 07/33] clk: Add K210 clock support
On 3/4/20 1:58 AM, Rick Chen wrote:
> Hi Sean
>
>> Due to the large number of clocks, I decided to use the CCF. The overall
>> structure is modeled after the imx code. Clocks are stored in several
>> arrays. There are some translation macros (FOOIFY()) which allow for more
>> dense packing. A possible improvement could be to only store the
>> parameters we need, instead of the whole CCF struct.
>>
>> Signed-off-by: Sean Anderson
>> ---
>
> Please checkpatch and fix
> total: 4 errors, 4 warnings, 18 checks, 662 lines checked
>
> Thanks
> Rick
>
Here is the output of checkpatch
> drivers/clk/kendryte/clk.c:82: warning: static const char * array should
> probably be static const char * const
> drivers/clk/kendryte/clk.c:83: warning: static const char * array should
> probably be static const char * const
These arrays can't have both consts because it needs to have the actual
name of the in0 clock added.
> drivers/clk/kendryte/clk.c:122: check: Please use a blank line after
> function/struct/union/enum declarations
This is due to using macros in the style
#define FOO_LIST \
FOO(bar) \
FOO(baz)
#define FOO(x) FOO_##x,
enum foo_ids {
FOO_LIST
};
#undef FOO
I think sticking the undefinition of FOO immediately after the closing
enum bracket makes it clearer that FOO is only used with that definition
during the declaration of that enum. It is closing the scope, so to
speak. If you'd like I can add a newline after enums declared this way.
> drivers/clk/kendryte/clk.c:124: error: space prohibited before open square
> bracket '['
This is due to macros declared like
#define FOO(x) [FOO_##x] = { \
.y = (x), \
}
Where there is clearly a space before the [, but it is necessary for the
macro. I could declare it like
#define FOO(X) \
[FOO_##x] = { \
.y = (x), \
}
but I think that the former style is more elegant. However, this can
also be changed if needed.
> drivers/clk/kendryte/clk.c:133: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:180: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:182: error: space prohibited before open square
> bracket '['
> drivers/clk/kendryte/clk.c:189: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:208: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:210: error: space prohibited before open square
> bracket '['
> drivers/clk/kendryte/clk.c:210: check: Macro argument reuse 'parents' -
> possible side-effects?
No possible side-effects here, since this macro argument doesn't make
sense unless it is a compile-time constant.
> drivers/clk/kendryte/clk.c:220: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:230: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:235: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:241: check: Macro argument reuse 'id' - possible
> side-effects?
> drivers/clk/kendryte/clk.c:249: check: Macro argument reuse 'id' - possible
> side-effects?
> drivers/clk/kendryte/clk.c:292: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:306: check: Please use a blank line after
> function/struct/union/enum declarations
> drivers/clk/kendryte/clk.c:329: error: do not initialise statics to false
> drivers/clk/kendryte/clk.c:361: check: Macro argument reuse 'clocks' -
> possible side-effects?
> drivers/clk/kendryte/clk.c:386: warning: line over 80 characters
> drivers/clk/kendryte/clk.c:397: warning: line over 80 characters
Unfortunately, I don't see any way to keep these two lines under 80
characters without seriously sacrificing readability. For reference, the
lines look like
clk_dm(K210_CLK_PLL2,
clk_register_composite_struct("pll2", pll2_sels,
ARRAY_SIZE(pll2_sels),
_clk_comps[COMPIFY(K210_CLK_PLL2)]));
The only way to further reduce the length would be to split the array
access over two lines, which I think harms readability too much.
> drivers/clk/kendryte/clk.c:399: check: Macro argument reuse 'id' - possible
> side-effects?
> drivers/clk/kendryte/clk.c:410: check: Macro argument reuse 'id' - possible
> side-effects?
> drivers/clk/kendryte/clk.c:438: check: Macro argument reuse 'id' - possible
> side-effects?
> drivers/clk/kendryte/clk.c:447: check: Macro argument reuse 'id' - possible
> side-effects?
> :0: warning: DT binding docs and includes should be a separate
> patch. See: Documentation/devicetree/bindings/submitting-patches.txt
> :0: warning: DT binding docs and includes should
[PATCH 4/4] mx6cuboxi: enable OF_CONTROL and DM in SPL
In order to take the beneficts of DT and DM in SPL, like reusing the code and avoid redundancy, enable SPL_OF_CONTROL, SPL_DM and SPL_DM_MMC. With this new configuration SPL image is 50 KB, higher than the 38 KB from the previous version, but it still under the 68 KB limit. Signed-off-by: Walter Lozano --- configs/mx6cuboxi_defconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig index bee7d280f0..7ea79b9064 100644 --- a/configs/mx6cuboxi_defconfig +++ b/configs/mx6cuboxi_defconfig @@ -25,6 +25,7 @@ CONFIG_USE_PREBOOT=y CONFIG_PREBOOT="if hdmidet; then usb start; setenv stdin serial,usbkbd; setenv stdout serial,vga; setenv stderr serial,vga; else setenv stdin serial; setenv stdout serial; setenv stderr serial; fi;" CONFIG_BOUNCE_BUFFER=y CONFIG_BOARD_EARLY_INIT_F=y +CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_FS_EXT4=y CONFIG_SPL_I2C_SUPPORT=y CONFIG_SPL_WATCHDOG_SUPPORT=y @@ -37,6 +38,7 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4_WRITE=y # CONFIG_SPL_PARTITION_UUIDS is not set CONFIG_OF_CONTROL=y +CONFIG_SPL_OF_CONTROL=y CONFIG_DEFAULT_DEVICE_TREE="imx6dl-hummingboard2-emmc-som-v15" CONFIG_OF_LIST="imx6dl-hummingboard2-emmc-som-v15 imx6q-hummingboard2-emmc-som-v15" CONFIG_MULTI_DTB_FIT=y @@ -45,6 +47,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_DM=y +CONFIG_SPL_DM=y CONFIG_DWC_AHSATA=y CONFIG_DM_MMC=y CONFIG_FSL_USDHC=y -- 2.20.1
[PATCH 0/4] mx6cuboxi: enable support for OF_CONTROL and DM in SPL
Make an additional step to add full DM support to mx6cuboxi, including its support for SPL With this new configuration SPL image is 50 KB, higher than the 38 KB from the previous version, but it still under the 68 KB limit. Walter Lozano (4): mx6cuboxi: enable MMC and eMMC in DT for SPL mx6cuboxi: enable MMC iomux on board_early_init_f mx6cuboxi: customize board_boot_order to access eMMC mx6cuboxi: enable OF_CONTROL and DM in SPL ...qdl-hummingboard2-emmc-som-v15-u-boot.dtsi | 8 ++ board/solidrun/mx6cuboxi/mx6cuboxi.c | 74 +++ configs/mx6cuboxi_defconfig | 3 + 3 files changed, 85 insertions(+) -- 2.20.1
[PATCH 1/4] mx6cuboxi: enable MMC and eMMC in DT for SPL
Signed-off-by: Walter Lozano
---
.../dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi| 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
index d302b2e275..400b885e43 100644
--- a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
+++ b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
@@ -34,3 +34,11 @@
{
status = "disabled";
};
+
+ {
+ u-boot,dm-pre-reloc;
+};
+
+ {
+ u-boot,dm-pre-reloc;
+};
--
2.20.1
[PATCH 3/4] mx6cuboxi: customize board_boot_order to access eMMC
In SPL legacy code only one MMC device is created, based on BOOT_CFG
register, which can be either SD or eMMC. In this context
board_boot_order return always MMC1 when configure to boot from
SD/eMMC. After switching to DM both SD and eMMC devices are created
based on the information available on DT, but as board_boot_order
only returns MMC1 is not possible to boot from eMMC.
This patch customizes board_boot_order taking into account BOOT_CFG
register to point to correct MMC1 / MMC2 device.
Signed-off-by: Walter Lozano
---
board/solidrun/mx6cuboxi/mx6cuboxi.c | 49
1 file changed, 49 insertions(+)
diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c
b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 71c77ad2a2..3ce122e8b9 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -649,6 +649,55 @@ int board_fit_config_name_match(const char *name)
return strcmp(name, tmp_name);
}
+void board_boot_order(u32 *spl_boot_list)
+{
+ struct src *psrc = (struct src *)SRC_BASE_ADDR;
+ unsigned int reg = readl(>sbmr1) >> 11;
+ u32 boot_mode = imx6_src_get_boot_mode() & IMX6_BMODE_MASK;
+ unsigned int bmode = readl(_base->sbmr2);
+
+ /* If bmode is serial or USB phy is active, return serial */
+ if (((bmode >> 24) & 0x03) == 0x01 || is_usbotg_phy_active()) {
+ spl_boot_list[0] = BOOT_DEVICE_BOARD;
+ return;
+ }
+
+ switch (boot_mode >> IMX6_BMODE_SHIFT) {
+ case IMX6_BMODE_NAND_MIN ... IMX6_BMODE_NAND_MAX:
+ spl_boot_list[0] = BOOT_DEVICE_NAND;
+ break;
+ case IMX6_BMODE_SD:
+ case IMX6_BMODE_ESD:
+ case IMX6_BMODE_MMC:
+ case IMX6_BMODE_EMMC:
+ /*
+* Upon reading BOOT_CFG register the following map is done:
+* Bit 11 and 12 of BOOT_CFG register can determine the current
+* mmc port
+* 0x1 SD2
+* 0x2 SD3
+*/
+
+ reg &= 0x3; /* Only care about bottom 2 bits */
+ switch (reg) {
+ case 1:
+ spl_boot_list[0] = BOOT_DEVICE_MMC1;
+ break;
+ case 2:
+ spl_boot_list[0] = BOOT_DEVICE_MMC2;
+ break;
+ }
+ break;
+ default:
+ /* By default use USB downloader */
+ spl_boot_list[0] = BOOT_DEVICE_BOARD;
+ break;
+ }
+
+ /* As a last resort, use serial downloader */
+ spl_boot_list[1] = BOOT_DEVICE_BOARD;
+}
+
#ifdef CONFIG_SPL_BUILD
#include
static const struct mx6dq_iomux_ddr_regs mx6q_ddr_ioregs = {
--
2.20.1
[PATCH 2/4] mx6cuboxi: enable MMC iomux on board_early_init_f
MMC iomux is done on board_mmc_init which is valid when DM_MMC is not
enabled. After enabling it, the iomux setup needs to be moved to a
valid place.
This patch moves the MMC iomux to board_early_init_f where other iomux
is done.
Signed-off-by: Walter Lozano
---
board/solidrun/mx6cuboxi/mx6cuboxi.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c
b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 6a96f9ecdb..71c77ad2a2 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -179,6 +179,28 @@ int board_mmc_get_env_dev(int devno)
#define USDHC2_CD_GPIO IMX_GPIO_NR(1, 4)
+static int setup_iomux_mmc(void)
+{
+ struct src *psrc = (struct src *)SRC_BASE_ADDR;
+ unsigned reg = readl(>sbmr1) >> 11;
+
+ /*
+* Upon reading BOOT_CFG register the following map is done:
+* Bit 11 and 12 of BOOT_CFG register can determine the current
+* mmc port
+* 0x1 SD2
+* 0x2 SD3
+*/
+ switch (reg & 0x3) {
+ case 0x1:
+ SETUP_IOMUX_PADS(usdhc2_pads);
+ case 0x2:
+ SETUP_IOMUX_PADS(usdhc3_pads);
+ }
+
+ return 0;
+}
+
int board_mmc_getcd(struct mmc *mmc)
{
struct fsl_esdhc_cfg *cfg = mmc->priv;
@@ -432,9 +454,12 @@ int board_early_init_f(void)
{
setup_iomux_uart();
+ setup_iomux_mmc();
+
#ifdef CONFIG_CMD_SATA
setup_sata();
#endif
+
return 0;
}
--
2.20.1
Re: [U-Boot] [PATCH V3 15/27] imx8m: Fix MMU table issue for OPTEE memory
Hi Peng,
On Tue, Aug 27, 2019 at 9:38 AM Peng Fan wrote:
>
> When running with OPTEE, the MMU table in u-boot does not remove the OPTEE
> memory from its settings. So ARM speculative prefetch in u-boot may access
> that OPTEE memory. Due to trust zone is enabled by OPTEE and that memory
> is set to secure access, then the speculative prefetch will fail and cause
> various memory issue in u-boot.
> The fail address register and int_status register in trustzone has logged
> that speculative access from u-boot.
>
> Signed-off-by: Peng Fan
> ---
> arch/arm/mach-imx/imx8m/soc.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 5115471eff..dd393b581b 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -112,16 +112,18 @@ static struct mm_region imx8m_mem_map[] = {
> /* DRAM1 */
> .virt = 0x4000UL,
> .phys = 0x4000UL,
> - .size = 0xC000UL,
> + .size = PHYS_SDRAM_SIZE,
> .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> PTE_BLOCK_OUTER_SHARE
> +#ifdef PHYS_SDRAM_2_SIZE
> }, {
> /* DRAM2 */
> .virt = 0x1UL,
> .phys = 0x1UL,
> - .size = 0x04000UL,
> + .size = PHYS_SDRAM_2_SIZE,
> .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> PTE_BLOCK_OUTER_SHARE
> +#endif
> }, {
> /* List terminator */
> 0,
> @@ -130,6 +132,20 @@ static struct mm_region imx8m_mem_map[] = {
>
> struct mm_region *mem_map = imx8m_mem_map;
>
> +void enable_caches(void)
> +{
> + /*
> +* If OPTEE runs, remove OPTEE memory from MMU table to
> +* avoid speculative prefetch. OPTEE runs at the top of
> +* the first memory bank
> +*/
> + if (rom_pointer[1])
> + imx8m_mem_map[5].size -= rom_pointer[1];
> +
> + icache_enable();
> + dcache_enable();
> +}
> +
> static u32 get_cpu_variant_type(u32 type)
> {
> struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR;
> --
> 2.16.4
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Unfortunately, I'm still facing a similar issue when OP-TEE is loaded on
iMX8MM-based module, despite BL32_BASE(0xbe00) +
BL32_SIZE(0x200) region is excluded.
Based on debug ouput the latest one is:
idx=2 PTE bfff7010 at level 1: bfffd003
Checking if pte fits for virt=bde0 size=20 blocksize=4000
addr=bde0 level=2
idx=2 PTE bfff7010 at level 1: bfffd003
idx=1ef PTE bfffdf78 at level 2: 0
Checking if pte fits for virt=bde0 size=20 blocksize=20
Setting PTE bfffdf78 to block virt=bde0
U-Boot hangs just after writing to System Control Register in
arch/arm/cpu/armv8/cache_v8.c:419:
/* enable the mmu */
set_sctlr(get_sctlr() | CR_M);
In my setup mainline U-Boot/TF-A/OP-TEE are used:
u-boot: 133276f14a ("Merge branch '2020-02-25-master-imports'")
tf-a: 6e46981f84 ("Merge "Update pathnames in maintainers.rst file"
into integration")
op-tee: a67dc424ff ("ta: pkcs11: API for slot/token information")
OP-TEE is compiled with these flags:
CFG_ARM64_core=y
CFG_NXP_CAAM=n
CFG_NXPCRYPT=n
PLATFORM=imx-mx8mmevk
CFG_TEE_CORE_LOG_LEVEL=4
DEBUG=y
CFG_TEE_CORE_DEBUG=y
With DCACHE disabled obviously everything works. Also I don't face any issues
if TF-A is compiled without SPD=opteed.
Maybe it rings the bell and you already saw similar issues before?
Thanks
--
Best regards - Freundliche Grüsse - Meilleures salutations
Igor Opaniuk
mailto: igor.opan...@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk
Re: [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
Hello Marek,
On Wed, 2020-03-04 at 15:30 +0100, Marek Vasut wrote:
> On 3/4/20 3:23 PM, Harald Seiler wrote:
> > From: Claudius Heine
> >
> > imx8m has the only implementation of `reset_cpu` which does not ignore
> > the addr parameter and instead gives it some meaning as the base address
> > of watchdog registers. This breaks convention with the rest of U-Boot
> > where the parameter is ignored and callers are passing in 0.
> >
> > Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
> > Co-Authored-by: Harald Seiler
> > Signed-off-by: Claudius Heine
> > Signed-off-by: Harald Seiler
> > ---
> > arch/arm/mach-imx/imx8m/soc.c | 5 +
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> > index 7fcbd53f3020..2d3afc61a452 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
> > #if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
> > void reset_cpu(ulong addr)
> > {
> > - struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
> > -
> > - if (!addr)
> > - wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> > + struct watchdog_regs *wdog = (struct watchdog_regs
> > *)WDOG1_BASE_ADDR;
> >
> > /* Clear WDA to trigger WDOG_B immediately */
> > writew((WCR_WDE | WCR_SRS), >wcr);
>
> Can't we remove the param altogether ?
Yes, that is what I would suggest as well. Although I'd do that as
a separate follow up because it is not ARM nor i.MX specific.
> Otherwise
> Reviewed-by: Marek Vasut
--
Harald
Re: [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
On 3/4/20 3:23 PM, Harald Seiler wrote:
> From: Claudius Heine
>
> imx8m has the only implementation of `reset_cpu` which does not ignore
> the addr parameter and instead gives it some meaning as the base address
> of watchdog registers. This breaks convention with the rest of U-Boot
> where the parameter is ignored and callers are passing in 0.
>
> Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
> Co-Authored-by: Harald Seiler
> Signed-off-by: Claudius Heine
> Signed-off-by: Harald Seiler
> ---
> arch/arm/mach-imx/imx8m/soc.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 7fcbd53f3020..2d3afc61a452 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
> #if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
> void reset_cpu(ulong addr)
> {
> - struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
> -
> - if (!addr)
> -wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> + struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
>
> /* Clear WDA to trigger WDOG_B immediately */
> writew((WCR_WDE | WCR_SRS), >wcr);
Can't we remove the param altogether ?
Otherwise
Reviewed-by: Marek Vasut
Re: [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
On 3/4/20 3:23 PM, Harald Seiler wrote: > From: Claudius Heine > > In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available > anywere, even if SYSRESET is disabled for SPL/TPL. > > 'do_reset' is called from SPL for instance from the panic handler and > PANIC_HANG is not set > > Signed-off-by: Claudius Heine Reviewed-by: Marek Vasut
Re: [PATCH 2/3] imx: imx8m*: Remove do_reset from board files
On 3/4/20 3:23 PM, Harald Seiler wrote: > From: Claudius Heine > > Use the `do_reset` implementation of `arch/arm/lib/reset.c` in SPL > instead. It is very close to what is done here, anyway, and plays > more nicely with the rest of U-Boot than adding a custom `do_reset` > implementation into board files. > > `do_reset` from `arch/arm/lib/reset.c` calls `reset_cpu` with 0 as the > addr parameter while the boards are passing WDOG1_BASE_ADDR. This is > ok because the `reset_cpu` implementation uses WDOG1_BASE_ADDR by > default if 0 is passed in. > > Co-Authored-by: Harald Seiler > Signed-off-by: Claudius Heine > Signed-off-by: Harald Seiler > --- > board/freescale/imx8mm_evk/spl.c | 9 - > board/freescale/imx8mn_evk/spl.c | 9 - > board/freescale/imx8mp_evk/spl.c | 9 - > board/toradex/verdin-imx8mm/spl.c | 9 - > 4 files changed, 36 deletions(-) Reviewed-by: Marek Vasut
[PATCH 2/3] imx: imx8m*: Remove do_reset from board files
From: Claudius Heine
Use the `do_reset` implementation of `arch/arm/lib/reset.c` in SPL
instead. It is very close to what is done here, anyway, and plays
more nicely with the rest of U-Boot than adding a custom `do_reset`
implementation into board files.
`do_reset` from `arch/arm/lib/reset.c` calls `reset_cpu` with 0 as the
addr parameter while the boards are passing WDOG1_BASE_ADDR. This is
ok because the `reset_cpu` implementation uses WDOG1_BASE_ADDR by
default if 0 is passed in.
Co-Authored-by: Harald Seiler
Signed-off-by: Claudius Heine
Signed-off-by: Harald Seiler
---
board/freescale/imx8mm_evk/spl.c | 9 -
board/freescale/imx8mn_evk/spl.c | 9 -
board/freescale/imx8mp_evk/spl.c | 9 -
board/toradex/verdin-imx8mm/spl.c | 9 -
4 files changed, 36 deletions(-)
diff --git a/board/freescale/imx8mm_evk/spl.c b/board/freescale/imx8mm_evk/spl.c
index 5d17f397cb68..4d34622465b3 100644
--- a/board/freescale/imx8mm_evk/spl.c
+++ b/board/freescale/imx8mm_evk/spl.c
@@ -161,12 +161,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts ("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/board/freescale/imx8mn_evk/spl.c b/board/freescale/imx8mn_evk/spl.c
index 7aed14c52b68..45417b24464d 100644
--- a/board/freescale/imx8mn_evk/spl.c
+++ b/board/freescale/imx8mn_evk/spl.c
@@ -114,12 +114,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/board/freescale/imx8mp_evk/spl.c b/board/freescale/imx8mp_evk/spl.c
index 0b20668e2b30..39c1dae684ac 100644
--- a/board/freescale/imx8mp_evk/spl.c
+++ b/board/freescale/imx8mp_evk/spl.c
@@ -149,12 +149,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/board/toradex/verdin-imx8mm/spl.c
b/board/toradex/verdin-imx8mm/spl.c
index a5dc54082054..dc5bd84f332e 100644
--- a/board/toradex/verdin-imx8mm/spl.c
+++ b/board/toradex/verdin-imx8mm/spl.c
@@ -169,12 +169,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
--
2.24.1
[PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used
Hello, continuing on the discussion around Claudius' patch for fixing reset in SPL [1] we have taken a closer look at the issue. To quickly summarize the situation: The original patch was to enable the generic ARM implementation of `do_reset` if CONFIG_SYSRESET is not enabled in SPL. This would break compilation for some boards which define their own `do_reset` in `board/*/spl.c`. To be more specific, the following 4 boards have this custom `do_reset`: - toradex/verdin-imx8mm - freescale/imx8mn_evk - freescale/imx8mm_evk - freescale/imx8mp_evk I hope we can all agree that `do_reset` is not at all meant to be implemented in board files. From looking at the related code for imx8m, it feels like this was just a workaround hack to archieve the same thing which Claudius has fixed. So this patch series reverts the addition of `do_reset` implementations in imx8m board files and instead switches to using the proper fix provided by Claudius. Additionally, the custom do_reset implementations were passing an address (WDOG1_BASE_ADDR) to `reset_cpu()` instead of 0. This is the only place in the entire U-Boot tree where this happens. Instead, all other implementations simply ignore the parameter and 0 is passed by callers. It looks a lot like this is a legacy left-over which makes me think that using it for a (hard-coded) watchdog address is not a good idea as it breaks convention with the rest of U-Boot. [1]: https://patchwork.ozlabs.org/patch/1201959 Claudius Heine (3): ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them imx: imx8m*: Remove do_reset from board files imx: imx8m: Don't use the addr parameter of reset_cpu arch/arm/lib/Makefile | 2 +- arch/arm/mach-imx/imx8m/soc.c | 5 + board/freescale/imx8mm_evk/spl.c | 9 - board/freescale/imx8mn_evk/spl.c | 9 - board/freescale/imx8mp_evk/spl.c | 9 - board/toradex/verdin-imx8mm/spl.c | 9 - 6 files changed, 2 insertions(+), 41 deletions(-) -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
[PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
From: Claudius Heine
imx8m has the only implementation of `reset_cpu` which does not ignore
the addr parameter and instead gives it some meaning as the base address
of watchdog registers. This breaks convention with the rest of U-Boot
where the parameter is ignored and callers are passing in 0.
Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
Co-Authored-by: Harald Seiler
Signed-off-by: Claudius Heine
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/imx8m/soc.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 7fcbd53f3020..2d3afc61a452 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
void reset_cpu(ulong addr)
{
- struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
-
- if (!addr)
- wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
+ struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
/* Clear WDA to trigger WDOG_B immediately */
writew((WCR_WDE | WCR_SRS), >wcr);
--
2.24.1
[PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
From: Claudius Heine In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL. 'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set Signed-off-by: Claudius Heine --- arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 8482f5446c5c..b839aa7a5096 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -57,7 +57,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif -- 2.24.1
Re: [PATCH v3 1/3] usb: dwc3-of-simple: Drop redundant inclding header file
On 3/4/20 1:59 AM, Kever Yang wrote: > The fdtdec.h is no use in this file, remove the include code. > > Signed-off-by: Kever Yang > --- > > Changes in v3: None There's typo in the subject, I'll fix it when applying.
Re: [PATCH v3 3/3] usb: Migrate to support live DT for some driver
On 3/4/20 1:59 AM, Kever Yang wrote: > Use ofnode_ instead of fdt_ APIs so that the drivers can support live DT. > This patch updates usb_get_dr_mode() and usb_get_maximum_speed() to use > ofnode as parameter instead of fdt offset. And all the drivers who use > these APIs update to use live dt APIs at the same time. > > Signed-off-by: Kever Yang I'll wait for RB from Simon on 2/3 and 3/3 , however they look good to me.
[PULL] u-boot-socfpga/master
The following changes since commit 9e1d65f36b83c5422ece3c0ea28d07a2246cb07f: configs: Resync with savedefconfig (2020-02-28 13:28:38 -0500) are available in the Git repository at: git://git.denx.de/u-boot-socfpga.git master for you to fetch changes up to 468ba8d00b5af7828302e297736ae23d4873cfb0: ARM: socfpga: Add initial support for the ABB SECU board (2020-03-03 22:11:36 +0100) Holger Brunck (1): ARM: socfpga: Add initial support for the ABB SECU board Marek Vasut (3): rtc: m41t62: add compatible for m41st87 ARM: socfpga: Permit overriding the default timer frequency ARM: socfpga: Add missing Denali NAND config options arch/arm/dts/Makefile | 1 + arch/arm/dts/socfpga_arria5_secu1.dts | 130 ++ arch/arm/mach-socfpga/Kconfig | 10 ++ board/keymile/Kconfig | 11 +- board/keymile/common/ivm.c | 19 ++- board/keymile/secu1/MAINTAINERS | 5 + board/keymile/secu1/Makefile| 7 + board/keymile/secu1/qts/iocsr_config.h | 694 + board/keymile/secu1/qts/pinmux_config.h | 218 + board/keymile/secu1/qts/pll_config.h| 83 board/keymile/secu1/qts/sdram_config.h | 327 board/keymile/secu1/socfpga.c | 67 + configs/socfpga_secu1_defconfig | 84 drivers/rtc/m41t62.c| 1 + include/configs/socfpga_arria5_secu1.h | 131 ++ include/configs/socfpga_common.h| 4 +- 16 files changed, 1787 insertions(+), 5 deletions(-) create mode 100644 arch/arm/dts/socfpga_arria5_secu1.dts create mode 100644 board/keymile/secu1/MAINTAINERS create mode 100644 board/keymile/secu1/Makefile create mode 100644 board/keymile/secu1/qts/iocsr_config.h create mode 100644 board/keymile/secu1/qts/pinmux_config.h create mode 100644 board/keymile/secu1/qts/pll_config.h create mode 100644 board/keymile/secu1/qts/sdram_config.h create mode 100644 board/keymile/secu1/socfpga.c create mode 100644 configs/socfpga_secu1_defconfig create mode 100644 include/configs/socfpga_arria5_secu1.h
Re: [PATCH 2/2] configs: socfpga: Add QSPI boot for Arria 10 SoCDK
On 3/4/20 10:31 AM, Ley Foon Tan wrote:
> On Tue, Mar 3, 2020 at 8:16 PM Marek Vasut wrote:
>>
>> On 3/3/20 10:21 AM, Ley Foon Tan wrote:
>>> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut wrote:
On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan
> wrote:
>>
>> Add QSPI boot settings for Arria 10 SoCDK.
>>
>> Signed-off-by: Ley Foon Tan
>> ---
>> include/configs/socfpga_arria10_socdk.h | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/include/configs/socfpga_arria10_socdk.h
>> b/include/configs/socfpga_arria10_socdk.h
>> index 645e66e6b0..e1d01c095f 100644
>> --- a/include/configs/socfpga_arria10_socdk.h
>> +++ b/include/configs/socfpga_arria10_socdk.h
>> @@ -39,6 +39,15 @@
>> /* SPL memory allocation configuration, this is for FAT implementation
>> */
>> #define CONFIG_SYS_SPL_MALLOC_SIZE 0x00015000
>>
>> +#define KERNEL_FIT_ADDR__stringify(0x120)
>> +
>> +#define SOCFPGA_BOOT_SETTINGS \
>> + "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
>> + "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
>> + "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
>> + "bootm ${scriptaddr}\0" \
>> + "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
>> +
>> /* The rest of the configuration is shared */
>> #include
>>
>
> Any comment on this patch?
Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
kernel address ?
>>> Yes, I know SPI flash can use UBI FS too, but we only enable jffs2 now.
>>
>> JFFS2 is dead for a very long time. I only ever met it on some archaic
>> altera hardware, everywhere else it's UBI, which makes me wonder -- what
>> is the reason Altera is sticking to this antique ?
>
> We haven't enable UBI for Gen5. Tien Fong did enable UBI for A10 and
> submit the patches before.
> But, he is busy on other tasks now and haven't continue rework the patches.
> But, Gen5 haven't enable UBI yet.
Well, just enable it, it's two config options.
>>> kernelfit_addr is for kernel fit image offset in SPI flash, it is
>>> different from kernel_addr_r.
>>
>> Shouldn't the MTD layout of the SPI NOR be described in mtdparts ?
> It doesn't use mtdparts now. I will try enable it.
Thanks!
[PULL] u-boot-usb/master
The following changes since commit 9e1d65f36b83c5422ece3c0ea28d07a2246cb07f: configs: Resync with savedefconfig (2020-02-28 13:28:38 -0500) are available in the Git repository at: git://git.denx.de/u-boot-usb.git master for you to fetch changes up to 491cabf067c6a8bb4a5859ab9f7e0ddc5b5347c8: gadget: f_thor: add missing line breaks for pr_err() (2020-03-01 21:58:54 +0100) Andy Shevchenko (1): dfu: Reset timeout in case of DFU request Seung-Woo Kim (1): gadget: f_thor: add missing line breaks for pr_err() drivers/usb/gadget/f_dfu.c | 5 + drivers/usb/gadget/f_thor.c | 24 2 files changed, 17 insertions(+), 12 deletions(-)
Re: [PATCH] arm: socfpga: arria10: Add save_boot_params()
On 3/4/20 1:36 AM, Tan, Ley Foon wrote: > > >> -Original Message- >> From: Marek Vasut >> Sent: Tuesday, March 3, 2020 8:13 PM >> To: Ley Foon Tan >> Cc: Tan, Ley Foon ; u-boot@lists.denx.de; Simon >> Goldschmidt ; See, Chin Liang >> ; Chee, Tien Fong >> Subject: Re: [PATCH] arm: socfpga: arria10: Add save_boot_params() >> >> On 3/3/20 10:14 AM, Ley Foon Tan wrote: >>> On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut wrote: On 3/2/20 8:20 AM, Tan, Ley Foon wrote: Hi, [...] >> On 2/26/20 8:01 PM, Ley Foon Tan wrote: >> [...] >>> +#define BOOTROM_SHARED_MEM_ADDR >> (CONFIG_SYS_INIT_RAM_ADDR + 0x4 \ >>> +- 0x800) #define >>> +RST_STATUS_SHARED_ADDR >> (BOOTROM_SHARED_MEM_ADDR + 0x438) >> >> Are all these magic values needed or is there some more descriptive >> macro name available for them ? > 0x4 is onchip ram size and 0x800 is 2KB size. > I can convert these to 2 macos. Aren't those already defined in include/configs/socfpga_common.h ? >>> socfpga_common.h have this: >>> #define CONFIG_SYS_INIT_RAM_SIZE(0x4 - >> CONFIG_SYS_SPL_MALLOC_SIZE) >>> >>> But, we can't use it here. >> >> Why ? > CONFIG_SYS_INIT_RAM_SIZE is minus with CONFIG_SYS_SPL_MALLOC_SIZE, not 256KB > (0x4). Then define the init ram size macro somewhere in arch/arm/mach-socfpga/include and use it in both the configs/ and your code ? :)
[PATCH 0/1] arm: mediatek: remove unused binman config
This patch fix mt7622 and mt7623 boot problem caused by BINMAN_FDT Sam Shih (1): arm: mediatek: remove unuse binman config arch/arm/Kconfig | 1 - arch/arm/mach-mediatek/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) -- 2.17.1
[PATCH 1/1] arm: mediatek: remove unused binman config
The binman-option BINMAN_FDT is introduced by this commit:
commit 3c10dc95bdd0 ("binman: Add a library to access binman entries")
BINMAN_FDT being selected when BINMAN=y that resulting in mt7623
and mt7622 are unable to boot. The root cause of this issue is commit:
commit cbd2fba1eca1 ("arm: MediaTek: add basic support for MT7629 boards")
select BINMAN=y in all mediatek SoCs, and others mediatek SoCs not
expect to use BINMAN_FDT.
This patch remove BINMAN=y option when ARCH_MEDIATEK=y and
move this to the specify SoCs part config.
Signed-off-by: Sam Shih
---
Used:
https://patchwork.ozlabs.org/patch/1242084/
Post this patch separately because it's not related to pwm
---
arch/arm/Kconfig | 1 -
arch/arm/mach-mediatek/Kconfig | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 8d9f7fcce7..9ca3d90aa2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -778,7 +778,6 @@ config ARCH_MESON
config ARCH_MEDIATEK
bool "MediaTek SoCs"
- select BINMAN
select DM
select OF_CONTROL
select SPL_DM if SPL
diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
index 17b84db5a8..0042e57017 100644
--- a/arch/arm/mach-mediatek/Kconfig
+++ b/arch/arm/mach-mediatek/Kconfig
@@ -36,6 +36,7 @@ config TARGET_MT7629
bool "MediaTek MT7629 SoC"
select CPU_V7A
select SPL
+ select BINMAN
help
The MediaTek MT7629 is a ARM-based SoC with a dual-core Cortex-A7
including DDR3, crypto engine, 3x3 11n/ac Wi-Fi, Gigabit Ethernet,
--
2.17.1
Re: [PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT
Quoting Andre Przywara (2020-03-04 11:59:40) > On Tue, 3 Mar 2020 16:08:00 +0100 > Jonas Smedegaard wrote: > > Tested-by: Jonas Smedegaard > > Some nit: This is somewhat implicit when you are the author. At least that's > the hope ;-) Ah, ok. Will keep that in mind for future patch proposals. > > Signed-off-by: Jonas Smedegaard > > Reviewed-by: Andre Przywara Thanks! > > Series-Cc: Jagan Teki > > Series-Cc: Lukasz Majewski > > Series-Cc: Andre Przywara > > Is this because of patman? If this applies to the whole series, I > typically just add CC:s to the git send-email command line. that keeps > the commits cleaner. I am wondering if this tag should be added to the > cover letter then, because patman requires those tags only in one > commit of a series. You mean you just memorize relevant Cc addresses and add them manually? I am new to patman and to contributing patches like this, and just fumbled my way to something that seemed to work. I shall try adopt your described approach for future patch proposals, thanks! - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: signature
Re: [PATCH v3 1/2] sunxi: fix support board-specific CONFIG_PREBOOT
On Tue, 3 Mar 2020 16:08:00 +0100
Jonas Smedegaard wrote:
Hi,
> commit 37304aaf60bf ("Convert CONFIG_USE_PREBOOT and CONFIG_PREBOOT to
> Kconfig") intended to support CONFIG_PREBOOT, but
> include/configs/sunxi-common.h hardcodes preboot as part of internally
> defined CONSOLE_STDIN_SETTINGS, silently ignoring any board-specific
> CONFIG_PREBOOT.
>
> This commit moves sunxi-specific CONFIG_PREBOOT to Kconfig,
> which supports board-specific override.
Yes, thanks for that! Actually seems to fix some minor annoyance as well, were
preboot was defined twice in the default environment.
> Tested-by: Jonas Smedegaard
Some nit: This is somewhat implicit when you are the author. At least that's
the hope ;-)
> Signed-off-by: Jonas Smedegaard
Reviewed-by: Andre Przywara
> Series-Cc: Jagan Teki
> Series-Cc: Lukasz Majewski
> Series-Cc: Andre Przywara
Is this because of patman? If this applies to the whole series, I typically
just add CC:s to the git send-email command line. that keeps the commits
cleaner. I am wondering if this tag should be added to the cover letter then,
because patman requires those tags only in one commit of a series.
Thanks,
Andre.
> ---
>
>
> Changes in v3:
> - move default setting to KConfig, thanks to Andre Przywara and Lukasz
> Majewski
>
> Changes in v2:
> - Rephrase commit message to clarify relationship with KConfig entries
>
> ---
> arch/arm/mach-sunxi/Kconfig| 3 +++
> include/configs/sunxi-common.h | 1 -
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 3a3b673430..9f16d903a0 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -48,6 +48,9 @@ config DRAM_SUN50I_H6
> Select this dram controller driver for some sun50i platforms,
> like H6.
>
> +config PREBOOT
> + default "usb start" if USB_KEYBOARD
> +
> config SUN6I_P2WI
> bool "Allwinner sun6i internal P2WI controller"
> help
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 0ef289fd64..69ef65193e 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -429,7 +429,6 @@ extern int soft_i2c_gpio_scl;
>
> #ifdef CONFIG_USB_KEYBOARD
> #define CONSOLE_STDIN_SETTINGS \
> - "preboot=usb start\0" \
> "stdin=serial,usbkbd\0"
> #else
> #define CONSOLE_STDIN_SETTINGS \
Re: [PATCH 2/2] configs: socfpga: Add QSPI boot for Arria 10 SoCDK
On Tue, Mar 3, 2020 at 8:16 PM Marek Vasut wrote:
>
> On 3/3/20 10:21 AM, Ley Foon Tan wrote:
> > On Mon, Mar 2, 2020 at 6:40 PM Marek Vasut wrote:
> >>
> >> On 3/2/20 10:33 AM, Ley Foon Tan wrote:
> >>> On Fri, Feb 21, 2020 at 9:25 AM Ley Foon Tan
> >>> wrote:
>
> Add QSPI boot settings for Arria 10 SoCDK.
>
> Signed-off-by: Ley Foon Tan
> ---
> include/configs/socfpga_arria10_socdk.h | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/configs/socfpga_arria10_socdk.h
> b/include/configs/socfpga_arria10_socdk.h
> index 645e66e6b0..e1d01c095f 100644
> --- a/include/configs/socfpga_arria10_socdk.h
> +++ b/include/configs/socfpga_arria10_socdk.h
> @@ -39,6 +39,15 @@
> /* SPL memory allocation configuration, this is for FAT implementation
> */
> #define CONFIG_SYS_SPL_MALLOC_SIZE 0x00015000
>
> +#define KERNEL_FIT_ADDR__stringify(0x120)
> +
> +#define SOCFPGA_BOOT_SETTINGS \
> + "kernelfit_addr=" KERNEL_FIT_ADDR "\0" \
> + "qspiboot=setenv bootargs " CONFIG_BOOTARGS \
> + "root=/dev/mtdblock1 rw rootfstype=jffs2;" \
> + "bootm ${scriptaddr}\0" \
> + "qspiload=sf probe; sf read ${scriptaddr} ${kernelfit_addr}\0" \
> +
> /* The rest of the configuration is shared */
> #include
>
> >>>
> >>> Any comment on this patch?
> >>
> >> Can we get rid of rootfstype=jffs2 ? It's archaic and you can use UBI on
> >> top of SPI NOR too. Also, isn't there already some kernel_addr_r for the
> >> kernel address ?
> > Yes, I know SPI flash can use UBI FS too, but we only enable jffs2 now.
>
> JFFS2 is dead for a very long time. I only ever met it on some archaic
> altera hardware, everywhere else it's UBI, which makes me wonder -- what
> is the reason Altera is sticking to this antique ?
We haven't enable UBI for Gen5. Tien Fong did enable UBI for A10 and
submit the patches before.
But, he is busy on other tasks now and haven't continue rework the patches.
But, Gen5 haven't enable UBI yet.
>
> > kernelfit_addr is for kernel fit image offset in SPI flash, it is
> > different from kernel_addr_r.
>
> Shouldn't the MTD layout of the SPI NOR be described in mtdparts ?
It doesn't use mtdparts now. I will try enable it.
Regards
Ley Foon
