Re: [PATCH] riscv: dts: starfive: jh7110-common: fix jh7110 SoC boot from SD-card.

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6519 bytes --]

On Wed, Jul 29, 2026 at 02:07:07PM -0700, E Shattow wrote:
> 
> On 7/29/26 03:59, Conor Dooley wrote:
> > On Wed, Jul 29, 2026 at 02:39:50AM -0700, E Shattow wrote:
> >>
> >>
> >> On 7/28/26 11:02, Conor Dooley wrote:
> >>> From: Conor Dooley <mail@conchuod.ie>
> >>>
> >>> On Fri, 06 Mar 2026 20:19:39 +0300, Ilya Sorochan wrote:
> >>>> Add bootph-pre-ram to mmc1_pins clk-pins.
> >>>>
> >>>> U-Boot pruned their overrides recently in favor of Linux which broke booting
> >>>> from sd-card for me and Heinrich Schuchardt [1]. Pruning commit:
> >>>> 27f617019dd070cb61f2 ("riscv: dts: starfive: prune redundant jh7110-common overrides")
> >>>>
> >>>> [1]  https://lore.kernel.org/all/ffdfc550-559b-4c59-9873-3f040fc3bb0e@canonical.com/
> >>>>
> >>>> [...]
> >>>
> >>> Right, I am going to try this again. I don't think there's actually a
> >>> valid technical argument against it, since it just affects the stage at
> >>> which configuration is applied rather than configuration itself.
> >>> If the configuration is wrong, and more devices don't use this configuration
> >>> than do, people need to move the node to where they think should be. That
> >>> shall be done separately.
> >>>
> >>> I don't want to hear about the politics about whether or not Starfive want
> >>> to support it or reverse engineering of binaries etc. All that I care about
> >>> is discussion of the hardware itself and which boards need this and which do
> >>> not.
> >>>
> >>> Applied to riscv-dt-fixes, thanks!
> >>>
> >>> [1/1] riscv: dts: starfive: jh7110-common: fix jh7110 SoC boot from SD-card.
> >>>       https://git.kernel.org/conor/c/42c57c049054
> >>>
> >>> Thanks,
> >>> Conor.
> >>
> >> Second NAK to this being applied, again, do not apply as-is.
> >>
> >> As you're not interested in the technical discussion, then you won't
> >> have one.
> > 
> > I am interested in technical discussion of the hardware configuration. I'm
> > not interested in reverse engineering of binaries, GPL compliance or
> > whether Starfive want this to work or not because I don't see how they
> > relate to the correctness of the hardware description.
> 
> The commit message suggesting "...broke booting from sd-card..." is
> missing information or should be clearer i.e. "The bootph-pre-ram
> hint(s) allow that following the SD Boot mode of StarFive loader in
> MaskROM the U-Boot SPL will have all necessary devicetree nodes to
> initialize the MMC driver and continue booting U-Boot Main from SD
> card". It has not to do with booting Linux from SD card, and is for want
> of a more generalized description affecting any SPL that filters on the
> patterns of bootph-* hints but U-Boot seems to be one-of-one the only
> consumer of this that I am aware of.
> 
> Placement of this bootph-pre-ram hint in jh7110-common is (probably?)
> not appropriate, as SD Card is not valid on all boards or even at that
> mmc device, namely StarFive VisionFive 2 Lite eMMC, Milk-V Mars CM, and
> requires testing to know if it breaks existing functionality on boards
> with swapped arrangement of mmc0 and mmc1 with respect to SDIO module
> and eMMC (i.e. OrangePi RV ? It has been awhile I may misremember this).

This kinda comes back to the point I was making about it not changing the
configuration, just the point at which the configuration is applied. All
boards get this configuration eventually, it just seems to be that this
particular boot method needs it earlier?
I'm not sure that applying it earlier would break any boards (although
of course it could), I'd be more likely to think that having the
configuration at all is probably harmful in the cases where it is
harmful.

The other thing I don't really get is why the clk-pins node is needed at
the pre-ram stage but not the mmc-pins node. Surely both are needed? I
suppose it may be the case that the latter matches the default
configuration for the pins but not the former? If that's the case, the
node mmc-pins should get the property too IMO. I've added Hal into the
thread, maybe he can clear stuff up somewhat in that regard.

> The functionality of adding this hint is not even possible for more than
> half the boards, and no new boards will have it.

Ignoring the rest of this for now, these are the supported boards and
inferring sd from cd-gpios:

jh7110-deepcomputing-fml13v01.dts

Looks like sd on 1

jh7110-milkv-marscm-emmc.dts
jh7110-milkv-marscm-lite.dts

looks like sd on 0

jh7110-milkv-mars.dts

looks like sd on 1

jh7110-orangepi-rv.dts

ditto

jh7110-pine64-star64.dts

ditto

jh7110-starfive-visionfive-2-lite.dts

looks like sd on 0

jh7110-starfive-visionfive-2-lite-emmc.dts
jh7110-starfive-visionfive-2-v1.2a.dts

looks like sd on 1

jh7110-starfive-visionfive-2-v1.3b.dts

ditto

Would you be happier if only the boards with sd on mmc1 got the
property?

I'd like to say I will read the decompliation, but I look after this
platform as a favour and I just don't have the time to dedicate to
understanding it. There's a reason it took me ages to even circle back
to this.

> 
> Repeating myself, Hal offered a description of the boot process but I
> find that does not exactly match the hardware based on my
> reverse-engineering effort. More eyes should be on this to explain what
> the hardware does in context of the patch commit message. Are we
> correctly describing this hardware, or not?

> The "reverse engineering of binaries" is the StarFive loader in hardware
> MaskROM that immediately precedes the supposed consumer of this proposed
> bootph-pre-ram hint as stated in the patch commit message. It is the
> hardware specifically the functionality that we are purporting to
> describe. So there it is an excerpt and if you want an explanation of
> what that all does to save you the trouble of reading it I am sorry to
> disappoint as I don't I don't understand a line of it. What are we
> describing and are there more bootph-pre-ram hints needed?

In theory, the state of the hardware prior to the boot loader stage
consuming bootph-pre-ram shouldn't even matter, any node needed by the
stage should be marked with it even if that node matches the default
state of the pinctrl hardware. What the prior stage is or does shouldn't
matter, although of course people tend to only add pinctrl nodes or
annotations when software requirements/expecations don't match the state
it finds the hardware in.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]