Re: [RFC PATCH 00/16] arm: Add Rockchip RK3588 support

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Mon, 13 Mar 2023 16:21:36 +0200
> From: Eugen Hristev <eugen.hristev@collabora.com>
> 
> On 3/13/23 12:00, Jonas Karlman wrote:
> > On 2023-03-13 09:42, Eugen Hristev wrote:
> >> On 3/13/23 00:34, Jonas Karlman wrote:
> >>> Hi Eugen,
> >>>
> >>> On 2023-03-08 09:57, Eugen Hristev wrote:
> >>>> On 1/29/23 11:04, Jonas Karlman wrote:
> >>>>> On 2023-01-27 14:21, Jagan Teki wrote:
> >>>>>> On Fri, 27 Jan 2023 at 05:13, Jonas Karlman <jonas@kwiboo.se> wrote:
> >>>>>>>
> >>>>>>> On 2023-01-26 23:16, Jonas Karlman wrote:
> >>>>>>>> Hi Jagan,
> >>>>>>>> On 2023-01-26 20:17, Jagan Teki wrote:
> >>>>>>>>> On Fri, 27 Jan 2023 at 00:33, Jonas Karlman <jonas@kwiboo.se> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On 2023-01-26 19:26, Jagan Teki wrote:
> >>>>>>>>>>> Hi Simon,
> >>>>>>>>>>>
> >>>>>>>>>>> On Thu, 26 Jan 2023 at 23:34, Simon Glass <sjg@chromium.org> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Hi Jagan,
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Thu, 26 Jan 2023 at 10:42, Jagan Teki <jagan@edgeble.ai> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Thu, 26 Jan 2023 at 22:28, Jonas Karlman <jonas@kwiboo.se> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Hi Jagan,
> >>>>>>>>>>>>>> On 2023-01-26 17:51, Jagan Teki wrote:
> >>>>>>>>>>>>>>> Hi Jonas,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On Thu, 26 Jan 2023 at 04:17, Jonas Karlman <jonas@kwiboo.se> wrote:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Hi Jagan,
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> On 2023-01-25 23:27, Jagan Teki wrote:
> >>>>>>>>>>>>>>>>> This series support Rockchip RK3588. All the device tree files are
> >>>>>>>>>>>>>>>>> synced from linux-next with the proper SHA1 mentioned in the commit
> >>>>>>>>>>>>>>>>> messages.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Unfortunately, the BL31 from rkbin is not compatible with U-Boot so
> >>>>>>>>>>>>>>>>> it is failing to load ATF entry from SPL and hang.
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Verified below BL31 versions,
> >>>>>>>>>>>>>>>>>      bl31-v1.15
> >>>>>>>>>>>>>>>>>      bl31-v1.21
> >>>>>>>>>>>>>>>>>      bl31-v1.22
> >>>>>>>>>>>>>>>>>      bl31-v1.23
> >>>>>>>>>>>>>>>>>      bl31-v1.24
> >>>>>>>>>>>>>>>>>      bl31-v1.25
> >>>>>>>>>>>>>>>>>      bl31-v1.26
> >>>>>>>>>>>>>>>>>
> >>>>>>
> >>>>>> < snip >
> >>>>>>
> >>>>>>>>
> >>>>>>>> As you can see in the logs above there is timeout waiting for data.
> >>>>>>>>
> >>>>>>>> I managed to find the issue and have a workaround that gets me longer
> >>>>>>>> in the boot process, there still seem to be other issue with the rk3588
> >>>>>>>> startup.
> >>>>>>>>
> >>>>>>>> --------
> >>>>>>>> U-Boot SPL 2023.01 (Jan 26 2023 - 22:03:00 +0000)
> >>>>>>>> Trying to boot from MMC1
> >>>>>>>> ## Checking hash(es) for config config-1 ... OK
> >>>>>>>> ## Checking hash(es) for Image atf-1 ... sha256+ OK
> >>>>>>>> ## Checking hash(es) for Image u-boot ... sha256+ OK
> >>>>>>>> ## Checking hash(es) for Image fdt-1 ... sha256+ OK
> >>>>>>>> ## Checking hash(es) for Image atf-2 ... sha256+ OK
> >>>>>>>> ## Checking hash(es) for Image atf-3 ... sha256+ OK
> >>>>>>>> INFO:    Preloader serial: 2
> >>>>>>>> NOTICE:  BL31: v2.3():v2.3-468-ge529a2760:derrick.huang
> >>>>>>>> NOTICE:  BL31: Built : 09:59:49, Nov 21 2022
> >>>>>>>> INFO:    spec: 0x1
> >>>>>>>> INFO:    ext 32k is not valid
> >>>>>>>> INFO:    ddr: stride-en 4CH
> >>>>>>>> INFO:    GICv3 without legacy support detected.
> >>>>>>>> INFO:    ARM GICv3 driver initialized in EL3
> >>>>>>>> INFO:    valid_cpu_msk=0xff bcore0_rst = 0x0, bcore1_rst = 0x0
> >>>>>>>> INFO:    system boots from cpu-hwid-0
> >>>>>>>> INFO:    idle_st=0x21fff, pd_st=0x11fff9, repair_st=0xfff70001
> >>>>>>>> INFO:    dfs DDR fsp_params[0].freq_mhz= 2112MHz
> >>>>>>>> INFO:    dfs DDR fsp_params[1].freq_mhz= 528MHz
> >>>>>>>> INFO:    dfs DDR fsp_params[2].freq_mhz= 1068MHz
> >>>>>>>> INFO:    dfs DDR fsp_params[3].freq_mhz= 1560MHz
> >>>>>>>> INFO:    BL31: Initialising Exception Handling Framework
> >>>>>>>> INFO:    BL31: Initializing runtime services
> >>>>>>>> WARNING: No OPTEE provided by BL2 boot loader, Booting device without OPTEE initialization. SMC`s destined for OPTEE will return SMC_UNK
> >>>>>>>> ERROR:   Error initializing runtime service opteed_fast
> >>>>>>>> INFO:    BL31: Preparing for EL3 exit to normal world
> >>>>>>>> INFO:    Entry point address = 0xa00000
> >>>>>>>> INFO:    SPSR = 0x3c9
> >>>>>>>> "Synchronous Abort" handler, esr 0x96000000
> >>>>>>>> elr: 0000000000a23650 lr : 0000000000a24d9c
> >>>>>>>> x0 : 0000000000b7fbe8 x1 : 350003402a0003f3
> >>>>>>>> x2 : 0000000000000000 x3 : 0000000000b80ff0
> >>>>>>>> x4 : 0000000000b80ff0 x5 : 0000000000b80e88
> >>>>>>>> x6 : 0000000000000054 x7 : 0000000000000044
> >>>>>>>> x8 : 000000000000000a x9 : 0000000000000000
> >>>>>>>> x10: 0000000000000034 x11: 0000000000000002
> >>>>>>>> x12: 0000000000001988 x13: 0000000000b7fadc
> >>>>>>>> x14: 0000000000a7e808 x15: 0000000000a7e808
> >>>>>>>> x16: 0000000000000000 x17: 0000000000000000
> >>>>>>>> x18: 0000000000b7fe50 x19: 0000000000b7fbe8
> >>>>>>>> x20: 000000003c14dc00 x21: 000000003c14dc00
> >>>>>>>> x22: 0000000000a7e808 x23: 0000000000000000
> >>>>>>>> x24: 0000000000000000 x25: 0000000000000000
> >>>>>>>> x26: 0000000000000000 x27: 0000000000000000
> >>>>>>>> x28: 0000000000000000 x29: 0000000000b7fb80
> >>>>>>>>
> >>>>>>>> Code: f90013f5 f9400001 b4000201 f9400021 (f9403435)
> >>>>>>>> Resetting CPU ...
> >>>>>>>> --------
> >>>>>>>>
> >>>>>>>> This was running on top of u-boot-dm/master 060a65e899859dcbf42049a18be20ce7118e7c0e
> >>>>>>>> with some rk3568 patches and this series, see [1].
> >>>>>>>>
> >>>>>>>> The last 3 commits contains workaround to issue with sdmmc clock.
> >>>>>>>> dwmmc driver set sclk to (uint)-2, my workaround just adds a
> >>>>>>>> fallback to default 400khz clock.
> >>>>>>>>
> >>>>>>>> Next issue is the sync abort, looks it happens when u-boot
> >>>>>>>> tries to set clock rates based on devicetree. this is the
> >>>>>>>> last debug line before the crash.
> >>>>>>>>
> >>>>>>>> clk_set_rate(clk=0000000000b7fba8, rate=1008000000)
> >>>>>>>
> >>>>>>> With the commit at [2] I can now successfully run U-Boot proper.
> >>>>>>>
> >>>>>>> Source of the two main issues to get this series to run have been the scmi clocks.
> >>>>>>> Vendor u-boot first load its scmi driver before trying to set the cpu clocks.
> >>>>>>> We can just remove it and leave setting a faster cpu rate to linux.
> >>>>>>>
> >>>>>>> I also noticed that my sdram size series only detect the first two channels of memory,
> >>>>>>> will respin a v2 of that series to add detection of all 4 channels of memory.
> >>>>>>>
> >>>>>>> [2] https://github.com/Kwiboo/u-boot-rockchip/commit/7e4fee945e725c0ec9ef3905c41ce367e77833b3
> >>>>>>>
> >>>>>> Okay. We need to find a way to handle the clock value 400Khz
> >>>>>> generically via the CLK framework or eMMC can be worth checking as it
> >>>>>> doesn't involve SCMI and have a working patch set before MW. I did
> >>>>>> that and was able to detect eMMC in U-Boot proper but got some issues
> >>>>>> while booting from eMMC [3]
> >>>>>
> >>>>> I have an updated branch at [4] that should support booting from sdmmc and sdhci.
> >>>>> Tested booting from both sd-card and emmc on my Radxa ROCK 5 Model B.
> >>>>>
> >>>>> This branch is based on u-boot/master f147aa80f52989c7455022ca1ab959e8545feccc
> >>>>> and I have included some rk3568 patches and your rk3588 rfc series.
> >>>>> I added a few fixup on top of that and a few additional patches, please see commit message
> >>>>> for a very brief note on why the change was needed.
> >>>>> Feel free to squash fixups and pick commits up to and possible including
> >>>>> "board: rockchip: Sync evb-rk3568_defconfig with evb-rk3588_defconfig"
> >>>>> for a v2 of this series.
> >>>>>
> >>>>> The remaining sdhci patches needs a little bit more work,
> >>>>> I can send a separate series with emmc patches once they are fully ready.
> >>>>>
> >>>>> The last commit adds a hack to mkimage to keep the data for atf-3 embedded in the FIT.
> >>>>> This ensures we do not run into problems trying to use dma from emmc into secure pmu sram.
> >>>>> I think this is a more appropriate way to work around this issue, instead of patching
> >>>>> u-boot spl_fit or sdhci core to use bounce buffers in a very special case.
> >>>>>
> >>>>> [4] https://github.com/Kwiboo/u-boot-rockchip/commits/rk3588-master-v2
> >>>>>
> >>>> Hello Jagan, Jonas,
> >>>>
> >>>> I wanted to chip into this discussion, to ask whether you did anything
> >>>> more on the SD clock matter ?
> >>>
> >>> I have been busy this past week but have now had time to take a new look
> >>> at the sdmmc issue, along with completing some other fixes.
> >>>
> >>>>
> >>>> I am currently using your workaround that creates dummy clocks in the DT
> >>>> and then the SD node just uses those, and it works, in the SPL, if and
> >>>> only if the bootrom also loads the SPL from SD.
> >>>> I tried to have the SPL inside the SPI flash, or load it to DDR using
> >>>> the rockusb protocol, and then, initializing the SD-Card from SPL fails
> >>>> utterly, namely, it cannot communicate with the card.
> >>>> I did some changes to have the pinctrl working at SPL level, which
> >>>> appears to be fine, but I would like to see what can be done about the
> >>>> clocks.
> >>>> Having the cru and all the required drivers at SPL level is the way to
> >>>> go ? The SPL should run before SCMI is required so it should be able to
> >>>> change all the clocks at the clock controller level ?
> >>>
> >>> I fully agree that we should have some sort of scmi clk driver so that
> >>> we can control the sdmmc clk in both SPL and U-Boot proper.
> >>>
> >>> As you have noticed the current workaround only works because bootrom
> >>> leave the clocks in a working state after it has loaded TPL/SPL from
> >>> the sdmmc device. When TPL/SPL is loaded from any other source it is
> >>> not be possible to read from the sdmmc device in u-boot.
> >>>
> >>> After having played around with the scmi agent driver and being inspired
> >>> by the dummy scmi clk driver in vendor u-boot I have managed to create
> >>> something that could work. See top three commits from [5] for a working
> >>> proof-of-concept.
> >>>
> >>> What I did was to enable the scmi agent driver for use in u-boot proper
> >>> and keeping it disabled in SPL build. Then added a new scmi clk driver
> >>> that is enabled in SPL build, the rk3588 clk driver hooks the new clk
> >>> driver to the scmi_clk ofnode. I only implemented the get/set rate ops
> >>> for the two sdmmc clocks. With this both SPL and U-Boot proper should
> >>> be able to configure the sdmmc clocks.
> >>>
> >>> The initial fixes commits in that branch should hit the list soon.
> >>> Will send the sdmmc related commits once I have had some time to do
> >>> more testing.
> >>>
> >>> [5] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-fixes
> >>>
> >> Hi Jonas,
> >>
> >> I managed to get it working in SPL as well, with a similar code inspired
> >> from the vendor tree.
> >> About the solution, I am not sure whether it should be called a scmi
> >> clock, and whether it should be part of clk_rk3588
> >> Honestly by looking at the DT node, I find the description of the clock
> >> tree wrong to begin with.
> >> The SD should not have the 2 clocks tied to the scmi agent, because this
> >> is true only if it's running in normal world with a secure world agent
> >> that will act as a middle man. So if we are running at a higher EL, as
> >> in the SPL case, the clocks should be tied directly to the CRU.
> >> So the node itself is described bad IMO.
> >>
> >> Anyway, if the node is set in stone from Linux, there isn;t much we can do.
> >> I am also thinking whether we should have the SCMI enabled in SPL as
> >> well, but the agent/clock should just access the CRU directly, meaning
> >> to do something at the agent level :
> >> #ifdef CONFIG_SPL_BUILD
> >> -> access CRU
> >> #else
> >> -> send SCMI message
> >> #endif
> >> Does this make more sense ?
> >>
> >> Or have some kind of wrapper driver that would act as a dispatcher
> >> depending on the SPL/proper build ?
> > 
> > Looks like the sdmmc clock is controlled by the securecru registers and I
> > expect that this can only be configured in secure world, not sure how this
> > could have been modeled differently.
> 
> Since it can work with a clock given by either scmi or securecru, I 
> would expect all clocks to be in the list, and the driver could start 
> the needed clock as per the EL level it's running into :)
> 
> e.g.
> clocks = <&scru CCLK_SD>, <&scru HCLK_SD>, <&scmi CCLK_SD>, <&scmi 
> HCLK_SD> , <&cru basic other clocks..>
> 
> Just my vision of how it could be modeled

That is a change to the device tree bindings.  The goal is to have
U-Boot use the same bindings and device tree as the official device
tree (which currently is the one in mainline Linux).

> > With the approach that I took I think the normal clk framework will behave
> > as such dispatcher and clock gets tied to cru in SPL.
> > 
> > #ifdef CONFIG_SPL_BUILD
> > -> bind protocol@14 to rk3588 securecru driver -> read/write securecru regs
> > #else
> > -> bind protocol@14 to scmi clk driver -> send SCMI message
> > #endif
> 
> This looks right, just that we would have to bring a lot of bloat to SPL
> , firmware subsystem and things we may not want.

I don't see why this would bring in a lot of bloat into SPL.  The SPL
device tree will grow a little bit since it will have to include the
scmi nodes.  And a little bit of additional code in the rk3588 clock
driver.  Navigating the Kconfig stuff is a bit hard, but I don't think
this needs to pull in the firmware subsystem in SPL.

> > 
> > Naming and placement of the SPL securecru driver could be improved.
> > Not sure if any other soc beside rk3588 will need this at this moment.
> 
> securecru sounds better for me, I don't think we can have a scmi clock 
> fake driver like in vendor uboot, it has nothing 'scmi' about it.
> 
> >> Meanwhile, I will test your patches to see how they work on my setup, I
> >> also have some things in progress including pinctrl in SPL for the rock5b.
> >> Thanks for your detailed description,
> > 
> > Thanks for the hint at pinctrl, I made some updates to [5] and could verify
> > that sdmmc works when booting from emmc thanks to your pinctrl commit.
> > Also enabled SPI NOR and eMMC for rock5b in [6], and could do a sf probe
> > and build a spi firmware image.
> > 
> > There is an issue with non-DMA access in sdhci that requires the HACK commit
> > for proper loading of atf, was hoping to disable SDMA in SPL like what is done
> > using fifo-mode for sdmmc. But only first sector is read successfully then it
> > fails, see below. Will take a closer look before I post eMMC series.
> 
> In your patches, I see this :
> https://github.com/Kwiboo/u-boot-rockchip/blob/rk35xx-fixes/arch/arm/dts/rk3588s-u-boot.dtsi#L61
> Perhaps the fifo-mode is intended for the sdhci(emmc) and not for 
> sdmmc(sd-card) ? My understanding was that sdhci cannot do DMA to SRAM, 
> but the sdmmc can ?
> one possible workaround is to have DMA to DRAM and then relocate it to 
> SRAM using the CPU. Having DMA disabled for the whole IP may have 
> downsides, but this is U-boot, we don't expect to have anything else to 
> do with the CPU while the DMA master works its magic, and the CPU should 
> be faster.
> 
> I tested your patches together with the series I sent now
> ( https://marc.info/?l=u-boot&m=167871206530072&w=2 ) and it appears to 
> boot correctly from SD-Card
> Once you send them to the ML I can retest them.
> Thanks !
> 
> > 
> >    Trying to boot from MMC2
> >    1
> >       - 0 'mmc@fe2c0000'
> >       - 1 'mmc@fe2e0000'
> >       - found
> >    blk_find_device: uclass_id=67, devnum=1: mmc@fe2c0000.blk, 67, 0
> >    blk_find_device: uclass_id=67, devnum=1: mmc@fe2e0000.blk, 67, 1
> >    clock is disabled (0Hz)
> >    clock is enabled (400000Hz)
> >    clk_set_rate(clk=5000d8, rate=400000)
> >    size=200, ptr=3b8, limit=100000: 5001b8
> >    clock is enabled (25000000Hz)
> >    clk_set_rate(clk=5000d8, rate=25000000)
> >    clk_set_rate(clk=5000d8, rate=25000000)
> >    clock is enabled (52000000Hz)
> >    clk_set_rate(clk=5000d8, rate=52000000)
> >    spl: mmc boot mode: raw
> >    blk_find_device: uclass_id=67, devnum=1: mmc@fe2c0000.blk, 67, 0
> >    blk_find_device: uclass_id=67, devnum=1: mmc@fe2e0000.blk, 67, 1
> >    hdr read sector 4000, count=1
> >    Found FIT
> >    size=a00, ptr=dc0, limit=100000: aligned to 5003c0
> >    blk_find_device: uclass_id=67, devnum=1: mmc@fe2c0000.blk, 67, 0
> >    blk_find_device: uclass_id=67, devnum=1: mmc@fe2e0000.blk, 67, 1
> >    fit read sector 4000, sectors=5, dst=5003c0, count=0, size=0xa00
> >    mmc_load_image_raw_sector: mmc block read error
> >    spl: mmc boot mode: fs
> >    Trying to boot from MMC1
> > 
> > [6] https://github.com/Kwiboo/u-boot-rockchip/commits/rk3588-emmc
> > 
> > Regards,
> > Jonas
> > 
> > 
> >> Eugen
> >>
> >>>
> >>> Regards,
> >>> Jonas
> >>>
> >>>>
> >>>> Thanks,
> >>>> Eugen
> >>>>
> >>>>>
> >>>>> Regards,
> >>>>> Jonas
> >>>>>
> >>>>>>
> >>>>>> [3] https://github.com/edgeble/u-boot/commit/1389822228ac3dfe8d3cd3513db6a32a5c898b7f
> >>>>>>
> >>>>>> Jagan.
> >>>>>
> >>>>
> >>>
> >>
> > 
> 
>