Hi Quentin,
On 2024-04-02 12:38, Quentin Schulz wrote:
Hi Jonas,
On 3/29/24 20:01, Jonas Karlman wrote:
Extend the Generic RK3566/RK3568 target to also include support for SPI flash, USB OTG, RockUSB and UMS.
Also fix sdmmc alias, include missing pinctrl and add broken-cd prop to fix use of SD-card in linux.
I think we would have benefit with more and smaller commits there, but since you're the one mainly maintaining those generic devices, up to you.
I can try to split it in a v2.
[...]
&sdmmc0 {
- broken-cd; bus-width = <4>; cap-sd-highspeed; disable-wp; no-mmc; no-sdio; pinctrl-names = "default";
- pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd>;
- pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
This is... surprising.
`broken-cd` but we still mux the SDMMC_DET pin in the SD card detect function?
According to the dt binding, if broken-cd is provided, we should do polling. If neither cd-gpios nor broken-cd is passed, host native card detect will be used (which I assume means the SD card controller handles this internally).
c.f. https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bind...
What are we supposed to do there actually, because this seems to be contradicting itself?
The generic DTs is intended to be able to be use as control FDT in U-Boot with any board that mostly follows reference hw dedign as close as possible.
broken-cd was added to fool U-Boot (and possible Linux) into thinking a card is present. And the sdmmc0_det pinctrl was added to satisfy the controller logic and auto jtag.
When I tried to boot into linux with the control FDT prior to this, only eMMC was detected and working, after these changes SD-card was working.
Will re-try without broken-cd for v2.
status = "okay"; };
+&sfc {
- #address-cells = <1>;
- #size-cells = <0>;
- status = "okay";
- flash@0 {
compatible = "jedec,spi-nor";reg = <0>;spi-max-frequency = <24000000>;Random thought, but shouldn't we update common/spl/spl_spi.c to read this value instead of using CONFIG_SF_DEFAULT_SPEED? (Nothing to do in this patch series though :) ).
For U-Boot proper the value from this prop is used, SF_DEFAULT_SPEED is only used in SPL.
[...]
diff --git a/configs/generic-rk3568_defconfig b/configs/generic-rk3568_defconfig index e7d5e55bbfd8..b458080cd539 100644 --- a/configs/generic-rk3568_defconfig +++ b/configs/generic-rk3568_defconfig @@ -3,17 +3,22 @@ CONFIG_SKIP_LOWLEVEL_INIT=y CONFIG_COUNTER_FREQUENCY=24000000 CONFIG_ARCH_ROCKCHIP=y CONFIG_NR_DRAM_BANKS=2 +CONFIG_SF_DEFAULT_SPEED=24000000 CONFIG_DEFAULT_DEVICE_TREE="rk3568-generic" CONFIG_ROCKCHIP_RK3568=y +CONFIG_ROCKCHIP_SPI_IMAGE=y CONFIG_SPL_SERIAL=y CONFIG_DEBUG_UART_BASE=0xFE660000 CONFIG_DEBUG_UART_CLOCK=24000000 +CONFIG_SPL_SPI_FLASH_SUPPORT=y +CONFIG_SPL_SPI=y CONFIG_SYS_LOAD_ADDR=0xc00800 CONFIG_DEBUG_UART=y CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_SPL_FIT_SIGNATURE=y CONFIG_SPL_LOAD_FIT=y +# CONFIG_BOOTMETH_VBE is not set CONFIG_LEGACY_IMAGE_FORMAT=y CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-generic.dtb" # CONFIG_DISPLAY_CPUINFO is not set @@ -21,32 +26,58 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_SPL_MAX_SIZE=0x40000 CONFIG_SPL_PAD_TO=0x7f8000 # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set +CONFIG_SPL_SPI_LOAD=y +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000 CONFIG_SPL_ATF=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPT=y CONFIG_CMD_MMC=y +CONFIG_CMD_ROCKUSB=y +CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_CMD_SETEXPR is not set # CONFIG_SPL_DOS_PARTITION is not set CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" +CONFIG_SYS_RELOC_GD_ENV_ADDR=y +# CONFIG_NET is not set
This seems surprising, do you really want to get rid of network support for the generic board defconfig?
My intention with the generic targets is that they only contain bare minimum to boot from emmc/sd-card/spi flash. And a future possible use could be to replace vendor usbplug blob and/or for other flashing or recovery purposes. So network support should not be needed, also saves a little on binary size and boot speed.
Regards, Jonas
Cheers, Quentin