RE: [PATCH 00/16] usb: gadget: update
Hi Lukasz, Are you fine with this patchset from NXP downstream? Thanks, Peng. > Subject: [PATCH 00/16] usb: gadget: update > > From: Peng Fan > > This patchset is to upstream NXP downstream gadget driver patches for > feature update. > > Jun Li (2): > usb: gadget: set correct usb_configuration for os_desc_config > usb: gadget: update os_desc_config when add config > > Li Jun (13): > usb: gadget: don't change ep name for dwc3 while ep autoconfig > usb: gadget: OS String support > usb: gadget: move utf8_to_utf16le to header file > usb: gadget: OS Feature Descriptors support > usb: gadget: add WCID support for mfgtool > usb: gadget: fastboot: add ext properties for WCID > usb: gadget: add super speed support > usb: fastboot: add super speed support > usb: gadget: dnl: set dnl to be super speed > usb: composite: force gadget to be USB2 for HS only function > usb: udc: ci: update speed handling > usb: gadget: fastboot: use correct max packet size > usb: gaget: ci: set ep's desc when enable ep > > Ye Li (1): > usb: gadget: Add ep_config call back to usb_gadget_ops > > drivers/usb/gadget/ci_udc.c | 5 +- > drivers/usb/gadget/composite.c | 392 > +--- > drivers/usb/gadget/epautoconf.c | 6 + > drivers/usb/gadget/f_fastboot.c | 83 ++- > drivers/usb/gadget/g_dnl.c | 1 + > drivers/usb/gadget/u_os_desc.h | 123 ++ > drivers/usb/gadget/usbstring.c | 74 +- > include/linux/usb/composite.h | 71 ++ > include/linux/usb/gadget.h | 9 + > include/linux/utf.h | 75 ++ > 10 files changed, 734 insertions(+), 105 deletions(-) create mode 100644 > drivers/usb/gadget/u_os_desc.h create mode 100644 include/linux/utf.h > > -- > 2.28.0
Re: [PATCH 00/16] Kconfig: Tidy up the top-level kconfig menu
On Thu, Sep 10, 2020 at 08:21:11PM -0600, Simon Glass wrote: > At present this menu is pretty messy, with quite a few minor options shown > at the top level. This series creates a few new menus and moves things > around so that the top-level menu is cleaner. > > There is more to do, but this is a start. > > > Simon Glass (16): > Kconfig: Add a 'Boot options' menu > Kconfig: Move boot menu into common/ > Kconfig: Move boot timing under boot options > Kconfig: Move boot media under boot options > Kconfig: Move autoboot options under boot options > Kconfig: Move CONFIG_BOOTDELAY under autoboot options > Kconfig: Move misc boot options under 'boot options' > Kconfig: Move SUPPORT_RAW_INITRD under boot options > Kconfig: Move DEFAULT_FDT_FILE under boot options > Kconfig: Create a new 'init options' menu > Kconfig: Move startup hooks under init options > Kconfig: MISC_INIT_R and BOARD_LATE_INIT -> start-up hooks > Kconfig: Move VERSION_VARIABLE under environment > Kconfig: Move BOUNCE_BUFFER under driver options > Kconfig: Move BOARD_TYPES under init options > Kconfig: Create a new tools menu > > Kconfig | 340 +--- > cmd/Kconfig | 117 -- > common/Kconfig | 505 ++-- > common/Kconfig.boot | 894 +++ > drivers/core/Kconfig | 11 + > dts/Kconfig | 9 - > env/Kconfig | 9 + > tools/Kconfig| 12 + > 8 files changed, 955 insertions(+), 942 deletions(-) > create mode 100644 common/Kconfig.boot > create mode 100644 tools/Kconfig For the series, applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH v2 1/1] sandbox: make SDL window resizable
Without resizing the SDL window showed by
./u-boot -D -l
is not legible on a high resolution screen.
Allow resizing the window
Signed-off-by: Heinrich Schuchardt
---
v2:
Do not maximize the window.
---
arch/sandbox/cpu/sdl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c
index 7dc3dab32e..911247123f 100644
--- a/arch/sandbox/cpu/sdl.c
+++ b/arch/sandbox/cpu/sdl.c
@@ -127,7 +127,8 @@ int sandbox_sdl_init_display(int width, int height, int
log2_bpp,
sdl.pitch = sdl.width * sdl.depth / 8;
SDL_Window *screen = SDL_CreateWindow("U-Boot", SDL_WINDOWPOS_UNDEFINED,
SDL_WINDOWPOS_UNDEFINED,
- sdl.vis_width, sdl.vis_height, 0);
+ sdl.vis_width, sdl.vis_height,
+ SDL_WINDOW_RESIZABLE);
if (!screen) {
printf("Unable to initialise SDL screen: %s\n",
SDL_GetError());
--
2.28.0
Re: [PATCH 1/1] sandbox: make SDL window resizable
On 09.10.20 19:23, Simon Glass wrote: > Hi Heinrich, > > On Thu, 8 Oct 2020 at 03:46, Heinrich Schuchardt wrote: >> >> On 05.10.20 03:41, Simon Glass wrote: >>> Hi Heinrich, >>> >>> On Mon, 28 Sep 2020 at 19:11, Heinrich Schuchardt >>> wrote: Without resizing the SDL window showed by ./u-boot -D -l is not legible on a high resolution screen. Start with a maximized window and allow resizing. Signed-off-by: Heinrich Schuchardt --- arch/sandbox/cpu/sdl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> Have you troubled the --double_lcd option? >> >> That looks better. >> >> This option is not documented in doc/arch/sandbox.rst. >> >>> >>> Does this have any effect on performance? >> >> With --double_lcd you are already doing a resize operation. I could not >> observe any performance loss for a maximized window. The BitBlt >> operations in X11 are well accelerated by current GPUs. >> >>> >>> I actually don't like this as it makes the text hard to read, and full >>> screen is annoying. I wonder if this should be a flag? >> >> I understand that you do not want the window maximized. But is anything >> wrong about making the window resizable (SDL_WINDOW_RESIZABLE)? >> >> Shall I update the patch accordingly? > > I think that is OK...does it mean that the window has scroll bars, etc? No, it will only have dragable edges and a maximize button. Best regards Heinrich
[PATCH] ARM: dts: stm32: Consistently enable internal pull-ups for SD bus
The default state of SD bus and clock line is logical HI. SD card IO is
open-drain and pulls the bus lines LO. Always enable the SD bus pull ups
to guarantee this behavior. Note that on systems with bus voltage level
shifter on the SD bus, the pull ups might also be built into the level
shifter, however that should have no negative impact.
Signed-off-by: Marek Vasut
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
arch/arm/dts/stm32mp15-pinctrl.dtsi | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/stm32mp15-pinctrl.dtsi
b/arch/arm/dts/stm32mp15-pinctrl.dtsi
index 154832983c..2f8ff44a7a 100644
--- a/arch/arm/dts/stm32mp15-pinctrl.dtsi
+++ b/arch/arm/dts/stm32mp15-pinctrl.dtsi
@@ -1184,13 +1184,13 @@
; /* SDMMC1_CMD */
slew-rate = <1>;
drive-push-pull;
- bias-disable;
+ bias-pull-up;
};
pins2 {
pinmux = ; /* SDMMC1_CK */
slew-rate = <2>;
drive-push-pull;
- bias-disable;
+ bias-pull-up;
};
};
@@ -1340,13 +1340,13 @@
; /* SDMMC2_CMD */
slew-rate = <1>;
drive-push-pull;
- bias-disable;
+ bias-pull-up;
};
pins2 {
pinmux = ; /* SDMMC2_CK */
slew-rate = <2>;
drive-push-pull;
- bias-disable;
+ bias-pull-up;
};
};
--
2.28.0
Re: [Uboot-stm32] [PATCH 0/7] arm: cache: cp15: don't map reserved region with no-map property
On Fri, 9 Oct 2020 at 19:13, Ahmad Fatoum wrote: > > Hello Patrick, > > On 10/9/20 5:52 PM, Patrick DELAUNAY wrote: > > I checked DACR behavior and CheckDomain / CheckPermission > > > > In my case the cortex A7 try to access to part of DDR / mapped cacheable > > and bufferable, protected by firewall. > > > > So to use DACR I always need to configure the MMU with several Domain > > - unsecure part of DDR as Domain 0 (DCACHE_WRITEALLOC) > > - secure part of DDR as Domain 1 (DCACHE_OFF) > > > > For other part of MMU region, the I/O registers are mapped as register with > > Domain 0 (D_CACHE_OFF) > > > > Then I can set DACR = 0x > > => Client Accesses are checked against the access permission bits in the > > TLB entry > > > > You ar right, the access permission is check only for domain configurated > > as client in DACR > > > > But in ARM architecture > > > > B2.4.8 Access permission checking > > > > CheckPermission() pseudo code > > Only check perms.ap is checked > > And perms.xp is not checked > > > > But as the secure memory is mapped cacheable by secure OS, OP-TEE > > I think to avoid to map the region is the ARM preconized solution > > As explain in my answer to ard in [1] > > > > [1] > > http://u-boot.10912.n7.nabble.com/PATCH-0-7-arm-cache-cp15-don-t-map-reserved-region-with-no-map-property-tt428715.html#a428958 > > I don't think the aliasing described in "A3.5.7 Memory access restrictions" > applies if the > same memory is mapped twice only, once in secure and another in normal world. > Data is already segregated in the caches by means of a NS bit. Only thing you > should need > to do within normal world is mapping it XN, cacheable and not be in manager > domain. > Unmapping sounds unnecessary to me. (You don't unmap peripherals you aren't > using either. > Why handle OP-TEE DRAM specially?) > Ah good point. The secure DDR is in the secure physical address space, whereas the non-secure mapping refers to non-secure physical memory. So from a coherency point of view, they are really not aliases of one another, and so the mismatched attribute concern does not apply. But this actually reinforces my previous point too: the secure and non-secure physical address spaces are disjoint, and the DT can only describe non-secure memory, so these regions don't belong in the DT given to the OS in the first place. > > > > > >> Cheers > >> Ahmad > >> > >> -- > >> Pengutronix e.K. | | > >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | > >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH] common, autoboot: sync functionality with Kconfig description
Hi Heiko, On Fri, 9 Oct 2020 at 07:21, Tom Rini wrote: > > On Wed, Oct 07, 2020 at 08:06:54AM +0200, Heiko Schocher wrote: > > > add back again special case: -2 > > autoboot with no delay and no check for abort > > > > as described in Kconfig option, see common/Kconfig > > help text for option BOOTDELAY. > > > > Signed-off-by: Heiko Schocher > > Reviewed-by: Tom Rini Reviewed-by: Simon Glass
Re: [PATCH v2 2/2] timer: Return count from timer_ops.get_count
Hi Sean,
On Wed, 7 Oct 2020 at 12:38, Sean Anderson wrote:
>
> No timer drivers return an error from get_count. Instead of possibly
> returning an error, just return the count directly.
>
> Signed-off-by: Sean Anderson
> ---
>
> Changes in v2:
> - mchp-pit64b was added since v1, so convert it
> - Document when get_count may be called, and what assumptions the timer
> subsystem makes about drivers
>
> arch/riscv/lib/andes_plmt.c | 6 ++
> arch/riscv/lib/sifive_clint.c | 6 ++
> drivers/timer/ag101p_timer.c | 5 ++---
> drivers/timer/altera_timer.c | 6 ++
> drivers/timer/arc_timer.c | 6 ++
> drivers/timer/ast_timer.c | 6 ++
> drivers/timer/atcpit100_timer.c | 5 ++---
> drivers/timer/atmel_pit_timer.c | 6 ++
> drivers/timer/cadence-ttc.c | 6 ++
> drivers/timer/dw-apb-timer.c | 6 ++
> drivers/timer/mchp-pit64b-timer.c | 6 ++
> drivers/timer/mpc83xx_timer.c | 6 ++
> drivers/timer/mtk_timer.c | 6 ++
> drivers/timer/nomadik-mtu-timer.c | 6 ++
> drivers/timer/omap-timer.c| 6 ++
> drivers/timer/ostm_timer.c| 6 ++
> drivers/timer/riscv_timer.c | 21 +
> drivers/timer/rockchip_timer.c| 5 ++---
> drivers/timer/sandbox_timer.c | 6 ++
> drivers/timer/sti-timer.c | 6 ++
> drivers/timer/stm32_timer.c | 6 ++
> drivers/timer/timer-uclass.c | 3 ++-
> drivers/timer/tsc_timer.c | 6 ++
> include/timer.h | 9 ++---
> 24 files changed, 59 insertions(+), 97 deletions(-)
>
[..]
> diff --git a/include/timer.h b/include/timer.h
> index aa9d870619..a044cb034e 100644
> --- a/include/timer.h
> +++ b/include/timer.h
> @@ -67,11 +67,14 @@ struct timer_ops {
> *
> * @dev: The timer device
> *
> -* @count: pointer that returns the current 64-bit timer count
> +* This function may be called at any time after the driver is probed.
> +* All necessary initialization must be completed by the time probe()
> +* returns. The count returned by this functions should be monotonic.
> +* This function must succeed.
> *
> -* Return: 0 if OK, -ve on error
> +* Return: The current 64-bit timer count
> */
> - int (*get_count)(struct udevice *dev, u64 *count);
> + u64 (*get_count)(struct udevice *dev);
We can require this the driver be probed before this is called. We
have the early timer for the pre-DM case.
Reviewed-by: Simon Glass
Regards,
Simon
Re: [PATCH 4/6] README: sandbox: Change reference to sandbox details
On Wed, 7 Oct 2020 at 22:17, Naoki Hayama wrote: > > doc/arch/index.rst is a list of links to each architecture. > As for the sandbox details, doc/arch/sandbox.rst looks better. > > Signed-off-by: Naoki Hayama > --- > > README | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 1/1] sandbox: make SDL window resizable
Hi Heinrich, On Thu, 8 Oct 2020 at 03:46, Heinrich Schuchardt wrote: > > On 05.10.20 03:41, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 28 Sep 2020 at 19:11, Heinrich Schuchardt > > wrote: > >> > >> Without resizing the SDL window showed by > >> > >> ./u-boot -D -l > >> > >> is not legible on a high resolution screen. > >> > >> Start with a maximized window and allow resizing. > >> > >> Signed-off-by: Heinrich Schuchardt > >> --- > >> arch/sandbox/cpu/sdl.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > > > > Have you troubled the --double_lcd option? > > That looks better. > > This option is not documented in doc/arch/sandbox.rst. > > > > > Does this have any effect on performance? > > With --double_lcd you are already doing a resize operation. I could not > observe any performance loss for a maximized window. The BitBlt > operations in X11 are well accelerated by current GPUs. > > > > > I actually don't like this as it makes the text hard to read, and full > > screen is annoying. I wonder if this should be a flag? > > I understand that you do not want the window maximized. But is anything > wrong about making the window resizable (SDL_WINDOW_RESIZABLE)? > > Shall I update the patch accordingly? I think that is OK...does it mean that the window has scroll bars, etc? Regards, Simon
Re: [PATCH] env: sf: add support for env erase
On 10/9/20 12:43 PM, Harry Waschkeit wrote: > Hi Sean, > > thanks for your try and sorry for the inconvenience my beginner's mistakes > have > caused :-( > > It is definitely no good idea to use copy&paste with patch data, I should have > guessed that beforehand ... You *can* do it, you just have to configure your mail client correctly. However, it gets pretty tedious when you have a lot of patches :) I suggest configuring git send-email. If you are going to be making more patch series, also check out tools/patman. > > Anyway, concerning the complaints from checkpatch.pl: I indeed ran > checkpatch.pl > on my patch prior to sending it - the real one, not the one as pasted into the > mail ;-/ > > The alignment warnings simply result from the fact that I adhered to the style > used in that file already, you can easily verify that by running checkpatch.pl > on the complete file. Please keep new code in the correct style. For example, you have > + ret = spi_flash_read(env_flash, saved_offset, > +saved_size, saved_buffer); which is aligned properly, but later on you have >>> + ret = spi_flash_read(env_flash, saved_offset, >>> + saved_size, saved_buffer); which is not. > > For the warnings with "IS_ENABLED(CONFIG...)" I don't see what I could do to > get around them: all three occurrences are about compiling functions into the > code depending on CONFIG_CMD_ERASEENV. > Two times it is the new function env_sf_erase(), one variant for normal and > the > other for redundand environment handling. > The third time it is used to define this new method into the structure > U_BOOT_ENV_LOCATION(sf). The macro IS_ENABLED can be used both in C code and in preprocessor directives. See include/linux/kconfig.h for details. > > The sign-off problem I guess is probably caused by the check not accepting > name > in reverse order, isn't it? Yes. The format is "First Last ". > Anyway, I will change my user.name to match the order in the mail address so > next patch is hopefully correct. > > So please let me know what else I should do beside sending a properly > formatted > patch ;-/ See below. > I will take care of that before resending my patch (v2 then, right?). Yes. > > On 10/9/20 3:55 PM, Sean Anderson wrote: >> On 10/8/20 1:27 PM, Harry Waschkeit wrote: >>> Command "env erase" didn't work even though CONFIG_CMD_ERASEENV was >>> defined, because serial flash environment routines didn't implement >>> erase method. >>> >>> Signed-off-by: Waschkeit, Harry >>> --- >> >> Hi Harry, >> >> I wanted to test out your patch, but I couldn't get it to apply. It >> appears that your mail program has replaced the tabs with spaces, so git >> can't figure out how to apply it. I tried to fix it by performing the >> substitutions >> >> s/^\(.\) /\1\t/g >> s//\t/g >> >> but it still wouldn't apply. In addition, checkpatch has a few warnings: >> >>> $ scripts/checkpatch.pl env-sf-add-support-for-env-erase.patch >>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where >>> possible >>> #152: FILE: env/sf.c:149: >>> +#ifdef CONFIG_CMD_ERASEENV >>> >>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where >>> possible >>> #240: FILE: env/sf.c:318: >>> +#ifdef CONFIG_CMD_ERASEENV >>> >>> CHECK: Alignment should match open parenthesis >>> #260: FILE: env/sf.c:338: >>> +ret = spi_flash_read(env_flash, saved_offset, >>> +saved_size, saved_buffer); >>> >>> CHECK: Alignment should match open parenthesis >>> #269: FILE: env/sf.c:347: >>> +ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET, >>> +sector * CONFIG_ENV_SECT_SIZE); >>> >>> CHECK: Alignment should match open parenthesis >>> #276: FILE: env/sf.c:354: >>> +ret = spi_flash_write(env_flash, saved_offset, >>> +saved_size, saved_buffer); >>> >>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where >>> possible >>> #307: FILE: env/sf.c:437: >>> +#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR) >>> >>> WARNING: Missing Signed-off-by: line by nominal patch author 'Harry >>> Waschkeit ' >>> >>> total: 0 errors, 4 warnings, 3 checks, 158 lines checked >>> >>> NOTE: For some of the reported defects, checkpatch may be able to >>>mechanically convert to the typical style using --fix or >>> --fix-inplace. >>> >>> env-sf-add-support-for-env-erase.patch has style problems, please review. >>> >>> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX >>> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE >>> PREFER_ETHER_ADDR_COPY USLEEP_RANGE >>> >>> NOTE: If any of the errors are false positives, please report >>>them to the maintainer, see CHECKPATCH in MAINTAINERS. >> >> Please fix these issues and resend, thanks! >> >> --Sean >> >>> env/sf.c | 130 +
RE: [Uboot-stm32] [PATCH 0/7] arm: cache: cp15: don't map reserved region with no-map property
Hi Ard,
> From: Ard Biesheuvel
> Sent: mercredi 7 octobre 2020 15:16
>
> On Wed, 7 Oct 2020 at 13:53, Ahmad Fatoum wrote:
> >
> > Hello,
> >
> > On 10/7/20 1:23 PM, Ahmad Fatoum wrote:
> > > My findings[1] back then were that U-Boot did set the eXecute Never
> > > bit only on OMAP, but not for other platforms. So I could imagine
> > > this being the root cause of Patrick's issues as well:
> >
> > Rereading my own link, my memory is a little less fuzzy: eXecute Never
> > was being set, but was without effect due Manager mode being set in the
> DACR:
> >
> > > The ARM Architecture Reference Manual notes[1]:
> > > > When using the Short-descriptor translation table format, the XN
> > > > attribute is not checked for domains marked as Manager.
> > > > Therefore, the system must not include read-sensitive memory in
> > > > domains marked as Manager, because the XN bit does not prevent
> > > > speculative fetches from a Manager domain.
> >
> > > To avoid speculative access to read-sensitive memory-mapped
> > > peripherals on ARMv7, we'll need U-Boot to use client domain
> > > permissions, so the XN bit can function.
> >
> > > This issue has come up before and was fixed in de63ac278
> > > ("ARM: mmu: Set domain permissions to client access") for OMAP2 only.
> > > It's equally applicable to all ARMv7-A platforms where caches are
> > > enabled.
> > > [1]: B3.7.2 - Execute-never restrictions on instruction fetching
> >
> > Hope this helps,
> > Ahmad
> >
>
> It most definitely does, thanks a lot.
>
> U-boot's mmu_setup() currently sets DACR to manager for all domains, so this
> is
> broken for all non-LPAE configurations running on v7 CPUs (except OMAP and
> perhaps others that fixed it individually). This affects all device mappings:
> not just
> secure DRAM for OP-TEE, but any MMIO register for any peripheral that is
> mapped into the CPU's address space.
>
> Patrick, could you please check whether this fixes the issue as well?
>
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -202,9 +202,9 @@ static inline void mmu_setup(void)
> asm volatile("mcr p15, 0, %0, c2, c0, 0"
> : : "r" (gd->arch.tlb_addr) : "memory"); #endif
> - /* Set the access control to all-supervisor */
> + /* Set the access control to client (0b01) for each of the 16
> + domains */
> asm volatile("mcr p15, 0, %0, c3, c0, 0"
> -: : "r" (~0));
> +: : "r" (0x));
>
> arm_init_domains();
The test will take some time to be sure that solve my remaining issue because
issue is not always reproductible.
At fist chek, I wasn't sure of DACR bahavior, but I found in [1] the line :
The XN attribute is not checked for domains marked as Manager.
Read-sensitive memory must
not be included in domains marked as Manager, because the XN bit does
not prevent prefetches
in these cases.
So, I need to test your patch + DCACHE_OFF instead of INVALID
(to map with XN the OP-TEE region) in my patchset.
FYI: I found the same DACR configuration is done in:
arch/arm/cpu/armv7/ls102xa/cpu.c:199
[1]
https://developer.arm.com/documentation/ddi0406/b/System-Level-Architecture/Virtual-Memory-System-Architecture--VMSA-/Memory-access-control/The-Execute-Never--XN--attribute-and-instruction-prefetching?lang=en
Patrick
For information:
At the beginning I wasn't sure that the current DACR configuration is an issue
because in found
in pseudo code of
DDI0406B_arm_architecture_reference_manual_errata_markup_8_0.pdf
B3.13.3 Address translation
if CheckDomain(tlbrecord.domain, mva, tlbrecord.sectionnotpage,
iswrite) then
CheckPermission(tlbrecord.perms, mva, tlbrecord.sectionnotpage,
iswrite, ispriv);
B3.13.4 Domain checking
boolean CheckDomain(bits(4) domain, bits(32) mva, boolean
sectionnotpage, boolean iswrite)
bitpos = 2*UInt(domain);
case DACR of
when ‘00’ DataAbort(mva, domain, sectionnotpage,
iswrite, DAbort_Domain);
when ‘01’ permissioncheck = TRUE;
when ‘10’ UNPREDICTABLE;
when ‘11’ permissioncheck = FALSE;
return permissioncheck;
B2.4.8 Access permission checking
// CheckPermission()
// =
CheckPermission(Permissions perms, bits(32) mva,
boolean sectionnotpage, bits(4) domain, boolean iswrite,
boolean ispriv)
if SCTLR.AFE == ‘0’ then
perms.ap<0> = ‘1’;
case perms.ap of
when ‘000’ abort = TRUE;
when ‘001’ abort = !ispriv;
when ‘010’ abort = !ispriv && iswrite;
when ‘011’ abort = FALSE;
when ‘100’ UNPREDICTABLE;
Re: [PATCH] env: sf: add support for env erase
Hi Sean,
thanks for your try and sorry for the inconvenience my beginner's mistakes have
caused :-(
It is definitely no good idea to use copy&paste with patch data, I should have
guessed that beforehand ...
Anyway, concerning the complaints from checkpatch.pl: I indeed ran checkpatch.pl
on my patch prior to sending it - the real one, not the one as pasted into the
mail ;-/
The alignment warnings simply result from the fact that I adhered to the style
used in that file already, you can easily verify that by running checkpatch.pl
on the complete file.
For the warnings with "IS_ENABLED(CONFIG...)" I don't see what I could do to
get around them: all three occurrences are about compiling functions into the
code depending on CONFIG_CMD_ERASEENV.
Two times it is the new function env_sf_erase(), one variant for normal and the
other for redundand environment handling.
The third time it is used to define this new method into the structure
U_BOOT_ENV_LOCATION(sf).
The sign-off problem I guess is probably caused by the check not accepting name
in reverse order, isn't it?
Anyway, I will change my user.name to match the order in the mail address so
next patch is hopefully correct.
So please let me know what else I should do beside sending a properly formatted
patch ;-/
I will take care of that before resending my patch (v2 then, right?).
On 10/9/20 3:55 PM, Sean Anderson wrote:
On 10/8/20 1:27 PM, Harry Waschkeit wrote:
Command "env erase" didn't work even though CONFIG_CMD_ERASEENV was
defined, because serial flash environment routines didn't implement
erase method.
Signed-off-by: Waschkeit, Harry
---
Hi Harry,
I wanted to test out your patch, but I couldn't get it to apply. It
appears that your mail program has replaced the tabs with spaces, so git
can't figure out how to apply it. I tried to fix it by performing the
substitutions
s/^\(.\) /\1\t/g
s//\t/g
but it still wouldn't apply. In addition, checkpatch has a few warnings:
$ scripts/checkpatch.pl env-sf-add-support-for-env-erase.patch
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
#152: FILE: env/sf.c:149:
+#ifdef CONFIG_CMD_ERASEENV
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
#240: FILE: env/sf.c:318:
+#ifdef CONFIG_CMD_ERASEENV
CHECK: Alignment should match open parenthesis
#260: FILE: env/sf.c:338:
+ ret = spi_flash_read(env_flash, saved_offset,
+ saved_size, saved_buffer);
CHECK: Alignment should match open parenthesis
#269: FILE: env/sf.c:347:
+ ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
+ sector * CONFIG_ENV_SECT_SIZE);
CHECK: Alignment should match open parenthesis
#276: FILE: env/sf.c:354:
+ ret = spi_flash_write(env_flash, saved_offset,
+ saved_size, saved_buffer);
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
#307: FILE: env/sf.c:437:
+#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR)
WARNING: Missing Signed-off-by: line by nominal patch author 'Harry Waschkeit
'
total: 0 errors, 4 warnings, 3 checks, 158 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
env-sf-add-support-for-env-erase.patch has style problems, please review.
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
PREFER_ETHER_ADDR_COPY USLEEP_RANGE
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Please fix these issues and resend, thanks!
--Sean
env/sf.c | 130 ++-
1 file changed, 128 insertions(+), 2 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..9cda192a73 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -146,6 +146,78 @@ static int env_sf_save(void)
return ret;
}
+#ifdef CONFIG_CMD_ERASEENV
+static int env_sf_erase(void)
+{
+ char*saved_buffer = NULL;
+ u32 saved_size, saved_offset, sector;
+ ulong offset;
+ ulong offsets[2] = { CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND };
+ int i;
+ int ret;
+
+ ret = setup_flash_device();
+ if (ret)
+ return ret;
+
+ /* get temporary storage if sector is larger than env (i.e. embedded) */
+ if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+ saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+ saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
+ if (!saved_buffer) {
+ ret = -ENOMEM;
+ goto done;
+ }
+ }
+
+ sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+
+ /* simply erase both environments, re
RE: [Uboot-stm32] [PATCH 0/7] arm: cache: cp15: don't map reserved region with no-map property
Hi Ahmad, > From: Ahmad Fatoum > Sent: mercredi 7 octobre 2020 13:24 > > Hello Ard, Patrick, > > On 10/7/20 12:26 PM, Ard Biesheuvel wrote: > >> The issue is solved only when the region reserved by OP-TEE is no > >> more mapped in U-Boot (mapped as DEVICE/NON-CACHEABLE wasn't > enough) > >> as it is already done in Linux kernel. > >> > > > > Spurious peculative accesses to device regions would be a severe > > silicon bug, so I wonder what is going on here. > > > > (Apologies if we are rehashing stuff here that has already been > > discussed - I don't remember the details) > > > > Are you sure that the speculative accesses were not caused by > > misconfigured CPU or page tables, missing TLB maintenance, etc etc? > > Because it really does smell like a software issue not a hardware > > issue. > > I debugged a similar issue a year ago on an i.MX6 UltraLite (also a Cortex-A7) > that turned to ultimately be caused by barebox not mapping I/O memory as non- > executable. This led to very interesting effects. > > My findings[1] back then were that U-Boot did set the eXecute Never bit only > on > OMAP, but not for other platforms. So I could imagine this being the root > cause of > Patrick's issues as well: > The CPU is speculatively executing from the region that the firewalled DRAM is > mapped at. > > barebox now configures XN for non-RAM before it turns on the MMU. You should > do that as well (in ARM arch code, not only for stm32mp1). Additionally, you > will > want to XN map the region where your OP-TEE sits at. > > [1]: https://community.nxp.com/thread/511925 Thanks to point me this thread. I checked DACR behavior and CheckDomain / CheckPermission In my case the cortex A7 try to access to part of DDR / mapped cacheable and bufferable, protected by firewall. So to use DACR I always need to configure the MMU with several Domain - unsecure part of DDR as Domain 0 (DCACHE_WRITEALLOC) - secure part of DDR as Domain 1 (DCACHE_OFF) For other part of MMU region, the I/O registers are mapped as register with Domain 0 (D_CACHE_OFF) Then I can set DACR = 0x => Client Accesses are checked against the access permission bits in the TLB entry You ar right, the access permission is check only for domain configurated as client in DACR But in ARM architecture B2.4.8 Access permission checking CheckPermission() pseudo code Only check perms.ap is checked And perms.xp is not checked But as the secure memory is mapped cacheable by secure OS, OP-TEE I think to avoid to map the region is the ARM preconized solution As explain in my answer to ard in [1] [1] http://u-boot.10912.n7.nabble.com/PATCH-0-7-arm-cache-cp15-don-t-map-reserved-region-with-no-map-property-tt428715.html#a428958 > Cheers > Ahmad > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH v2 2/2] km/common: change ubicopy variable
Instead having a hard coded value for "cramfsaddr" after compile time,
we change it to take the variable "cramfsaddr" for the ubicopy variable.
This makes sure that ubicopy uses the right address, even when
the value for "cramfsaddr" has changed.
CC: Valentin Longchamp
CC: Heiko Schocher
CC: Tom Rini
Signed-off-by: Holger Brunck
---
include/configs/km/keymile-common.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/configs/km/keymile-common.h
b/include/configs/km/keymile-common.h
index 6a8c41529f..c1968048a7 100644
--- a/include/configs/km/keymile-common.h
+++ b/include/configs/km/keymile-common.h
@@ -143,8 +143,7 @@
#define CONFIG_KM_DEF_ENV_FLASH_BOOT \
"cramfsaddr=" __stringify(CONFIG_KM_CRAMFS_ADDR) "\0" \
"cramfsloadkernel=cramfsload ${load_addr_r} ${uimage}\0"\
- "ubicopy=ubi read "__stringify(CONFIG_KM_CRAMFS_ADDR) \
- " bootfs${boot_bank}\0" \
+ "ubicopy=ubi read ${cramfsaddr} bootfs${boot_bank}\0" \
"uimage=" CONFIG_KM_UIMAGE_NAME \
CONFIG_KM_DEV_ENV_FLASH_BOOT_UBI
--
2.26.0
[PATCH v2 1/2] km: adapt defines and variables for new memory layout
Due to increasing kernel image sizes we get problems when decompressing
the kernel image. To fix this we need to change the addresses where we
load and where we extract the kernel. Also we need to adapt the address
where to load the CRAMFS image and where to load the DTB file.
While at it also harmonize all boards for PPC and ARM to have the
same values. Also we add a new variable "env_version", so that the
userspace is able to detect if this is a u-boot binary with updated
values or not.
CC: Valentin Longchamp
CC: Heiko Schocher
CC: Tom Rini
Signed-off-by: Holger Brunck
---
board/keymile/Kconfig | 12 +++-
include/configs/km/keymile-common.h | 1 +
include/configs/km/km-powerpc.h | 4
include/configs/km/km_arm.h | 3 +++
4 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/board/keymile/Kconfig b/board/keymile/Kconfig
index e20c017436..e5906906f3 100644
--- a/board/keymile/Kconfig
+++ b/board/keymile/Kconfig
@@ -37,26 +37,20 @@ config KM_RESERVED_PRAM
config KM_CRAMFS_ADDR
hex "CRAMFS Address"
- default 0x240 if ARCH_KIRKWOOD
- default 0xC0 if MPC83xx
- default 0x200 if MPC85xx
+ default 0x300
depends on !ARCH_SOCFPGA
help
Start address of the CRAMFS containing the Linux kernel.
config KM_KERNEL_ADDR
hex "Kernel Load Address"
- default 0x200 if ARCH_KIRKWOOD
- default 0x40 if MPC83xx
- default 0x100 if MPC85xx || ARCH_SOCFPGA
+ default 0x200
help
Address where to load Linux kernel in RAM.
config KM_FDT_ADDR
hex "FDT Load Address"
- default 0x23E if ARCH_KIRKWOOD || ARCH_SOCFPGA
- default 0xB8 if MPC83xx
- default 0x1F8 if MPC85xx
+ default 0x2FC
help
Address where to load flattened device tree in RAM.
diff --git a/include/configs/km/keymile-common.h
b/include/configs/km/keymile-common.h
index e9e3981060..6a8c41529f 100644
--- a/include/configs/km/keymile-common.h
+++ b/include/configs/km/keymile-common.h
@@ -160,6 +160,7 @@
"pnvramsize=" __stringify(CONFIG_KM_PNVRAM) "\0"\
"testbootcmd=setenv boot_bank ${test_bank}; " \
"run ${subbootcmds}; reset\0" \
+ "env_version=1\0" \
""
#ifndef CONFIG_KM_DEF_ENV
diff --git a/include/configs/km/km-powerpc.h b/include/configs/km/km-powerpc.h
index fde8487178..7bfe12fecb 100644
--- a/include/configs/km/km-powerpc.h
+++ b/include/configs/km/km-powerpc.h
@@ -21,6 +21,9 @@
/* Reserve 4 MB for malloc */
#define CONFIG_SYS_MALLOC_LEN (4 * 1024 * 1024)
+/* Increase max size of compressed kernel */
+#define CONFIG_SYS_BOOTM_LEN 0x200 /* 32 MB */
+
/**
* (PRAM usage)
* ... ---
@@ -53,6 +56,7 @@
"protect on " __stringify(BOOTFLASH_START) " +${filesize}\0"\
"set_fdthigh=true\0"\
"checkfdt=true\0" \
+ "bootm_mapsize=" __stringify(CONFIG_SYS_BOOTM_LEN) "\0" \
""
#endif /* __CONFIG_KEYMILE_POWERPC_H */
diff --git a/include/configs/km/km_arm.h b/include/configs/km/km_arm.h
index 79edfa728a..98e0ce1c24 100644
--- a/include/configs/km/km_arm.h
+++ b/include/configs/km/km_arm.h
@@ -35,6 +35,9 @@
/* Reserve 4 MB for malloc */
#define CONFIG_SYS_MALLOC_LEN (4 * 1024 * 1024)
+/* Increase max size of compressed kernel */
+#define CONFIG_SYS_BOOTM_LEN (32 << 20)
+
#include "asm/arch/config.h"
#define CONFIG_SYS_LOAD_ADDR 0x0080 /* default load adr- 8M */
--
2.26.0
Re: [PATCH] env: sf: add support for env erase
On 10/8/20 1:27 PM, Harry Waschkeit wrote:
> Command "env erase" didn't work even though CONFIG_CMD_ERASEENV was
> defined, because serial flash environment routines didn't implement
> erase method.
>
> Signed-off-by: Waschkeit, Harry
> ---
Hi Harry,
I wanted to test out your patch, but I couldn't get it to apply. It
appears that your mail program has replaced the tabs with spaces, so git
can't figure out how to apply it. I tried to fix it by performing the
substitutions
s/^\(.\) /\1\t/g
s//\t/g
but it still wouldn't apply. In addition, checkpatch has a few warnings:
> $ scripts/checkpatch.pl env-sf-add-support-for-env-erase.patch
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> possible
> #152: FILE: env/sf.c:149:
> +#ifdef CONFIG_CMD_ERASEENV
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> possible
> #240: FILE: env/sf.c:318:
> +#ifdef CONFIG_CMD_ERASEENV
>
> CHECK: Alignment should match open parenthesis
> #260: FILE: env/sf.c:338:
> + ret = spi_flash_read(env_flash, saved_offset,
> + saved_size, saved_buffer);
>
> CHECK: Alignment should match open parenthesis
> #269: FILE: env/sf.c:347:
> + ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
> + sector * CONFIG_ENV_SECT_SIZE);
>
> CHECK: Alignment should match open parenthesis
> #276: FILE: env/sf.c:354:
> + ret = spi_flash_write(env_flash, saved_offset,
> + saved_size, saved_buffer);
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> possible
> #307: FILE: env/sf.c:437:
> +#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR)
>
> WARNING: Missing Signed-off-by: line by nominal patch author 'Harry Waschkeit
> '
>
> total: 0 errors, 4 warnings, 3 checks, 158 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> env-sf-add-support-for-env-erase.patch has style problems, please review.
>
> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
> PREFER_ETHER_ADDR_COPY USLEEP_RANGE
>
> NOTE: If any of the errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
Please fix these issues and resend, thanks!
--Sean
> env/sf.c | 130 ++-
> 1 file changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/env/sf.c b/env/sf.c
> index 937778aa37..9cda192a73 100644
> --- a/env/sf.c
> +++ b/env/sf.c
> @@ -146,6 +146,78 @@ static int env_sf_save(void)
> return ret;
> }
>
> +#ifdef CONFIG_CMD_ERASEENV
> +static int env_sf_erase(void)
> +{
> + char*saved_buffer = NULL;
> + u32 saved_size, saved_offset, sector;
> + ulong offset;
> + ulong offsets[2] = { CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND };
> + int i;
> + int ret;
> +
> + ret = setup_flash_device();
> + if (ret)
> + return ret;
> +
> + /* get temporary storage if sector is larger than env (i.e. embedded)
> */
> + if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
> + saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
> + saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
> + if (!saved_buffer) {
> + ret = -ENOMEM;
> + goto done;
> + }
> + }
> +
> + sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
> +
> + /* simply erase both environments, retaining non-env data (if any) */
> + for (i = 0; i < ARRAY_SIZE(offsets); i++) {
> + offset = offsets[i];
> +
> + if (saved_buffer) {
> + saved_offset = offset + CONFIG_ENV_SIZE;
> + ret = spi_flash_read(env_flash, saved_offset,
> +saved_size, saved_buffer);
> + if (ret)
> + goto done;
> + }
> +
> + if (i)
> + puts("Redund:");
> +
> + puts("Erasing SPI flash...");
> + ret = spi_flash_erase(env_flash, offset,
> + sector * CONFIG_ENV_SECT_SIZE);
> + if (ret)
> + goto done;
> +
> + if (saved_buffer) {
> + puts("Writing non-environment data to SPI flash...");
> + ret = spi_flash_write(env_flash, saved_offset,
> + saved_size, saved_buffer);
> + if (ret)
> + goto done;
> + }
> +
> + puts("done\n");
> + }
> +
> + /*
Re: [PATCH] common, autoboot: sync functionality with Kconfig description
On Wed, Oct 07, 2020 at 08:06:54AM +0200, Heiko Schocher wrote: > add back again special case: -2 > autoboot with no delay and no check for abort > > as described in Kconfig option, see common/Kconfig > help text for option BOOTDELAY. > > Signed-off-by: Heiko Schocher Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2] am335x_evm: Allow booting from usb-storage device
On Sun, Oct 04, 2020 at 02:23:21PM +0300, Matwey V. Kornilov wrote: > Ping? > > пн, 24 авг. 2020 г. в 21:00, Matwey V. Kornilov : > > > Signed-off-by: Matwey V. Kornilov > > --- > > include/configs/am335x_evm.h | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h > > index 9c4ef369c5..103c046137 100644 > > --- a/include/configs/am335x_evm.h > > +++ b/include/configs/am335x_evm.h > > @@ -66,6 +66,12 @@ > > #define BOOTENV_DEV_NAME_NAND(devtypeu, devtypel, instance) \ > > #devtypel #instance " " > > > > +#if CONFIG_IS_ENABLED(CMD_USB) > > +# define BOOT_TARGET_USB(func) func(USB, usb, 0) > > +#else > > +# define BOOT_TARGET_USB(func) > > +#endif > > + > > #if CONFIG_IS_ENABLED(CMD_PXE) > > # define BOOT_TARGET_PXE(func) func(PXE, pxe, na) > > #else > > @@ -84,6 +90,7 @@ > > func(MMC, mmc, 1) \ > > func(LEGACY_MMC, legacy_mmc, 1) \ > > func(NAND, nand, 0) \ > > + BOOT_TARGET_USB(func) \ > > BOOT_TARGET_PXE(func) \ > > BOOT_TARGET_DHCP(func) Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH] Define default CONFIG_PREBOOT with right config option
On Wed, Oct 07, 2020 at 08:37:57AM +, Patrick DELAUNAY wrote:
> Hi,
>
> > From: U-Boot On Behalf Of Peter Robinson
> > Sent: mardi 29 septembre 2020 11:48
> >
> > The 44758771ee commit removes CONFIG_PREBOOT but actually sets the
> > USE_PREBOOT Kconfig option which isn't CONFIG_PREBOOT and is also a bool
> > option which means we regress because 'usb start' isn't run when expected,
> > it
> > should also be run for devices that have USB storage because keyboards
> > aren't
> > the only thing we might need the USB bus for.
> >
> > Fixes: 44758771ee ("arm: move CONFIG_PREBOOT="usb start" to KConfig")
> > Signed-off-by: Peter Robinson
> > Cc: Jonas Smedegaard
> > Cc: Neil Armstrong
> > ---
> > common/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/Kconfig b/common/Kconfig index b1934b3a9c..9c20a9738e
> > 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -403,7 +403,6 @@ config BOOTCOMMAND
> >
> > config USE_PREBOOT
> > bool "Enable preboot"
> > - default "usb start" if USB_KEYBOARD
> > help
> > When this option is enabled, the existence of the environment
> > variable "preboot" will be checked immediately before starting the @@
> > -
> > 417,6 +416,7 @@ config USE_PREBOOT config PREBOOT
> > string "preboot default value"
> > depends on USE_PREBOOT && !USE_DEFAULT_ENV_FILE
> > + default "usb start" if USB_KEYBOARD || USB_STORAGE
> > default ""
> > help
> > This is the default of "preboot" environment variable.
> > --
> > 2.26.2
>
> For information, this patch cause unexpected 'usb start' on STM32MP15x boards
> and slow down the start-up in realease v2020.10.
>
> For me it is unexpected because
> - USB keyboard is not activated
> - USB storage is activated but USB boot is not supported (not managed by
> distro boot command)
>
> I sent a patch [1] for the associated defconfig but I'm afraid that other
> boards are impacted.
>
> As the USB storage boot initialization is correctly managed by distro boot
> command 'usb_boot'
> (defined in include/config_distro_bootcmd.h, it already include 'usb start'),
> I think that the
> USB_STORAGE test should be removed or limited by !DISTRO_DEFAULTS.
>
> [1] = "configs: stm32mp: force empty PREBOOT"
> http://patchwork.ozlabs.org/project/uboot/patch/20201007081020.30635-1-patrick.delau...@st.com/
Re-re-reading everything and this is a helpful explanation. This commit
is wrong as it did more than just fix 44758771ee, which put the default
in the wrong place, but added new logic that shouldn't be required.
Patrick, can you please send a new patch to fix this commit and in turn
NOT also default usb start on USB_STORAGE, only USB_KEYBOARD? Thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 1/1] rsa: fix retrieving public exponent on big-endian systems
On Wed, Oct 07, 2020 at 12:17:56AM +0200, Rasmus Villemoes wrote:
> On 07/10/2020 00.02, Simon Glass wrote:
> > Hi Rasmus,
> >
> > On Tue, 6 Oct 2020 at 04:10, Rasmus Villemoes
> > wrote:
> >>
> >> Commit fdf0819afb (rsa: fix alignment issue when getting public
> >> exponent) changed the logic to avoid doing an 8-byte access to a
> >> possibly-not-8-byte-aligned address.
> >>
> >> However, using rsa_convert_big_endian is wrong: That function converts
> >> an array of big-endian (32-bit) words with the most significant word
> >> first (aka a BE byte array) to an array of cpu-endian words with the
> >> least significant word first. While the exponent is indeed _stored_ as
> >> a big-endian 64-bit word (two BE words with MSW first), we want to
> >> extract it as a cpu-endian 64 bit word. On a little-endian host,
> >> swapping the words and byte-swapping each 32-bit word works, because
> >> that's the same as byte-swapping the whole 64 bit word. But on a
> >> big-endian host, the fdt32_to_cpu are no-ops, but
> >> rsa_convert_big_endian() still does the word-swapping, breaking
> >> verified boot.
> >>
> >> To fix that, while still ensuring we don't do unaligned accesses, add
> >> a little helper that first memcpy's the bytes to a local fdt64_t, then
> >> applies fdt64_to_cpu(). [The name is chosen based on the
> >> [bl]eXX_to_cpup in linux/byteorder/generic.h].
> >>
> >> Fixes: fdf0819afb ("rsa: fix alignment issue when getting public exponent")
> >> Signed-off-by: Rasmus Villemoes
> >> ---
> >> lib/rsa/rsa-mod-exp.c | 11 +--
> >> 1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >
> > Reviewed-by: Simon Glass
> >
> > Is there a way to add a test for this?
>
> Not that I can think of, other than finding some BE board and hooking it
> up in some CI. Apparently not very many people use verified boot on BE
> platforms :( or at least they don't follow upstream U-Boot closely.
We have tests for verified boot for sandbox. Can we not expand them to
run on qemu* including ppce500?
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 5/6] qemu-arm: Drop ARCH_SUPPORT_TFABOOT
On Wed, Sep 30, 2020 at 05:39:17PM +0100, Andre Przywara wrote: > CONFIG_ARCH_SUPPORT_TFABOOT was used on the qemu-arm64 platform to > guard a tweak to the flash bank configuration. U-Boot now reads the > current flash setup from the devicetree, so there is no need for > this option anymore. > > Signed-off-by: Andre Przywara > Reviewed-by: Tom Rini Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 6/6] qemu-arm64: Enable POSITION_INDEPENDENT
On Wed, Sep 30, 2020 at 05:39:18PM +0100, Andre Przywara wrote: > Now that PIE works when U-Boot is started from ROM, let's enable > CONFIG_POSITION_INDEPENDENT, which allows to load U-Boot also via > ARM Trusted-Firmware's fip.bin to DRAM, without tweaking the > configuration. > > To get a writable initial stack, we need to keep the fixed initial > stack pointer, which points to DRAM in our case. > > Signed-off-by: Andre Przywara > Reviewed-by: Stephen Warren Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 4/6] qemu-arm: Remove need to specify flash banks
On Wed, Sep 30, 2020 at 05:39:16PM +0100, Andre Przywara wrote:
> Currently we hard-code the number and initial addresses of QEMU's flash
> banks, even though our code is perfectly able to gather the same
> information from the DTB provided by QEMU.
> This is especially annoying, since we have two slightly different
> U-Boot configurations ("bare-metal" vs. loaded via Arm Trusted
> Firmware), which need to be selected at build time.
>
> Drop the two hard coded alternatives, and use
> CONFIG_SYS_MAX_FLASH_BANKS_DETECT instead, which relies on the DTB to
> figure out the actual flash configuration at runtime.
>
> Signed-off-by: Andre Przywara
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 3/6] arm64: PIE: Allow fixed stack pointer
On Wed, Sep 30, 2020 at 05:39:15PM +0100, Andre Przywara wrote: > Currently selecting CONFIG_POSITION_INDEPENDENT also forces us to use an > initial stack pointer relative to the beginning of the BSS section. > This makes some sense, because this should be writable memory anyway. > > However the BSS section is not cleared or used until later in the > setup process (after relocation), so memory nearby might not be > available early enough to host the initial stack. This is an issue if > U-Boot is loaded from (Flash-)ROM, for instance. > > Allow CONFIG_INIT_SP_RELATIVE to be turned off by a board's config, to > be able to select a fixed stack pointer, for instance in known good > DRAM. > > This will help QEMU utilising PIE, when it's loaded to (Flash-)ROM. > > Signed-off-by: Andre Przywara > Reviewed-by: Stephen Warren Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] arm: Kconfig: Explain TFABOOT
On Wed, Sep 30, 2020 at 03:45:07PM +0100, Andre Przywara wrote: > The CONFIG_TFABOOT option is more about what U-Boot DOES NOT need to do > than to support some features. > > Explain a bit more in the Kconfig help text to avoid misunderstandings. > > Signed-off-by: Andre Przywara > Reviewed-by: Tom Rini Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/6] arm64: PIE: Skip fixups if distance is zero
On Wed, Sep 30, 2020 at 05:39:14PM +0100, Andre Przywara wrote: > When the actual offset between link and runtime address is zero, there > is no need for patching up U-Boot early when running with > CONFIG_POSITION_INDEPENDENT. > > Skip the whole routine when the distance is 0. > > This helps when U-Boot is loaded into ROM, or in otherwise sensitive > memory locations. > > Signed-off-by: Andre Przywara > Reviewed-by: Stephen Warren Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/1] examples: make examples/ optional
On Wed, Sep 23, 2020 at 07:09:51PM +0200, Heinrich Schuchardt wrote: > Most users don't need the standalone API examples. Distributions like SUSE > do not supply libgcc for cross-compiling and we cannot do without on ARMv8 > for building examples/. > > Make examples selectable via symbol CONFIG_EXAMPLES. It defaults to > yes on ARCH_QEMU to ensure that we compile the API as part of our > continuous integration. > > Cc: Matthias Brugger > Signed-off-by: Heinrich Schuchardt > Reviewed-by: Tom Rini > Reviewed-by: Simon Glass > Reviewed-by: Matthias Brugger Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/6] arm64: PIE: Do not skip static relocation
On Wed, Sep 30, 2020 at 05:39:13PM +0100, Andre Przywara wrote: > When we build an arm64 target and enable POSITION_INDEPENDENT, we were > skipping our build-time dynamic relocation fixup routine (STATIC_RELA). > > This was probably done because we didn't need it in this case, as the > PIE fixup routine in start.S would take care of that at runtime. > > However when we now skip this routine (upon detecting that the fixup > offset is 0), this might lead to uninitialised pointers. > > Remove the exception, so that we always do the build-time relocation. > > NOTE: GNU binutils starting with v2.27.1 do this build-time relocation > automatically, to be in-line with other architecures. So on newer > toolchains our manual fixup is actually not needed. It doesn't hurt to > have it, though, so that we keep compatibility with the popular Linaro > toolchains, which lack this feature. > > Signed-off-by: Andre Przywara > Reviewed-by: Stephen Warren Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 11/12] riscv: add DT binding for BOOT button on Maix board
On Mon, Sep 14, 2020 at 11:02:05AM -0400, Sean Anderson wrote: > From: Heinrich Schuchardt > > Add a device tree binding for the BOOT button on the Maix board. > > Signed-off-by: Heinrich Schuchardt > Reviewed-by: Sean Anderson > Signed-off-by: Sean Anderson > Reviewed-by: Rick Chen Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 10/12] riscv: Add pinmux and gpio bindings for Kendryte K210
On Mon, Sep 14, 2020 at 11:02:04AM -0400, Sean Anderson wrote: > This patch adds the necessary device tree bindings. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass > Acked-by: Rick Chen Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 09/12] test: dm: Test for default led naming
On Mon, Sep 14, 2020 at 11:02:03AM -0400, Sean Anderson wrote: > This modifies the existing led test to check for default led naming as > added in the previous patch. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 12/12] riscv: Add FPIOA and GPIO support for Kendryte K210
On Mon, Sep 14, 2020 at 11:02:06AM -0400, Sean Anderson wrote: > This patch adds the necessary configs and docs for FPIOA and GPIO support > on the K210. > > Signed-off-by: Sean Anderson Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] wdt: designware: fix timeout calculation due to expecting KHz
On Thu, Sep 17, 2020 at 10:30:40AM +0100, Jack Mitchell wrote: > The timeout calculation is based on the clk being in KHz but > the clk api returns the clk value in Hz. Convert this to KHz > to calculate the correct timeout value. > > Signed-off-by: Jack Mitchell Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 08/12] led: gpio: Default to using node name if label is absent
On Mon, Sep 14, 2020 at 11:02:02AM -0400, Sean Anderson wrote: > This more closely mirrors Linux's behaviour, and will make it easier to > transition to using function+color in the future. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 07/12] gpio: dw: Return output value when direction is out
On Mon, Sep 14, 2020 at 11:02:01AM -0400, Sean Anderson wrote: > dm_gpio_ops.get_value can be called when the gpio is either input or > output. The current dw code always returns the input value, which is > invalid if the direction is set to out. > > Signed-off-by: Sean Anderson > Reviewed-by: Ley Foon Tan Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 05/12] gpio: dw: Fix warnings about casting int to pointer
On Mon, Sep 14, 2020 at 11:01:59AM -0400, Sean Anderson wrote: > Change the type of gpio_dwabp_platdata.base from fdt_addr_t to a void > pointer, since we pass it to readl. > > Signed-off-by: Sean Anderson > Reviewed-by: Ley Foon Tan > Reviewed-by: Bin Meng > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 06/12] gpio: dw: Add a trailing underscore to generated name
On Mon, Sep 14, 2020 at 11:02:00AM -0400, Sean Anderson wrote: > Previously, if there was no bank-name property, it was easy to have > confusing gpio names like "gpio1@08", instead of "gpio1@0_8". This patch > follows the example of the sifive gpio driver. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 03/12] test: pinmux: Add test for pin muxing
On Mon, Sep 14, 2020 at 11:01:57AM -0400, Sean Anderson wrote: > This extends the pinctrl-sandbox driver to support pin muxing, and adds a > test for that behaviour. The test is done in C and not python (like the > existing tests for the pinctrl uclass) because it needs to call > pinctrl_select_state. Another option could be to add a command that > invokes pinctrl_select_state and then test everything in > test/py/tests/test_pinmux.py. > > The pinctrl-sandbox driver now mimics the way that many pinmux devices > work. There are two groups of pins which are muxed together, as well as > four pins which are muxed individually. I have tried to test all normal > paths. However, very few error cases are explicitly checked for. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 01/12] pinctrl: Add pinmux property support to pinctrl-generic
On Mon, Sep 14, 2020 at 11:01:55AM -0400, Sean Anderson wrote: > The pinmux property allows for smaller and more compact device trees, > especially when there are many pins which need to be assigned individually. > Instead of specifying an array of strings to be parsed as pins and a > function property, the pinmux property contains an array of integers > representing pinmux groups. A pinmux group consists of the pin identifier > and mux settings represented as a single integer or an array of integers. > Each individual pin controller driver specifies the exact format of a > pinmux group. As specified in the Linux documentation, a pinmux group may > be multiple integers long. However, no existing drivers use multi-integer > pinmux groups, so I have chosen to omit this feature. This makes the > implementation easier, since there is no need to allocate a buffer to do > endian conversions. > > Support for the pinmux property is done differently than in Linux. As far > as I can tell, inversion of control is used when implementing support for > the pins and groups properties to avoid allocating. This results in some > duplication of effort; every property in a config node is parsed once for > each pin in that node. This is not such an overhead with pins and groups > properties, since having multiple pins in one config node does not occur > especially often. However, the semantics of the pinmux property make such a > configuration much more appealing. A future patch could parse all config > properties at once and store them in an array. This would make it easier to > create drivers which do not function solely as callbacks from > pinctrl-generic. > > This commit increases the size of the sandbox build by approximately 48 > bytes. However, it also decreases the size of the K210 device tree by 2 > KiB from the previous version of this series. > > The documentation has been updated from the last Linux commit before it was > split off into yaml files. > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 04/12] pinctrl: Add support for Kendryte K210 FPIOA
On Mon, Sep 14, 2020 at 11:01:58AM -0400, Sean Anderson wrote: > 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. However, GPIO functionality remains > largely unimplemented (in favor of the dedicated GPIO peripherals). > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 02/12] pinctrl: Reformat documentation in dm/pinctrl.h
On Mon, Sep 14, 2020 at 11:01:56AM -0400, Sean Anderson wrote: > This normalizes the documentation to conform to kernel-doc style [1]. It > also moves the documentation for pinctrl_ops inline, and adds argument and > return-value documentation. I have kept the usual function style for these > comments. I could not find any existing examples of function documentation > inside structs. > > [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html > > Signed-off-by: Sean Anderson > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2] ram: add ddr4 dual x8 configuration
On Mon, Sep 07, 2020 at 04:25:07PM +0800, Dylan Hung wrote: > the aspeed ddr sdram controller needs to know if the memory chip mounted on > the board is dual x8 die or not. Or it may get the wrong size of the > memory space. > > Signed-off-by: Dylan Hung > Reviewed-by: Ryan Chen Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/2] ram: move aspeed ram driver into drivers/ directory
On Mon, Sep 07, 2020 at 04:25:06PM +0800, Dylan Hung wrote: > to improve the maintainability. It is more easier to modify and add > configurations of the driver in the centralized ram driver directory. > > Signed-off-by: Dylan Hung > Reviewed-by: Ryan Chen Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
RE: [PATCH 1/2] i2c: ocores: add i2c driver for OpenCores I2C controller
Hi Sean,
>-Original Message-
>From: Sean Anderson
>Sent: 09 October 2020 17:58
>To: Pragnesh Patel ; u-boot@lists.denx.de
>Cc: atish.pa...@wdc.com; palmerdabb...@google.com; bmeng...@gmail.com;
>Paul Walmsley ( Sifive) ; anup.pa...@wdc.com;
>Sagar Kadam ; r...@andestech.com; Heiko
>Schocher
>Subject: Re: [PATCH 1/2] i2c: ocores: add i2c driver for OpenCores I2C
>controller
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>On 10/9/20 1:46 AM, Pragnesh Patel wrote:
>> Add support for the OpenCores I2C controller IP core (See
>> http://www.opencores.org/projects.cgi/web/i2c/overview).
>>
>> This driver implementation is inspired from the Linux OpenCores I2C
>> driver available.
>>
>> Thanks to Peter Korsgaard for writing Linux
>> OpenCores I2C driver.
>>
>> Signed-off-by: Pragnesh Patel
>> ---
>> drivers/i2c/Kconfig | 7 +
>> drivers/i2c/Makefile | 1 +
>> drivers/i2c/ocores_i2c.c | 638
>> +++
>> 3 files changed, 646 insertions(+)
>> create mode 100644 drivers/i2c/ocores_i2c.c
>>
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig index
>> 8ae54e1e93..adf37177c4 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -342,6 +342,13 @@ config SYS_I2C_NEXELL
>> have several I2C ports and all are provided, controlled by the
>> device tree.
>>
>> +config SYS_I2C_OCORES
>> + bool "ocores I2C driver"
>> + depends on DM_I2C
>> + help
>> + Add support for ocores I2C controller. For details see
>> + http://www.opencores.org/projects.cgi/web/i2c/overview
>
>This link 404s for me. https://opencores.org/projects/i2c works.
Ooops, This is copied from Linux I2C driver.
I will update in v2.
>
>> +
>> config SYS_I2C_OMAP24XX
>> bool "TI OMAP2+ I2C driver"
>> depends on ARCH_OMAP2PLUS || ARCH_K3 diff --git
>> a/drivers/i2c/Makefile b/drivers/i2c/Makefile index
>> bd248cbf52..37dc8ada6d 100644
>> --- a/drivers/i2c/Makefile
>> +++ b/drivers/i2c/Makefile
>> @@ -32,6 +32,7 @@ obj-$(CONFIG_SYS_I2C_MESON) += meson_i2c.o
>> obj-$(CONFIG_SYS_I2C_MVTWSI) += mvtwsi.o
>> obj-$(CONFIG_SYS_I2C_MXC) += mxc_i2c.o
>> obj-$(CONFIG_SYS_I2C_NEXELL) += nx_i2c.o
>> +obj-$(CONFIG_SYS_I2C_OCORES) += ocores_i2c.o
>> obj-$(CONFIG_SYS_I2C_OCTEON) += octeon_i2c.o
>> obj-$(CONFIG_SYS_I2C_OMAP24XX) += omap24xx_i2c.o
>> obj-$(CONFIG_SYS_I2C_RCAR_I2C) += rcar_i2c.o diff --git
>> a/drivers/i2c/ocores_i2c.c b/drivers/i2c/ocores_i2c.c new file mode
>> 100644 index 00..b42b55625f
>> --- /dev/null
>> +++ b/drivers/i2c/ocores_i2c.c
>> @@ -0,0 +1,638 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * ocores-i2c.c: I2C bus driver for OpenCores I2C controller
>> + * (https://opencores.org/project/i2c/overview)
>> + *
>> + * (C) Copyright Peter Korsgaard
>> + *
>> + * Copyright (C) 2020 SiFive, Inc.
>> + * Pragnesh Patel
>> + *
>> + * Support for the GRLIB port of the controller by
>> + * Andreas Larsson */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +/* registers */
>> +#define OCI2C_PRELOW 0
>> +#define OCI2C_PREHIGH1
>> +#define OCI2C_CONTROL2
>> +#define OCI2C_DATA 3
>> +#define OCI2C_CMD4 /* write only */
>> +#define OCI2C_STATUS 4 /* read only, same address as OCI2C_CMD */
>> +
>> +#define OCI2C_CTRL_IEN 0x40
>> +#define OCI2C_CTRL_EN0x80
>> +
>> +#define OCI2C_CMD_START 0x91
>> +#define OCI2C_CMD_STOP 0x41
>> +#define OCI2C_CMD_READ 0x21
>> +#define OCI2C_CMD_WRITE 0x11
>> +#define OCI2C_CMD_READ_ACK 0x21
>> +#define OCI2C_CMD_READ_NACK 0x29
>> +#define OCI2C_CMD_IACK 0x01
>> +
>> +#define OCI2C_STAT_IF0x01
>> +#define OCI2C_STAT_TIP 0x02
>> +#define OCI2C_STAT_ARBLOST 0x20
>> +#define OCI2C_STAT_BUSY 0x40
>> +#define OCI2C_STAT_NACK 0x80
>> +
>> +#define STATE_DONE 0
>> +#define STATE_START 1
>> +#define STATE_WRITE 2
>> +#define STATE_READ 3
>> +#define STATE_ERROR 4
>> +
>> +#define TYPE_OCORES 0
>> +#define TYPE_GRLIB 1
>> +#define TYPE_SIFIVE_REV0 2
>> +
>> +#define OCORES_FLAG_BROKEN_IRQ BIT(1) /* Broken IRQ for FU540-C000
>> +SoC */
>> +
>> +struct ocores_i2c_bus {
>> + void __iomem *base;
>> + u32 reg_shift;
>> + u32 reg_io_width;
>> + unsigned long flags;
>> + struct i2c_msg *msg;
>> + int pos;
>> + int nmsgs;
>> + int state; /* see STATE_ */
>> + struct clk clk;
>> + int ip_clk_khz;
>> + int bus_clk_khz;
>> + void (*setreg)(struct ocores_i2c_bus *i2c, int reg, u8 value);
>> + u8 (*getreg)(struct ocores_i2c_bus *i2c, int reg); };
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +/* Boolea
Re: [PATCH 1/2] i2c: ocores: add i2c driver for OpenCores I2C controller
On 10/9/20 1:46 AM, Pragnesh Patel wrote:
> Add support for the OpenCores I2C controller IP core
> (See http://www.opencores.org/projects.cgi/web/i2c/overview).
>
> This driver implementation is inspired from the Linux OpenCores
> I2C driver available.
>
> Thanks to Peter Korsgaard for writing Linux
> OpenCores I2C driver.
>
> Signed-off-by: Pragnesh Patel
> ---
> drivers/i2c/Kconfig | 7 +
> drivers/i2c/Makefile | 1 +
> drivers/i2c/ocores_i2c.c | 638 +++
> 3 files changed, 646 insertions(+)
> create mode 100644 drivers/i2c/ocores_i2c.c
>
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index 8ae54e1e93..adf37177c4 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -342,6 +342,13 @@ config SYS_I2C_NEXELL
> have several I2C ports and all are provided, controlled by the
> device tree.
>
> +config SYS_I2C_OCORES
> + bool "ocores I2C driver"
> + depends on DM_I2C
> + help
> + Add support for ocores I2C controller. For details see
> + http://www.opencores.org/projects.cgi/web/i2c/overview
This link 404s for me. https://opencores.org/projects/i2c works.
> +
> config SYS_I2C_OMAP24XX
> bool "TI OMAP2+ I2C driver"
> depends on ARCH_OMAP2PLUS || ARCH_K3
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index bd248cbf52..37dc8ada6d 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_SYS_I2C_MESON) += meson_i2c.o
> obj-$(CONFIG_SYS_I2C_MVTWSI) += mvtwsi.o
> obj-$(CONFIG_SYS_I2C_MXC) += mxc_i2c.o
> obj-$(CONFIG_SYS_I2C_NEXELL) += nx_i2c.o
> +obj-$(CONFIG_SYS_I2C_OCORES) += ocores_i2c.o
> obj-$(CONFIG_SYS_I2C_OCTEON) += octeon_i2c.o
> obj-$(CONFIG_SYS_I2C_OMAP24XX) += omap24xx_i2c.o
> obj-$(CONFIG_SYS_I2C_RCAR_I2C) += rcar_i2c.o
> diff --git a/drivers/i2c/ocores_i2c.c b/drivers/i2c/ocores_i2c.c
> new file mode 100644
> index 00..b42b55625f
> --- /dev/null
> +++ b/drivers/i2c/ocores_i2c.c
> @@ -0,0 +1,638 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ocores-i2c.c: I2C bus driver for OpenCores I2C controller
> + * (https://opencores.org/project/i2c/overview)
> + *
> + * (C) Copyright Peter Korsgaard
> + *
> + * Copyright (C) 2020 SiFive, Inc.
> + * Pragnesh Patel
> + *
> + * Support for the GRLIB port of the controller by
> + * Andreas Larsson
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* registers */
> +#define OCI2C_PRELOW 0
> +#define OCI2C_PREHIGH1
> +#define OCI2C_CONTROL2
> +#define OCI2C_DATA 3
> +#define OCI2C_CMD4 /* write only */
> +#define OCI2C_STATUS 4 /* read only, same address as OCI2C_CMD */
> +
> +#define OCI2C_CTRL_IEN 0x40
> +#define OCI2C_CTRL_EN0x80
> +
> +#define OCI2C_CMD_START 0x91
> +#define OCI2C_CMD_STOP 0x41
> +#define OCI2C_CMD_READ 0x21
> +#define OCI2C_CMD_WRITE 0x11
> +#define OCI2C_CMD_READ_ACK 0x21
> +#define OCI2C_CMD_READ_NACK 0x29
> +#define OCI2C_CMD_IACK 0x01
> +
> +#define OCI2C_STAT_IF0x01
> +#define OCI2C_STAT_TIP 0x02
> +#define OCI2C_STAT_ARBLOST 0x20
> +#define OCI2C_STAT_BUSY 0x40
> +#define OCI2C_STAT_NACK 0x80
> +
> +#define STATE_DONE 0
> +#define STATE_START 1
> +#define STATE_WRITE 2
> +#define STATE_READ 3
> +#define STATE_ERROR 4
> +
> +#define TYPE_OCORES 0
> +#define TYPE_GRLIB 1
> +#define TYPE_SIFIVE_REV0 2
> +
> +#define OCORES_FLAG_BROKEN_IRQ BIT(1) /* Broken IRQ for FU540-C000 SoC */
> +
> +struct ocores_i2c_bus {
> + void __iomem *base;
> + u32 reg_shift;
> + u32 reg_io_width;
> + unsigned long flags;
> + struct i2c_msg *msg;
> + int pos;
> + int nmsgs;
> + int state; /* see STATE_ */
> + struct clk clk;
> + int ip_clk_khz;
> + int bus_clk_khz;
> + void (*setreg)(struct ocores_i2c_bus *i2c, int reg, u8 value);
> + u8 (*getreg)(struct ocores_i2c_bus *i2c, int reg);
> +};
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* Boolean attribute values */
> +enum {
> + FALSE = 0,
> + TRUE,
> +};
> +
> +static void oc_setreg_8(struct ocores_i2c_bus *i2c, int reg, u8 value)
> +{
> + writeb(value, i2c->base + (reg << i2c->reg_shift));
> +}
> +
> +static void oc_setreg_16(struct ocores_i2c_bus *i2c, int reg, u8 value)
> +{
> + writew(value, i2c->base + (reg << i2c->reg_shift));
> +}
> +
> +static void oc_setreg_32(struct ocores_i2c_bus *i2c, int reg, u8 value)
> +{
> + writel(value, i2c->base + (reg << i2c->reg_shift));
> +}
> +
> +static void oc_setreg_16be(struct ocores_i2c_bus *i2c, int reg, u8 value)
> +{
> + out_be16(i2c->base + (reg << i2c->reg_shift), value);
> +}
> +
> +static void oc_setreg_32
Re: [PATCH 0/7] arm: cache: cp15: don't map reserved region with no-map property
On Fri, 9 Oct 2020 at 13:19, Patrick DELAUNAY wrote:
>
> Hi Ard,
>
> > From: Ard Biesheuvel
> > Sent: mercredi 7 octobre 2020 12:27
> >
> > On Tue, 6 Oct 2020 at 18:36, Patrick Delaunay
> > wrote:
> > >
> > >
> > > Hi,
> > >
> > > On STM32MP15x platform we can use OP-TEE, loaded in DDR in a region
> > > protected by a firewall. This region is reserved in device with "no-map"
> > > property.
> > >
> > > But sometime the platform boot failed in U-boot on a Cortex A7 access
> > > to this region (depending of the binary and the issue can change with
> > > compiler version or with code alignment), then the firewall raise a
> > > error, for example:
> > >
> > > E/TC:0 tzc_it_handler:19 TZC permission failure
> > > E/TC:0 dump_fail_filter:420 Permission violation on filter 0
> > > E/TC:0 dump_fail_filter:425 Violation @0xde5c6bf0, non-secure privileged
> > read,
> > > AXI ID 5c0
> > > E/TC:0 Panic
> > >
> > > After investigation, the forbidden access is a speculative request
> > > performed by the Cortex A7 because all the DDR is mapped as MEMORY
> > > with CACHEABLE property.
> > >
> > > The issue is solved only when the region reserved by OP-TEE is no more
> > > mapped in U-Boot (mapped as DEVICE/NON-CACHEABLE wasn't enough) as it
> > > is already done in Linux kernel.
> > >
> >
> > Spurious peculative accesses to device regions would be a severe silicon
> > bug, so
> > I wonder what is going on here.
> >
> > (Apologies if we are rehashing stuff here that has already been discussed -
> > I don't
> > remember the details)
> >
> > Are you sure that the speculative accesses were not caused by misconfigured
> > CPU or page tables, missing TLB maintenance, etc etc?
> > Because it really does smell like a software issue not a hardware issue.
> >
> > > I think that can be a general issue for ARM architecture: the no-map
> > > tag in device should be respected by U-boot, so I propose a generic
> > > solution in arm/lib/cache-cp15.c:dram_bank_mmu_setup().
> > >
> > > This serie is composed by 7 patches
> > > - 1..4/7: preliminary steps to support flags in library in lmb
> > > (as it is done in memblock.c in Linux)
> > > - 5/7: unitary test on the added feature in lmb lib
> > > - 6/7: save the no-map flags in lmb when the device tree is parsed
> > > - 7/7: update the generic behavior for "no-map" region in
> > >arm/lib/cache-cp15.c::dram_bank_mmu_setup()
> > >
> > > It is a rebase on master branch of previous RFC [2].
> > >
> > > I can change this last patch if this feature is note required by other
> > > ARM architecture; it is a weak function so I can avoid to map the
> > > region with "no-map" property in device tree only for STM32MP
> > > architecture (in arch/arm/mach-stm32mp/cpu.c).
> > >
> > > See also [1] which handle same speculative access on armv8 for area
> > > with Executable attribute.
> > >
> > > [1]
> > > http://patchwork.ozlabs.org/project/uboot/patch/20200903000106.5016-1-
> > > marek.bykow...@gmail.com/ [2]
> > > http://patchwork.ozlabs.org/project/uboot/list/?series=199486&archive=
> > > both&state=*
> > >
> > > Regards
> > > Patrick
> > >
> > >
> > > Patrick Delaunay (7):
> > > lmb: Add support of flags for no-map properties
> > > lmb: add lmb_is_reserved_flags
> > > lmb: remove lmb_region.size
> > > lmb: add lmb_dump_region() function
> > > test: lmb: add test for lmb_reserve_flags
> > > image-fdt: save no-map parameter of reserve-memory
> > > arm: cache: cp15: don't map the reserved region with no-map property
> > >
> > > arch/arm/include/asm/system.h | 3 +
> > > arch/arm/lib/cache-cp15.c | 19 ++-
> > > common/image-fdt.c| 23 +---
> > > include/lmb.h | 22 +++-
> > > lib/lmb.c | 100 +++---
> > > test/lib/lmb.c| 89 ++
> > > 6 files changed, 212 insertions(+), 44 deletions(-)
> > >
> > > --
> > > 2.17.1
> > >
>
> No, I don't yet described the issue in details on mailing list.
> So I will explain the investigation done and my conclusion.
>
> On STM32MP15x platform we have firewall to protect the access to DDR (TZC400
> IP).
>
> When OP-TEE is used, the boot chain is:
>
> 1/ ROM-Code (secure)
> 2/ TF-A (BL2) in SYSRAM (secure)
> 3/ OP-TEE in SYSRAM and DDR (secure)
> 4/ U-Boot in DDR
> 5/ Linux lernel
>
> OP-TEE is loaded by TF-A BL2
> - in SYRAM (for pager part)
> - in DDR (for pageable part).
>
> U-Boot is loaded by TF-A BL2 in DDR.
>
> When OP-TEE is execute, it protects with TZC400 firewall the reserved part
> of DDR as defined in device tree :
>
> For example : ./arch/arm/dts/stm32mp157a-ed1-u-boot.dtsi:42:
> reserved-memory {
> optee@fe00 {
> reg = <0xfe00 0x0200>;
> no-map;
> };
> };
>
> When OP-TEE is running in secure world (secure monitor is resident),
> it jump to normal world (unsecure) =
optee: FIT image sub-image rejected
Hi guys,
I'm trying to boot a FIT image, containing a Linux Kernel, an FDT and
optee. I am using U-boot 2018.03, as it came in the NXP i.MX6UL Yocto
distribution, patched it with Bryan O'Donoghue's optee patches of
March 2018, and out-configured NXP's CONFIG_IMX_OPTEE.
Below the DTS for my for the FIT image:
/dts-v1/;
/ {
description = "XXX i.MX6UL Linux";
#address-cells = <1>;
images {
kernel@1 {
description = "Linux kernel";
data = /incbin/("/home/XXX/zImage.bin");
type = "kernel";
arch = "arm";
os = "linux";
compression = "none";
load = <0x8080>;
entry = <0x8080>;
};
fdt@1 {
description = "Flattened Device Tree";
data = /incbin/("/home/XXX/XXX.dtb");
type = "flat_dt";
arch = "arm";
load = <0x8300>;
compression = "none";
};
optee@1 {
description = "OP/TEE";
data = /incbin/("/home/XXX/tee.bin");
type = "kernel";
arch = "arm";
os = "tee";
compression = "none";
load = <0x8de4>;
entry = <0x8e00>;
};
};
configurations {
default = "tee@1";
tee@1 {
description = "Boot Linux kernel through OP/TEE";
kernel = "optee@1";
fdt = "fdt@1";
loadables = "kernel@1";
};
bare@1 {
description = "Boot bare Linux kernel";
kernel = "kernel@1";
fdt = "fdt@1";
};
};
};
Booting this image using bootm 0x8500#bare works, so both Kernel
sub-image and FDT sub-image are OK.
Booting the default configuration (tee@1) however, produces an error:
=> bootm 0x8500
## Loading kernel from FIT Image at 8500 ...
Using 'tee@1' configuration
Trying 'optee@1' kernel subimage
Description: OP/TEE
Type: Kernel Image
Compression: uncompressed
Data Start: 0x85887e5c
Data Size:315732 Bytes = 308.3 KiB
Architecture: ARM
OS: Trusted Execution Environment
Load Address: 0x8de4
Entry Point: 0x8e00
Verifying Hash Integrity ... OK
No Trusted Execution Environment ARM Kernel Image Image
This error is triggered in .../common/image-fit.c:
if ((!type_ok || !os_ok) && image_type != IH_TYPE_LOADABLE) {
fit_image_get_os(fit, noffset, &os);
printf("No %s %s %s Image\n",
genimg_get_os_name(os),
genimg_get_arch_name(arch),
genimg_get_type_name(image_type));
bootstage_error(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
return -EIO;
type_ok = 1
os_ok = 0
image_type = 2
The failing argument here is os_ok, which is set shortly before:
os_ok = image_type == IH_TYPE_FLATDT ||
image_type == IH_TYPE_FPGA ||
fit_image_check_os(fit, noffset, IH_OS_LINUX) ||
fit_image_check_os(fit, noffset, IH_OS_U_BOOT) ||
fit_image_check_os(fit, noffset, IH_OS_OPENRTOS);
The optee sub-image in my FIT image is of type IH_OS_TEE, clearly not
in this list, but neither are the vast majority of the other OSes.
I could set 'os' in my DTS file to 'linux', but everybody else seems
to use 'tee', most even set 'type' to 'tee'. But even the current
master branch is not including IH_OS_TEE in setting os_ok.
Should it be included, or am I doing something else wrong? (BTW,
including IH_OS_TEE still fails to launch optee, but I'd like to start
at the most obvious error first.)
With kind regards,
Robert.
--
DISCLAIMER
De informatie, verzonden in of met dit e-mailbericht, is
vertrouwelijk en uitsluitend voor de geadresseerde(n) bestemd. Het gebruik
van de informatie in dit bericht, de openbaarmaking, vermenigvuldiging,
verspreiding en|of verstrekking daarvan aan derden is niet toegestaan.
Gebruik van deze informatie door anderen dan geadresseerde(n) is strikt
verboden. Aan deze informatie kunnen geen rechten worden ontleend. U wordt
verzocht bij onjuiste adressering de afzender direct te informeren door het
bericht te retourneren en het bericht uit uw computersysteem te verwijderen.
RE: [PATCH 0/7] arm: cache: cp15: don't map reserved region with no-map property
Hi Ard,
> From: Ard Biesheuvel
> Sent: mercredi 7 octobre 2020 12:27
>
> On Tue, 6 Oct 2020 at 18:36, Patrick Delaunay
> wrote:
> >
> >
> > Hi,
> >
> > On STM32MP15x platform we can use OP-TEE, loaded in DDR in a region
> > protected by a firewall. This region is reserved in device with "no-map"
> > property.
> >
> > But sometime the platform boot failed in U-boot on a Cortex A7 access
> > to this region (depending of the binary and the issue can change with
> > compiler version or with code alignment), then the firewall raise a
> > error, for example:
> >
> > E/TC:0 tzc_it_handler:19 TZC permission failure
> > E/TC:0 dump_fail_filter:420 Permission violation on filter 0
> > E/TC:0 dump_fail_filter:425 Violation @0xde5c6bf0, non-secure privileged
> read,
> > AXI ID 5c0
> > E/TC:0 Panic
> >
> > After investigation, the forbidden access is a speculative request
> > performed by the Cortex A7 because all the DDR is mapped as MEMORY
> > with CACHEABLE property.
> >
> > The issue is solved only when the region reserved by OP-TEE is no more
> > mapped in U-Boot (mapped as DEVICE/NON-CACHEABLE wasn't enough) as it
> > is already done in Linux kernel.
> >
>
> Spurious peculative accesses to device regions would be a severe silicon bug,
> so
> I wonder what is going on here.
>
> (Apologies if we are rehashing stuff here that has already been discussed - I
> don't
> remember the details)
>
> Are you sure that the speculative accesses were not caused by misconfigured
> CPU or page tables, missing TLB maintenance, etc etc?
> Because it really does smell like a software issue not a hardware issue.
>
> > I think that can be a general issue for ARM architecture: the no-map
> > tag in device should be respected by U-boot, so I propose a generic
> > solution in arm/lib/cache-cp15.c:dram_bank_mmu_setup().
> >
> > This serie is composed by 7 patches
> > - 1..4/7: preliminary steps to support flags in library in lmb
> > (as it is done in memblock.c in Linux)
> > - 5/7: unitary test on the added feature in lmb lib
> > - 6/7: save the no-map flags in lmb when the device tree is parsed
> > - 7/7: update the generic behavior for "no-map" region in
> >arm/lib/cache-cp15.c::dram_bank_mmu_setup()
> >
> > It is a rebase on master branch of previous RFC [2].
> >
> > I can change this last patch if this feature is note required by other
> > ARM architecture; it is a weak function so I can avoid to map the
> > region with "no-map" property in device tree only for STM32MP
> > architecture (in arch/arm/mach-stm32mp/cpu.c).
> >
> > See also [1] which handle same speculative access on armv8 for area
> > with Executable attribute.
> >
> > [1]
> > http://patchwork.ozlabs.org/project/uboot/patch/20200903000106.5016-1-
> > marek.bykow...@gmail.com/ [2]
> > http://patchwork.ozlabs.org/project/uboot/list/?series=199486&archive=
> > both&state=*
> >
> > Regards
> > Patrick
> >
> >
> > Patrick Delaunay (7):
> > lmb: Add support of flags for no-map properties
> > lmb: add lmb_is_reserved_flags
> > lmb: remove lmb_region.size
> > lmb: add lmb_dump_region() function
> > test: lmb: add test for lmb_reserve_flags
> > image-fdt: save no-map parameter of reserve-memory
> > arm: cache: cp15: don't map the reserved region with no-map property
> >
> > arch/arm/include/asm/system.h | 3 +
> > arch/arm/lib/cache-cp15.c | 19 ++-
> > common/image-fdt.c| 23 +---
> > include/lmb.h | 22 +++-
> > lib/lmb.c | 100 +++---
> > test/lib/lmb.c| 89 ++
> > 6 files changed, 212 insertions(+), 44 deletions(-)
> >
> > --
> > 2.17.1
> >
No, I don't yet described the issue in details on mailing list.
So I will explain the investigation done and my conclusion.
On STM32MP15x platform we have firewall to protect the access to DDR (TZC400
IP).
When OP-TEE is used, the boot chain is:
1/ ROM-Code (secure)
2/ TF-A (BL2) in SYSRAM (secure)
3/ OP-TEE in SYSRAM and DDR (secure)
4/ U-Boot in DDR
5/ Linux lernel
OP-TEE is loaded by TF-A BL2
- in SYRAM (for pager part)
- in DDR (for pageable part).
U-Boot is loaded by TF-A BL2 in DDR.
When OP-TEE is execute, it protects with TZC400 firewall the reserved part of
DDR as defined in device tree :
For example : ./arch/arm/dts/stm32mp157a-ed1-u-boot.dtsi:42:
reserved-memory {
optee@fe00 {
reg = <0xfe00 0x0200>;
no-map;
};
};
When OP-TEE is running in secure world (secure monitor is resident),
it jump to normal world (unsecure) = U-Boot
After relocation, U-Boot maps all the DDR as normal memory
(cacheable, bufferable, executable) and activate data cahe
Then, sometime (because depending of the compilation), the firewall detect an
unsecure access
to OP-TEE secured region when U-Boot is running.
We have investigate this access wit
[PATCH v2] net: lx2160a.c: Update to set ECx_PMUX precedence
As per hardware documentation, ECx_PMUX has precedence
over SerDes protocol.
For LX2160/LX2162 if DPMACs 17 and 18 are enabled as SGMII
through SerDes protocol but ECx_PMUX configured them as RGMII,
then the ports will be configured as RGMII and not SGMII.
Signed-off-by: Razvan Ionut Cirjan
---
arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 20 +--
.../asm/arch-fsl-layerscape/immap_lsch3.h | 2 +-
drivers/net/ldpaa_eth/lx2160a.c | 4 ++--
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index 33d72f194558..33920bad26f0 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -1144,17 +1144,17 @@ int arch_early_init_r(void)
/* some dpmacs in armv8a based freescale layerscape SOCs can be
* configured via both serdes(sgmii, xfi, xlaui etc) bits and via
* EC*_PMUX(rgmii) bits in RCW.
-* e.g. dpmac 17 and 18 in LX2160A and LX2162A can be configured as
-* SGMII from serdes bits and as RGMII via EC1_PMUX/EC2_PMUX bits
-* Now if a dpmac is enabled by serdes bits then it takes precedence
-* over EC*_PMUX bits. i.e. in LX2160A if we select serdes protocol
-* that configures dpmac17 as SGMII and set the EC1_PMUX as RGMII,
-* then the dpmac is SGMII and not RGMII.
+* e.g. dpmac 17 and 18 in LX2160A can be configured as SGMII from
+* serdes bits and as RGMII via EC1_PMUX/EC2_PMUX bits
+* Now if a dpmac is enabled as RGMII through ECx_PMUX then it takes
+* precedence over SerDes protocol. i.e. in LX2160A if we select serdes
+* protocol that configures dpmac17 as SGMII and set the EC1_PMUX as
+* RGMII, then the dpmac is RGMII and not SGMII.
*
-* Therefore, move the fsl_rgmii_init after fsl_serdes_init. in
-* fsl_rgmii_init function of SOC, we will check if the dpmac is enabled
-* or not? if it is (fsl_serdes_init has already enabled the dpmac),
-* then don't enable it.
+* Therefore, even thought fsl_rgmii_init is after fsl_serdes_init
+* function of SOC, the dpmac will be enabled as RGMII even if it was
+* also enabled before as SGMII. If ECx_PMUX is not configured for
+* RGMII, DPMAC will remain configured as SGMII from fsl_serdes_init().
*/
fsl_rgmii_init();
#endif
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
index 2971262736f4..d579af9558b6 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch3.h
@@ -396,7 +396,7 @@ struct ccsr_gur {
#define FSL_CHASSIS3_EC2_REGSR 27
#define FSL_CHASSIS3_EC1_REGSR_PRTCL_MASK 0x0003
#define FSL_CHASSIS3_EC1_REGSR_PRTCL_SHIFT 0
-#define FSL_CHASSIS3_EC2_REGSR_PRTCL_MASK 0x0007
+#define FSL_CHASSIS3_EC2_REGSR_PRTCL_MASK 0x000C
#define FSL_CHASSIS3_EC2_REGSR_PRTCL_SHIFT 2
#define FSL_CHASSIS3_RCWSR28_SRDS1_PRTCL_MASK 0x001F
#define FSL_CHASSIS3_RCWSR28_SRDS1_PRTCL_SHIFT 16
diff --git a/drivers/net/ldpaa_eth/lx2160a.c b/drivers/net/ldpaa_eth/lx2160a.c
index 41eff0dd1cff..90ac5db1fec0 100644
--- a/drivers/net/ldpaa_eth/lx2160a.c
+++ b/drivers/net/ldpaa_eth/lx2160a.c
@@ -93,7 +93,7 @@ void fsl_rgmii_init(void)
& FSL_CHASSIS3_EC1_REGSR_PRTCL_MASK;
ec >>= FSL_CHASSIS3_EC1_REGSR_PRTCL_SHIFT;
- if (!ec && (wriop_is_enabled_dpmac(17) == -ENODEV))
+ if (!ec)
wriop_init_dpmac_enet_if(17, PHY_INTERFACE_MODE_RGMII_ID);
#endif
@@ -102,7 +102,7 @@ void fsl_rgmii_init(void)
& FSL_CHASSIS3_EC2_REGSR_PRTCL_MASK;
ec >>= FSL_CHASSIS3_EC2_REGSR_PRTCL_SHIFT;
- if (!ec && (wriop_is_enabled_dpmac(18) == -ENODEV))
+ if (!ec)
wriop_init_dpmac_enet_if(18, PHY_INTERFACE_MODE_RGMII_ID);
#endif
}
--
2.17.1
RE: [PATCH] Define default CONFIG_PREBOOT with right config option
Hi Simon and Tom
> From: Tom Rini
> Sent: mercredi 7 octobre 2020 17:45
>
> On Wed, Oct 07, 2020 at 07:26:55AM -0600, Simon Glass wrote:
> > Hi Patrick,
> >
> > On Wed, 7 Oct 2020 at 02:38, Patrick DELAUNAY
> wrote:
> > >
> > > Hi,
> > >
> > > > From: U-Boot On Behalf Of Peter
> > > > Robinson
> > > > Sent: mardi 29 septembre 2020 11:48
> > > >
> > > > The 44758771ee commit removes CONFIG_PREBOOT but actually sets the
> > > > USE_PREBOOT Kconfig option which isn't CONFIG_PREBOOT and is also
> > > > a bool option which means we regress because 'usb start' isn't run
> > > > when expected, it should also be run for devices that have USB
> > > > storage because keyboards aren't the only thing we might need the USB
> bus for.
> > > >
> > > > Fixes: 44758771ee ("arm: move CONFIG_PREBOOT="usb start" to
> > > > KConfig")
> > > > Signed-off-by: Peter Robinson
> > > > Cc: Jonas Smedegaard
> > > > Cc: Neil Armstrong
> > > > ---
> > > > common/Kconfig | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/Kconfig b/common/Kconfig index
> > > > b1934b3a9c..9c20a9738e
> > > > 100644
> > > > --- a/common/Kconfig
> > > > +++ b/common/Kconfig
> > > > @@ -403,7 +403,6 @@ config BOOTCOMMAND
> > > >
> > > > config USE_PREBOOT
> > > > bool "Enable preboot"
> > > > - default "usb start" if USB_KEYBOARD
> > > > help
> > > > When this option is enabled, the existence of the environment
> > > > variable "preboot" will be checked immediately before
> > > > starting the @@ -
> > > > 417,6 +416,7 @@ config USE_PREBOOT config PREBOOT
> > > > string "preboot default value"
> > > > depends on USE_PREBOOT && !USE_DEFAULT_ENV_FILE
> > > > + default "usb start" if USB_KEYBOARD || USB_STORAGE
> > > > default ""
> > > > help
> > > > This is the default of "preboot" environment variable.
> > > > --
> > > > 2.26.2
> > >
> > > For information, this patch cause unexpected 'usb start' on
> > > STM32MP15x boards and slow down the start-up in realease v2020.10.
> > >
> > > For me it is unexpected because
> > > - USB keyboard is not activated
> > > - USB storage is activated but USB boot is not supported (not
> > > managed by distro boot command)
> > >
> > > I sent a patch [1] for the associated defconfig but I'm afraid that other
> > > boards
> are impacted.
> > >
> > > As the USB storage boot initialization is correctly managed by distro boot
> command 'usb_boot'
> > > (defined in include/config_distro_bootcmd.h, it already include 'usb
> > > start'), I think that the USB_STORAGE test should be removed or limited by
> !DISTRO_DEFAULTS.
> >
> > Perhaps PREBOOT should depend on USE_PREBOOT?
>
> It does. And ARCH_STM32MP does "imply USE_PREBOOT" which is maybe the
> root problem?
> --
> Tom
I activate CONFIG_USE_PREBOOT to handle the "preboot" variable in
common/main.c::main_loop()
if (IS_ENABLED(CONFIG_USE_PREBOOT))
run_preboot_environment_command();
In my case the preboot variable is dynamically build in
arch/arm/mach-stm32mp/cpu.c::setup_boot_mode()
to handle the reboot reason provided by Linux kernel in a TAMPER register
And it is why I activate the USE_PREBOOT with imply in mach-stm32mp Kconfig.
So the expected configuration for me is
- CONFIG_USE_PREBOOT=y => variable preboot is handle
- CONFIG_PREBOOT="" => default value of variable
After this patch, the value CONFIG_PREBOOT change because
CONFIG_ USB_STORAGE is also activated in stm32mp15 defconfig.
Anyway I will solve the issue for my defconfig with [1]
But I am still confuse with this this patch:
1/ I understood why preboot = "usb start" when the USB keyboard is activated
keyboard can be use to interrupt the boot and enter new command,
2/ I don't understood why it is need for USB storage by default
'usb start' can't be handle in bootcmd, as it is done in distro bootcmd ?
Why usb device had to be available before the bootcmd is executed in main
loop ?
I don't found boot use-case where it is needed when distro bootcmd is
used...
I just ask some clarification on this part:
+ default "usb start" iUSB_STORAGE
,
And raise a potential issue for other platforms with
- CONFIG_USB_STORAGE=y
- CONFIG_USE_PREBOOT=y
- CONFIG_PREBOOT not defined
- preboot build dynamically:
arch/arm/mach-imx/mx6/opos6ul.c:68: env_set("preboot", "");
arch/arm/mach-rockchip/boot_mode.c:98: env_set("preboot", "setenv
preboot; fastboot usb0");
arch/arm/mach-rockchip/boot_mode.c:102: env_set("preboot", "setenv
preboot; ums mmc 0");
board/syteco/zmx25/zmx25.c:162: env_set("preboot", "run
gs_slow_boot");
board/syteco/zmx25/zmx25.c:164: env_set("preboot", "run
gs_fast_boot");
board/boundary/nitrogen6x/nitrogen6x.c:966:
env_set("preboot", cmd);
board/rockchip/kylin_rk3036/kylin_rk3036.c:46: env_set("p
RE: [PATCH] configs: stm32mp: force empty PREBOOT
Hi Tom,
> From: Tom Rini
> Sent: jeudi 8 octobre 2020 16:16
>
> On Wed, Oct 07, 2020 at 10:10:20AM +0200, Patrick Delaunay wrote:
>
> > This patch remove the default preboot command 'usb start' for
> > STMicroelectronics board. These command is added by the commit
> > 324d77998ed6 ("Define default CONFIG_PREBOOT with right config
> > option")' and commit 44758771eefb ("arm: move CONFIG_PREBOOT="usb
> start"
> > to KConfig").
> >
> > The USB storage boot (not activated in stm32mp1.h) is correctly
> > managed by distro boot command 'usb_boot' (defined in
> > include/config_distro_bootcmd.h, it include 'usb start') and USB
> > keyboard is not supported in stm32mp15 defconfig.
> >
> > So this patch avoids unnecessary USB initialization which slows down
> > the start-up:
> > starting USB...
> > Bus usbh-ehci@5800d000: USB EHCI 1.00
> > scanning bus usbh-ehci@5800d000 for devices... 3 USB Device(s) found
> > scanning usb for storage devices... 1 Storage Device(s) found
> >
> > Cc: Peter Robinson
> > Cc: Jonas Smedegaard
> > Cc: Neil Armstrong
> > Signed-off-by: Patrick Delaunay
> > ---
> >
> > configs/stm32mp15_basic_defconfig | 1 +
> > configs/stm32mp15_trusted_defconfig | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/configs/stm32mp15_basic_defconfig
> > b/configs/stm32mp15_basic_defconfig
> > index a8c4112dbe..f937a0278d 100644
> > --- a/configs/stm32mp15_basic_defconfig
> > +++ b/configs/stm32mp15_basic_defconfig
> > @@ -19,6 +19,7 @@ CONFIG_DEFAULT_DEVICE_TREE="stm32mp157c-
> ev1"
> > CONFIG_DISTRO_DEFAULTS=y
> > CONFIG_FIT=y
> > CONFIG_BOOTCOMMAND="run bootcmd_stm32mp"
> > +CONFIG_PREBOOT=""
> > CONFIG_BOARD_EARLY_INIT_F=y
> > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION=y
> > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=3
> > diff --git a/configs/stm32mp15_trusted_defconfig
> > b/configs/stm32mp15_trusted_defconfig
> > index 0792884a9d..b0be064cc3 100644
> > --- a/configs/stm32mp15_trusted_defconfig
> > +++ b/configs/stm32mp15_trusted_defconfig
> > @@ -12,6 +12,7 @@ CONFIG_DEFAULT_DEVICE_TREE="stm32mp157c-
> ev1"
> > CONFIG_DISTRO_DEFAULTS=y
> > CONFIG_FIT=y
> > CONFIG_BOOTCOMMAND="run bootcmd_stm32mp"
> > +CONFIG_PREBOOT=""
> > CONFIG_SYS_PROMPT="STM32MP> "
> > CONFIG_CMD_ADTIMG=y
> > # CONFIG_CMD_ELF is not set
>
> In this case you should disable CONFIG_USE_PREBOOT.
I activate CONFIG_USE_PREBOOT to handle the "preboot" variable in
common/main.c::main_loop()
if (IS_ENABLED(CONFIG_USE_PREBOOT))
run_preboot_environment_command();
But I expect that the "preboot" varibale in empty by default,
as it is the case until now.
In my case the preboot variable is dynamically build in
arch/arm/mach-stm32mp/cpu.c::setup_boot_mode()
it is why I activate the USE_PREBOOT by default:
with imply in mach-stm32mp Kconfig.
I use this feature to handle the 'forced' boot request,
and I preferred don't override the bootcmd
(I handle the fastboot 'continue' command without any effort:
the bootcmd is executed after preboot=fastboot)
And it is managed by linux with reboot mode driver:
tamp: tamp@5c00a000 {
compatible = "simple-bus", "syscon", "simple-mfd";
reg = <0x5c00a000 0x400>;
reboot-mode {
compatible = "syscon-reboot-mode";
offset = <0x150>; /* reg20 */
mask = <0xff>;
mode-normal = <0>;
mode-fastboot = <0x1>;
mode-recovery = <0x2>;
mode-stm32cubeprogrammer = <0x3>;
mode-ums_mmc0 = <0x10>;
mode-ums_mmc1 = <0x11>;
mode-ums_mmc2 = <0x12>;
};
};
So the expected configuration for me is
- CONFIG_USE_PREBOOT=y => variable preboot is handle
- CONFIG_PREBOOT="" => default value of variable
> --
> Tom
Regards
Patrick
Re: [PATCH] ARM: dts: stm32: Do not set eth1addr if KS8851 has EEPROM
Hi Marek
On 10/8/20 3:14 PM, Marek Vasut wrote:
> In case the KS8851 has external EEPROM attached to it, do not set
> eth1addr at all. The network stack will read the MAC out of the
> KS8851 and set eth1addr accordingly.
>
> Signed-off-by: Marek Vasut
> Cc: Patrice Chotard
> Cc: Patrick Delaunay
> ---
> board/dhelectronics/dh_stm32mp1/board.c | 40 ++---
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/board/dhelectronics/dh_stm32mp1/board.c
> b/board/dhelectronics/dh_stm32mp1/board.c
> index c9abe3cc6d..f42d395098 100644
> --- a/board/dhelectronics/dh_stm32mp1/board.c
> +++ b/board/dhelectronics/dh_stm32mp1/board.c
> @@ -81,6 +81,11 @@
> */
> DECLARE_GLOBAL_DATA_PTR;
>
> +#define KS_CCR 0x08
> +#define KS_CCR_EEPROMBIT(9)
> +#define KS_BE0 BIT(12)
> +#define KS_BE1 BIT(13)
> +
> int setup_mac_address(void)
> {
> unsigned char enetaddr[6];
> @@ -97,12 +102,39 @@ int setup_mac_address(void)
> if (off < 0) {
> /* ethernet1 is not present in the system */
> skip_eth1 = true;
> - } else {
> - ret = eth_env_get_enetaddr("eth1addr", enetaddr);
> - if (ret)/* eth1addr is already set */
> - skip_eth1 = true;
> + goto out_set_ethaddr;
> + }
> +
> + ret = eth_env_get_enetaddr("eth1addr", enetaddr);
> + if (ret) {
> + /* eth1addr is already set */
> + skip_eth1 = true;
> + goto out_set_ethaddr;
> + }
> +
> + ret = fdt_node_check_compatible(gd->fdt_blob, off, "micrel,ks8851-mll");
> + if (ret)
> + goto out_set_ethaddr;
> +
> + /*
> + * KS8851 with EEPROM may use custom MAC from EEPROM, read
> + * out the KS8851 CCR register to determine whether EEPROM
> + * is present. If EEPROM is present, it must contain valid
> + * MAC address.
> + */
> + u32 reg, ccr;
> + reg = fdt_get_base_address(gd->fdt_blob, off);
> + if (!reg)
> + goto out_set_ethaddr;
> +
> + writew(KS_BE0 | KS_BE1 | KS_CCR, reg + 2);
> + ccr = readw(reg);
> + if (ccr & KS_CCR_EEPROM) {
> + skip_eth1 = true;
> + goto out_set_ethaddr;
> }
>
> +out_set_ethaddr:
> if (skip_eth0 && skip_eth1)
> return 0;
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
