Hi Jonas,
On 4/2/24 14:37, Jonas Karlman wrote:
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/bindings/mmc/mmc-controller.yaml#L33
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.
You shouldn't need to fool auto jtag anymore on RK3588 since
https://source.denx.de/u-boot/u-boot/-/commit/5d710738bb1e0ff2bb93ce7baf4c9691ce919b53
since it would be disabled except if you enable it by hand via the symbol.
I assume you need a similar patch for RK3568.
Linux disables auto jtag automatically for RK3588 since
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/soc/rockchip/grf.c?id=6f6878ec6faf16a5f36761c93da6ea9cf09adb33.
I guess you need a similar patch for RK3568?
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.
I think it'd make sense to properly patch this by disabling autojtag
feature instead :)
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.
Yup, but I assume we have DM support for SPI flashes in SPL, so we could
do this instead of having to rely on a defconfig symbol :)
[...]
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.
netboot/pxe/tftp is nice for debugging, but I understand :)
Cheers,
Quentin
