[PATCH v2 1/1] riscv: fix building with CONFIG_SPL_SMP=n
Building with CONFIG_SPL_SMP=n results in: arch/riscv/lib/spl.c: In function ‘jump_to_image_no_args’: arch/riscv/lib/spl.c:33:6: error: unused variable ‘ret’ [-Werror=unused-variable] 33 | int ret; | ^~~ Define the variable ret as __maybe_unused. Fixes: 191636e44898 ("riscv: Introduce SPL_SMP Kconfig option for U-Boot SPL") Fixes: 8c59f2023cc8 ("riscv: add SPL support") Signed-off-by: Heinrich Schuchardt Reviewed-by: Bin Meng Reviewed-by: Simon Glass Reviewed-by: Rick Chen --- v2: Do not break Fixes line --- arch/riscv/lib/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/lib/spl.c b/arch/riscv/lib/spl.c index c47dcd46ce..ef00ec2bcc 100644 --- a/arch/riscv/lib/spl.c +++ b/arch/riscv/lib/spl.c @@ -30,7 +30,7 @@ void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) { typedef void __noreturn (*image_entry_riscv_t)(ulong hart, void *dtb); void *fdt_blob; - int ret; + __maybe_unused int ret; #if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL) fdt_blob = spl_image->fdt_addr; -- 2.27.0
Re: [PATCH 1/1] clk: kendryte/pll.h: do not redefine nop()
On 8/5/20 2:19 PM, Heinrich Schuchardt wrote: > On 05.08.20 13:45, Sean Anderson wrote: >> On 8/3/20 2:20 PM, Heinrich Schuchardt wrote: >>> On 03.08.20 16:08, Sean Anderson wrote: Maybe. Because we are configuring the PLL, the CPU clock is temporarily set to the in0 oscillator, so the timer would give an incorrect delay. However, it would probably be fine even if incorrect. The original reason I used nops is because that is what the SDK does in its PLL configuration function [1]. I believe I tried using udelay at one point, but I don't remember whether I had any issues with it. --Sean [1] https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/sysctl.c#L844 >>> >>> This is what udelay(1) gets compiled to: >>> >>> 004a <.LBE106>: >>> udelay(1); >>> 4a: 4505li a0,1 >>> >>> 004c <.LVL21>: >>> 4c: 0097auipc ra,0x0 >>> 50: 80e7jalrra # 4c <.LVL21> >>> >>> Somehow udelay() seems not to be fully implemented and the platform. So >>> I think we should stay with the nop() macro. >> >> Hm, I don't see this generated code. What function is that a disassembly >> of? > > I was disassembling > > riscv64-linux-gnu-objdump -D -S drivers/clk/kendryte/pll.o > > If I disassemble the file u-boot function k210_pll_enable seems to be > ok. nop()s replaced by udelay(1): > > writel(reg, pll->reg); > 80012524: 7518ld a4,40(a0) > __iowmb(); > 80012526: 055f fence ow,ow > reg |= K210_PLL_RESET; > 8001252a: 003006b7 lui a3,0x300 > 8001252e: 8fd5or a5,a5,a3 > __arch_putl(val, addr); > 80012530: c31csw a5,0(a4) > udelay(1); > 80012532: 4505li a0,1 > 80012534: 612200ef jal ra,80032b46 > writel(reg, pll->reg); > 80012538: 741cld a5,40(s0) > __iowmb(); > 8001253a: 055f fence ow,ow > > So it was simply the link step missing to show the call. > > But anyway which way do you want the patch? Hello Sean, did I miss you reply? Best regards Heinrich > > Best regards > > Heinrich > >> >> $ riscv64-linux-gnu-objdump -S --disassemble=__udelay u-boot >> >> u-boot: file format elf64-littleriscv >> >> >> Disassembly of section .text: >> >> Disassembly of section .text_rest: >> >> 800197f8 <__udelay>: >> do_div(tick, 100); >> return tick; >> } >> >> void __weak __udelay(unsigned long usec) >> { >> 800197f8:1101addisp,sp,-32 >> 800197fa:ec06sd ra,24(sp) >> 800197fc:e822sd s0,16(sp) >> 800197fe:e426sd s1,8(sp) >> 80019800:84aamv s1,a0 >> uint64_t tmp; >> >> tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ >> 80019802:f7bff0efjal ra,8001977c >> 80019806:842amv s0,a0 >> 80019808:8526mv a0,s1 >> 8001980a:fcbff0efjal ra,800197d4 >> >> 8001980e:942aadd s0,s0,a0 >> >> while (get_ticks() < tmp+1) /* loop till event */ >> 80019810:0405addis0,s0,1 >> 80019812:f6bff0efjal ra,8001977c >> 80019816:fe856ee3bltua0,s0,80019812 >> <__udelay+0x1a> >> /*NOP*/; >> } >> 8001981a:60e2ld ra,24(sp) >> 8001981c:6442ld s0,16(sp) >> 8001981e:64a2ld s1,8(sp) >> 80019820:6105addisp,sp,32 >> 80019822:8082ret >> >> --Sean >> >
Pull request for UEFI sub-system for efi-2020-10-rc3 (2)
Dear Tom, The following changes since commit cdcf591d9b20534e5f5c58aa2a2b07b3b173f5a1: Merge https://gitlab.denx.de/u-boot/custodians/u-boot-marvell (2020-08-13 08:25:25 -0400) are available in the Git repository at: https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2020-10-rc3-2 for you to fetch changes up to a4bda5ebab8246004caaca2e17bc865d265bf57a: riscv: load addresses for Sipeed MAIX (2020-08-14 17:22:50 +0200) No problems where reported by Gitlab CI and Travis CI: https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/4424 https://travis-ci.org/github/xypron2/u-boot/builds/717955462 Concerning the RISC-V patch: Rick asked me to take this one as it was originally part of a series concerning UEFI testing of the Sipeed MAIX board. Pull request for UEFI sub-system for efi-2020-10-rc3 (2) This series includes bug fixes for: * UEFI secure boot - images with multiple signatures * UEFI secure boot - support for intermediate certificates * corrections for UEFI unit tests * missing loadaddr on MAIX board AKASHI Takahiro (7): efi_loader: variable: keep temporary buffer during the authentication efi_loader: signature: rework for intermediate certificates support test/py: efi_secboot: small rework for adding a new test test/py: efi_secboot: add test for intermediate certificates efi_loader: variable: fix secure state initialization efi_loader: signature: correct a behavior against multiple signatures test/py: efi_secboot: modify 'multiple signatures' test case Heinrich Schuchardt (2): cmd/efidebug: missing initialization of load_options riscv: load addresses for Sipeed MAIX cmd/efidebug.c | 2 +- include/configs/sipeed-maix.h | 9 + include/efi_loader.h | 17 +- lib/efi_loader/Kconfig | 1 + lib/efi_loader/efi_image_loader.c | 33 +- lib/efi_loader/efi_signature.c | 413 - lib/efi_loader/efi_variable.c | 42 ++- test/py/tests/test_efi_secboot/conftest.py | 131 ++- test/py/tests/test_efi_secboot/defs.py | 10 +- test/py/tests/test_efi_secboot/openssl.cnf | 48 +++ test/py/tests/test_efi_secboot/test_signed.py | 26 +- .../py/tests/test_efi_secboot/test_signed_intca.py | 135 +++ 12 files changed, 554 insertions(+), 313 deletions(-) create mode 100644 test/py/tests/test_efi_secboot/openssl.cnf create mode 100644 test/py/tests/test_efi_secboot/test_signed_intca.py
Re: [PATCH 1/1] clk: kendryte/pll.h: do not redefine nop()
On 8/15/20 3:53 AM, Heinrich Schuchardt wrote: > On 8/5/20 2:19 PM, Heinrich Schuchardt wrote: >> On 05.08.20 13:45, Sean Anderson wrote: >>> On 8/3/20 2:20 PM, Heinrich Schuchardt wrote: On 03.08.20 16:08, Sean Anderson wrote: > Maybe. Because we are configuring the PLL, the CPU clock is temporarily > set to the in0 oscillator, so the timer would give an incorrect delay. > However, it would probably be fine even if incorrect. The original > reason I used nops is because that is what the SDK does in its PLL > configuration function [1]. I believe I tried using udelay at one point, > but I don't remember whether I had any issues with it. > > --Sean > > [1] > https://github.com/kendryte/kendryte-standalone-sdk/blob/develop/lib/drivers/sysctl.c#L844 > This is what udelay(1) gets compiled to: 004a <.LBE106>: udelay(1); 4a: 4505li a0,1 004c <.LVL21>: 4c: 0097auipc ra,0x0 50: 80e7jalrra # 4c <.LVL21> Somehow udelay() seems not to be fully implemented and the platform. So I think we should stay with the nop() macro. >>> >>> Hm, I don't see this generated code. What function is that a disassembly >>> of? >> >> I was disassembling >> >> riscv64-linux-gnu-objdump -D -S drivers/clk/kendryte/pll.o >> >> If I disassemble the file u-boot function k210_pll_enable seems to be >> ok. nop()s replaced by udelay(1): >> >> writel(reg, pll->reg); >> 80012524: 7518ld a4,40(a0) >> __iowmb(); >> 80012526: 055f fence ow,ow >> reg |= K210_PLL_RESET; >> 8001252a: 003006b7 lui a3,0x300 >> 8001252e: 8fd5or a5,a5,a3 >> __arch_putl(val, addr); >> 80012530: c31csw a5,0(a4) >> udelay(1); >> 80012532: 4505li a0,1 >> 80012534: 612200ef jal ra,80032b46 >> writel(reg, pll->reg); >> 80012538: 741cld a5,40(s0) >> __iowmb(); >> 8001253a: 055f fence ow,ow >> >> So it was simply the link step missing to show the call. >> >> But anyway which way do you want the patch? I think the patch as-is is fine. --Sean > > Hello Sean, > > did I miss you reply?> > Best regards > > Heinrich > >> >> Best regards >> >> Heinrich >> >>> >>> $ riscv64-linux-gnu-objdump -S --disassemble=__udelay u-boot >>> >>> u-boot: file format elf64-littleriscv >>> >>> >>> Disassembly of section .text: >>> >>> Disassembly of section .text_rest: >>> >>> 800197f8 <__udelay>: >>> do_div(tick, 100); >>> return tick; >>> } >>> >>> void __weak __udelay(unsigned long usec) >>> { >>> 800197f8: 1101addisp,sp,-32 >>> 800197fa: ec06sd ra,24(sp) >>> 800197fc: e822sd s0,16(sp) >>> 800197fe: e426sd s1,8(sp) >>> 80019800: 84aamv s1,a0 >>> uint64_t tmp; >>> >>> tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */ >>> 80019802: f7bff0efjal ra,8001977c >>> 80019806: 842amv s0,a0 >>> 80019808: 8526mv a0,s1 >>> 8001980a: fcbff0efjal ra,800197d4 >>> >>> 8001980e: 942aadd s0,s0,a0 >>> >>> while (get_ticks() < tmp+1) /* loop till event */ >>> 80019810: 0405addis0,s0,1 >>> 80019812: f6bff0efjal ra,8001977c >>> 80019816: fe856ee3bltua0,s0,80019812 >>> <__udelay+0x1a> >>> /*NOP*/; >>> } >>> 8001981a: 60e2ld ra,24(sp) >>> 8001981c: 6442ld s0,16(sp) >>> 8001981e: 64a2ld s1,8(sp) >>> 80019820: 6105addisp,sp,32 >>> 80019822: 8082ret >>> >>> --Sean >>> >> >
HDMi Output on OrangePi Win/Win Plus Allwinner A64 (SUN50I)
Hello. Does u-boot support HDMI output on OrangePi Win/Win Plus Allwinner A64 (SUN50I) platform? At current time HDMI output only work after linux kernel is loaded. Before - blank screen on a monitor (no signal). Change setting's for video-mode=sunxi.. did not give any results. Output only in serial console I try: setenv video-mode=sunxi:1024x768-24@60,monitor=dvi,hpd=0,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=dvi,hpd=1,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=dvi,hpd=0,edid=0 setenv video-mode=sunxi:1024x768-24@60,monitor=hdmi,hpd=0,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=hdmi,hpd=1,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=hdmi,hpd=0,edid=0 and setenv hdmi.audio=EDID:0 disp.screen0_output_mode=1024x768p60 setenv hdmi.audio=EDID:0 disp.screen0_output_mode=EDID:1024x768p60 Need some help to debug this. Full output of U-BOOT load === U-Boot SPL 2020.07-STLK1-dirty (Aug 14 2020 - 02:43:26 +0300) DRAM: 2048 MiB Trying to boot from MMC1 NOTICE: BL31: v2.3(debug):v2.3-420-gf0b1864f8 NOTICE: BL31: Built : 15:05:30, Jul 31 2020 NOTICE: BL31: Detected Allwinner A64/H64/R18 SoC (1689) NOTICE: BL31: Found U-Boot DTB at 0x4099690, model: OrangePi Win/Win Plus INFO:ARM GICv2 driver initialized INFO:Configuring SPC Controller INFO:PMIC: Probing AXP803 on RSB INFO:PMIC: dcdc1 voltage: 3.300V INFO:PMIC: dcdc5 voltage: 1.500V INFO:PMIC: dcdc6 voltage: 1.100V INFO:PMIC: dldo1 voltage: 3.300V INFO:PMIC: dldo2 voltage: 3.300V INFO:PMIC: dldo4 voltage: 3.300V INFO:BL31: Platform setup done INFO:BL31: Initializing runtime services INFO:BL31: cortex_a53: CPU workaround for 843419 was applied INFO:BL31: cortex_a53: CPU workaround for 855873 was applied NOTICE: PSCI: System suspend is unavailable INFO:BL31: Preparing for EL3 exit to normal world INFO:Entry point address = 0x4a00 INFO:SPSR = 0x3c9 alloc space exhausted U-Boot 2020.07-STLK1-dirty (Aug 14 2020 - 02:43:26 +0300) Allwinner Technology CPU: Allwinner A64 (SUN50I) Model: OrangePi Win/Win Plus DRAM: 2 GiB MMC: mmc@1c0f000: 0, mmc@1c1: 1 Loading Environment from FAT... Card did not respond to voltage select! In:serial Out: serial Err: serial Model: OrangePi Win/Win Plus Net: phy interface7 eth0: ethernet@1c3 starting USB... Bus usb@1c1a000: USB EHCI 1.00 Bus usb@1c1a400: USB OHCI 1.0 Bus usb@1c1b000: USB EHCI 1.00 Bus usb@1c1b400: USB OHCI 1.0 scanning bus usb@1c1a000 for devices... 1 USB Device(s) found scanning bus usb@1c1a400 for devices... 1 USB Device(s) found scanning bus usb@1c1b000 for devices... 1 USB Device(s) found scanning bus usb@1c1b400 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found Autoboot in 2 seconds, press to stop switch to partitions #0, OK mmc0 is current device Scanning mmc 0:1... Found U-Boot script /boot/boot.scr 3098 bytes read in 2 ms (1.5 MiB/s) ## Executing script at 4fc0 U-boot loaded from SD Boot script loaded from mmc 104 bytes read in 2 ms (50.8 KiB/s) 39702 bytes read in 5 ms (7.6 MiB/s) 3821 bytes read in 5 ms (746.1 KiB/s) Applying kernel provided DT fixup script (sun50i-a64-fixup.scr) ## Executing script at 4400 11888288 bytes read in 550 ms (20.6 MiB/s) 17162752 bytes read in 818 ms (20 MiB/s) ## Loading init Ramdisk from Legacy Image at 4fe0 ... Image Name: uInitrd Image Type: AArch64 Linux RAMDisk Image (gzip compressed) Data Size:11888224 Bytes = 11.3 MiB Load Address: Entry Point: Verifying Checksum ... OK ## Flattened Device Tree blob at 4fa0 Booting using the fdt blob at 0x4fa0 EHCI failed to shut down host controller. Loading Ramdisk to 494a9000, end 49fff660 ... OK Loading Device Tree to 49436000, end 494a8fff ... OK Starting kernel ...
HDMi Output on OrangePi Win/Win Plus Allwinner A64 (SUN50I)
Hello. Does u-boot support HDMI output on OrangePi Win/Win Plus Allwinner A64 (SUN50I) platform? At current time HDMI output only work after linux kernel is loaded. Before - blank screen on a monitor (no signal). Change setting's for video-mode=sunxi.. did not give any results. Output only in serial console I try: setenv video-mode=sunxi:1024x768-24@60,monitor=dvi,hpd=0,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=dvi,hpd=1,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=dvi,hpd=0,edid=0 setenv video-mode=sunxi:1024x768-24@60,monitor=hdmi,hpd=0,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=hdmi,hpd=1,edid=1 setenv video-mode=sunxi:1024x768-24@60,monitor=hdmi,hpd=0,edid=0 and setenv hdmi.audio=EDID:0 disp.screen0_output_mode=1024x768p60 setenv hdmi.audio=EDID:0 disp.screen0_output_mode=EDID:1024x768p60 Need some help to debug this.
Re: [PATCH 1/1] riscv: riscv_get_time() implementation for SMODE
On Sat, Aug 15, 2020 at 12:57 AM Heinrich Schuchardt wrote: > > On 8/14/20 8:38 PM, Anup Patel wrote: > > On Fri, Aug 14, 2020 at 11:35 PM Heinrich Schuchardt > > wrote: > >> > >> On 14.08.20 19:52, Anup Patel wrote: > >>> On Fri, Aug 14, 2020 at 11:15 PM Heinrich Schuchardt > >>> wrote: > > On the Kendryte K210 OpenBSI cannot emulate the rdtime instruction. So we > have to use the Sifive CLINT driver to provide riscv_get_time() in SMODE. > >>> > >>> Can you elaborate why ? > >>> > >>> The rdtime instruction should generate an illegal instruction fault which > >>> OpenSBI will emulate. > >> > >> The RISC-V Instruction Set Manual Volume II Privileged architectur 1.11 > >> has incompatible changes relative to version 1.9.1 > >> > >> mtval on the Kendryte K210 delivers the bad virtual address and not the > >> instruction: > > > > Ahh, I see. The MTVAL CSR is actually legacy MBADADDR CSR and this > > CSR. > > > > The S-mode software always has working rdtime instruction for all > > RISC-V systems. If HW does not implement TIME CSR then OpenSBI > > emulates it. Please don't break this convention for U-Boot S-mode > > because we do have RISC-V systems where TIME CSR is implemeted > > in HW so this will patch will break U-Boot S-mode system because > > on those system we are supposed to use TIME CSR from S-mode. > > This patch does not change anything for existing systems. It only allows > me to customize U-Boot for the K210 differently. I understand that > fixing OpenSBI is a better approach. Currently, on most RISC-V systems the CLINT timer interrupts and IPI interrupts are hard-wired to M-mode timer and software interrupt lines. In other words, the CLINT is integrated as M-mode only device on most RISC-V systems. Due to above reason, CLINT driver is M-mode only driver for Linux kernel The Linux S-mode will use: 1. TIME CSR as free running counter 2. SBI calls for timer interrupts 3. SBI calls for injecting IPIs For #1 above, the M-mode firmware will trap-n-emulate TIME CSR for S-mode if HW does not implement TIME CSR. Based on above mentioned convention, the U-Boot CLINT driver should be M-mode only and U-Boot S-mode should use TIME CSR as a free running counter. > > > > >> > >> lib/sbi/sbi_illegal_insn.c(123) sbi_illegal_insn_handler: > >> insn 0x4114121602, epc 0x8058c168. > >> > >> The illegal instruction being > >> c01027f3rdtime a5 > >> > >> In the long run it may make sense to provide backwards compatibility in > >> OpenSBI. > > > > Yes, let's try to explore possible fixes in OpenSBI. > > > > Instead of this patch, try the following change in OpenSBI ... > > > > diff --git a/lib/sbi/sbi_illegal_insn.c b/lib/sbi/sbi_illegal_insn.c > > index 0e5523f..c8f2e4a 100644 > > --- a/lib/sbi/sbi_illegal_insn.c > > +++ b/lib/sbi/sbi_illegal_insn.c > > @@ -119,12 +119,10 @@ int sbi_illegal_insn_handler(ulong insn, struct > > sbi_trap_regs *regs) > > struct sbi_trap_info uptrap; > > > > if (unlikely((insn & 3) != 3)) { > > Why do put sbi_get_insn() behind this if and not before it? Currently, OpenSBI only deals with 32bit (or longer) illegal instructions. If we see insn == 0 OR insn is 16bit then we double-check instruction encoding using unprivileged access. The PC in RISC-V is always 2-byte aligned address so if MTVAL has fault instruction address instead of instruction encoding then "(insn & 3) != 3" will be TRUE and we will be forced to double-check. The "insn == 0" check below is causing problem RISC-V v1.9 spec (i.e. MTVAL having instruction address) and it is totally harmless to remove the "insn == 0" check for RISC-V v1.10 (or higher) hence my suggestion to remove the check. > > > - if (insn == 0) { > > - insn = sbi_get_insn(regs->mepc, &uptrap); > > - if (uptrap.cause) { > > - uptrap.epc = regs->mepc; > > - return sbi_trap_redirect(regs, &uptrap); > > - } > > + insn = sbi_get_insn(regs->mepc, &uptrap); > > + if (uptrap.cause) { > > + uptrap.epc = regs->mepc; > > + return sbi_trap_redirect(regs, &uptrap); > > } > > if ((insn & 3) != 3) > > return truly_illegal_insn(insn, regs); > > > > For this illegal instruction exception the fix works. But I think the > change is in the wrong place. > > We have to cover all exceptions, e.g. unaligned access. So we better > should determine the instruction in sbi_trap_handler(). That's incorrect. For RISC-V v1.10 (or higher), the MTVAL contains: 1. Instruction encoding for illegal instruction trap 2. Fault address for unaligned traps, page faults, and access faults For RISC-V v1.10 (or higher), the unaligned trap does not provide fault instruction encoding so we end-up doing unpriv access. Base on above, it's best to work-around your issue in sbi_illegal_insn_
Re: [PATCH 1/1] riscv: riscv_get_time() implementation for SMODE
Am 15. August 2020 16:06:41 MESZ schrieb Anup Patel : >On Sat, Aug 15, 2020 at 12:57 AM Heinrich Schuchardt > wrote: >> >> On 8/14/20 8:38 PM, Anup Patel wrote: >> > On Fri, Aug 14, 2020 at 11:35 PM Heinrich Schuchardt > wrote: >> >> >> >> On 14.08.20 19:52, Anup Patel wrote: >> >>> On Fri, Aug 14, 2020 at 11:15 PM Heinrich Schuchardt > wrote: >> >> On the Kendryte K210 OpenBSI cannot emulate the rdtime >instruction. So we >> have to use the Sifive CLINT driver to provide riscv_get_time() >in SMODE. >> >>> >> >>> Can you elaborate why ? >> >>> >> >>> The rdtime instruction should generate an illegal instruction >fault which >> >>> OpenSBI will emulate. >> >> >> >> The RISC-V Instruction Set Manual Volume II Privileged architectur >1.11 >> >> has incompatible changes relative to version 1.9.1 >> >> >> >> mtval on the Kendryte K210 delivers the bad virtual address and >not the >> >> instruction: >> > >> > Ahh, I see. The MTVAL CSR is actually legacy MBADADDR CSR and this >> > CSR. >> > >> > The S-mode software always has working rdtime instruction for all >> > RISC-V systems. If HW does not implement TIME CSR then OpenSBI >> > emulates it. Please don't break this convention for U-Boot S-mode >> > because we do have RISC-V systems where TIME CSR is implemeted >> > in HW so this will patch will break U-Boot S-mode system because >> > on those system we are supposed to use TIME CSR from S-mode. >> >> This patch does not change anything for existing systems. It only >allows >> me to customize U-Boot for the K210 differently. I understand that >> fixing OpenSBI is a better approach. > >Currently, on most RISC-V systems the CLINT timer interrupts and IPI >interrupts are hard-wired to M-mode timer and software interrupt lines. >In other words, the CLINT is integrated as M-mode only device on most >RISC-V systems. > >Due to above reason, CLINT driver is M-mode only driver for Linux >kernel > >The Linux S-mode will use: >1. TIME CSR as free running counter >2. SBI calls for timer interrupts >3. SBI calls for injecting IPIs > >For #1 above, the M-mode firmware will trap-n-emulate TIME CSR >for S-mode if HW does not implement TIME CSR. > >Based on above mentioned convention, the U-Boot CLINT driver >should be M-mode only and U-Boot S-mode should use TIME CSR >as a free running counter. > >> >> > >> >> >> >> lib/sbi/sbi_illegal_insn.c(123) sbi_illegal_insn_handler: >> >> insn 0x4114121602, epc 0x8058c168. >> >> >> >> The illegal instruction being >> >> c01027f3rdtime a5 >> >> >> >> In the long run it may make sense to provide backwards >compatibility in >> >> OpenSBI. >> > >> > Yes, let's try to explore possible fixes in OpenSBI. >> > >> > Instead of this patch, try the following change in OpenSBI ... >> > >> > diff --git a/lib/sbi/sbi_illegal_insn.c >b/lib/sbi/sbi_illegal_insn.c >> > index 0e5523f..c8f2e4a 100644 >> > --- a/lib/sbi/sbi_illegal_insn.c >> > +++ b/lib/sbi/sbi_illegal_insn.c >> > @@ -119,12 +119,10 @@ int sbi_illegal_insn_handler(ulong insn, >struct >> > sbi_trap_regs *regs) >> > struct sbi_trap_info uptrap; >> > >> > if (unlikely((insn & 3) != 3)) { >> >> Why do put sbi_get_insn() behind this if and not before it? > >Currently, OpenSBI only deals with 32bit (or longer) illegal >instructions. If we see insn == 0 OR insn is 16bit then we >double-check instruction encoding using unprivileged access. > >The PC in RISC-V is always 2-byte aligned address so if MTVAL >has fault instruction address instead of instruction encoding then >"(insn & 3) != 3" will be TRUE and we will be forced to double-check. >The "insn == 0" check below is causing problem RISC-V v1.9 spec >(i.e. MTVAL having instruction address) and it is totally harmless to >remove the "insn == 0" check for RISC-V v1.10 (or higher) hence >my suggestion to remove the check. > Thank you for your detailed explanation. Maybe you can add a comment to the code. >> >> > - if (insn == 0) { >> > - insn = sbi_get_insn(regs->mepc, &uptrap); >> > - if (uptrap.cause) { >> > - uptrap.epc = regs->mepc; >> > - return sbi_trap_redirect(regs, >&uptrap); >> > - } >> > + insn = sbi_get_insn(regs->mepc, &uptrap); >> > + if (uptrap.cause) { >> > + uptrap.epc = regs->mepc; >> > + return sbi_trap_redirect(regs, &uptrap); >> > } >> > if ((insn & 3) != 3) >> > return truly_illegal_insn(insn, regs); >> > >> >> For this illegal instruction exception the fix works. But I think the >> change is in the wrong place. >> >> We have to cover all exceptions, e.g. unaligned access. So we better >> should determine the instruction in sbi_trap_handler(). > >That's incorrect. > >For RISC-V v1.10 (or higher), the MTVAL contains: >1. Instruction encoding for illegal instruction trap >2. Fau
[PATCH v5 02/11] pinctrl: Reformat documentation in dm/pinctrl.h
This normalizes the documentatation 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 --- (no changes since v4) Changes in v4: - New include/dm/pinctrl.h | 363 --- 1 file changed, 242 insertions(+), 121 deletions(-) diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h index d50af1ce38..5b994f22cc 100644 --- a/include/dm/pinctrl.h +++ b/include/dm/pinctrl.h @@ -11,11 +11,10 @@ /** * struct pinconf_param - pin config parameters - * - * @property: property name in DT nodes - * @param: ID for this config parameter - * @default_value: default value for this config parameter used in case - * no value is specified in DT nodes + * @property: Property name in DT nodes + * @param: ID for this config parameter + * @default_value: default value for this config parameter used in case + * no value is specified in DT nodes */ struct pinconf_param { const char * const property; @@ -27,111 +26,228 @@ struct pinconf_param { * struct pinctrl_ops - pin control operations, to be implemented by * pin controller drivers. * - * The @set_state is the only mandatory operation. You can implement your - * pinctrl driver with its own @set_state. In this case, the other callbacks - * are not required. Otherwise, generic pinctrl framework is also available; - * use pinctrl_generic_set_state for @set_state, and implement other operations + * set_state() is the only mandatory operation. You can implement your pinctrl + * driver with its own @set_state. In this case, the other callbacks are not + * required. Otherwise, generic pinctrl framework is also available; use + * pinctrl_generic_set_state for @set_state, and implement other operations * depending on your necessity. - * - * @get_pins_count: return number of selectable named pins available - * in this driver. (necessary to parse "pins" property in DTS) - * @get_pin_name: return the pin name of the pin selector, - * called by the core to figure out which pin it shall do - * operations to. (necessary to parse "pins" property in DTS) - * @get_groups_count: return number of selectable named groups available - * in this driver. (necessary to parse "groups" property in DTS) - * @get_group_name: return the group name of the group selector, - * called by the core to figure out which pin group it shall do - * operations to. (necessary to parse "groups" property in DTS) - * @get_functions_count: return number of selectable named functions available - * in this driver. (necessary for pin-muxing) - * @get_function_name: return the function name of the muxing selector, - * called by the core to figure out which mux setting it shall map a - * certain device to. (necessary for pin-muxing) - * @pinmux_set: enable a certain muxing function with a certain pin. - * The @func_selector selects a certain function whereas @pin_selector - * selects a certain pin to be used. On simple controllers one of them - * may be ignored. (necessary for pin-muxing against a single pin) - * @pinmux_group_set: enable a certain muxing function with a certain pin - * group. The @func_selector selects a certain function whereas - * @group_selector selects a certain set of pins to be used. On simple - * controllers one of them may be ignored. - * (necessary for pin-muxing against a pin group) - * @pinmux_property_set: enable a pinmux group. @pinmux_group should specify the - * pin identifier and mux settings. The exact format of a pinmux group is - * left up to the driver. The pin selector for the mux-ed pin should be - * returned on success. (necessary to parse the "pinmux" property in DTS) - * @pinconf_num_params: number of driver-specific parameters to be parsed - * from device trees (necessary for pin-configuration) - * @pinconf_params: list of driver_specific parameters to be parsed from - * device trees (necessary for pin-configuration) - * @pinconf_set: configure an individual pin with a given parameter. - * (necessary for pin-configuration against a single pin) - * @pinconf_group_set: configure all pins in a group with a given parameter. - * (necessary for pin-configuration against a pin group) - * @set_state: do pinctrl operations specified by @config, a pseudo device - * pointing a config node. (necessary for pinctrl_full) - * @set_state_simple: do needed pinctrl operations for a peripherl @periph. - * (necessary for pinctrl_simple) - * @get_pin_muxing: display the muxing of a given pin. - * @gpio_request_enable: requests and enables GPI
[PATCH v5 01/11] pinctrl: Add pinmux property support to pinctrl-generic
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 --- (no changes since v2) Changes in v2: - New .../pinctrl/pinctrl-bindings.txt | 65 - drivers/pinctrl/pinctrl-generic.c | 125 ++ include/dm/pinctrl.h | 21 +-- 3 files changed, 168 insertions(+), 43 deletions(-) diff --git a/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt b/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt index b73c96d24f..603796f169 100644 --- a/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt +++ b/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt @@ -119,7 +119,8 @@ For example: The contents of each of those pin configuration child nodes is defined entirely by the binding for the individual pin controller device. There -exists no common standard for this content. +exists no common standard for this content. The pinctrl framework only +provides generic helper bindings that the pin controller driver can use. The pin configuration nodes need not be direct children of the pin controller device; they may be grandchildren, for example. Whether this is legal, and @@ -156,6 +157,29 @@ state_2_node_a { pins = "mfio29", "mfio30"; }; +For hardware where pin multiplexing configurations have to be specified for +each single pin the number of required sub-nodes containing "pin" and +"function" properties can quickly escalate and become hard to write and +maintain. + +For cases like this, the pin controller driver may use the pinmux helper +property, where the pin identifier is provided with mux configuration settings +in a pinmux group. A pinmux group consists of the pin identifier and mux +settings represented as a single integer or an array of integers. + +The pinmux property accepts an array of pinmux groups, each of them describing +a single pin multiplexing configuration. + +pincontroller { + state_0_node_a { + pinmux = , , ...; + }; +}; + +Each individual pin controller driver bindings documentation shall specify +how pin IDs and pin multiplexing configuration are defined and assembled +together in a pinmux group. + == Generic pin configuration node content == Many data items that are represented in a pin configuration node are common @@ -168,12 +192,15 @@ structure of the DT nodes that contain these properties. Supported generic properties are: pins - the list of pins that properties in the node - apply to (either this or "group" has to be + apply to (either this, "group" or "pinmux" has to be specified) group - the group to apply the properties to, if the driver supports configuration of whole groups rather than - individual pins (either this or "pins" has to be - specified) + individual pins (either this, "pins" or "pinmux" has + to be specified) +pinmux - the list of numeric pin ids and their mux settings +
[PATCH v5 00/11] riscv: Add FPIOA and GPIO support for Kendryte K210
This patch series adds support for pinmuxing, gpios, and leds on the Kendyte K210. This patch series was previously part of https://patchwork.ozlabs.org/project/uboot/list/?series=161576 This patch series depends on https://patchwork.ozlabs.org/project/uboot/list/?series=178480 Changes in v5: - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded by commit 2548493ab4. - Rebase onto u-boot/master Changes in v4: - Add sandbox dt-binding headers to MAINTAINERS - Add test for led behavior - Add test/dm/pinmux.c to patch - Move sandbox_* variables into a priv structure. This resets them to the default state every time we re-probe. - Reformat documentation in dm/pinctrl.h Changes in v3: - Add dt-bindings/pinctrl/sandbox-pinmux.h to patch Changes in v2: - Add test for pinmuxing - Don't clear existing pinctrl settings on probe - Re-order GPIOs to match the defaults more closely - Rebase onto v13 of "riscv: Add Sipeed Maix support" - Rewrite FPIOA driver to use pinmux property - Support muxing the output enable signal for each function in the FPIOA - Support output and input inversion in the pinmux driver - Support pinmux property in pinctrl-generic Sean Anderson (11): pinctrl: Add pinmux property support to pinctrl-generic pinctrl: Reformat documentation in dm/pinctrl.h test: pinmux: Add test for pin muxing pinctrl: Add support for Kendryte K210 FPIOA gpio: dw: Fix warnings about casting int to pointer gpio: dw: Add a trailing underscore to generated name gpio: dw: Return output value when direction is out led: gpio: Default to using node name if label is absent test: dm: Test for default led naming riscv: Add pinmux and gpio bindings for Kendryte K210 riscv: Add FPIOA and GPIO support for Kendryte K210 MAINTAINERS | 3 + arch/riscv/dts/k210-maix-bit.dts | 104 +++ arch/riscv/dts/k210.dtsi | 12 + arch/sandbox/dts/test.dts | 47 +- board/sipeed/maix/Kconfig | 9 + configs/sipeed_maix_bitm_defconfig| 2 + doc/board/sipeed/maix.rst | 64 +- .../pinctrl/kendryte,k210-fpioa.txt | 102 +++ .../pinctrl/pinctrl-bindings.txt | 65 +- drivers/gpio/dwapb_gpio.c | 33 +- drivers/led/led_gpio.c| 7 +- drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/kendryte/Kconfig | 7 + drivers/pinctrl/kendryte/Makefile | 1 + drivers/pinctrl/kendryte/pinctrl.c| 678 ++ drivers/pinctrl/kendryte/pinctrl.h| 70 ++ drivers/pinctrl/pinctrl-generic.c | 125 +++- drivers/pinctrl/pinctrl-sandbox.c | 186 +++-- include/dm/pinctrl.h | 360 +++--- include/dt-bindings/pinctrl/k210-pinctrl.h| 277 +++ include/dt-bindings/pinctrl/sandbox-pinmux.h | 19 + test/dm/Makefile | 3 + test/dm/led.c | 3 +- test/dm/pinmux.c | 57 ++ test/py/tests/test_pinmux.py | 36 +- 26 files changed, 2028 insertions(+), 244 deletions(-) create mode 100644 doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt create mode 100644 drivers/pinctrl/kendryte/Kconfig create mode 100644 drivers/pinctrl/kendryte/Makefile create mode 100644 drivers/pinctrl/kendryte/pinctrl.c create mode 100644 drivers/pinctrl/kendryte/pinctrl.h create mode 100644 include/dt-bindings/pinctrl/k210-pinctrl.h create mode 100644 include/dt-bindings/pinctrl/sandbox-pinmux.h create mode 100644 test/dm/pinmux.c -- 2.28.0
[PATCH v5 07/11] gpio: dw: Return output value when direction is out
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 --- This patch was previously submitted as part of https://patchwork.ozlabs.org/project/uboot/list/?series=161576 (no changes since v3) Changes in v3: - Undo reorder to prevent use-before-declared error Changes in v2: - Reorder changes to minimize diff drivers/gpio/dwapb_gpio.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c index a52c9e18e3..37916e7771 100644 --- a/drivers/gpio/dwapb_gpio.c +++ b/drivers/gpio/dwapb_gpio.c @@ -66,13 +66,6 @@ static int dwapb_gpio_direction_output(struct udevice *dev, unsigned pin, return 0; } -static int dwapb_gpio_get_value(struct udevice *dev, unsigned pin) -{ - struct gpio_dwapb_platdata *plat = dev_get_platdata(dev); - return !!(readl(plat->base + GPIO_EXT_PORT(plat->bank)) & (1 << pin)); -} - - static int dwapb_gpio_set_value(struct udevice *dev, unsigned pin, int val) { struct gpio_dwapb_platdata *plat = dev_get_platdata(dev); @@ -98,6 +91,18 @@ static int dwapb_gpio_get_function(struct udevice *dev, unsigned offset) return GPIOF_INPUT; } +static int dwapb_gpio_get_value(struct udevice *dev, unsigned pin) +{ + struct gpio_dwapb_platdata *plat = dev_get_platdata(dev); + u32 value; + + if (dwapb_gpio_get_function(dev, pin) == GPIOF_OUTPUT) + value = readl(plat->base + GPIO_SWPORT_DR(plat->bank)); + else + value = readl(plat->base + GPIO_EXT_PORT(plat->bank)); + return !!(value & BIT(pin)); +} + static const struct dm_gpio_ops gpio_dwapb_ops = { .direction_input= dwapb_gpio_direction_input, .direction_output = dwapb_gpio_direction_output, -- 2.28.0
[PATCH v5 06/11] gpio: dw: Add a trailing underscore to generated name
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 --- This patch was previously submitted as part of https://patchwork.ozlabs.org/project/uboot/list/?series=161576 (no changes since v1) drivers/gpio/dwapb_gpio.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c index bf324f5239..a52c9e18e3 100644 --- a/drivers/gpio/dwapb_gpio.c +++ b/drivers/gpio/dwapb_gpio.c @@ -186,7 +186,15 @@ static int gpio_dwapb_bind(struct udevice *dev) * Fall back to node name. This means accessing pins * via bank name won't work. */ - plat->name = ofnode_get_name(node); + char name[32]; + + snprintf(name, sizeof(name), "%s_", +ofnode_get_name(node)); + plat->name = strdup(name); + if (!plat->name) { + kfree(plat); + return -ENOMEM; + } } ret = device_bind_ofnode(dev, dev->driver, plat->name, -- 2.28.0
[PATCH v5 04/11] pinctrl: Add support for Kendryte K210 FPIOA
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 --- This patch was previously submitted as part of https://patchwork.ozlabs.org/project/uboot/list/?series=161576 Changes from that version include: - Reformat to reduce errors from checkpatch (no changes since v2) Changes in v2: - Don't clear existing pinctrl settings on probe - Rewrite to use pinmux property - Support muxing the output enable signal for each function - Support output and input inversion - Update binding documentation MAINTAINERS | 2 + .../pinctrl/kendryte,k210-fpioa.txt | 102 +++ drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/kendryte/Kconfig | 7 + drivers/pinctrl/kendryte/Makefile | 1 + drivers/pinctrl/kendryte/pinctrl.c| 678 ++ drivers/pinctrl/kendryte/pinctrl.h| 70 ++ include/dt-bindings/pinctrl/k210-pinctrl.h| 277 +++ 9 files changed, 1139 insertions(+) create mode 100644 doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt create mode 100644 drivers/pinctrl/kendryte/Kconfig create mode 100644 drivers/pinctrl/kendryte/Makefile create mode 100644 drivers/pinctrl/kendryte/pinctrl.c create mode 100644 drivers/pinctrl/kendryte/pinctrl.h create mode 100644 include/dt-bindings/pinctrl/k210-pinctrl.h diff --git a/MAINTAINERS b/MAINTAINERS index 24de698433..3abe52570f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -929,7 +929,9 @@ RISC-V KENDRYTE M: Sean Anderson S: Maintained F: doc/device-tree-bindings/mfd/kendryte,k210-sysctl.txt +F: doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt F: drivers/clk/kendryte/ +F: drivers/pinctrl/kendryte/ F: include/kendryte/ RNG diff --git a/doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt b/doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt new file mode 100644 index 00..06a9cc060f --- /dev/null +++ b/doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt @@ -0,0 +1,102 @@ +Kendryte K210 FPIOA + +This binding describes the Fully-Programmable Input/Output Array (FPIOA) found +in Kendryte K210 SoCs. Any of the 256 functions can be mapped to any of the 48 +pins. + +Required properties: +- compatible: should be "kendryte,k210-fpioa" +- reg: address and length of the FPIOA registers +- kendryte,sysctl: phandle to the "sysctl" register map node +- kendryte,power-offset: offset in the register map of the power bank control + register (in bytes) + +Configuration nodes + +Pin configuration nodes are documented in pinctrl-bindings.txt + +Required properties for pin-configuration nodes or sub-nodes are: +- groups: list of power groups to which the configuration applies. Valid groups + are: + A0, A1, A2, B0, B1, B2, C0, C1 + (either this or "pinmux" must be specified) +- pinmux: integer array representing pin multiplexing configuration. In addition + to the 256 standard functions, each pin can also output the direction + indicator (DO) of any function. This signal is high whenever the function + would normally drive the output. Helper macros to ease assembling the "pinmux" + arguments from the pin and function are provided by the FPIOA header file at: + + Integer values in the "pinmux" argument list are assembled as: + ((PIN << 16) | (DO << 8) | (FUNC)) + Valid values for PIN are numbers 0 through 47. + Valid values for DO are 0 or 1. + Valid values for FUNC are numbers 0 through 255. For a complete list of + acceptable functions, consult the FPIOA header file. + (either this or "groups" must be specified) + +Optional properties for "pinmux" nodes are: + bias-disable, bias-pull-down, bias-pull-up, drive-strength, + drive-strength-ua, input-enable, input-disable, input-schmitt-enable, + input-schmitt-disable, output-low, output-high, output-enable, + output-disable, slew-rate, output-polarity-invert, input-polarity-invert + +Optional properties for "groups" nodes are: + power-source + +Notes on specific properties include: +- bias-pull-up, -down, and -pin-default: The pull strength cannot be configured. +- drive-strength: There are 8 drive strength settings between 11 and 50 mA. +- input- and output-polarity-invert: Invert the polarity of either the input or + the output, respectively. +- power-source: Controls the output voltage of a bank of pins. Either + K210_PC_POWER_1V8 or K210_PC_POWER_3V3 may be specified. +- slew-rate: Specifying this property reduces the slew rate. + +Example: +fpioa: pinmux@502B { +
[PATCH v5 03/11] test: pinmux: Add test for pin muxing
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 --- (no changes since v4) Changes in v4: - Add sandbox dt-binding headers to MAINTAINERS - Add test/dm/pinmux.c to patch - Move sandbox_* variables into a priv structure. This resets them to the default state every time we re-probe. Changes in v3: - Add dt-bindings/pinctrl/sandbox-pinmux.h to patch Changes in v2: - New MAINTAINERS | 1 + arch/sandbox/dts/test.dts| 45 - drivers/pinctrl/pinctrl-sandbox.c| 186 ++- include/dt-bindings/pinctrl/sandbox-pinmux.h | 19 ++ test/dm/Makefile | 3 + test/dm/pinmux.c | 57 ++ test/py/tests/test_pinmux.py | 36 ++-- 7 files changed, 274 insertions(+), 73 deletions(-) create mode 100644 include/dt-bindings/pinctrl/sandbox-pinmux.h create mode 100644 test/dm/pinmux.c diff --git a/MAINTAINERS b/MAINTAINERS index 17ac45587b..24de698433 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -953,6 +953,7 @@ SANDBOX M: Simon Glass S: Maintained F: arch/sandbox/ +F: include/dt-bindings/*/sandbox*.h SH M: Marek Vasut diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 491893a17d..0255c97cf5 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -2,6 +2,7 @@ #include #include +#include / { model = "sandbox"; @@ -1024,30 +1025,60 @@ pinctrl { compatible = "sandbox,pinctrl"; - pinctrl-names = "default"; - pinctrl-0 = <&gpios>; + pinctrl-names = "default", "alternate"; + pinctrl-0 = <&pinctrl_gpios>, <&pinctrl_i2s>; + pinctrl-1 = <&pinctrl_spi>, <&pinctrl_i2c>; - gpios: gpios { + pinctrl_gpios: gpios { gpio0 { - pins = "GPIO0"; + pins = "P5"; + function = "GPIO"; bias-pull-up; input-disable; }; gpio1 { - pins = "GPIO1"; + pins = "P6"; + function = "GPIO"; output-high; drive-open-drain; }; gpio2 { - pins = "GPIO2"; + pinmux = ; bias-pull-down; input-enable; }; gpio3 { - pins = "GPIO3"; + pinmux = ; bias-disable; }; }; + + pinctrl_i2c: i2c { + groups { + groups = "I2C_UART"; + function = "I2C"; + }; + + pins { + pins = "P0", "P1"; + drive-open-drain; + }; + }; + + pinctrl_i2s: i2s { + groups = "SPI_I2S"; + function = "I2S"; + }; + + pinctrl_spi: spi { + groups = "SPI_I2S"; + function = "SPI"; + + cs { + pinmux = , +; + }; + }; }; hwspinlock@0 { diff --git a/drivers/pinctrl/pinctrl-sandbox.c b/drivers/pinctrl/pinctrl-sandbox.c index ac0119d198..d27f74248d 100644 --- a/drivers/pinctrl/pinctrl-sandbox.c +++ b/drivers/pinctrl/pinctrl-sandbox.c @@ -1,57 +1,70 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * Copyright (C) 2015 Masahiro Yamada + * Copyright (C) 2020 Sean Anderson + * Copyright (C) 2015 Masahiro Yamada */ -/* #define DEBUG */ - #include #include -#include #include +#include +#include #include +/* + * This driver emulates a pin controller with the following rules:
[PATCH v5 05/11] gpio: dw: Fix warnings about casting int to pointer
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 --- This patch was previously submitted as part of https://patchwork.ozlabs.org/project/uboot/list/?series=161576 (no changes since v1) drivers/gpio/dwapb_gpio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c index e5e3518194..bf324f5239 100644 --- a/drivers/gpio/dwapb_gpio.c +++ b/drivers/gpio/dwapb_gpio.c @@ -40,7 +40,7 @@ struct gpio_dwapb_platdata { const char *name; int bank; int pins; - fdt_addr_t base; + void __iomem*base; }; static int dwapb_gpio_direction_input(struct udevice *dev, unsigned pin) @@ -176,7 +176,7 @@ static int gpio_dwapb_bind(struct udevice *dev) if (!plat) return -ENOMEM; - plat->base = base; + plat->base = (void *)base; plat->bank = bank; plat->pins = ofnode_read_u32_default(node, "snps,nr-gpios", 0); -- 2.28.0
[PATCH v5 08/11] led: gpio: Default to using node name if label is absent
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 --- This patch was previously submitted as part of https://patchwork.ozlabs.org/project/uboot/list/?series=161576 (no changes since v1) drivers/led/led_gpio.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/led/led_gpio.c b/drivers/led/led_gpio.c index ef9b61ee62..2cdb0269f4 100644 --- a/drivers/led/led_gpio.c +++ b/drivers/led/led_gpio.c @@ -99,11 +99,8 @@ static int led_gpio_bind(struct udevice *parent) const char *label; label = ofnode_read_string(node, "label"); - if (!label) { - debug("%s: node %s has no label\n", __func__, - ofnode_get_name(node)); - return -EINVAL; - } + if (!label) + label = ofnode_get_name(node); ret = device_bind_driver_to_node(parent, "gpio_led", ofnode_get_name(node), node, &dev); -- 2.28.0
[PATCH v5 09/11] test: dm: Test for default led naming
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 --- (no changes since v4) Changes in v4: - New arch/sandbox/dts/test.dts | 2 +- test/dm/led.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 0255c97cf5..a0a57143f0 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -503,7 +503,7 @@ default_off { gpios = <&gpio_a 6 0>; - label = "sandbox:default_off"; + /* label intentionally omitted */ default-state = "off"; }; }; diff --git a/test/dm/led.c b/test/dm/led.c index 8b587d0a22..ac6ee36394 100644 --- a/test/dm/led.c +++ b/test/dm/led.c @@ -40,7 +40,8 @@ static int dm_test_led_default_state(struct unit_test_state *uts) ut_assertok(led_get_by_label("sandbox:default_on", &dev)); ut_asserteq(LEDST_ON, led_get_state(dev)); - ut_assertok(led_get_by_label("sandbox:default_off", &dev)); + /* Also tests default label behaviour */ + ut_assertok(led_get_by_label("default_off", &dev)); ut_asserteq(LEDST_OFF, led_get_state(dev)); return 0; -- 2.28.0
[PATCH v5 10/11] riscv: Add pinmux and gpio bindings for Kendryte K210
This patch adds the necessary device tree bindings. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v2) Changes in v2: - Convert to use pinmux property - Don't hog ISP on boot - Re-order GPIOs to match the defaults more closely arch/riscv/dts/k210-maix-bit.dts | 104 +++ arch/riscv/dts/k210.dtsi | 12 2 files changed, 116 insertions(+) diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts index 5b32c5fd5f..e840e04ada 100644 --- a/arch/riscv/dts/k210-maix-bit.dts +++ b/arch/riscv/dts/k210-maix-bit.dts @@ -17,6 +17,22 @@ stdout-path = "serial0:115200"; }; + gpio-leds { + compatible = "gpio-leds"; + + green { + gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>; + }; + + red { + gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>; + }; + + blue { + gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>; + }; + }; + sound { compatible = "simple-audio-card"; simple-audio-card,format = "i2s"; @@ -39,9 +55,97 @@ }; &uarths0 { + pinctrl-0 = <&fpioa_uarths>; + pinctrl-names = "default"; + status = "okay"; +}; + +&gpio0 { + pinctrl-0 = <&fpioa_gpiohs>; + pinctrl-names = "default"; + status = "okay"; +}; + +&gpio1 { + pinctrl-0 = <&fpioa_gpio>; + pinctrl-names = "default"; status = "okay"; }; &i2s0 { #sound-dai-cells = <1>; + pinctrl-0 = <&fpioa_i2s0>; + pinctrl-names = "default"; +}; + +&fpioa { + status = "okay"; + + fpioa_uarths: uarths { + pinmux = , +; + }; + + fpioa_gpio: gpio { + pinmux = , +, +, +, +, +, +, +; + }; + + fpioa_gpiohs: gpiohs { + pinmux = , +, +, +, +, +, +, +, +, +, +, +, +; + }; + + fpioa_i2s0: i2s0 { + pinmux = , +, +; + }; + + fpioa_dvp: dvp { + pinmux = , +, +, +, +, +, +, +; + }; + + fpioa_spi0: spi0 { + pinmux = , /* cs */ +, /* rst */ +, /* dc */ +; /* wr */ + }; + + fpioa_spi1: spi1 { + pinmux = , +, +, +; + }; +}; + +&dvp0 { + pinctrl-0 = <&fpioa_dvp>; + pinctrl-names = "default"; }; diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi index 2546c7d4e0..fc7986b326 100644 --- a/arch/riscv/dts/k210.dtsi +++ b/arch/riscv/dts/k210.dtsi @@ -5,6 +5,7 @@ #include #include +#include #include / { @@ -367,7 +368,18 @@ reg = <0x502B 0x100>; clocks = <&sysclk K210_CLK_FPIOA>; resets = <&sysrst K210_RST_FPIOA>; + kendryte,sysctl = <&sysctl>; + kendryte,power-offset = ; + pinctrl-0 = <&fpioa_jtag>; + pinctrl-names = "default"; status = "disabled"; + + fpioa_jtag: jtag { + pinmux = , +, +, +; + }; }; sha256: sha256@502C { -- 2.28.0
[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210
This patch adds the necessary configs and docs for FPIOA and GPIO support on the K210. The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not changed). It also boots when changes are made to the device tree and then committed. I don't know why this happens. These breakages only occur after bf2fb81ad3. Signed-off-by: Sean Anderson --- Changes in v5: - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded by commit 2548493ab4. Changes in v3: - Document pins 6 and 7 as not set Changes in v2: - Remove SPI flash related Kconfig settings board/sipeed/maix/Kconfig | 9 + configs/sipeed_maix_bitm_defconfig | 2 + doc/board/sipeed/maix.rst | 64 +- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/board/sipeed/maix/Kconfig b/board/sipeed/maix/Kconfig index 0cdcd32adc..4c42dd2087 100644 --- a/board/sipeed/maix/Kconfig +++ b/board/sipeed/maix/Kconfig @@ -44,4 +44,13 @@ config BOARD_SPECIFIC_OPTIONS imply RESET_SYSCON imply SYSRESET imply SYSRESET_SYSCON + imply PINCTRL + imply PINCONF + imply PINCTRL_K210 + imply DM_GPIO + imply DWAPB_GPIO + imply SIFIVE_GPIO + imply CMD_GPIO + imply LED + imply LED_GPIO endif diff --git a/configs/sipeed_maix_bitm_defconfig b/configs/sipeed_maix_bitm_defconfig index 459bf0d530..0b038ff0a1 100644 --- a/configs/sipeed_maix_bitm_defconfig +++ b/configs/sipeed_maix_bitm_defconfig @@ -2,6 +2,8 @@ CONFIG_RISCV=y CONFIG_TARGET_SIPEED_MAIX=y CONFIG_ARCH_RV64I=y CONFIG_STACK_SIZE=0x10 +# FIXME: Dirty hack to get boot working! +CONFIG_LOGLEVEL=5 # CONFIG_NET is not set # CONFIG_INPUT is not set # CONFIG_DM_ETH is not set diff --git a/doc/board/sipeed/maix.rst b/doc/board/sipeed/maix.rst index b1894f3a6f..3811eac61f 100644 --- a/doc/board/sipeed/maix.rst +++ b/doc/board/sipeed/maix.rst @@ -156,7 +156,7 @@ To run legacy images, use the ``bootm`` command: Load Address: 8000 Entry Point: 8000 -$ picocom -b 115200 /dev/ttyUSB0i +$ picocom -b 115200 /dev/ttyUSB0 => loady ## Ready for binary (ymodem) download to 0x8000 at 115200 bps... C @@ -187,6 +187,66 @@ To run legacy images, use the ``bootm`` command: argv[0] = "" Hit any key to exit ... +Pin Assignment +-- + +The K210 contains a Fully Programmable I/O Array (FPIOA), which can remap any of +its 256 input functions to any any of 48 output pins. The following table has +the default pin assignments for the BitM. + += == === +Pin Function Comment += == === +IO_0 JTAG_TCLK +IO_1 JTAG_TDI +IO_2 JTAG_TMS +IO_3 JTAG_TDO +IO_4 UARTHS_RX +IO_5 UARTHS_TX +IO_6 Not set +IO_7 Not set +IO_8 GPIO_0 +IO_9 GPIO_1 +IO_10 GPIO_2 +IO_11 GPIO_3 +IO_12 GPIO_4 Green LED +IO_13 GPIO_5 Red LED +IO_14 GPIO_6 Blue LED +IO_15 GPIO_7 +IO_16 GPIOHS_0 ISP +IO_17 GPIOHS_1 +IO_18 I2S0_SCLK MIC CLK +IO_19 I2S0_WSMIC WS +IO_20 I2S0_IN_D0 MIC SD +IO_21 GPIOHS_5 +IO_22 GPIOHS_6 +IO_23 GPIOHS_7 +IO_24 GPIOHS_8 +IO_25 GPIOHS_9 +IO_26 SPI1_D1MMC MISO +IO_27 SPI1_SCLK MMC CLK +IO_28 SPI1_D0MMC MOSI +IO_29 GPIOHS_13 MMC CS +IO_30 GPIOHS_14 +IO_31 GPIOHS_15 +IO_32 GPIOHS_16 +IO_33 GPIOHS_17 +IO_34 GPIOHS_18 +IO_35 GPIOHS_19 +IO_36 GPIOHS_20 Panel CS +IO_37 GPIOHS_21 Panel RST +IO_38 GPIOHS_22 Panel DC +IO_39 SPI0_SCK Panel WR +IO_40 SCCP_SDA +IO_41 SCCP_SCLK +IO_42 DVP_RST +IO_43 DVP_VSYNC +IO_44 DVP_PWDN +IO_45 DVP_HSYNC +IO_46 DVP_XCLK +IO_47 DVP_PCLK += == === + Over- and Under-clocking @@ -361,5 +421,5 @@ AddressSize Description 0x8801C000 0x1000riscv priv spec 1.9 config 0x8801D000 0x2000flattened device tree (contains only addresses and interrupts) -0x8801f000 0x1000credits +0x8801F000 0x1000credits == = === -- 2.28.0
Re: [PATCH 1/1] riscv: riscv_get_time() implementation for SMODE
On Sat, Aug 15, 2020 at 8:37 PM Heinrich Schuchardt wrote: > > Am 15. August 2020 16:06:41 MESZ schrieb Anup Patel : > >On Sat, Aug 15, 2020 at 12:57 AM Heinrich Schuchardt > > wrote: > >> > >> On 8/14/20 8:38 PM, Anup Patel wrote: > >> > On Fri, Aug 14, 2020 at 11:35 PM Heinrich Schuchardt > > wrote: > >> >> > >> >> On 14.08.20 19:52, Anup Patel wrote: > >> >>> On Fri, Aug 14, 2020 at 11:15 PM Heinrich Schuchardt > > wrote: > >> > >> On the Kendryte K210 OpenBSI cannot emulate the rdtime > >instruction. So we > >> have to use the Sifive CLINT driver to provide riscv_get_time() > >in SMODE. > >> >>> > >> >>> Can you elaborate why ? > >> >>> > >> >>> The rdtime instruction should generate an illegal instruction > >fault which > >> >>> OpenSBI will emulate. > >> >> > >> >> The RISC-V Instruction Set Manual Volume II Privileged architectur > >1.11 > >> >> has incompatible changes relative to version 1.9.1 > >> >> > >> >> mtval on the Kendryte K210 delivers the bad virtual address and > >not the > >> >> instruction: > >> > > >> > Ahh, I see. The MTVAL CSR is actually legacy MBADADDR CSR and this > >> > CSR. > >> > > >> > The S-mode software always has working rdtime instruction for all > >> > RISC-V systems. If HW does not implement TIME CSR then OpenSBI > >> > emulates it. Please don't break this convention for U-Boot S-mode > >> > because we do have RISC-V systems where TIME CSR is implemeted > >> > in HW so this will patch will break U-Boot S-mode system because > >> > on those system we are supposed to use TIME CSR from S-mode. > >> > >> This patch does not change anything for existing systems. It only > >allows > >> me to customize U-Boot for the K210 differently. I understand that > >> fixing OpenSBI is a better approach. > > > >Currently, on most RISC-V systems the CLINT timer interrupts and IPI > >interrupts are hard-wired to M-mode timer and software interrupt lines. > >In other words, the CLINT is integrated as M-mode only device on most > >RISC-V systems. > > > >Due to above reason, CLINT driver is M-mode only driver for Linux > >kernel > > > >The Linux S-mode will use: > >1. TIME CSR as free running counter > >2. SBI calls for timer interrupts > >3. SBI calls for injecting IPIs > > > >For #1 above, the M-mode firmware will trap-n-emulate TIME CSR > >for S-mode if HW does not implement TIME CSR. > > > >Based on above mentioned convention, the U-Boot CLINT driver > >should be M-mode only and U-Boot S-mode should use TIME CSR > >as a free running counter. > > > >> > >> > > >> >> > >> >> lib/sbi/sbi_illegal_insn.c(123) sbi_illegal_insn_handler: > >> >> insn 0x4114121602, epc 0x8058c168. > >> >> > >> >> The illegal instruction being > >> >> c01027f3rdtime a5 > >> >> > >> >> In the long run it may make sense to provide backwards > >compatibility in > >> >> OpenSBI. > >> > > >> > Yes, let's try to explore possible fixes in OpenSBI. > >> > > >> > Instead of this patch, try the following change in OpenSBI ... > >> > > >> > diff --git a/lib/sbi/sbi_illegal_insn.c > >b/lib/sbi/sbi_illegal_insn.c > >> > index 0e5523f..c8f2e4a 100644 > >> > --- a/lib/sbi/sbi_illegal_insn.c > >> > +++ b/lib/sbi/sbi_illegal_insn.c > >> > @@ -119,12 +119,10 @@ int sbi_illegal_insn_handler(ulong insn, > >struct > >> > sbi_trap_regs *regs) > >> > struct sbi_trap_info uptrap; > >> > > >> > if (unlikely((insn & 3) != 3)) { > >> > >> Why do put sbi_get_insn() behind this if and not before it? > > > >Currently, OpenSBI only deals with 32bit (or longer) illegal > >instructions. If we see insn == 0 OR insn is 16bit then we > >double-check instruction encoding using unprivileged access. > > > >The PC in RISC-V is always 2-byte aligned address so if MTVAL > >has fault instruction address instead of instruction encoding then > >"(insn & 3) != 3" will be TRUE and we will be forced to double-check. > >The "insn == 0" check below is causing problem RISC-V v1.9 spec > >(i.e. MTVAL having instruction address) and it is totally harmless to > >remove the "insn == 0" check for RISC-V v1.10 (or higher) hence > >my suggestion to remove the check. > > > > Thank you for your detailed explanation. Maybe you can add a comment to the > code. Sure will do. > > >> > >> > - if (insn == 0) { > >> > - insn = sbi_get_insn(regs->mepc, &uptrap); > >> > - if (uptrap.cause) { > >> > - uptrap.epc = regs->mepc; > >> > - return sbi_trap_redirect(regs, > >&uptrap); > >> > - } > >> > + insn = sbi_get_insn(regs->mepc, &uptrap); > >> > + if (uptrap.cause) { > >> > + uptrap.epc = regs->mepc; > >> > + return sbi_trap_redirect(regs, &uptrap); > >> > } > >> > if ((insn & 3) != 3) > >> > return truly_illegal_insn(insn, regs); > >> > > >> > >> For this illegal instruction excep
[PATCH 1/1] efi_loader: document parameters of do_bootefi_exec()
Add the missing description of the load_options parameter. Signed-off-by: Heinrich Schuchardt --- cmd/bootefi.c | 4 1 file changed, 4 insertions(+) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index fbfed54e85..06563d28ca 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -304,7 +304,11 @@ efi_status_t efi_install_fdt(void *fdt) /** * do_bootefi_exec() - execute EFI binary * + * The image indicated by @handle is started. When it returns the allocated + * memory for the @load_options is freed. + * * @handle:handle of loaded image + * @load_options: load options * Return: status code * * Load the EFI binary into a newly assigned memory unwinding the relocation -- 2.28.0
[PATCH 1/1] efi_loader: remove empty comment line
Remove a line leading to a warning in make htmldocs. Signed-off-by: Heinrich Schuchardt --- include/efi_variable.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/efi_variable.h b/include/efi_variable.h index 60491cb640..4704a3c16e 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -274,7 +274,6 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_na * @data: buffer to which the variable value is copied * @timep: authentication time (seconds since start of epoch) * Return: status code - */ efi_status_t __efi_runtime efi_get_variable_mem(u16 *variable_name, const efi_guid_t *vendor, u32 *attributes, -- 2.28.0
[PATCH 1/1] configs: defconfig for Sipeed Maix in S-mode
Provide a defconfig that can be used to build U-Boot for the Maix boards running upon OpenSBI. Update the documentation. Signed-off-by: Heinrich Schuchardt --- configs/sipeed_maix_smode_defconfig | 10 ++ doc/board/sipeed/maix.rst | 49 + 2 files changed, 59 insertions(+) create mode 100644 configs/sipeed_maix_smode_defconfig diff --git a/configs/sipeed_maix_smode_defconfig b/configs/sipeed_maix_smode_defconfig new file mode 100644 index 00..2516bb7258 --- /dev/null +++ b/configs/sipeed_maix_smode_defconfig @@ -0,0 +1,10 @@ +CONFIG_RISCV=y +CONFIG_SYS_TEXT_BASE=0x8002 +CONFIG_TARGET_SIPEED_MAIX=y +CONFIG_ARCH_RV64I=y +CONFIG_RISCV_SMODE=y +CONFIG_STACK_SIZE=0x10 +# CONFIG_NET is not set +# CONFIG_INPUT is not set +# CONFIG_DM_ETH is not set +# CONFIG_EFI_UNICODE_CAPITALIZATION is not set diff --git a/doc/board/sipeed/maix.rst b/doc/board/sipeed/maix.rst index b1894f3a6f..efcde9aebf 100644 --- a/doc/board/sipeed/maix.rst +++ b/doc/board/sipeed/maix.rst @@ -75,6 +75,49 @@ console shall be opened immediately. Boot output should look like the following: Err: serial@3800 => +OpenSBI +^^^ + +OpenSBI is an open source supervisor execution environment implementing the +RISC-V Supervisor Binary Interface Specification [1]. One of its features is +to intercept run-time exceptions, e.g. for unaligned access or illegal +instructions, and to emulate the failing instructions. + +The OpenSBI source can be downloaded via: + +.. code-block:: bash + +git clone https://github.com/riscv/opensbi + +As OpenSBI will be loaded at 0x8000 we have to adjust the U-Boot text base. +Furthermore we have to enable building U-Boot for S-mode:: + +CONFIG_SYS_TEXT_BASE=0x8002 +CONFIG_RISCV_SMODE=y + +Both settings are contained in sipeed_maix_smode_defconfig so we can build +U-Boot with: + +.. code-block:: bash + +make sipeed_maix_smode_defconfig +make + +To build OpenSBI with U-Boot as a payload: + +.. code-block:: bash + +cd opensbi +make \ +PLATFORM=kendryte/k210 \ +FW_PAYLOAD=y \ +FW_PAYLOAD_OFFSET=0x2 \ +FW_PAYLOAD_PATH=/u-boot-dtb.bin + +The value of FW_PAYLOAD_OFFSET must match CONFIG_SYS_TEXT_BASE - 0x8000. + +The file to flash is build/platform/kendryte/k210/firmware/fw_payload.bin. + Loading Images ^^ @@ -363,3 +406,9 @@ AddressSize Description interrupts) 0x8801f000 0x1000credits == = === + +Links +- + +[1] https://github.com/riscv/riscv-sbi-doc +RISC-V Supervisor Binary Interface Specification -- 2.28.0
Re: [RFC PATCH v2 0/3] RFC: tiny-dm: Proposal for using driver model in SPL
Hi Walter, On Sun, 26 Jul 2020 at 20:45, Walter Lozano wrote: > > Hi Simon, > > > On 10/7/20 01:12, Walter Lozano wrote: > > Hi Simon, > > > > On 2/7/20 18:10, Simon Glass wrote: > >> This series provides a proposed enhancement to driver model to reduce > >> overhead in SPL. > >> > >> These patches should not be reviewed other than to comment on the > >> approach. The code is all lumped together in a few patches and so cannot > >> be applied as is. > >> > >> For now, the source tree is available at: > >> > >> https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working > >> > >> Comments welcome! > >> > >> Benefits (good news) > >> > >> > >> As an example of the impact of tiny-dm, chromebook_jerry is converted to > >> use it. This shows approximately a 30% reduction in code and data > >> size and > >> a 85% reduction in malloc() space over of-platdata: > >> > >> text databssdechexfilename > >>25248 1836 12 27096 69d8 spl/u-boot-spl > >> (original with DT) > >>19727 3436 12 23175 5a87 spl/u-boot-spl > >> (OF_PLATDATA) > >> 78%187%100% 86% as %age of original > >> > >>13784 1408 12 15204 3b64 spl/u-boot-spl > >> (SPL_TINY) > >> 70% 41%100% 66% as %age of platdata > >> 55% 77%100% 56% as %age of original > >> > >> SPL malloc() usage drops from 944 bytes (OF_PLATDATA) to 116 (SPL_TINY). > >> > >> Overall the 'overhead' of tiny-dm is much less than the full driver > >> model. Code size is currently about 600 bytes for these functions on > >> Thumb2: > >> > >> 0054 T tiny_dev_probe > >> 0034 T tiny_dev_get_by_drvdata > >> 0024 T tiny_dev_find > >> 001a T tiny_dev_get > >> 003c T tinydev_alloc_data > >> 002a t tinydev_lookup_data > >> 0022 T tinydev_ensure_data > >> 0014 T tinydev_get_data > >> 0004 T tinydev_get_parent > >> > >> Effort (bad news) > >> - > >> > >> Unfortunately it is quite a bit of work to convert drivers over to > >> tiny-dm. First, the of-platdata conversion must be done. But on top of > >> that, tiny-dm needs entirely separate code for dealing with devices. > >> This > >> means that instead of 'struct udevice' and 'struct uclass' there is just > >> 'struct tinydev'. Each driver and uclass must be modified to support > >> both, pulling common code into internal static functions. > >> > >> Another option > >> -- > >> > >> Note: It is assumed that any board that is space-contrained should use > >> of-platdata in SPL (see doc/driver-model/of-plat.rst). This is shown to > >> reduce device-tree overhead by approximately 4KB. > >> > >> Designing tiny-dm has suggested a number of things that could be changed > >> in the current driver model to make it more space-efficient for TPL and > >> SPL. The ones with least impact on driver code are (CS=reduces code > >> size, > >> DS=reduces data size): > >> > >> CS - drop driver_bind() and create devices (struct udevice) at > >> build-time > >> CS - allocate all device- and uclass-private data at build-time > >> CS - remove all but the basic operations for each uclass (e.g. SPI > >> flash only supports reading) > >> DS - use 8-bit indexes instead of 32/64-bit pointers for device > >> pointers possible since these are created at build-time) > >> DS - use singly-linked lists > >> DS - use 16-bit offsets to private data, instead of 32/64-bit > >> pointers > >> (possible since it is all in SRAM relative to malloc() base, > >> presumably word-aligned and < 256KB) > >> DS - move private pointers into a separate data structure so that > >> NULLs > >> are not stored > >> CS / DS - Combine req_seq and seq and calculate the new value at > >> build-time > >> > >> More difficult are: > >> > >> DS - drop some of the lesser-used driver and uclass methods > >> DS - drop all uclass methods except init() > >> DS - drop all driver methods except probe() > >> CS / DS - drop uclasses and require drivers to manually call uclass > >> functions > >> > >> Even with all of this we would not reach tiny-dm and it would muddy > >> up the > >> driver-model datas structures. But we might come close to tiny-dm on > >> size > >> and there are some advantages: > >> > >> - much of the benefit would apply to all boards that use of-platdata > >> (i.e. > >>with very little effort on behalf of board maintainers) > >> - the impact on the code base is much less (we keep a single, unified > >>driver mode in SPL and U-Boot proper) > >> > >> Overall I think it is worth looking at this option. While it doesn't > >> have > >> the 'nuclear' impact of tiny-dm, neither does it mess with the U-Boot > >> driver code as much and it is easier to learn. > > >
Re: [RFC PATCH v2 0/3] RFC: tiny-dm: Proposal for using driver model in SPL
Hi Simon, On 16/8/20 00:06, Simon Glass wrote: Hi Walter, On Sun, 26 Jul 2020 at 20:45, Walter Lozano wrote: Hi Simon, On 10/7/20 01:12, Walter Lozano wrote: Hi Simon, On 2/7/20 18:10, Simon Glass wrote: This series provides a proposed enhancement to driver model to reduce overhead in SPL. These patches should not be reviewed other than to comment on the approach. The code is all lumped together in a few patches and so cannot be applied as is. For now, the source tree is available at: https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working Comments welcome! Benefits (good news) As an example of the impact of tiny-dm, chromebook_jerry is converted to use it. This shows approximately a 30% reduction in code and data size and a 85% reduction in malloc() space over of-platdata: text databssdechexfilename 25248 1836 12 27096 69d8 spl/u-boot-spl (original with DT) 19727 3436 12 23175 5a87 spl/u-boot-spl (OF_PLATDATA) 78%187%100% 86% as %age of original 13784 1408 12 15204 3b64 spl/u-boot-spl (SPL_TINY) 70% 41%100% 66% as %age of platdata 55% 77%100% 56% as %age of original SPL malloc() usage drops from 944 bytes (OF_PLATDATA) to 116 (SPL_TINY). Overall the 'overhead' of tiny-dm is much less than the full driver model. Code size is currently about 600 bytes for these functions on Thumb2: 0054 T tiny_dev_probe 0034 T tiny_dev_get_by_drvdata 0024 T tiny_dev_find 001a T tiny_dev_get 003c T tinydev_alloc_data 002a t tinydev_lookup_data 0022 T tinydev_ensure_data 0014 T tinydev_get_data 0004 T tinydev_get_parent Effort (bad news) - Unfortunately it is quite a bit of work to convert drivers over to tiny-dm. First, the of-platdata conversion must be done. But on top of that, tiny-dm needs entirely separate code for dealing with devices. This means that instead of 'struct udevice' and 'struct uclass' there is just 'struct tinydev'. Each driver and uclass must be modified to support both, pulling common code into internal static functions. Another option -- Note: It is assumed that any board that is space-contrained should use of-platdata in SPL (see doc/driver-model/of-plat.rst). This is shown to reduce device-tree overhead by approximately 4KB. Designing tiny-dm has suggested a number of things that could be changed in the current driver model to make it more space-efficient for TPL and SPL. The ones with least impact on driver code are (CS=reduces code size, DS=reduces data size): CS - drop driver_bind() and create devices (struct udevice) at build-time CS - allocate all device- and uclass-private data at build-time CS - remove all but the basic operations for each uclass (e.g. SPI flash only supports reading) DS - use 8-bit indexes instead of 32/64-bit pointers for device pointers possible since these are created at build-time) DS - use singly-linked lists DS - use 16-bit offsets to private data, instead of 32/64-bit pointers (possible since it is all in SRAM relative to malloc() base, presumably word-aligned and < 256KB) DS - move private pointers into a separate data structure so that NULLs are not stored CS / DS - Combine req_seq and seq and calculate the new value at build-time More difficult are: DS - drop some of the lesser-used driver and uclass methods DS - drop all uclass methods except init() DS - drop all driver methods except probe() CS / DS - drop uclasses and require drivers to manually call uclass functions Even with all of this we would not reach tiny-dm and it would muddy up the driver-model datas structures. But we might come close to tiny-dm on size and there are some advantages: - much of the benefit would apply to all boards that use of-platdata (i.e. with very little effort on behalf of board maintainers) - the impact on the code base is much less (we keep a single, unified driver mode in SPL and U-Boot proper) Overall I think it is worth looking at this option. While it doesn't have the 'nuclear' impact of tiny-dm, neither does it mess with the U-Boot driver code as much and it is easier to learn. Thanks for your hard work on this topic. I think that there is great value in this research and in this conclusion. It is clear that there two different approaches, but I feel that the improvement to the current DM implementation would have a higher impact in the community. Since the first version of this proposal I have been thinking in a solution that takes some of the advantages of tiny-dm idea but that does not require so much effort. This seems to be aligned with what you h
Re: [PATCH 1/3] gpio: at91: use dev_read_addr() to get base address
On Mon, 3 Aug 2020 at 23:15, Masahiro Yamada wrote: > > It is strange to use devfdt_get_addr_ptr(), then cast the pointer > back to uint32 because you could use devfdt_get_addr() without casting. > > Convert it to dev_read_addr(), which is capable to CONFIG_OF_LIVE. > > Signed-off-by: Masahiro Yamada > --- > > drivers/gpio/at91_gpio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 3/3] treewide: convert devfdt_get_addr_ptr() to dev_read_addr_ptr()
On Mon, 3 Aug 2020 at 23:15, Masahiro Yamada wrote: > > When you enable CONFIG_OF_LIVE, you will end up with a lot of > conversions. > > To help this tedious work, this commit converts devfdt_get_addr_ptr() > to dev_read_addr_ptr() by coccinelle. I also removed redundant casts > because dev_read_addr_ptr() returns an opaque pointer. > > To generate this commit, I ran the following semantic patch > excluding include/dm/. > > > @@ > type T; > expression dev; > @@ > -(T *)devfdt_get_addr_ptr(dev) > +dev_read_addr_ptr(dev) > @@ > expression dev; > @@ > -devfdt_get_addr_ptr(dev) > +dev_read_addr_ptr(dev) > > > Signed-off-by: Masahiro Yamada > --- > > drivers/clk/aspeed/clk_ast2500.c | 2 +- > drivers/clk/at91/pmc.c| 4 ++-- > drivers/gpio/hsdk-creg-gpio.c | 2 +- > drivers/i2c/ast_i2c.c | 2 +- > drivers/i2c/designware_i2c.c | 2 +- > drivers/i2c/mv_i2c.c | 2 +- > drivers/i2c/mvtwsi.c | 4 ++-- > drivers/mmc/gen_atmel_mci.c | 2 +- > drivers/mmc/snps_dw_mmc.c | 2 +- > drivers/pinctrl/mvebu/pinctrl-mvebu.c | 2 +- > drivers/reset/reset-socfpga.c | 2 +- > drivers/serial/serial_mvebu_a3700.c | 2 +- > drivers/spi/uniphier_spi.c| 2 +- > drivers/sysreset/sysreset_socfpga.c | 2 +- > drivers/timer/ast_timer.c | 2 +- > drivers/timer/atmel_pit_timer.c | 2 +- > drivers/usb/host/ehci-zynq.c | 2 +- > drivers/watchdog/ast_wdt.c| 2 +- > 18 files changed, 20 insertions(+), 20 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v1 04/15] x86: coreboot: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/coreboot/coreboot/Makefile | 2 +- > board/coreboot/coreboot/start.S | 12 > 2 files changed, 1 insertion(+), 13 deletions(-) > delete mode 100644 board/coreboot/coreboot/start.S > Reviewed-by: Simon Glass
Re: [PATCH v2 03/21] clk: bind clk to new parent device
Hi Claudiu, On Thu, 6 Aug 2020 at 02:24, wrote: > > Hi Simon, > > On 05.08.2020 18:11, Claudiu Beznea wrote: > > Clock re-parenting is not binding the clock's device to its new > > parent device, it only calls the clock's ops->set_parent() API. The > > changes in this commit re-parent the clock device to its new parent > > so that subsequent operations like clk_get_parent() to point to the > > proper parent. > > > > Signed-off-by: Claudiu Beznea > > Reviewed-by: Simon Glass > > I added your Reviewed-by here but at the same time I also added the sandbox > test in this patch. Let me know if you want the sandbox test in a separate > patch. Same thing for patch 4/21 in this series. I think it is nice to have the test in the same patch in many cases, since it clearly shows that there is a test for the change / new functionality. Regards, Simon
Re: [PATCH v2 04/21] clk: do not disable clock if it is critical
On Wed, 5 Aug 2020 at 09:12, Claudiu Beznea wrote: > > Do not disable clock if it is a critical one. > > Signed-off-by: Claudiu Beznea > Reviewed-by: Simon Glass > --- > drivers/clk/clk-uclass.c | 3 +++ > test/dm/clk_ccf.c| 32 +++- > 2 files changed, 34 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 1/2] cmd: clk: cosmetic: correct code alignment in show_clks
On Thu, 30 Jul 2020 at 06:04, Patrick Delaunay wrote: > > Correct code alignment in show_clks() function. > > Signed-off-by: Patrick Delaunay > --- > > cmd/clk.c | 32 > 1 file changed, 16 insertions(+), 16 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v1 10/15] x86: intel: crownbay: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/crownbay/Makefile | 2 +- > board/intel/crownbay/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/intel/crownbay/start.S > Reviewed-by: Simon Glass
Re: [PATCH v1 12/15] x86: intel: galileo: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/galileo/Makefile | 2 +- > board/intel/galileo/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/intel/galileo/start.S Reviewed-by: Simon Glass
Re: [PATCH v1 15/15] x86: qemu: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/emulation/qemu-x86/Makefile | 2 -- > board/emulation/qemu-x86/start.S | 8 > 2 files changed, 10 deletions(-) > delete mode 100644 board/emulation/qemu-x86/start.S Reviewed-by: Simon Glass
Re: [PATCH v4 20/27] Makefile: Warn against using CONFIG_SPL_FIT_GENERATOR
Hi Michal, On Fri, 14 Aug 2020 at 07:28, Michal Simek wrote: > > Hi Simon, > > ne 19. 7. 2020 v 22:06 odesílatel Simon Glass napsal: > > > > This option is used to run arch-specific shell scripts which produce .its > > files which are used to produce FIT images. We already have binman which > > is designed to produce firmware images. It is more powerful and has tests. > > > > So this option should be deprecated and not used. Existing uses should be > > migrated. > > > > Mentions of this in code reviews over the last year or so do not seem to > > have resulted in action, and things are getting worse. > > > > So let's add a warning. > > > > Signed-off-by: Simon Glass > > Reviewed-by: Bin Meng > > --- > > > > (no changes since v1) > > > > Makefile | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/Makefile b/Makefile > > index f1b5be1882..d73c10a973 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1148,6 +1148,13 @@ ifneq ($(CONFIG_DM_ETH),y) > > @echo >&2 "See doc/driver-model/migration.rst for more info." > > @echo >&2 "" > > endif > > +endif > > +ifneq ($(CONFIG_SPL_FIT_GENERATOR),) > > + @echo >&2 "= WARNING ==" > > + @echo >&2 "This board uses CONFIG_SPL_FIT_GENERATOR. Please migrate" > > + @echo >&2 "to binman instead, to avoid the proliferation of" > > + @echo >&2 "arch-specific scripts with no tests." > > + @echo >&2 "" > > endif > > @# Check that this build does not use CONFIG options that we do not > > @# know about unless they are in Kconfig. All the existing CONFIG > > @@ -1345,6 +1352,8 @@ endif > > > > # Boards with more complex image requirements can provide an .its source > > file > > # or a generator script > > +# NOTE: Please do not use this. We are migrating away from Makefile rules > > to use > > +# binman instead. > > ifneq ($(CONFIG_SPL_FIT_SOURCE),"") > > U_BOOT_ITS := u-boot.its > > $(U_BOOT_ITS): $(subst ",,$(CONFIG_SPL_FIT_SOURCE)) > > -- > > 2.28.0.rc0.105.gf9edc3c819-goog > > > > I just got to this conversion and I am curious how that transition > should look like. > I found how FIT image is created which is fine but I didn't find any > reference on how to generate images based on CONFIG_OF_LIST. > If you look at arch/arm/mach-zynqmp/mkimage_fit_atf.sh you will see > that I loop over this entry and create multiple DT nodes and the same > amount of configurations to cover it. Is this supported by binman? > If yes, what's the syntax for it? The easiest way is probably to create a new entry type, like zynq-fit. Then you can generate the DT using the sequence writer functions. See _ReadSubNodes() in fit.py for an example. You can perhaps have a template subnode and use that in a for loop to generate the nodes. > > I tried several configurations and we can use that for generating qspi > images and also images with different configurations to have them > ready > but first I need to be able to handle the case above. I was thinking of converting sunxi which has the same need, but it sounds like you are on the case. Let me know if you need help. Regards, Simon
Re: [PATCH v1 14/15] x86: intel: slimbootloader: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Cc: Aiden Park > Signed-off-by: Andy Shevchenko > --- > board/intel/slimbootloader/Makefile | 2 +- > board/intel/slimbootloader/start.S | 9 - > 2 files changed, 1 insertion(+), 10 deletions(-) > delete mode 100644 board/intel/slimbootloader/start.S Reviewed-by: Simon Glass
Re: x86: apl: Acessing SPI flash when booting with Coreboot/U-Boot
Hi Wolfgang, On Fri, 14 Aug 2020 at 08:04, Wolfgang Wallner wrote: > > Hi Simon, > > Since commit 609b90a6a9c0 ("x86: spi: Rewrite logic for obtaining the SPI > memory map") I have trouble accessing the SPI flash on my Apollo Lake board > when booting with Coreboot and having U-Boot as the payload. > > Accessing the SPI flash returns -91 (EPROTOTYPE). > > My understanding of what happens is the following: > > - ich_spi_ofdata_to_platdata() calls ich_spi_get_basics(), the parameter >'can_probe' is hardcoded to true > - There is no PCH device in my devicetree (there is also no PCH driver >contained in my build) > - As can_probe is true, ich_spi_get_basics() tries to find a PCH device >and returns -EPROTOTYPE as it finds none > > As far as I see the PCH is not used in the code paths relevant to Apollo Lake. > I think this behavior can not be triggered in a standalone U-Boot on Apollo > Lake, > as in this case arch_cpu_init_tpl() requires an LPC, which is below the PCH, > and so a PCH has to be part of the devicetree anyway. In this case a PCH would > be found in ich_spi_get_basics(), but it would not get used. > > As a quick workaround [1], I have set can_probe hardcoded to false, and with > this change I can access the SPI flash again. > > Do you have any advice on how to fix this? > How about making the value for can_probe depend on a check for > ich_version == ICHV_APL? That might be OK I think. I am quite unhappy with how this has turned out. It seems like we might be able to refactor it to reduce the number of cases. > > regards, Wolfgang > > > [1] Workaround patch: > > --- a/drivers/spi/ich.c > +++ b/drivers/spi/ich.c > @@ -1016,7 +1016,7 @@ static int ich_spi_ofdata_to_platdata(struct udevice > *dev) > #if !CONFIG_IS_ENABLED(OF_PLATDATA) > struct ich_spi_priv *priv = dev_get_priv(dev); > > - ret = ich_spi_get_basics(dev, true, &priv->pch, &plat->ich_version, > + ret = ich_spi_get_basics(dev, false, &priv->pch, &plat->ich_version, > > > Regards, Simon
Re: [PATCH] x86: mtrr: Fix parsing of "mtrr list" command
On Fri, 14 Aug 2020 at 01:55, Wolfgang Wallner wrote: > > The command 'mtrr' does not recognize the 'list' subcommand any more > since the code restructuring in commit b2a76b3fe75a ("x86: mtrr: > Restructure so command execution is in one place"). > > The if-else parsing the command arguments does not take 'list' into > account: the if-branch is intended for no subcommands, the else-branch > is intended for the non-list subcommands (which all expect additional > arguments). Calling the 'mtrr list' subcommand leads to a "return > CMD_RET_USAGE" in the else-branch. > > Fix this by changing the else-branch to explicitly checking for > if (cmd != 'l'). > > Fixes: b2a76b3fe75a ("x86: mtrr: Restructure so command execution is in one > place") > > Signed-off-by: Wolfgang Wallner > > --- > > cmd/x86/mtrr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass Ah yes, I never actually use it - since it is implicit. Regards, Simon
Re: [PATCH 3/7] wdt: dw: Fix clock rate being off by 1000
On Wed, 5 Aug 2020 at 13:14, Sean Anderson wrote: > > The clock subsystem returns clock rates in Hz. We need to divide by 1000 so > the rate is in kHz. > > Fixes: cf89ef8d10f240554541c20b2e1bdcdd58d1d7e6 > Signed-off-by: Sean Anderson > --- > > drivers/watchdog/designware_wdt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 5/7] wdt: dw: Free the clock on error
On Wed, 5 Aug 2020 at 13:14, Sean Anderson wrote: > > The clock subsystem requires that clk_free be called on clocks obtained via > clk_get_*. > > Signed-off-by: Sean Anderson > --- > > drivers/watchdog/designware_wdt.c | 17 - > 1 file changed, 12 insertions(+), 5 deletions(-) > Reviewed-by: Simon Glass
Re: pytest to test reset
Hi Peng, On Wed, 12 Aug 2020 at 02:13, Peng Fan wrote: > > Hi All, > > Any ideas how to test uboot reset cmd many times with pytest? Can you use sysreset_allowed[] perhaps? Regards, Simon
Re: [PATCH 2/2] cmd: clk: correctly handle depth for clk dump
On Thu, 30 Jul 2020 at 06:04, Patrick Delaunay wrote: > > Update depth only when clock uclass is found to have correct display > of command "clk dump". > > Without this patch, the displayed depth is the binding depth for > all the uclass and that can be strange as only clock uclass nodes > are displayed. > > Signed-off-by: Patrick Delaunay > --- > > cmd/clk.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH v2 0/8] regmap: Add managed API, regmap fields, regmap config
Hi Pratyush, On Wed, 5 Aug 2020 at 02:07, Pratyush Yadav wrote: > > Hi Simon, > > On 06/06/20 02:00AM, Pratyush Yadav wrote: > > Hi, > > > > This series is a re-spin of Jean-Jacques' earlier effort [0], the goal > > of which was to facilitate porting drivers from the Linux kernel. It > > adds the managed API, using the same API as Linux. It also adds support > > for regmap fields. > > > > Jean-Jacques' series added support for custom regmap read/write > > callbacks. The design was argued against by Simon [1]. He argued that > > using the driver model was a better idea instead of the custom > > functions. That would mean slightly more memory usage and a more > > involved change. > > > > The main aim of adding the custom functions is to support the Cadence > > Sierra PHY driver from Linux [2]. The driver's custom functions aren't > > very complicated, so they can be replaced by some simple formatting > > options. So, add the struct regmap_config which contains fields to alter > > the behaviour of the regmap. This includes specifying the size of the > > read/write operations via 'width', specifying the start and size of > > range from code instead of device tree via 'r_start' and 'r_size', and > > specifying a left shift of the register offset before access via > > 'reg_offset_shift'. The driver can't be ported verbatim now, but this > > allows the changes to be very isolated and minimal. > > > > These config options allow us to avoid converting to driver model until > > we really need it. > > > > The patches are based on [3] which fixes a segmentation fault in sandbox > > which didn't allow the tests to complete. > > > > [0] > > https://patchwork.ozlabs.org/project/uboot/cover/20191105114700.24989-1-jjhib...@ti.com/ > > [1] https://patchwork.ozlabs.org/comment/2426186/ > > [2] > > https://elixir.bootlin.com/linux/latest/source/drivers/phy/cadence/phy-cadence-sierra.c > > [3] > > https://patchwork.ozlabs.org/project/uboot/patch/20200526120557.26240-1-p.ya...@ti.com/ > > I don't see this series in v2020.10-rc1. I was under the impression that > this series was good to go. Has it fell through the cracks somehow? You can check in patchwork to see whose queue it is in. +Tom Rini Regards, Simon
Re: [PATCH v2 01/21] clk: check hw and hw->dev before dereference it
On Wed, 5 Aug 2020 at 09:12, Claudiu Beznea wrote: > > Check hw and hw->dev before dereference it. > > Signed-off-by: Claudiu Beznea > --- > drivers/clk/clk.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 02/21] dm: core: add support for device re-parenting
On Wed, 5 Aug 2020 at 09:12, Claudiu Beznea wrote: > > In common clock framework the relation b/w parent and child clocks is > determined based on the udevice parent/child information. A clock > parent could be changed based on devices needs. In case this is happen > the functionalities for clock who's parent is changed are broken. Add > a function that reparent a device. This will be used in clk-uclass.c > to reparent a clock device. > > Signed-off-by: Claudiu Beznea > --- > drivers/core/device.c| 22 ++ > include/dm/device-internal.h | 9 +++ > test/dm/core.c | 160 > +++ > 3 files changed, 191 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH v2 0/2] reset: Add a managed API
Hi Pratyush, On Wed, 5 Aug 2020 at 02:13, Pratyush Yadav wrote: > > Hi Simon, > > On 12/06/20 05:38PM, Pratyush Yadav wrote: > > Hi, > > > > This is the 4th of a few series that are re-rolls of Jean-Jacques' > > earlier efforts. The goal is to facilitate porting drivers from the > > Linux kernel. > > > > This particular series is about reset controllers. It adds a managed API, > > close to that of Linux. The main difference is that bulk and reset_ctl > > are handled with different functions. > > I don't see this series in v2020.10-rc1. Has it fell through the cracks > somehow? I'm not sure...can you check patchwork? Regards, Simon
Re: [PATCH v1 02/15] x86: advantech: som-db5800-som-6867: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Cc: George McCollister > Signed-off-by: Andy Shevchenko > --- > board/advantech/som-db5800-som-6867/Makefile | 2 +- > board/advantech/som-db5800-som-6867/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/advantech/som-db5800-som-6867/start.S > Reviewed-by: Simon Glass
Re: [PATCH v1 09/15] x86: intel: cougarcanyon2: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/cougarcanyon2/Makefile | 2 +- > board/intel/cougarcanyon2/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/intel/cougarcanyon2/start.S Reviewed-by: Simon Glass
Re: [PATCH v1 11/15] x86: intel: edison: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/edison/Makefile | 2 +- > board/intel/edison/start.S | 12 > 2 files changed, 1 insertion(+), 13 deletions(-) > delete mode 100644 board/intel/edison/start.S > Reviewed-by: Simon Glass
Re: [PATCH 4/7] wdt: dw: Enable the clock before using it
On Wed, 5 Aug 2020 at 13:14, Sean Anderson wrote: > > The watchdog won't work if the clock isn't enabled. > > Fixes: cf89ef8d10f240554541c20b2e1bdcdd58d1d7e6 > Signed-off-by: Sean Anderson > --- > > drivers/watchdog/designware_wdt.c | 4 > 1 file changed, 4 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH v1 06/15] x86: efi: efi-x86_payload: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/efi/efi-x86_payload/Makefile | 2 +- > board/efi/efi-x86_payload/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/efi/efi-x86_payload/start.S > Reviewed-by: Simon Glass
Re: [PATCH v1 08/15] x86: intel: cherryhill: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/cherryhill/Makefile | 2 +- > board/intel/cherryhill/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/intel/cherryhill/start.S > Reviewed-by: Simon Glass
Re: [PATCH v1 07/15] x86: intel: bayleybay: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/bayleybay/Makefile | 2 +- > board/intel/bayleybay/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/intel/bayleybay/start.S > Reviewed-by: Simon Glass
Re: [PATCH 1/4] firmware: add new driver for SCMI firmwares
Hi Etienne, On Tue, 4 Aug 2020 at 03:33, Etienne Carriere wrote: > > Hello Simon, > > Thanks for the feedback, I'll fix the changes in my v2. > and sorry for these delayed answers. > > On Sun, 26 Jul 2020 at 16:55, Simon Glass wrote: > > > > Hi Etienne, > > > > On Fri, 17 Jul 2020 at 09:38, Etienne Carriere > > wrote: > > > > > > This change introduces SCMI agent driver in U-Boot in the firmware > > > U-class. > > > > > > SCMI agent driver is designed for platforms that embed a SCMI server in > > > a firmware hosted for example by a companion co-processor or the secure > > > world of the executing processor. > > > > > > SCMI protocols allow an SCMI agent to discover and access external > > > resources as clock, reset controllers and many more. SCMI agent and > > > server communicate following the SCMI specification [1]. SCMI agent > > > complies with the DT bindings defined in the Linux kernel source tree > > > regarding SCMI agent description since v5.8-rc1. > > > > > > These bindings describe 2 supported message transport layer: using > > > mailbox uclass devices or using Arm SMC invocation instruction. Both > > > use a piece or shared memory for message data exchange. > > > > > > In the current state, the SCMI agent driver does not bind to any SCMI > > > protocol to a U-Boot device driver. Former changes will implement > > > dedicated driver (i.e. an SCMI clock driver or an SCMI reset controller > > > driver) and add bind supported SCMI protocols in scmi_agent_bind(). > > > > > > Links: [1] > > > https://developer.arm.com/architectures/system-architectures/software-standards/scmi > > > Signed-off-by: Etienne Carriere > > > --- > > > > > > drivers/firmware/Kconfig | 15 ++ > > > drivers/firmware/Makefile | 1 + > > > drivers/firmware/scmi.c | 439 ++ > > > include/scmi.h| 82 +++ > > > 4 files changed, 537 insertions(+) > > > create mode 100644 drivers/firmware/scmi.c > > > create mode 100644 include/scmi.h > > > > > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > > > index b70a2063551..f7c7ee7a5aa 100644 > > > --- a/drivers/firmware/Kconfig > > > +++ b/drivers/firmware/Kconfig > > > @@ -1,6 +1,21 @@ > > > config FIRMWARE > > > bool "Enable Firmware driver support" > > > > > > +config SCMI_FIRMWARE > > > + bool "Enable SCMI support" > > > + select FIRMWARE > > > + select OF_TRANSLATE > > > + depends on DM_MAILBOX || ARM_SMCCC > > > + help > > > + An SCMI agent communicates with a related SCMI server firmware > > > > Please write out SCMI in full somewhere and add a link to the spec. > > > > > + located in another sub-system, as a companion micro controller > > > + or a companion host in the CPU system. > > > + > > > + Communications between agent (client) and the SCMI server are > > > + based on message exchange. Messages can be exchange over > > > tranport > > > + channels as a mailbox device or an Arm SMCCC service with some > > > + piece of identified shared memory. > > > + > > > config SPL_FIRMWARE > > > bool "Enable Firmware driver support in SPL" > > > depends on FIRMWARE > > > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile > > > index a0c250a473e..3965838179f 100644 > > > --- a/drivers/firmware/Makefile > > > +++ b/drivers/firmware/Makefile > > > @@ -2,4 +2,5 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o > > > obj-$(CONFIG_$(SPL_)ARM_PSCI_FW) += psci.o > > > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o > > > obj-$(CONFIG_SANDBOX) += firmware-sandbox.o > > > +obj-$(CONFIG_SCMI_FIRMWARE)+= scmi.o > > > obj-$(CONFIG_ZYNQMP_FIRMWARE) += firmware-zynqmp.o > > > diff --git a/drivers/firmware/scmi.c b/drivers/firmware/scmi.c > > > new file mode 100644 > > > index 000..fa8a91c3f3d > > > --- /dev/null > > > +++ b/drivers/firmware/scmi.c > > > @@ -0,0 +1,439 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright (c) 2015-2019, Arm Limited and Contributors. All rights > > > reserved. > > > + * Copyright (C) 2019-2020 Linaro Limited. > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define TIMEOUT_US_10MS1 > > > + > > > +struct error_code { > > > > Function comment > > > > > + int scmi; > > > + int errno; > > > +}; > > > + > > > +static const struct error_code scmi_linux_errmap[] = { > > > + { .scmi = SCMI_NOT_SUPPORTED, .errno = -EOPNOTSUPP, }, > > > + { .scmi = SCMI_INVALID_PARAMETERS, .errno = -EINVAL, }, > > > + { .scmi = SCMI_DENIED, .errno = -EACCES, }, > > > + { .scmi = SCMI
Re: [PATCH v2 05/21] clk: get clock pointer before proceeding
On Wed, 5 Aug 2020 at 09:12, Claudiu Beznea wrote: > > clk_get_by_indexed_prop() retrieves a clock with dev member being set > with the pointer to the udevice for the clock controller driver. But > in case of CCF each clock driver has set in dev member the reference > to its parent (the root of the clock tree is a fixed clock, every > node in clock tree is a clock registered with clk_register()). In this > case the subsequent operations like dev_get_clk_ptr() on clocks > retrieved by clk_get_by_indexed_prop() will fail. For this, get the > pointer to the proper clock registered (with clk_register()) using > clk_get_by_id() before proceeding. > > Fixes: 1d7993d1d0ef ("clk: Port Linux common clock framework [CCF] for imx6q > to U-boot (tag: v5.1.12)") > Signed-off-by: Claudiu Beznea > --- > drivers/clk/clk-uclass.c | 37 + > 1 file changed, 33 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v1 13/15] x86: intel: minnowmax: Remove dead code
On Thu, 6 Aug 2020 at 08:54, Andy Shevchenko wrote: > > start.S does nothing and can be safely removed. Makefile is still being used > by the build system, so simply drop the rule from it. > > Signed-off-by: Andy Shevchenko > --- > board/intel/minnowmax/Makefile | 2 +- > board/intel/minnowmax/start.S | 8 > 2 files changed, 1 insertion(+), 9 deletions(-) > delete mode 100644 board/intel/minnowmax/start.S > Reviewed-by: Simon Glass