Hi Anand, On 10/31/26 09:45, Anand Moon wrote: > Hi Hung-Chun, > > Thanks for testing this series > > On Sat, 31 Oct 2026 at 17:26, Hung-Chun Tseng <[email protected]> wrote: >> >> Hi Anand, >> >> Thanks for sending this. I tested the series on a Milk-V Jupiter, >> booting from SD. It reaches the U-Boot prompt, but the SPL no longer >> finds the PMIC: >> >> Get board name:k1-x_milkv-jupiter >> No PMIC (-19), leaving rails as found >> DDR firmware: [0xc0820740]:0xf0227179, size:0x8d98 >> >> With a Jupiter -u-boot.dtsi that has the same i2c8/pmic block this >> patch removes, the SPL sets the rails before DDR init: >> >> vdd_core, value:900000 >> vdd_1v8, value:1800000 >> vdd_1v8_mmc, value:3300000 >> >> The commit message says the regulator configuration moves to >> dts/upstream, but k1-milkv-jupiter.dts there has no bootph tags and no >> vdd_core/vdd_1v8/vdd_1v8_mmc names, so the PMIC is not in the SPL >> device tree and pmic_init() skips it. It works on this board only >> because the rails come up at usable voltages. How do you plan to let >> the SPL find the rails? >> >> The new file is also not picked up until 6/6. U-Boot includes >> <dtb>-u-boot.dtsi, $(SYS_SOC)-u-boot.dtsi, $(SYS_CPU)-u-boot.dtsi, >> $(SYS_VENDOR)-u-boot.dtsi and u-boot.dtsi (scripts/Makefile.lib), >> which on K1 means k1-u-boot.dtsi or spacemit-u-boot.dtsi. Nothing >> includes spacemit-k1-u-boot.dtsi until 6/6 sets >> CONFIG_DEVICE_TREE_INCLUDES, so 1/6 to 5/6 build without any K1 >> overlay, which breaks bisection. I have not built each step. Naming the >> file k1-u-boot.dtsi would get it included from this patch on, and 6/6 >> would not need DEVICE_TREE_INCLUDES. >> > I think configs/spacemit_k3_defconfig uses the same approach. > > CONFIG_DEVICE_TREE_INCLUDES="spacemit-k3-u-boot.dtsi" >
Please split up the dts reorganization changes into three-or-four patches [A], ([B]), [C], and [D]; The initial transition to this as [A] should be not adding or fixing any nodes: A1. New file 'spacemit-k1-binman.dtsi' with '#include "binman.dtsi"' at the top just after the copyright identifier A2. '&binman' sections from per-board k1-*-u-boot.dtsi overrides should merge and be moved appended to spacemit-k1-binman.dtsi A3. New file 'spacemit-k1-u-boot.dtsi' specified by CONFIG_DEVICE_TREE_INCLUDES, with '#include "spacemit-k1-binman.dtsi"' as the very last line of the file. A4. Merge and move from per-board k1-*-u-boot.dtsi files any properties in devicetree root that are rejected by upstream for devicetree-rebasing subtree. The content should be inserted after the copyright identifier and before the last line of the file '#include "spacemit-k1-binman.dtsi"'. An example of this is the memory controller reg override in which we specify a sensible fixed-amount value so that the bootloader bring-up can function, but this configuration would never be accepted upstream. If not for [B] this should only be the memory controller reg override, apparently. B. Resolve the presence of /aliases and /chosen nodes in k1-musepi-pro-u-boot.dtsi compared to the lack of such nodes in k1-bananapi-f3-u-boot.dtsi and then upstream and/or merge and move any resolution to spacemit-k1-u-boot.dtsi ; I do not understand the implications so maybe you would find that this can be ignored unchanged and then removed in [D] C(n). Import to per-board -u-boot.dtsi and spacemit-k1-u-boot.dtsi all the proposed upstream changes (i.e. bootph-pre-ram hints). These should be grouped by upstream patch discussion thread or commit id so that from view of u-boot git tree one upstream discussion thread (patch or commit id) is referenced in a comment within the dtsi file(s) that import it. D. Delete redundant nodes and properties from per-board dtsi places where they are redundant. This deletion is not combined with [C] because we should be able to later do a git revert of [C(n)] when a corresponding commit lands upstream. There is going to be a considerable amount of per-board overrides that are identical over a subset of per-board override -u-boot.dtsi and so duplication is preferable as it is (from U-Boot git log) logically grouped by commits that reference a particular upstream discussion or commit id. That is also to say further organizing some changesets in additional dtsi files to be included from a subset of per-board dtsi is considered, but I am not yet convinced it would be helpful rather than just duplicating it among the per-board dtsi files now and having a sensible git history for reverting later when appropriate. > It is consistent to use the same naming format for files. >> The MusePi Pro overlay also set sdhci0's cd-gpios to GPIO 80 active >> low, while k1-musepi-pro.dts in dts/upstream has it active high, so >> removing the override flips card detect on that board. Was the >> override fixing the upstream polarity? > I haven't verified this yet, but we can retain the override if you prefer >> >> Two other things I saw, neither caused by this series: the SPI NOR on >> the Jupiter is a GigaDevice part (JEDEC c8 40 17), which needs >> "configs: k1: enable GigaDevice SPI NOR flash support" [1], and >> U-Boot reports 8 GiB on this 4 GiB board, a known issue in >> ddr_get_density(). >> > Yep, you can enable CONFIG_SPI_FLASH_GIGADEVICE in the > configs/spacemit_k1_defconfig > >> [1] >> https://lore.kernel.org/u-boot/[email protected]/ >> > You likely need to update the regulator name in your upstream patches > to match the specific naming convention of your board. > > for example > regulators { > buck1_0v9: buck1 { > regulator-name = "vdd_core"; > bootph-pre-ram; > regulator-min-microvolt = <500000>; > regulator-max-microvolt = <1050000>; > regulator-ramp-delay = <5000>; > regulator-always-on; > }; > > buck4: buck4 { > bootph-pre-ram; > regulator-name = "vdd_1v8"; > regulator-min-microvolt = <500000>; > regulator-max-microvolt = <3300000>; > regulator-ramp-delay = <5000>; > regulator-always-on; > }; > > aldo1: aldo1 { > regulator-name = "vdd_1v8_mmc"; > bootph-pre-ram; > regulator-min-microvolt = <500000>; > regulator-max-microvolt = <3400000>; > regulator-boot-on; > }; > > You can verify this using the command below > > # dtc -I dtb -O dts spl/u-boot-spl.dtb | > grep -n -E \ > 'regulator|pmic|reset-controller|reset-gpios|vmmc-supply|vqmmc-supply' > 136: pmic@41 { > 149: regulators { > 152: regulator-name = "vdd_core"; > 153: > regulator-min-microvolt = <0x7a120>; > 154: > regulator-max-microvolt = <0x100590>; > 155: regulator-ramp-delay = > <0x1388>; > 156: regulator-always-on; > 161: regulator-name = "vdd_1v8"; > 162: > regulator-min-microvolt = <0x7a120>; > 163: > regulator-max-microvolt = <0x1b7740>; > 164: regulator-ramp-delay = > <0x1388>; > 165: regulator-always-on; > 170: regulator-name = > "vdd_1v8_mmc"; > 171: > regulator-min-microvolt = <0x7a120>; > 172: > regulator-max-microvolt = <0x33e140>; > 173: regulator-boot-on; > 298: vmmc-supply = <0x3d>; > 299: vqmmc-supply = <0x3e>; > > If you get this working on your boards, please consider updating it in > your patch series. > >> Thanks, >> Hung-Chun > > Thanks > -Anand -E
