Re: [PATCH] cmd: fdt: use U-Boot's FDT by default
Hi Caleb, the problem here is hidden conditional behavior.
On Sat, Aug 31, 2024 at 9:56 AM Caleb Connolly
wrote:
>
> When using the FDT command to inspect an arbitrary FDT in memory, it
> will always be necessary to explicitly set the FDT address. However it
> is also quite likely that the command is being used to inspect U-Boot's
> own FDT. Simplify that common workflow of running "fdt addr -c" to get
> the control address and set it by just making $fdtcontroladdr the
> default FDT if there isn't one.
>
> Signed-off-by: Caleb Connolly
> ---
> cmd/fdt.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/cmd/fdt.c b/cmd/fdt.c
> index d16b141ce32d..8909706e2483 100644
> --- a/cmd/fdt.c
> +++ b/cmd/fdt.c
> @@ -276,8 +276,17 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int
> argc, char *const argv[])
>
> return CMD_RET_SUCCESS;
> }
>
> + /* Try using U-Boot's FDT by default */
> + if (!working_fdt) {
> + struct fdt_header *addr;
> +
> + addr = (void *)env_get_hex("fdtcontroladdr", 0);
> + if (addr && fdt_check_header(&addr))
> + set_working_fdt_addr((phys_addr_t)addr);
> + }
> +
> if (!working_fdt) {
> puts("No FDT memory address configured. Please configure\n"
> "the FDT address via \"fdt addr \" command.\n"
> "Aborting!\n");
> --
> 2.46.0
>
The use of `fdt` command in the default action might be depended on by
some userspace script as a success/fail result. I don't imagine what
that might possibly be, just that the logic of scripts in u-boot
depend on that pattern of use.
Secondly there would need to be a warning to the user that some hidden
conditional action is being applied? i.e. "No valid FDT address is
configured to $fdt_addr_r or $fdt_addr so now configuring to use
$fdtcontroladdr instead." or however you would phrase that.
Otherwise I agree improvement to the fdt is welcome and my memory of
first using this command is that it has not-sensible defaults but I
then assumed it was designed this way because of possible use in
U-Boot scripts.
-E
Re: [PATCH v4 8/9] spl: starfive: star64: Setup USB fdt fixup function
"cdns3,usb2-phy\0cdns3,usb3-phy"));
> +}
> +
Code readability can be better with fdt_appendprop_string helper. As:
fdt_setprop_string(fdt, offset, "phy-names", "cdns3,usb2-phy");
fdt_appendprop_string(fdt, offset, "phy-names", "cdns3,usb3-phy");
Some people prefer it to have "\0" and avoid the fdt_appendprop_string
helper. What you should do is not my choice here to make.
Anyway, and again, I think this will not be our problem after
OF_UPSTREAM is realized for JH7110 boards in U-Boot.
> void spl_fdt_fixup_mars(void *fdt)
> {
> static const char compat[] = "milkv,mars\0starfive,jh7110";
> @@ -335,6 +398,9 @@ void spl_fdt_fixup_star64(void *fdt)
> break;
> }
> }
> + spl_fdt_fixup_usb_host(fdt);
> + spl_fdt_fixup_usb_vbus_pin(fdt, 25);
> + spl_fdt_fixup_set_usb3(fdt);
> }
>
> void spl_perform_fixups(struct spl_image_info *spl_image)
> --
> 2.17.1
>
I have now tested Mars CM Lite:
+ spl_fdt_fixup_usb_host(fdt);
+ spl_fdt_fixup_usb_vbus_pin(fdt, 25);
added to the Milk-V Mars CM fix-up routine: in U-Boot there is both
working USB and PCIe. I would note also an error for PCIe in Linux if
continuing from U-Boot with the fdt of U-Boot into Grub2, then Linux:
Loading Linux 6.11-rc4-riscv64 ...
Loading initial ramdisk ...
[ 11.718961] pcie-starfive 2b000000.pcie: error -ENODEV: failed to
get valid pcie domain
[ 11.727238] cadence-qspi 1301.spi: couldn't determine
trigger-address
[ 11.730479] pcie-starfive 2c00.pcie: error -ENODEV: failed to
get valid pcie domain
[ 11.734112] cadence-qspi 1301.spi: Cannot get mandatory OF
data.
Gave up waiting for suspend/resume device
This is only a problem when I force `fdtfile` path to be not found,
and U-Boot gives the internal fdt to the next boot software in
sequence (here this is Grub2, and then Debian Linux).
For now, Minda, the error with Mars CM Lite and U-Boot internal fdt
into 6.11-rc4 Linux causing PCIe to fail is something I consider a
regression if we do enable for Mars CM and Mars CM Lite. It is good
enough in this series with only the JH7110 boards that you have
available for testing.
Reviewed-by: E Shattow
Tested-by: E Shattow
Re: [PATCH 1/1] riscv: define find_{first, next}_zero_bit in asm/bitops.h
Series https://patchwork.ozlabs.org/project/uboot/list/?series=421417 Add Starfive JH7110 Cadence USB driver fails to compile anymore without this patch. With that, Tested-by: E Shattow wrote: > > 27.07.2024 13:35, E Shattow wrote: > > Is this a problem in Linux upstream? or specific to U-Boot, and is it > > a regression? > > > > refrerence > > https://lore.kernel.org/u-boot/20240504183354.GL2568172@bill-the-cat/ > > and reference > > https://lore.kernel.org/u-boot/bjxpr01mb0855813dd38ef86cca6dd5c8e6...@bjxpr01mb0855.chnpr01.prod.partner.outlook.cn/ > > > > It is very similar. But it comes from > > In file included from > /home/maximus/git/yadro/ymp-build/u-boot/include/linux/usb/composite.h:26, > from > /home/maximus/git/yadro/ymp-build/u-boot/include/g_dnl.h:12, > from > /home/maximus/git/yadro/ymp-build/u-boot/cmd/fastboot.c:12: > /home/maximus/git/yadro/ymp-build/u-boot/include/linux/bitmap.h: In > function ‘bitmap_find_next_zero_area’: > /home/maximus/git/yadro/ymp-build/u-boot/include/linux/bitmap.h:170:17: > error: implicit declaration of function ‘find_next_zero_bit’; did you > mean ‘find_next_bit’? [-Wimplicit-function-declaration] >170 | index = find_next_zero_bit(map, size, start); >| ^~ >| find_next_bit > > I've just tried v2024.07 and master. > > I tried to drop #include from composite.h, but it fails > on usb gadget compilation: > >CC drivers/usb/gadget/g_dnl.o > In file included from > /home/maximus/git/yadro/ymp-build/u-boot/drivers/usb/gadget/g_dnl.c:24: > /home/maximus/git/yadro/ymp-build/u-boot/drivers/usb/gadget/composite.c: > In function ‘reset_config’: > /home/maximus/git/yadro/ymp-build/u-boot/drivers/usb/gadget/composite.c:362:17: > error: implicit declaration of function ‘bitmap_zero’ > [-Wimplicit-function-declaration] >362 | bitmap_zero(f->endpoints, 32); >| ^~~ > > > >
Re: [PATCH v4 7/9] dts: starfive: Add JH7110 Cadence USB dts node
On Wed, Aug 28, 2024 at 9:47 PM Sumit Garg wrote:
>
> Hi,
>
> On Thu, 29 Aug 2024 at 07:01, Minda Chen wrote:
> >
> > Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> > is USB 2.0 device.
> >
> > Signed-off-by: Minda Chen
> > ---
> > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 ++
> > arch/riscv/dts/jh7110.dtsi| 53 +++
> > 2 files changed, 58 insertions(+)
>
> Can you try to evaluate if this SoC can support OF_UPSTREAM? I can see
> corresponding DT sources well supported by dts/upstream. AFAICS, you
> at least need to add a Makefile for RISC-V here:
> dts/upstream/src/riscv/ for which you can reference
> dts/upstream/src/arm64/Makefile.
>
> This is not something to be considered as a blocker for this series
> but kind of follow-up work.
>
> -Sumit
>
> >
> > diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > index e11babc1cd..44785bbee3 100644
> > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > @@ -378,3 +378,8 @@
> > };
> > };
> > };
> > +
> > +&usb_cdns3 {
> > + dr_mode = "peripheral";
> > + status = "okay";
> > +};
> > diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
> > index 2cdc683d49..7bf9b2a03a 100644
> > --- a/arch/riscv/dts/jh7110.dtsi
> > +++ b/arch/riscv/dts/jh7110.dtsi
> > @@ -371,6 +371,59 @@
> > status = "disabled";
> > };
> >
> > + usb0: usb@1010 {
> > + compatible = "starfive,jh7110-usb";
> > + ranges = <0x0 0x0 0x1010 0x10>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + starfive,stg-syscon = <&stg_syscon 0x4>;
> > + clocks = <&stgcrg JH7110_STGCLK_USB_LPM>,
> > +<&stgcrg JH7110_STGCLK_USB_STB>,
> > +<&stgcrg JH7110_STGCLK_USB_APB>,
> > +<&stgcrg JH7110_STGCLK_USB_AXI>,
> > +<&stgcrg JH7110_STGCLK_USB_UTMI_APB>;
> > + clock-names = "lpm", "stb", "apb", "axi",
> > "utmi_apb";
> > + resets = <&stgcrg JH7110_STGRST_USB_PWRUP>,
> > +<&stgcrg JH7110_STGRST_USB_APB>,
> > +<&stgcrg JH7110_STGRST_USB_AXI>,
> > +<&stgcrg JH7110_STGRST_USB_UTMI_APB>;
> > + reset-names = "pwrup", "apb", "axi", "utmi_apb";
> > +
> > + usb_cdns3: usb@0 {
> > + compatible = "cdns,usb3";
> > + reg = <0x0 0x1>,
> > + <0x1 0x1>,
> > + <0x2 0x1>;
> > + reg-names = "otg", "xhci", "dev";
> > + interrupts = <100>, <108>, <110>;
> > + interrupt-names = "host", "peripheral",
> > "otg";
> > + phys = <&usbphy0>;
> > + phy-names = "cdns3,usb2-phy";
> > + };
> > + };
> > +
> > + usbphy0: phy@1020 {
> > + compatible = "starfive,jh7110-usb-phy";
> > + reg = <0x0 0x1020 0x0 0x1>;
> > + clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
> > +<&stgcrg JH7110_STGCLK_USB_APP_125>;
> > + clock-names = "125m", "app_125m";
> > + starfive,sys-syscon = <&sys_syscon 0x18>;
> > + #phy-cells = <0>;
> > + };
> > +
> > + pciephy0: phy@1021 {
> > + compatible = "starfive,jh7110-pcie-phy";
> > + reg = <0x0 0x1021 0x0 0x1>;
> > + #phy-cells = <0>;
> > + };
> > +
> > + pciephy1: phy@1022 {
> > + compatible = "starfive,jh7110-pcie-phy";
> > + reg = <0x0 0x1022 0x0 0x1>;
> > + #phy-cells = <0>;
> > + };
> > +
> > stgcrg: clock-controller@1023 {
> > compatible = "starfive,jh7110-stgcrg";
> > reg = <0x0 0x1023 0x0 0x1>;
> > --
> > 2.17.1
> >
I note that Hal Feng saying earlier this week they would take on
OF_UPSTREAM of JH7110 boards:
https://lore.kernel.org/lkml/zq2pr01mb13073ef3bd7a64f2c098aa8fe6...@zq2pr01mb1307.chnpr01.prod.partner.outlook.cn/
It is a good time for this work to begin.
-E
Re: [PATCH 1/2] bootm: adjust the print format
On Sun, Aug 25, 2024 at 5:26 AM Dario Binacchi
wrote:
>
> All three addresses printed are in hexadecimal format, but only the
> first two have the "0x" prefix. The patch aligns the format of the
> "end" address with the other two by adding the "0x" prefix.
>
> Signed-off-by: Dario Binacchi
> ---
>
> boot/bootm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 480f8e6a0e6e..951e549f19ff 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -703,7 +703,7 @@ static int bootm_load_os(struct bootm_headers *images,
> int boot_progress)
>
> /* Handle BOOTM_STATE_LOADOS */
> if (relocated_addr != load) {
> - printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
> + printf("Moving Image from 0x%lx to 0x%lx,
> end=0x%lx\n",
>load, relocated_addr,
>relocated_addr + image_size);
> memmove((void *)relocated_addr, load_buf, image_size);
> --
> 2.43.0
>
>From U-Boot documentation, alpha-numeric input is assumed to be
hexadecimal except when it is not, and generally does not accept "0x"
prefix on input. So the correct action would be to make this
consistent over the whole U-Boot code base, or remove the "0x"
prefixes (not add more of them) ?
-E
Re: xPL terminology
Hi Simon, On Sun, Aug 25, 2024 at 6:07 AM Simon Glass wrote: > > Hi, > > We have the term 'SPL', which has a dual meaning. It is both a > particular phase of U-Boot (the one that loads U-Boot proper) and a > generic name for any pre-proper phase. ZBL or ZSBL: Zeroth Stage Boot Loader (usually in MaskROM) SPL: Secondary Program Loader is what gets loaded by ZSBL and jump code execution to next Everything after SPL seems like an invention of naming, and yes it is confusing! > > You can see that in a few areas, but for example CONFIG_SPL_BUILD is > enabled for TPL and VPL builds, not just SPL. > > I propose to rename the generic term from SPL to xPL (meaning any PL > phase), leaving SPL to just refer to the phase before U-Boot proper. > > The symbol would be CONFIG_XPL but in documentation we would talk of > xPL, with a lower-case X, so it is more obvious that it refers to any > phase. > > What do you think? > > Regards, > Simon Secondary / Tertiary / Verifying / U-Boot from documentation: https://docs.u-boot.org/en/latest/usage/spl_boot.html ... does not add much clarity to what these programs do, excepting "verifying" which is at least saying what it does. When code execution is transferred to U-Boot (as SPL from ZSBL) then it is simply U-Boot Program if you need a short acronym: UP UBP UPRG UPGM UBPG Add to this the descriptive short word name modifier of what that U-Boot Program does: TINY VERIFY SETUP MAIN Taking for example UP and TINY we would have UP_TINY UP_VERIFY UP_SETUP UP_MAIN Any of these would be the SPL if they are loaded by the ZSBL, simple enough. Also keeping the same mnemonic first letter to what it is already now for making it less disruptive. My comment then is that if you want it to be less confusing we should get away from "SPL" (x)PL naming so it looks nothing like ZSBL or SPL acronym. Also I observe that U-Boot code base norms are unnecessarily cryptic and weirdly abbreviated descriptions in the guise of "reducing code size" at the expense of readability... and it is welcome to me if you can improve this with _less_ cryptic naming and descriptions. Sorry "xPL" is more cryptic than it is now and keeps the overloading of the "SPL" terminology which is the cause of confusion. -E
Re: [PATCH 1/1] riscv: define find_{first, next}_zero_bit in asm/bitops.h
Is this a problem in Linux upstream? or specific to U-Boot, and is it
a regression?
refrerence https://lore.kernel.org/u-boot/20240504183354.GL2568172@bill-the-cat/
and reference
https://lore.kernel.org/u-boot/bjxpr01mb0855813dd38ef86cca6dd5c8e6...@bjxpr01mb0855.chnpr01.prod.partner.outlook.cn/
On Fri, Jul 26, 2024 at 5:44 AM Maxim Kochetkov wrote:
>
> These seem to be missing, and trying to build fastboot cmd without
> them is causing errors due to these being missing.
>
> Signed-off-by: Maxim Kochetkov
> ---
> arch/riscv/include/asm/bitops.h | 40 +
> 1 file changed, 40 insertions(+)
>
> diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
> index 35f1368b83..2f2994c4dd 100644
> --- a/arch/riscv/include/asm/bitops.h
> +++ b/arch/riscv/include/asm/bitops.h
> @@ -138,6 +138,43 @@ static inline unsigned long ffz(unsigned long word)
> return k;
> }
>
> +static inline int find_next_zero_bit(void *addr, int size, int offset)
> +{
> + unsigned long *p = ((unsigned long *)addr) + (offset / BITS_PER_LONG);
> + unsigned long result = offset & ~(BITS_PER_LONG - 1);
> + unsigned long tmp;
> +
> + if (offset >= size)
> + return size;
> + size -= result;
> + offset &= (BITS_PER_LONG - 1);
> + if (offset) {
> + tmp = *(p++);
> + tmp |= ~0UL >> (BITS_PER_LONG - offset);
> + if (size < BITS_PER_LONG)
> + goto found_first;
> + if (~tmp)
> + goto found_middle;
> + size -= BITS_PER_LONG;
> + result += BITS_PER_LONG;
> + }
> + while (size & ~(BITS_PER_LONG - 1)) {
> + tmp = *(p++);
> + if (~tmp)
> + goto found_middle;
> + result += BITS_PER_LONG;
> + size -= BITS_PER_LONG;
> + }
> + if (!size)
> + return result;
> + tmp = *p;
> +
> +found_first:
> + tmp |= ~0UL << size;
> +found_middle:
> + return result + ffz(tmp);
> +}
> +
> /*
> * ffs: find first bit set. This is defined the same way as
> * the libc and compiler builtin ffs routines, therefore
> @@ -158,6 +195,9 @@ static inline unsigned long ffz(unsigned long word)
> #define hweight16(x) generic_hweight16(x)
> #define hweight8(x) generic_hweight8(x)
>
> +#define find_first_zero_bit(addr, size) \
> + find_next_zero_bit((addr), (size), 0)
> +
> #define test_and_set_bit __test_and_set_bit
> #define test_and_clear_bit __test_and_clear_bit
>
> --
> 2.45.2
>
-E
Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
On Tue, Jul 23, 2024 at 8:24 PM Minda Chen wrote:
>
>
>
> > -邮件原件-----
> > 发件人: E Shattow
> > 发送时间: 2024年7月23日 21:06
> > 收件人: Minda Chen
> > 抄送: Marek Vasut ; Tom Rini ; Roger
> > Quadros ; Neil Armstrong ;
> > Alexey Romanov ; Sumit Garg
> > ; Mark Kettenis ; Nishanth
> > Menon ; Rick Chen ; Leo Yu-Chi Liang
> > ; u-boot@lists.denx.de; Heinrich Schuchardt
> > ; Simon Glass
> > 主题: Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
> >
> > On Tue, Jul 23, 2024 at 3:16 AM Minda Chen
> > wrote:
> > >
> > >
> > >
> > > >
> > > > On Mon, Jul 22, 2024 at 6:29 PM Minda Chen
> > > >
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > On Sat, Jul 20, 2024 at 6:47 PM E Shattow wrote:
> > > > > > >
> > > > > > > Hi, I am testing on Milk-V Mars CM Lite, and I add to these
> > > > > > > devicetree changes at runtime from
> > > > > > > board/starfive/visionfive2/spl.c
> > > > > > >
> > > > > > > On Thu, Jul 18, 2024 at 6:38 PM Minda Chen
> > > > > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> > > > > > > > is USB
> > > > > > > > 2.0 device.
> > > > > > > >
> > > > > > > > Signed-off-by: Minda Chen
> > > > > > > > ---
> > > > > > > > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 ++
> > > > > > > > arch/riscv/dts/jh7110.dtsi| 52
> > > > > > +++
> > > > > > > > 2 files changed, 57 insertions(+)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > > > a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > > > b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > > > index e11babc1cd..44785bbee3 100644
> > > > > > > > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > > > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > > > @@ -378,3 +378,8 @@
> > > > > > > > };
> > > > > > > > };
> > > > > > > > };
> > > > > > > > +
> > > > > > > > +&usb_cdns3 {
> > > > > > > > + dr_mode = "peripheral";
> > > > > > > > + status = "okay";
> > > > > > > > +};
> > > > > > > > diff --git a/arch/riscv/dts/jh7110.dtsi
> > > > > > > > b/arch/riscv/dts/jh7110.dtsi index 2cdc683d49..1eee924e1d
> > > > > > > > 100644
> > > > > > > > --- a/arch/riscv/dts/jh7110.dtsi
> > > > > > > > +++ b/arch/riscv/dts/jh7110.dtsi
> > > > > > > > @@ -371,6 +371,58 @@
> > > > > > > > status = "disabled";
> > > > > > > > };
> > > > > > > >
> > > > > > > > + usb0: usb@1010 {
> > > > > > > > + compatible = "starfive,jh7110-usb";
> > > > > > > > + ranges = <0x0 0x0 0x1010
> > 0x10>;
> > > > > > > > + #address-cells = <1>;
> > > > > > > > + #size-cells = <1>;
> > > > > > > > + starfive,stg-syscon = <&stg_syscon
> > 0x4>;
> > > > > > > > + clocks = <&stgcrg
> > > > > > JH7110_STGCLK_USB_LPM>,
> > > > > > > > +<&stgcrg
> > > > > > JH7110_STGCLK_USB_STB>,
> > > > > > > > +<&stgcrg
> > > > > > JH7110_STGCLK_USB_APB>,
> > > > > > > > +<&stgcrg
> &g
Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
On Tue, Jul 23, 2024 at 3:16 AM Minda Chen wrote:
>
>
>
> >
> > On Mon, Jul 22, 2024 at 6:29 PM Minda Chen
> > wrote:
> > >
> > >
> > >
> > > >
> > > > On Sat, Jul 20, 2024 at 6:47 PM E Shattow wrote:
> > > > >
> > > > > Hi, I am testing on Milk-V Mars CM Lite, and I add to these
> > > > > devicetree changes at runtime from
> > > > > board/starfive/visionfive2/spl.c
> > > > >
> > > > > On Thu, Jul 18, 2024 at 6:38 PM Minda Chen
> > > > >
> > > > wrote:
> > > > > >
> > > > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting is
> > > > > > USB
> > > > > > 2.0 device.
> > > > > >
> > > > > > Signed-off-by: Minda Chen
> > > > > > ---
> > > > > > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 ++
> > > > > > arch/riscv/dts/jh7110.dtsi| 52
> > > > +++
> > > > > > 2 files changed, 57 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > index e11babc1cd..44785bbee3 100644
> > > > > > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > > > @@ -378,3 +378,8 @@
> > > > > > };
> > > > > > };
> > > > > > };
> > > > > > +
> > > > > > +&usb_cdns3 {
> > > > > > + dr_mode = "peripheral";
> > > > > > + status = "okay";
> > > > > > +};
> > > > > > diff --git a/arch/riscv/dts/jh7110.dtsi
> > > > > > b/arch/riscv/dts/jh7110.dtsi index 2cdc683d49..1eee924e1d 100644
> > > > > > --- a/arch/riscv/dts/jh7110.dtsi
> > > > > > +++ b/arch/riscv/dts/jh7110.dtsi
> > > > > > @@ -371,6 +371,58 @@
> > > > > > status = "disabled";
> > > > > > };
> > > > > >
> > > > > > + usb0: usb@1010 {
> > > > > > + compatible = "starfive,jh7110-usb";
> > > > > > + ranges = <0x0 0x0 0x1010 0x10>;
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <1>;
> > > > > > + starfive,stg-syscon = <&stg_syscon 0x4>;
> > > > > > + clocks = <&stgcrg
> > > > JH7110_STGCLK_USB_LPM>,
> > > > > > +<&stgcrg
> > > > JH7110_STGCLK_USB_STB>,
> > > > > > +<&stgcrg
> > > > JH7110_STGCLK_USB_APB>,
> > > > > > +<&stgcrg
> > > > JH7110_STGCLK_USB_AXI>,
> > > > > > +<&stgcrg
> > > > JH7110_STGCLK_USB_UTMI_APB>;
> > > > > > + clock-names = "lpm", "stb", "apb",
> > > > > > + "axi",
> > > > "utmi_apb";
> > > > > > + resets = <&stgcrg
> > > > JH7110_STGRST_USB_PWRUP>,
> > > > > > +<&stgcrg
> > > > JH7110_STGRST_USB_APB>,
> > > > > > +<&stgcrg
> > > > JH7110_STGRST_USB_AXI>,
> > > > > > +<&stgcrg
> > > > JH7110_STGRST_USB_UTMI_APB>;
> > > > > > + reset-names = "pwrup", "apb", "axi",
> > > > > > + "utmi_apb";
> > > > > > +
> > > > > > + usb_cdns3: usb@0 {
> > > > > > + compatible = "cdns,usb3";
> > > > > > + reg = <0x0 0x1000
Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
On Mon, Jul 22, 2024 at 6:29 PM Minda Chen wrote:
>
>
>
> >
> > On Sat, Jul 20, 2024 at 6:47 PM E Shattow wrote:
> > >
> > > Hi, I am testing on Milk-V Mars CM Lite, and I add to these devicetree
> > > changes at runtime from board/starfive/visionfive2/spl.c
> > >
> > > On Thu, Jul 18, 2024 at 6:38 PM Minda Chen
> > wrote:
> > > >
> > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting is USB
> > > > 2.0 device.
> > > >
> > > > Signed-off-by: Minda Chen
> > > > ---
> > > > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 ++
> > > > arch/riscv/dts/jh7110.dtsi| 52
> > +++
> > > > 2 files changed, 57 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > index e11babc1cd..44785bbee3 100644
> > > > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > > > @@ -378,3 +378,8 @@
> > > > };
> > > > };
> > > > };
> > > > +
> > > > +&usb_cdns3 {
> > > > + dr_mode = "peripheral";
> > > > + status = "okay";
> > > > +};
> > > > diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
> > > > index 2cdc683d49..1eee924e1d 100644
> > > > --- a/arch/riscv/dts/jh7110.dtsi
> > > > +++ b/arch/riscv/dts/jh7110.dtsi
> > > > @@ -371,6 +371,58 @@
> > > > status = "disabled";
> > > > };
> > > >
> > > > + usb0: usb@1010 {
> > > > + compatible = "starfive,jh7110-usb";
> > > > + ranges = <0x0 0x0 0x1010 0x10>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <1>;
> > > > + starfive,stg-syscon = <&stg_syscon 0x4>;
> > > > + clocks = <&stgcrg
> > JH7110_STGCLK_USB_LPM>,
> > > > +<&stgcrg
> > JH7110_STGCLK_USB_STB>,
> > > > +<&stgcrg
> > JH7110_STGCLK_USB_APB>,
> > > > +<&stgcrg
> > JH7110_STGCLK_USB_AXI>,
> > > > +<&stgcrg
> > JH7110_STGCLK_USB_UTMI_APB>;
> > > > + clock-names = "lpm", "stb", "apb", "axi",
> > "utmi_apb";
> > > > + resets = <&stgcrg
> > JH7110_STGRST_USB_PWRUP>,
> > > > +<&stgcrg
> > JH7110_STGRST_USB_APB>,
> > > > +<&stgcrg
> > JH7110_STGRST_USB_AXI>,
> > > > +<&stgcrg
> > JH7110_STGRST_USB_UTMI_APB>;
> > > > + reset-names = "pwrup", "apb", "axi",
> > > > + "utmi_apb";
> > > > +
> > > > + usb_cdns3: usb@0 {
> > > > + compatible = "cdns,usb3";
> > > > + reg = <0x0 0x1>,
> > > > + <0x1 0x1>,
> > > > + <0x2 0x1>;
> > > > + reg-names = "otg", "xhci", "dev";
> > > > + interrupts = <100>, <108>, <110>;
> > > > + interrupt-names = "host",
> > "peripheral", "otg";
> > > > + phys = <&usbphy0>;
> > > > + phy-names = "cdns3,usb2-phy";
> > > > + };
> > > > + };
> > > > +
> > > > + usbphy0: phy@1020 {
> > > > + compatible = "starfive,jh7110-usb-phy";
> > > > +
Re: [PATCH 1/1] board: fix compatible property Milk-V Mars CM
On Sun, Jul 21, 2024 at 2:42 PM Heinrich Schuchardt
wrote:
>
> On 7/21/24 23:27, E Shattow wrote:
> > P.S. my suggestion below
> >
> > On Fri, Jul 19, 2024 at 5:06 PM E Shattow wrote:
> >>
> >> On Fri, Jul 19, 2024 at 4:12 PM Heinrich Schuchardt
> >> wrote:
> >>>
> >>> For the Milk-V Mars CM (lite) we have only been copying sizeof(void *)
> >>> bytes to the compatible property instead of the whole string list.
> >>
> >> This const char array must be so that we may get an accurate data
> >> length with sizeof() but it highlights there are helper functions for
> >> get of fdt stringlist and not for set of fdt stringlist.
> >>
> >>>
> >>> Fixes: de3229599d4f ("board: add support for Milk-V Mars CM")
> >>> Reported-by: E Shattow
> >>> Signed-off-by: Heinrich Schuchardt
> >>> ---
> >>> board/starfive/visionfive2/spl.c | 15 ---
> >>> 1 file changed, 12 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/board/starfive/visionfive2/spl.c
> >>> b/board/starfive/visionfive2/spl.c
> >>> index b794b73b6bd..f55c6b5d34c 100644
> >>> --- a/board/starfive/visionfive2/spl.c
> >>> +++ b/board/starfive/visionfive2/spl.c
> >>> @@ -170,23 +170,32 @@ void spl_fdt_fixup_mars_cm(void *fdt)
> >>> {
> >>> const char *compat;
> >>> const char *model;
> >>> + int compat_size;
> >>>
> >>> spl_fdt_fixup_mars(fdt);
> >>>
> >>> if (!get_mmc_size_from_eeprom()) {
> >>> int offset;
> >>> + static const char
> >>> + compat_cm_lite[] = "milkv,mars-cm-lite\0starfive,jh7110";
> >>>
> >>> model = "Milk-V Mars CM Lite";
> >>> - compat = "milkv,mars-cm-lite\0starfive,jh7110";
> >>> + compat = compat_cm_lite;
> >>> + compat_size = sizeof(compat_cm_lite);
> >>>
> >>> offset = fdt_path_offset(fdt,
> >>> "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> >>> /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE,
> >>> GPI_NONE) */
> >>> fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> >>> } else {
> >>> + static const char
> >>> + compat_cm[] = "milkv,mars-cm\0starfive,jh7110";
> >>> +
> >>> model = "Milk-V Mars CM";
> >>> - compat = "milkv,mars-cm\0starfive,jh7110";
> >>> + compat = compat_cm;
> >>> + compat_size = sizeof(compat_cm);
> >>> }
> >>> - fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> >>> sizeof(compat));
> >>> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"),
> >>> + "compatible", compat, compat_size);
> >>> fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> >>> model);
> >>> }
> >>>
> >>> --
> >>> 2.45.2
> >>>
> >>
> >> -E
> >
> > What about something like as follows?
> >
> > void spl_fdt_fixup_mars(void *fdt)
> > {
> > - static const char compat[] = "milkv,mars\0starfive,jh7110";
> > u32 phandle;
> > u8 i;
> > int offset;
> > int ret;
> >
> > - fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible",
> > compat, sizeof(compat));
> > - fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> > - "Milk-V Mars");
> > + offset = fdt_path_offset(fdt, "/");
> > + fdt_setprop_string(fdt,offset, "compatible", "milkv,mars");
> > + fdt_appendprop_string(fdt, offset, "compatible", "starfive,jh7110");
> > + fdt_setprop_string(fdt,offset, "model", "Milk-V Mars");
> >
> > /* gmac0 */
> > offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
> > @@ -164,30 +22
Re: [PATCH 1/1] board: fix compatible property Milk-V Mars CM
P.S. my suggestion below
On Fri, Jul 19, 2024 at 5:06 PM E Shattow wrote:
>
> On Fri, Jul 19, 2024 at 4:12 PM Heinrich Schuchardt
> wrote:
> >
> > For the Milk-V Mars CM (lite) we have only been copying sizeof(void *)
> > bytes to the compatible property instead of the whole string list.
>
> This const char array must be so that we may get an accurate data
> length with sizeof() but it highlights there are helper functions for
> get of fdt stringlist and not for set of fdt stringlist.
>
> >
> > Fixes: de3229599d4f ("board: add support for Milk-V Mars CM")
> > Reported-by: E Shattow
> > Signed-off-by: Heinrich Schuchardt
> > ---
> > board/starfive/visionfive2/spl.c | 15 ---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/board/starfive/visionfive2/spl.c
> > b/board/starfive/visionfive2/spl.c
> > index b794b73b6bd..f55c6b5d34c 100644
> > --- a/board/starfive/visionfive2/spl.c
> > +++ b/board/starfive/visionfive2/spl.c
> > @@ -170,23 +170,32 @@ void spl_fdt_fixup_mars_cm(void *fdt)
> > {
> > const char *compat;
> > const char *model;
> > + int compat_size;
> >
> > spl_fdt_fixup_mars(fdt);
> >
> > if (!get_mmc_size_from_eeprom()) {
> > int offset;
> > + static const char
> > + compat_cm_lite[] = "milkv,mars-cm-lite\0starfive,jh7110";
> >
> > model = "Milk-V Mars CM Lite";
> > - compat = "milkv,mars-cm-lite\0starfive,jh7110";
> > + compat = compat_cm_lite;
> > + compat_size = sizeof(compat_cm_lite);
> >
> > offset = fdt_path_offset(fdt,
> > "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> > /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE)
> > */
> > fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> > } else {
> > + static const char
> > + compat_cm[] = "milkv,mars-cm\0starfive,jh7110";
> > +
> > model = "Milk-V Mars CM";
> > - compat = "milkv,mars-cm\0starfive,jh7110";
> > + compat = compat_cm;
> > + compat_size = sizeof(compat_cm);
> > }
> > - fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> > sizeof(compat));
> > + fdt_setprop(fdt, fdt_path_offset(fdt, "/"),
> > + "compatible", compat, compat_size);
> > fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> > }
> >
> > --
> > 2.45.2
> >
>
> -E
What about something like as follows?
void spl_fdt_fixup_mars(void *fdt)
{
- static const char compat[] = "milkv,mars\0starfive,jh7110";
u32 phandle;
u8 i;
int offset;
int ret;
- fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible",
compat, sizeof(compat));
- fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
- "Milk-V Mars");
+ offset = fdt_path_offset(fdt, "/");
+ fdt_setprop_string(fdt,offset, "compatible", "milkv,mars");
+ fdt_appendprop_string(fdt, offset, "compatible", "starfive,jh7110");
+ fdt_setprop_string(fdt,offset, "model", "Milk-V Mars");
/* gmac0 */
offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
@@ -164,30 +226,31 @@ void spl_fdt_fixup_mars(void *fdt)
break;
}
}
}
void spl_fdt_fixup_mars_cm(void *fdt)
{
- const char *compat;
- const char *model;
+ int offset;
spl_fdt_fixup_mars(fdt);
if (!get_mmc_size_from_eeprom()) {
- int offset;
-
- model = "Milk-V Mars CM Lite";
- compat = "milkv,mars-cm-lite\0starfive,jh7110";
+ offset = fdt_path_offset(fdt, "/");
+ fdt_setprop_string(fdt, offset, "compatible",
"milkv,mars-cm-lite");
+ fdt_appendprop_string(fdt, offset, "compatible",
"starfive,jh7110");
+ fdt_setprop_string(fdt, offset, "model", "Milk-V Mars CM Lite");
offset = fdt_path_offset(fdt,
"/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
/* GPIOMUX(22, GPOUT
Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
On Sat, Jul 20, 2024 at 6:47 PM E Shattow wrote:
>
> Hi, I am testing on Milk-V Mars CM Lite, and I add to these devicetree
> changes at runtime from board/starfive/visionfive2/spl.c
>
> On Thu, Jul 18, 2024 at 6:38 PM Minda Chen
> wrote:
> >
> > Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> > is USB 2.0 device.
> >
> > Signed-off-by: Minda Chen
> > ---
> > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 ++
> > arch/riscv/dts/jh7110.dtsi| 52 +++
> > 2 files changed, 57 insertions(+)
> >
> > diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > index e11babc1cd..44785bbee3 100644
> > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > @@ -378,3 +378,8 @@
> > };
> > };
> > };
> > +
> > +&usb_cdns3 {
> > + dr_mode = "peripheral";
> > + status = "okay";
> > +};
> > diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
> > index 2cdc683d49..1eee924e1d 100644
> > --- a/arch/riscv/dts/jh7110.dtsi
> > +++ b/arch/riscv/dts/jh7110.dtsi
> > @@ -371,6 +371,58 @@
> > status = "disabled";
> > };
> >
> > + usb0: usb@1010 {
> > + compatible = "starfive,jh7110-usb";
> > + ranges = <0x0 0x0 0x1010 0x10>;
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + starfive,stg-syscon = <&stg_syscon 0x4>;
> > + clocks = <&stgcrg JH7110_STGCLK_USB_LPM>,
> > +<&stgcrg JH7110_STGCLK_USB_STB>,
> > +<&stgcrg JH7110_STGCLK_USB_APB>,
> > +<&stgcrg JH7110_STGCLK_USB_AXI>,
> > +<&stgcrg JH7110_STGCLK_USB_UTMI_APB>;
> > + clock-names = "lpm", "stb", "apb", "axi",
> > "utmi_apb";
> > + resets = <&stgcrg JH7110_STGRST_USB_PWRUP>,
> > +<&stgcrg JH7110_STGRST_USB_APB>,
> > +<&stgcrg JH7110_STGRST_USB_AXI>,
> > +<&stgcrg JH7110_STGRST_USB_UTMI_APB>;
> > + reset-names = "pwrup", "apb", "axi", "utmi_apb";
> > +
> > + usb_cdns3: usb@0 {
> > + compatible = "cdns,usb3";
> > + reg = <0x0 0x1>,
> > + <0x1 0x1>,
> > + <0x2 0x1>;
> > + reg-names = "otg", "xhci", "dev";
> > + interrupts = <100>, <108>, <110>;
> > + interrupt-names = "host", "peripheral",
> > "otg";
> > + phys = <&usbphy0>;
> > + phy-names = "cdns3,usb2-phy";
> > + };
> > + };
> > +
> > + usbphy0: phy@1020 {
> > + compatible = "starfive,jh7110-usb-phy";
> > + reg = <0x0 0x1020 0x0 0x1>;
> > + clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
> > +<&stgcrg JH7110_STGCLK_USB_APP_125>;
> > + clock-names = "125m", "app_125m";
> > + #phy-cells = <0>;
> > + };
> > +
> > + pciephy0: phy@1021 {
> > + compatible = "starfive,jh7110-pcie-phy";
> > + reg = <0x0 0x1021 0x0 0x1>;
> > + #phy-cells = <0>;
> > + };
> > +
> > + pciephy1: phy@1022 {
> > + compatible = "starfive,jh7110-pcie-phy";
> > + reg = &l
Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
n: Load access fault
EPC: fff85ce2 RA: fff85cdc TVAL: 0004
EPC: 40246ce2 RA: 40246cdc reloc adjusted
Code: 9863 3ee7 8526 f0ef c37f 651c 3a03 0105 (43dc)
resetting ...
when I add only these:
int offset;
offset = fdt_path_offset(fdt, "/soc/pinctrl@1304"); /* &sysgpio */
fdt_add_subnode(fdt, offset, "usb0-0");
fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
"usb_pins", "/soc/pinctrl@1304/usb0-0");
offset = fdt_path_offset(fdt, "/soc/pinctrl@1304/usb0-0");
/* usb_pins */
fdt_create_phandle(fdt, offset);
fdt_add_subnode(fdt, offset, "driver-vbus-pin");
offset = fdt_path_offset(fdt,
"/soc/pinctrl@1304/usb0-0/driver-vbus-pin");
fdt_setprop_u32(fdt, offset, "pinmux", 0xff070019); /*
GPIOMUX(25, GPOUT_SYS_USB_DRIVE_VBUS, GPOEN_ENABLE, GPI_NONE) */
fdt_setprop_empty(fdt, offset, "bias-disable");
fdt_setprop_empty(fdt, offset, "input-disable");
fdt_setprop_empty(fdt, offset, "input-schmitt-disable");
fdt_setprop_u32(fdt, offset, "slew-rate", 0);
offset = fdt_path_offset(fdt, "/soc/usb@1010"); /* &usb0 */
fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
fdt_setprop_u32(fdt, offset, "pinctrl-0", fdt_get_phandle(fdt,
fdt_path_offset(fdt, "/soc/pinctrl@1304/usb0-0")));
fdt_setprop_string(fdt, offset, "status", "okay");
offset = fdt_path_offset(fdt, "/soc/usb@1010/usb@0"); /*
&usb_cdns3 */
fdt_setprop_string(fdt, offset, "dr_mode", "host");
Success USB is working but PCI disabled if instead I add all of this:
int offset;
offset = fdt_path_offset(fdt, "/soc/pinctrl@1304"); /* &sysgpio */
fdt_add_subnode(fdt, offset, "usb0-0");
fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
"usb_pins", "/soc/pinctrl@1304/usb0-0");
offset = fdt_path_offset(fdt, "/soc/pinctrl@1304/usb0-0");
/* usb_pins */
fdt_create_phandle(fdt, offset);
fdt_add_subnode(fdt, offset, "driver-vbus-pin");
offset = fdt_path_offset(fdt,
"/soc/pinctrl@1304/usb0-0/driver-vbus-pin");
fdt_setprop_u32(fdt, offset, "pinmux", 0xff070019); /*
GPIOMUX(25, GPOUT_SYS_USB_DRIVE_VBUS, GPOEN_ENABLE, GPI_NONE) */
fdt_setprop_empty(fdt, offset, "bias-disable");
fdt_setprop_empty(fdt, offset, "input-disable");
fdt_setprop_empty(fdt, offset, "input-schmitt-disable");
fdt_setprop_u32(fdt, offset, "slew-rate", 0);
offset = fdt_path_offset(fdt, "/soc/pcie@2b00"); /* &pcie0 */
fdt_setprop_string(fdt, offset, "status", "disabled");
offset = fdt_path_offset(fdt, "/soc/phy@1021"); /* &pciephy0 */
fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/sys_syscon@1303"))); /* = <&sys_syscon> */
fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18);
/* append */
fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/stg_syscon@1024"))); /* = <&stg_syscon> */
fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148);
/* append */
fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4);
/* append */
fdt_setprop_string(fdt, offset, "status", "okay");
offset = fdt_path_offset(fdt, "/soc/usb@1010"); /* &usb0 */
fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
fdt_setprop_u32(fdt, offset, "pinctrl-0", fdt_get_phandle(fdt,
fdt_path_offset(fdt, "/soc/pinctrl@1304/usb0-0")));
fdt_setprop_string(fdt, offset, "status", "okay");
offset = fdt_path_offset(fdt, "/soc/usb@1010/usb@0"); /*
&usb_cdns3 */
fdt_setprop_u32(fdt,offset, "phys",
fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@1020"))); /* =
<&usbphy0> */
fdt_appendprop_u32(fdt, offset, "phys",
fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@1021"))); /*
append <&pciephy0> */
fdt_setprop(fdt,offset, "phy-names",
"cdns3,usb2-phy\0cdns3,usb3-phy",
sizeof("cdns3,usb2-phy\0cdns3,usb3-phy"));
fdt_setprop_string(fdt, offset, "dr_mode", "host");
I have made some mistake for devicetree and USB2.0 with keeping pcie0
(not disable)? or is there a problem with the implementation?
Best regards, -E Shattow
Re: [PATCH 1/1] board: fix compatible property Milk-V Mars CM
On Fri, Jul 19, 2024 at 4:12 PM Heinrich Schuchardt
wrote:
>
> For the Milk-V Mars CM (lite) we have only been copying sizeof(void *)
> bytes to the compatible property instead of the whole string list.
This const char array must be so that we may get an accurate data
length with sizeof() but it highlights there are helper functions for
get of fdt stringlist and not for set of fdt stringlist.
>
> Fixes: de3229599d4f ("board: add support for Milk-V Mars CM")
> Reported-by: E Shattow
> Signed-off-by: Heinrich Schuchardt
> ---
> board/starfive/visionfive2/spl.c | 15 ---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index b794b73b6bd..f55c6b5d34c 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -170,23 +170,32 @@ void spl_fdt_fixup_mars_cm(void *fdt)
> {
> const char *compat;
> const char *model;
> + int compat_size;
>
> spl_fdt_fixup_mars(fdt);
>
> if (!get_mmc_size_from_eeprom()) {
> int offset;
> + static const char
> + compat_cm_lite[] = "milkv,mars-cm-lite\0starfive,jh7110";
>
> model = "Milk-V Mars CM Lite";
> - compat = "milkv,mars-cm-lite\0starfive,jh7110";
> + compat = compat_cm_lite;
> + compat_size = sizeof(compat_cm_lite);
>
> offset = fdt_path_offset(fdt,
> "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
> fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> } else {
> + static const char
> + compat_cm[] = "milkv,mars-cm\0starfive,jh7110";
> +
> model = "Milk-V Mars CM";
> - compat = "milkv,mars-cm\0starfive,jh7110";
> + compat = compat_cm;
> + compat_size = sizeof(compat_cm);
> }
> - fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> sizeof(compat));
> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"),
> + "compatible", compat, compat_size);
> fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> }
>
> --
> 2.45.2
>
-E
Re: [PATCH] riscv: dts: jh7110: Enable PLL node in SPL
On Tue, Jul 16, 2024 at 3:59 AM Leo Liang wrote: > > On Thu, Jul 11, 2024 at 12:55:05PM -0700, E Shattow wrote: > > [EXTERNAL MAIL] > > > > Ping. This regression still exists and is now in stable release. > > Should we revert this change or how must it be fixed? > > > > -E > > > > Hi all, > > I think I could revert this commit for now > if we cannot find the root cause and solution right away. > > Best regards, > Leo > Yes I agree this needs to be reverted. There are more than just failures of older SD card profiles within U-Boot, there is also the failure of newer SD card profile device access from Linux Kernel when U-Boot internal fdt is passed on to Grub2 and then Linux Kernel (6.10 upstream). We should test for these problems before bringing this change back. -E
Re: [PATCH 1/1] efi_loader: find distro device-path for media devices
On Sat, Jul 13, 2024 at 4:30 AM Heinrich Schuchardt
wrote:
>
> The auto-generated load options for media device do not contain a partition
> node. We cannot expect the simple file protocol here.
>
> Get the partition device-path via the loaded image protocol.
>
> Fixes: e91b68fd6b83 ("efi_loader: load distro dtb in bootmgr")
> Reported-by: E Shattow
> Signed-off-by: Heinrich Schuchardt
> ---
> include/efi_loader.h | 2 +-
> lib/efi_loader/efi_bootmgr.c | 2 +-
> lib/efi_loader/efi_fdt.c | 33 +++--
> 3 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 6c993e1a694..2a06fbe4704 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -1205,6 +1205,6 @@ efi_status_t efi_load_option_dp_join(struct
> efi_device_path **dp,
>
> int efi_get_distro_fdt_name(char *fname, int size, int seq);
>
> -void efi_load_distro_fdt(void **fdt, efi_uintn_t *fdt_size);
> +void efi_load_distro_fdt(efi_handle_t handle, void **fdt, efi_uintn_t
> *fdt_size);
>
> #endif /* _EFI_LOADER_H */
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 304ed43595c..589d3996b68 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -1277,7 +1277,7 @@ efi_status_t efi_bootmgr_run(void *fdt)
> if (fdt_lo)
> fdt = fdt_lo;
> if (!fdt) {
> - efi_load_distro_fdt(&fdt_distro, &fdt_size);
> + efi_load_distro_fdt(handle, &fdt_distro, &fdt_size);
> fdt = fdt_distro;
> }
> }
> diff --git a/lib/efi_loader/efi_fdt.c b/lib/efi_loader/efi_fdt.c
> index 86ba00c2bdd..4ccf2055be3 100644
> --- a/lib/efi_loader/efi_fdt.c
> +++ b/lib/efi_loader/efi_fdt.c
> @@ -75,28 +75,34 @@ int efi_get_distro_fdt_name(char *fname, int size, int
> seq)
> /**
> * efi_load_distro_fdt() - load distro device-tree
> *
> + * @handle:handle of loaded image
> * @fdt: on return device-tree, must be freed via efi_free_pages()
> * @fdt_size: buffer size
> */
> -void efi_load_distro_fdt(void **fdt, efi_uintn_t *fdt_size)
> +void efi_load_distro_fdt(efi_handle_t handle, void **fdt, efi_uintn_t
> *fdt_size)
> {
> - struct efi_device_path *rem, *dp;
> + struct efi_device_path *dp;
> efi_status_t ret;
> + struct efi_handler *handler;
> + struct efi_loaded_image *loaded_image;
> efi_handle_t device;
>
> *fdt = NULL;
>
> - dp = efi_get_dp_from_boot(NULL);
> - if (!dp)
> + /* Get boot device from loaded image protocol */
> + ret = efi_search_protocol(handle, &efi_guid_loaded_image, &handler);
> + if (ret != EFI_SUCCESS)
> return;
> - device = efi_dp_find_obj(dp, NULL, &rem);
> - ret = efi_search_protocol(device,
> &efi_simple_file_system_protocol_guid,
> - NULL);
> + loaded_image = handler->protocol_interface;
> + device = loaded_image->device_handle;
> +
> + /* Get device path of boot device */
> + ret = efi_search_protocol(device, &efi_guid_device_path, &handler);
> if (ret != EFI_SUCCESS)
> - goto err;
> - memcpy(rem, &END, sizeof(END));
> + return;
> + dp = handler->protocol_interface;
>
> - /* try the various available names */
> + /* Try the various available names */
> for (int seq = 0; ; ++seq) {
> struct efi_device_path *file;
> char buf[255];
> @@ -108,10 +114,9 @@ void efi_load_distro_fdt(void **fdt, efi_uintn_t
> *fdt_size)
> break;
> ret = efi_load_image_from_path(true, file, fdt, fdt_size);
> efi_free_pool(file);
> - if (ret == EFI_SUCCESS)
> + if (ret == EFI_SUCCESS) {
> + log_debug("Fdt %pD loaded\n", file);
> break;
> + }
> }
> -
> -err:
> - efi_free_pool(dp);
> }
> --
> 2.45.2
>
Tested-by: E Shattow
Re: [PATCH] dm: button: support remapping phone keys
Adding my casual opinion to this naming decision:
On Sun, Jul 14, 2024 at 11:24 PM Caleb Connolly
wrote:
>
> Hi Dragan,
>
> On 14/07/2024 22:47, Dragan Simic wrote:
> > Hello Caleb,
> >
> > On 2024-07-14 21:49, Caleb Connolly wrote:
> >> We don't have audio support in U-Boot, but we do have boot menus. Add an
> >> option to re-map the volume and power buttons to up/down/enter so that
> >> in situations where these are the only available buttons (such as on
> >> mobile phones) it's still possible to navigate menus built in U-Boot or
> >> an external EFI app like GRUB or systemd-boot.
> >>
> >> Signed-off-by: Caleb Connolly
> >> ---
> >> Cc: u-boot-q...@groups.io
> >
> > Very nice, thanks for this patch! Looking good to me, with a few
> > suggestions available below. Anyway, please feel free to add:
> >
> > Reviewed-by: Dragan Simic
> >
> >> ---
> >> drivers/button/Kconfig | 11 +++
> >> drivers/button/button-uclass.c | 22 +-
> >> 2 files changed, 32 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
> >> index 3918b05ae03e..6cae16fcc8bf 100644
> >> --- a/drivers/button/Kconfig
> >> +++ b/drivers/button/Kconfig
> >> @@ -8,8 +8,19 @@ config BUTTON
> >>U-Boot provides a uclass API to implement this feature. Button
> >> drivers
> >>can provide access to board-specific buttons. Use of the device
> >> tree
> >>for configuration is encouraged.
> >>
> >> +config BUTTON_REMAP_PHONE_KEYS
> >> +bool "Remap phone keys for navigation"
> >> +depends on BUTTON
> >> +help
> >> + Enable remapping of phone keys to navigation keys. This is
> >> useful for
> >> + devices with phone keys that are not used in U-Boot. The phone
> >> keys
> >> + are remapped to the following navigation keys:
> >> + - Volume up: Up
> >> + - Volume down: Down
> >> + - Power: Enter
> >> +
> >
> > Frankly, "phone keys" sounds a bit strange to me, maybe because there
> > are also tablets that have the same style of reduced-set keys. Thus,
> > I'd suggest that the following language is used:
> >
> > - "BUTTON_REMAP_PHONE_KEYS" instead of "BUTTON_REMAP_REDUCED_KEYS"
> > - "reduced smartphone-style keys" instead of "phone keys"
>
> I would have assumed that anyone working on a tablet would immediately
> guess what this option does and that it might be useful given the name.
> I would argue that seeing "BUTTON_REMAP_REDUCED_KEYS" instead makes it
> harder to guess what this option does.
>
> I think of it not as "this option remaps the keys on your phone" but as
> "this option remaps the keys that phones have", as in, the volume and
> power buttons.
>
> If you'd prefer, maybe we can meet somewhere in the middle with "mobile"?
>
> how's BUTTON_REMAP_MOBILE_KEYS?
The distinction is about what it is not, rather than what it is. It is
not a full keyboard layout but there may be some limited keyboard or
button events. That is not necessarily a mobile device, nor a phone,
or any other specific device. Certainly "reduced" makes sense but may
not translate well for all people who would need to interact with this
code.
What I have known over the existence of keyboards is that "media keys"
are describing functionality that is not typical for a keyboard input
device. The other thought is "menu" keys which is more UX-derived but
confusingly may be assumed to be arrow or pagination keys that do
exist on a full keyboard layout. Most keyboards sold (circa 2024) now
include these media keys and are listed as such, so would be familiar.
> >
> > Using "reduced" would also allow us to have this remapping logic more
> > easily extended to also cover some other buttons found on some other
> > devices with reduced-set keys.
>
> If such a device exists and gains support in U-Boot, the switch/case
> could be extended, or a new option added if it doesn't make sense to
> lump everything together. Without knowing about such a device I think
> it's impossible to make a judgement here.
>
> >
> >> config BUTTON_ADC
> >> bool "Button adc"
> >> depends on BUTTON
> >> depends on ADC
> >> diff --git a/drivers/button/button-uclass.c
> >> b/drivers/button/button-uclass.c
> >> index cda243389df3..729983d58701 100644
> >> --- a/drivers/button/button-uclass.c
> >> +++ b/drivers/button/button-uclass.c
> >> @@ -9,8 +9,9 @@
> >>
> >> #include
> >> #include
> >> #include
> >> +#include
> >>
> >> int button_get_by_label(const char *label, struct udevice **devp)
> >> {
> >> struct udevice *dev;
> >> @@ -36,16 +37,35 @@ enum button_state_t button_get_state(struct
> >> udevice *dev)
> >>
> >> return ops->get_state(dev);
> >> }
> >>
> >> +static int button_remap_phone_keys(int code)
> >
> > Pretty much the same suggestion as above applies here.
> >
> >> +{
> >> +switch (code) {
> >> +case KEY_VOLUMEUP:
> >> +return KEY_UP;
> >> +case KEY_VOLUMEDOWN:
> >> +return KEY_DOWN;
> >> +
Re: [PATCH] riscv: dts: jh7110: Enable PLL node in SPL
Ping. This regression still exists and is now in stable release.
Should we revert this change or how must it be fixed?
-E
On Sat, Apr 20, 2024 at 3:56 AM E Shattow wrote:
>
> On Fri, Apr 19, 2024 at 5:51 PM Bo Gan wrote:
> >
> ...snip...
> >
> > If without the change (reverted), can you read/write the same SD media in
> > U-boot
> > proper? (U-boot proper will switch BUS_ROOT to PLL2).
>
> I tested again this change in commit e6b7aeef, before this change in
> parent commit e6b7aeef~, af04f37a HEAD from today 19th Apr 2024 (which
> due to not matching EEPROM product_id will be in the fall-through case
> of board/starfive/visionfive2/spl.c), af04f37a with applied patchset
> "board: starfive: add Milk-V Mars CM support" from 15th Apr 2024, and
> af04f37a reverting changes from e6b7aeef also with applied patchset
> "board: starfive: add Milk-V Mars CM support" from 15th Apr 2024.
>
> In all builds is OpenSBI at commit d4d2582e HEAD from today 19 Apr 2024.
>
> For each build tested per vendor Milk-V the Mars CM Lite (SD Card only
> non-eMMC) has pinmux of GPIO22 instead of GPIO62:
>
> -- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> @@ -233,7 +233,7 @@
>
> mmc0_pins: mmc0-pins {
> mmc0-pins-rest {
> - pinmux = + pinmux =GPOEN_ENABLE, GPI_NONE)>;
> bias-pull-up;
> drive-strength = <12>;
>
> U-Boot config is simply starfive_visionfive2_defconfig.
>
> Results are as follows.
>
> StarFive # mac
> EEPROM INFO
> Vendor : MILK-V
> Product full SN: MARC-V10-2340-D004E000-06DF
> data version: 0x2
> PCB revision: 0xc1
> BOM revision: A
> Ethernet MAC0 address: 6c:cf:39:00:83:11
> Ethernet MAC1 address: 6c:cf:39:00:83:12
> EEPROM INFO
>
> e6b7aeef: 2GB microSD (no speed class markings)
> af04f37a: 2GB microSD (no speed class markings)
> af04f37a with Mars CM patchset: 2GB microSD (no speed class markings)
> StarFive # mmc rescan ; mmc info
> unable to select a mode
> unable to select a mode
>
> e6b7aeef~: 2GB microSD (no speed class markings)
> af04f37a revert e6b7aeef with Mars CM patchset: 2GB microSD (no speed
> class markings)
> StarFive # mmc rescan ; mmc info
> Device: mmc@1601
> Manufacturer ID: 1c
> OEM: 5356
> Name: USD
> Bus Speed: 5000
> Mode: SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 2.0
> High Capacity: No
> Capacity: 1.9 GiB
> Bus Width: 1-bit
> Erase Group Size: 512 Bytes
>
> e6b7aeef: 8GB microSD Class 4
> e6b7aeef~: 8GB microSD Class 4
> af04f37a: 8GB microSD Class 4
> af04f37a with Mars CM patchset: 8GB microSD Class 4
> af04f37a revert e6b7aeef with Mars CM patchset: 8GB microSD Class 4
> StarFive # mmc rescan ; mmc info
> Device: mmc@1601
> Manufacturer ID: 2
> OEM: 544d
> Name: SA08G
> Bus Speed: 5000
> Mode: SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 7.4 GiB
> Bus Width: 1-bit
> Erase Group Size: 512 Bytes
>
> e6b7aeef: 8GB microSD Class 10
> e6b7aeef~: 8GB microSD Class 10
> af04f37a: 8GB microSD Class 10
> af04f37a with Mars CM patchset: 8GB microSD Class 10
> af04f37a revert e6b7aeef with Mars CM patchset: 8GB microSD Class 10
> StarFive # mmc rescan ; mmc info
> Device: mmc@1601
> Manufacturer ID: 74
> OEM: 4a60
> Name: USD
> Bus Speed: 5000
> Mode: SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 7.5 GiB
> Bus Width: 1-bit
> Erase Group Size: 512 Bytes
>
> e6b7aeef: 32GB microSD Class 10 A1 U1 HC1
> e6b7aeef~: 32GB microSD Class 10 A1 U1 HC1
> af04f37a: 32GB microSD Class 10 A1 U1 HC1
> af04f37a with Mars CM patchset: 32GB microSD Class 10 A1 U1 HC1
> af04f37a revert e6b7aeef with Mars CM patchset: 32GB microSD Class 10 A1 U1
> HC1
> StarFive # mmc rescan ; mmc info
> Device: mmc@1601
> Manufacturer ID: 3
> OEM: 5344
> Name: SC32G
> Bus Speed: 5000
> Mode: SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 29.7 GiB
> Bus Width: 1-bit
> Erase Group Size: 512 Bytes
>
> e6b7aeef: 200GB microSD Class 10 A1 U1 XC1
> e6b7aeef~: 200GB microSD Class 10 A1 U1 XC1
> af04f37a: 200GB microSD Class 10 A1 U1 XC1
> af04f37a with Mars CM patchset: 200GB microSD Class 10 A1 U1 XC1
> af04f37a revert e6b7aeef with Mars CM patchset: 200GB microSD Class 10 A1 U1
> XC1
> StarFive # mmc rescan ; mmc info
> Device: mmc@16
Re: [PATCH v2 0/7] Add Starfive JH7110 Cadence USB driver
| 7 + > > drivers/phy/starfive/phy-jh7110-pcie.c| 202 ++ > > drivers/phy/starfive/phy-jh7110-usb2.c| 135 > > drivers/pinctrl/starfive/pinctrl-jh7110-sys.c | 11 +- > > drivers/usb/cdns3/Kconfig | 7 + > > drivers/usb/cdns3/Makefile| 2 + > > drivers/usb/cdns3/cdns3-starfive.c| 183 > > drivers/usb/cdns3/core.c | 25 +++ > > 15 files changed, 661 insertions(+), 2 deletions(-) > > create mode 100644 drivers/phy/starfive/Kconfig > > create mode 100644 drivers/phy/starfive/Makefile > > create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c > > create mode 100644 drivers/phy/starfive/phy-jh7110-usb2.c > > create mode 100644 drivers/usb/cdns3/cdns3-starfive.c > > > > > > base-commit: 8937bb265a7f2251c1bd999784a4ef10e9c6080d > Thanks very much Minda, I am happy to see some USB Host function on Milk-V Mars CM Lite here. -E Shattow
Re: [PATCH v1 0/7] Add Starfive JH7110 Cadence USB driver
Hi, On Sun, Jun 23, 2024 at 6:28 PM Minda Chen wrote: > > > > > > > Minda, can you test USB Host function on VisionFive2? I guess that it is > > connected to the USB-C port. For the boards with JH7110 and not any > > VL805 USB controller this Cadence USB is the only way to have host USB. It > > is > > very much wanted to have host USB. Thanks! -E > > > > In VF2, PCIe0 connect with VL805 USB 3.0 host controller. Now PCIe driver have > commit to Uboot upstream code. USB 3.0 can be used in uboot upstream code. > Milk-v mars also connect VL805 and can use USB 3.0 host too. No no I am asking about Cadence USB of JH7110 CPU. This VL805 is not the question, sorry that my question was not easy to understand before. > > You can use "pci e" command to active USB 3.0 host controller and then "usb > reset" to > scan usb devices. If you have any issue about this. Also reply it in this. > Thanks. Can you show that Host USB is functioning on VF2 with the JH7110 CPU Cadence USB, not the VL805 controller? This is needed so Milk-V Mars CM and Pine64 Star64 can have USB Host. There is no use of VL805 and we need JH7110 Cadence USB then. Thanks! -E > > > On Sun, May 19, 2024 at 11:20 PM Minda Chen > > wrote: > > > > > > > > > > > > > > > > > Hi, there is a compile warning. I don't know why. > > > > > > > > On Sat, May 4, 2024 at 8:04 AM Minda Chen > > > > > > > > wrote: > > > > > > > > > > Add Starfive JH7110 Cadence USB driver and related PHY driver. > > > > > So the codes can be used in visionfive2 and milkv 7110 board. > > > > > > > > > > The driver is almost the same with kernel driver. > > > > > > > > > > patch1: Add set phy mode function in cdns3 core driver > > > > > which is used by Starfive. > > > > > > > > > > patch2-3: USB and PCIe 2.0 (usb 3.0) PHY drivier > > > > > patch4: Cadence USB wrapper driver. > > > > > patch5-7 dts, config and maintainers update. > > > > > > > > > > Minda Chen (7): > > > > > usb: cdns3: Set USB PHY mode in cdns3_probe() > > > > > phy: starfive: Add Starfive JH7110 USB 2.0 PHY driver > > > > > phy: starfive: Add Starfive JH7110 PCIe 2.0 PHY driver > > > > > usb: cdns: starfive: Add cdns USB driver > > > > > configs: starfive: Add visionfive2 cadence USB configuration > > > > > dts: starfive: Add JH7110 Cadence USB dts node > > > > > MAINTAINERS: Update Starfive visionfive2 maintain files. > > > > > > > > > > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 + > > > > > arch/riscv/dts/jh7110.dtsi| 52 + > > > > > board/starfive/visionfive2/MAINTAINERS| 2 + > > > > > configs/starfive_visionfive2_defconfig| 9 + > > > > > drivers/phy/Kconfig | 1 + > > > > > drivers/phy/Makefile | 1 + > > > > > drivers/phy/starfive/Kconfig | 19 ++ > > > > > drivers/phy/starfive/Makefile | 7 + > > > > > drivers/phy/starfive/phy-jh7110-pcie.c| 211 > > > > ++ > > > > > drivers/phy/starfive/phy-jh7110-usb2.c| 135 +++ > > > > > drivers/usb/cdns3/Kconfig | 7 + > > > > > drivers/usb/cdns3/Makefile| 2 + > > > > > drivers/usb/cdns3/cdns3-starfive.c| 184 > > +++ > > > > > drivers/usb/cdns3/core.c | 17 ++ > > > > > 14 files changed, 652 insertions(+) create mode 100644 > > > > > drivers/phy/starfive/Kconfig create mode 100644 > > > > > drivers/phy/starfive/Makefile create mode 100644 > > > > > drivers/phy/starfive/phy-jh7110-pcie.c > > > > > create mode 100644 drivers/phy/starfive/phy-jh7110-usb2.c > > > > > create mode 100644 drivers/usb/cdns3/cdns3-starfive.c > > > > > > > > > > > > > > > base-commit: 174ac987655c888017c82df1883c0c2ea0dc2495 > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > The compile warning as follows: > > > > > > > > In file included from > > > > /home/user/source/u-boot.git/drivers/usb/cdns3/gadget.c:70: > > > > /home/user/source/u-boot.git/include/linux/bitmap.h: In function > > > > ‘bitmap_find_next_zero_area’: > > > > /home/user/source/u-boot.git/include/linux/bitmap.h:170:17: warning: > > > > implicit declaration of function ‘find_next_zero_bit’; did you mean > > > > ‘find_next_bit’? [-Wimplicit-function-declaration] > > > > 170 | index = find_next_zero_bit(map, size, start); > > > > | ^~ > > > > | find_next_bit > > > > CC drivers/usb/cdns3/ep0.o > > > > In file included from > > > > /home/user/source/u-boot.git/include/linux/usb/composite.h:26, > > > > from > > > > /home/user/source/u-boot.git/drivers/usb/cdns3/ep0.c:19: > > > > /home/user/source/u-boot.git/include/linux/bitmap.h: In function > > > > ‘bitmap_find_next_zero_area’: > > > > /home/user/source/u-boot.git/include/linux/bitmap.h:170:17: warning: > > > > implicit declaration of function ‘find_next_zero_bit’; did you me
Re: [PATCH v1 0/7] Add Starfive JH7110 Cadence USB driver
Minda, can you test USB Host function on VisionFive2? I guess that it is connected to the USB-C port. For the boards with JH7110 and not any VL805 USB controller this Cadence USB is the only way to have host USB. It is very much wanted to have host USB. Thanks! -E On Sun, May 19, 2024 at 11:20 PM Minda Chen wrote: > > > > > -邮件原件----- > > 发件人: E Shattow > > 发送时间: 2024年5月20日 13:06 > > 收件人: Minda Chen > > 抄送: Marek Vasut ; Tom Rini ; Roger > > Quadros ; Neil Armstrong ; > > Alexey Romanov ; Sumit Garg > > ; Mark Kettenis ; Nishanth > > Menon ; Rick Chen ; Leo Yu-Chi Liang > > ; u-boot@lists.denx.de; Heinrich Schuchardt > > ; Simon Glass > > 主题: Re: [PATCH v1 0/7] Add Starfive JH7110 Cadence USB driver > > > > Hi, there is a compile warning. I don't know why. > > > > On Sat, May 4, 2024 at 8:04 AM Minda Chen > > wrote: > > > > > > Add Starfive JH7110 Cadence USB driver and related PHY driver. > > > So the codes can be used in visionfive2 and milkv 7110 board. > > > > > > The driver is almost the same with kernel driver. > > > > > > patch1: Add set phy mode function in cdns3 core driver > > > which is used by Starfive. > > > > > > patch2-3: USB and PCIe 2.0 (usb 3.0) PHY drivier > > > patch4: Cadence USB wrapper driver. > > > patch5-7 dts, config and maintainers update. > > > > > > Minda Chen (7): > > > usb: cdns3: Set USB PHY mode in cdns3_probe() > > > phy: starfive: Add Starfive JH7110 USB 2.0 PHY driver > > > phy: starfive: Add Starfive JH7110 PCIe 2.0 PHY driver > > > usb: cdns: starfive: Add cdns USB driver > > > configs: starfive: Add visionfive2 cadence USB configuration > > > dts: starfive: Add JH7110 Cadence USB dts node > > > MAINTAINERS: Update Starfive visionfive2 maintain files. > > > > > > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 + > > > arch/riscv/dts/jh7110.dtsi| 52 + > > > board/starfive/visionfive2/MAINTAINERS| 2 + > > > configs/starfive_visionfive2_defconfig| 9 + > > > drivers/phy/Kconfig | 1 + > > > drivers/phy/Makefile | 1 + > > > drivers/phy/starfive/Kconfig | 19 ++ > > > drivers/phy/starfive/Makefile | 7 + > > > drivers/phy/starfive/phy-jh7110-pcie.c| 211 > > ++ > > > drivers/phy/starfive/phy-jh7110-usb2.c| 135 +++ > > > drivers/usb/cdns3/Kconfig | 7 + > > > drivers/usb/cdns3/Makefile| 2 + > > > drivers/usb/cdns3/cdns3-starfive.c| 184 +++ > > > drivers/usb/cdns3/core.c | 17 ++ > > > 14 files changed, 652 insertions(+) > > > create mode 100644 drivers/phy/starfive/Kconfig create mode 100644 > > > drivers/phy/starfive/Makefile create mode 100644 > > > drivers/phy/starfive/phy-jh7110-pcie.c > > > create mode 100644 drivers/phy/starfive/phy-jh7110-usb2.c > > > create mode 100644 drivers/usb/cdns3/cdns3-starfive.c > > > > > > > > > base-commit: 174ac987655c888017c82df1883c0c2ea0dc2495 > > > -- > > > 2.17.1 > > > > > > > The compile warning as follows: > > > > In file included from > > /home/user/source/u-boot.git/drivers/usb/cdns3/gadget.c:70: > > /home/user/source/u-boot.git/include/linux/bitmap.h: In function > > ‘bitmap_find_next_zero_area’: > > /home/user/source/u-boot.git/include/linux/bitmap.h:170:17: warning: > > implicit declaration of function ‘find_next_zero_bit’; did you mean > > ‘find_next_bit’? [-Wimplicit-function-declaration] > > 170 | index = find_next_zero_bit(map, size, start); > > | ^~ > > | find_next_bit > > CC drivers/usb/cdns3/ep0.o > > In file included from > > /home/user/source/u-boot.git/include/linux/usb/composite.h:26, > > from > > /home/user/source/u-boot.git/drivers/usb/cdns3/ep0.c:19: > > /home/user/source/u-boot.git/include/linux/bitmap.h: In function > > ‘bitmap_find_next_zero_area’: > > /home/user/source/u-boot.git/include/linux/bitmap.h:170:17: warning: > > implicit declaration of function ‘find_next_zero_bit’; did you mean > > ‘find_next_bit’? [-Wimplicit-function-declaration] > > 170 | index = find_next_zero_bit(map, size, start); > > | ^~ > > | find_next_bit > > > > > > Is this something missing in the patch series? > > > > -E > > I have not noticed this. I just check this it is risc-v code do not contain > "find_next_zero_bit" macro define.
Re: Where to put DTB file for UEFI support on ARM
Hi Leith, On Wed, Jun 12, 2024 at 5:50 AM Leith Bade wrote: > > Hi all, > > I have a BananaPi BPI-R3 board that I am setting up to boot Ubuntu or > Debian. After figuring out the required config options I have managed to > get Uboot to boot the Ubuntu USB installer, and also boot off an SD card > once I installed Ubuntu there. > > I set up the bootmeth/bootdev/bootflow stuff so that the auto boot menu > works nicely with SD card. For USB I have to drop to console and do manual > "usb start" etc, but good enough for running the installer. It was a bit > difficult as there seems to be a lot of older documentation or Google > results related to using UEFI before the bootflow stuff was added. Also it > took me a while to figure out that a board/mediatek/mt7986.env file is now > needed as some of the config options set #define values that are not > included in /include/configs/mt7986.h board headers - due to .env files > replacing those #defines. It would be nice to have a default .env file for > this board that sets sensible defaults for the various memory loading > addresses as it took a bit of effort to come up with all the hex values. > > However I am struggling to work out where to place the device tree DTB file > for the Linux kernel. For this board I need a different DTB for > U-boot (which is included in the u-boot.bin and fip.bin) and the kernel as > U-boot's drivers do not like the Linux upstream device tree. > > I can see in the UEFI bootmeth code that it searches a location /dtb/ for a > file named in the $fdtfile environment variable. I tried putting the .dtb > file at /dtb/mt7986a-bananapi-bpi-r3.dtb on the SD card's FAT32 partition > that Ubuntu created, however it doesn't seem to load this and the U-boot > boots with the u-boot.bin DTB which means kernel doesn't start If I > manually load the correct .dtb file to $fdt_addr_r then the correct DTB is > used and the kernel starts. Correct, this is a limitation and the fix is not yet released with U-Boot; It is now queued up for U-Boot -next, but only for EFI bootmanager (not the global EFI boot). > > What is the correct location for the .dtb file when using UEFI? In theory you could just load it from the target Linux OS root filesystem at /usr/lib/linux-image-... but when I try this there's no support to read that filesystem within EFI bootmanager; the files referenced by EFI bootmanager are however accessible if on the EFI System Partition. Perhaps I am missing some option to support loading from partitions other than the EFI System Partition but I don't know how. > > Is there a recommended way of putting the kernel DTB into the NAND or NOR > chip when using UEFI? That way if the SD card gets wiped U-boot can still > boot the USB drive. I don't fully understand how the MTD partitions, FIT > files, UBI files all fit into the picture to have U-boot load the kernel > DTB from flash. U-Boot has its own internal FDT (firmware devicetree) which you note may or may not work well enough to boot Linux kernel and have a working OS userspace. It is ambiguous if a Linux OS kernel dtb of a specific board hardware target belongs in any of NAND or NOR flash since they tend to be tied to specific versions of the Linux kernel, as things are today. There's no consensus that I've read anywhere on how this should be done - again not specific to U-Boot. I can just say from experience that if it would be able to be loaded from the same rootfs that the Linux kernel resides then there is still the trouble of not knowing which exact Linux kernel version that the user will decide to load that boot cycle. > > Thanks, > Leith Bade > le...@bade.nz -E Shattow
Re: [PATCH v1 0/4] Sync StarFive JH7110 clock and reset dt-bindings with Linux
Hi Hal, Instead of manual dt-bindings sync can we please adopt OF_UPSTREAM for JH7110 ? -E On Mon, Jun 3, 2024 at 6:57 AM Hal Feng wrote: > > There are differences in clock / reset dt-bindings between U-Boot and > Linux. Sync them, so it is feasible to use OF_UPSTREAM for StarFive > JH7110 SoC. > > Hal Feng (4): > dt-bindings: clock: jh7110: Sync with Linux > dt-bindings: reset: jh7110: Sync with Linux > clk: starfive: jh7110: Sync clock definitions with Linux > riscv: dts: jh7110: Sync clock and reset definitions with Linux > > .../dts/jh7110-starfive-visionfive-2.dtsi | 6 +- > arch/riscv/dts/jh7110-u-boot.dtsi | 2 +- > arch/riscv/dts/jh7110.dtsi| 28 +-- > drivers/clk/starfive/clk-jh7110-pll.c | 6 +- > drivers/clk/starfive/clk-jh7110.c | 44 ++--- > .../dt-bindings/clock/starfive,jh7110-crg.h | 180 +++--- > .../dt-bindings/reset/starfive,jh7110-crg.h | 144 -- > 7 files changed, 243 insertions(+), 167 deletions(-) > > > base-commit: ea722aa5eb33740ae77e8816aeb72b385e621cd0 > -- > 2.43.2 >
Re: [PATCH v2 0/8] efi_loader: improve device-tree loading
Hi, On Tue, May 28, 2024 at 7:43 AM Heinrich Schuchardt wrote: > > In U-Boot EFI boot options can already specify both an EFI binary and > an initrd. With this series we can additionally define the matching > device-tree to be loaded in the boot option. > > With the last patch the boot manager will fall back the device-tree > specified by $fdtfile in directories '/dtb/', '/', or '/dtb/current/' > on the boot device if no device-tree is specified in the boot > option or via a bootefi command parameter. > As tested the $fdtfile environment variable has no effect on global EFI boot when i.e. EFI/BOOT/BOOTRISCV64.EFI on EFI System Partition and no user-added boot option; $fdtfile env variable is not used with "mmc 0" or whichever global boot option is enabled by default in the boot order. Adding a boot option for EFI/BOOT/BOOTRISCV64.EFI and giving this priority in the boot order allows $fdtfile to be effective here. This is consistent with what is described by the series. Would the global EFI boot also get support for $fdtfile either with this or a later series? > v2: > Update efi_dp_concat() instead of new function efi_dp_merge(). > Carve out a function efi_load_option_dp_join() which we can > use both for the eficonfig and the efidebug command. > Rename variables id_dp, final_dp_size. > Rename create_initrd_dp() to create_lo_dp_part(). > Use enum as parameter for create_lo_dp_part(). > Put all related changes into one patch. > > Heinrich Schuchardt (8): > efi_loader: allow concatenation with contained end node > cmd: eficonfig: add support for setting fdt > cmd: efidebug: add support for setting fdt > efi_loader: load device-tree specified in boot option > efi_loader: move distro_efi_get_fdt_name() > efi_loader: return binary from efi_dp_from_lo() > efi_loader: export efi_load_image_from_path > efi_loader: load distro dtb in bootmgr > > boot/bootmeth_efi.c| 60 +- > cmd/eficonfig.c| 83 + > cmd/efidebug.c | 130 +++-- > include/efi_loader.h | 24 +++- > lib/efi_loader/Makefile| 1 + > lib/efi_loader/efi_bootbin.c | 2 +- > lib/efi_loader/efi_bootmgr.c | 75 +++- > lib/efi_loader/efi_boottime.c | 3 +- > lib/efi_loader/efi_device_path.c | 40 --- > lib/efi_loader/efi_device_path_utilities.c | 2 +- > lib/efi_loader/efi_fdt.c | 117 +++ > lib/efi_loader/efi_helper.c | 44 +++ > 12 files changed, 445 insertions(+), 136 deletions(-) > create mode 100644 lib/efi_loader/efi_fdt.c > > -- > 2.43.0 > Tested-by: E Shattow
Re: [PATCH] riscv: dts: jh7110: Update qspi node with upstream
Hi,
On Mon, May 27, 2024 at 3:47 AM wrote:
>
> From: Matthias Brugger
>
> Upstream node uses a specific SoC compatible to make the kernel driver
> work. Copy over the upstream node to fullfill that need.
>
> Signed-off-by: Matthias Brugger
> ---
>
> .../jh7110-starfive-visionfive-2-u-boot.dtsi | 2 +-
> .../dts/jh7110-starfive-visionfive-2.dtsi | 29 ---
> arch/riscv/dts/jh7110.dtsi| 19 +++-
> 3 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> index 3012466b305..a69d8fcb391 100644
> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2-u-boot.dtsi
> @@ -40,7 +40,7 @@
> &qspi {
> bootph-pre-ram;
>
> - nor-flash@0 {
> + nor_flash@0 {
> bootph-pre-ram;
> };
> };
> diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> index e11babc1cde..375449b73a8 100644
> --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> @@ -305,17 +305,38 @@
> };
>
> &qspi {
> - spi-max-frequency = <25000>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> status = "okay";
>
> - nor-flash@0 {
> + nor_flash: nor_flash@0 {
> compatible = "jedec,spi-nor";
> - reg=<0>;
> - spi-max-frequency = <1>;
> + reg = <0>;
> + cdns,read-delay = <5>;
> + spi-max-frequency = <1200>;
> cdns,tshsl-ns = <1>;
> cdns,tsd2d-ns = <1>;
> cdns,tchsh-ns = <1>;
> cdns,tslch-ns = <1>;
> +
> + partitions {
> + compatible = "fixed-partitions";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + spl@0 {
> + reg = <0x0 0x8>;
> + };
> + uboot-env@f {
> + reg = <0xf 0x1>;
> + };
> + uboot@10 {
> + reg = <0x10 0x40>;
> + };
> + reserved-data@60 {
> + reg = <0x60 0xa0>;
> + };
> + };
> };
> };
>
> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
> index 2cdc683d49b..2b331e58497 100644
> --- a/arch/riscv/dts/jh7110.dtsi
> +++ b/arch/riscv/dts/jh7110.dtsi
> @@ -480,19 +480,22 @@
> };
>
> qspi: spi@1301 {
> - compatible = "cdns,qspi-nor";
> - reg = <0x0 0x1301 0x0 0x1
> - 0x0 0x2100 0x0 0x40>;
> - clocks = <&syscrg JH7110_SYSCLK_QSPI_REF>;
> - clock-names = "clk_ref";
> + compatible = "starfive,jh7110-qspi", "cdns,qspi-nor";
> + reg = <0x0 0x1301 0x0 0x1>,
> + <0x0 0x2100 0x0 0x40>;
> + interrupts = <25>;
> + clocks = <&syscrg JH7110_SYSCLK_QSPI_REF>,
> +<&syscrg JH7110_SYSCLK_QSPI_AHB>,
> +<&syscrg JH7110_SYSCLK_QSPI_APB>;
> + clock-names = "ref", "ahb", "apb";
> resets = <&syscrg JH7110_SYSRST_QSPI_APB>,
> <&syscrg JH7110_SYSRST_QSPI_AHB>,
> <&syscrg JH7110_SYSRST_QSPI_REF>;
> - reset-names = "rst_apb", "rst_ahb", "rst_ref";
> + reset-names = "qspi", "qspi-ocp", "rstc_ref";
> cdns,fifo-depth = <256>;
> cdns,fifo-width = <4>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> + cdns,trigger-address = <0x0>;
> + status = "disabled";
> };
>
> syscrg: clock-controller@1302 {
> --
> 2.44.0
>
Please consider OF_UPSTREAM. Linux Kernel has now merged the Milk-V
Mars dts patch series which introduces a jh7110-common.dtsi and also
the PLDA PCIe HOST patch set is queued for-next, so it is a good time
to do so.
-E
Re: [PATCH 2/2 v5] board: starfive: support Pine64 Star64 board
9E5-4433-87C0-68B6B72699C7 \ > + /dev/sdb > + > +Copy U-Boot to the SD card > + > +.. code-block:: bash > + > + sudo dd if=u-boot-spl.bin.normal.out of=/dev/sdb1 > + sudo dd if=u-boot.itb of=/dev/sdb2 > + > + sudo mount /dev/sdb3 /mnt/ > + sudo cp u-boot-spl.bin.normal.out /mnt/ > + sudo cp u-boot.itb /mnt/ > + sudo cp Image.gz /mnt/ > + sudo cp initramfs.cpio.gz /mnt/ > + sudo cp jh7110-starfive-visionfive-2.dtb /mnt/ > + sudo umount /mnt > + > +Booting > +~~~ > + > +Once you plugin the sdcard and power up, you should see the U-Boot prompt. > + > +Serial Number and MAC address issues > + > + > +U-Boot requires valid EEPROM data to determine which board-specific fix-up to > +apply at runtime. This affects the size of memory initialized, network mac > +address numbering, and tuning of the network PHYs. > + > +The Star64 does not currently ship with unique serial numbers per-device. > +Devices follow a pattern where the last mac address bytes are a sum of 0x7558 > +and the serial number (lower port mac0), or a sum of 0x7559 and the serial > +number (upper port mac1). > + > +As tested there are several 4gb model units where the serial number and > network > +mac addresses collide with other devices (serial > +``STAR64V1-2310-D004E000-0005``, MACs ``6c:cf:39:00:75:61``, > +``6c:cf:39:00:75:62``) > + > +Some early Star64 boards shipped with an uninitialized EEPROM and no write > +protect pull-up resistor in place. Later units of all 4gb and 8gb models > +sharing the same serial number in EEPROM data will have this problem that the > +network mac addresses are alike between different models and this may be > +corrected by defeating the write protect resistor to write new values. As an > +alternative to this, it may be worked around by overriding the mac addresses > +via U-Boot environment variables. > + > +It is required for any unit having uninitialized EEPROM and recommended for > +all later Star64 4gb model units (not properly serialized) to have decided > on a > +new 6-byte serial number. This serial number should be high enough to > +avoid collision with other JH7110 boards and low enough not to overflow i.e. > +between ``cafe00`` and ``f00d00``. > + > +Update EEPROM values > + > + > +1. Prepare EEPROM data in memory > + > +:: > + > + ## When there is no error to load existing data: > + mac read_eeprom > + > + ## When there is an error to load non-existing data: > + # "DRAM: Not a StarFive EEPROM data format - magic error" > + mac initialize > + > +2. Set Star64 values > + > +:: > + > + ## Common values > + mac vendor PINE64 > + mac pcb_revision c1 > + mac bom_revision A > + > + ## Device-specific values > + # Year 2023 week 10 production date, 8GB DRAM, optional eMMC, serial > cdef01 > + mac product_id STAR64V1-2310-D008E000-00cdef01 > + > + # Last three bytes mac0: 0x7558 + serial number 0xcdef01 > + mac mac0_address 6c:cf:39:ce:64:59 > + > + # Last three bytes mac1: 0x7559 + serial number 0xcdef01 > + mac mac1_address 6c:cf:39:ce:64:5a > + > +3. Defeat write-protect pull-up resistor (if installed) and write to EEPROM > + > +:: > + > + mac write_eeprom > + > +Set Variables in U-Boot > +^^^ > + > +.. note:: Changing just the serial number will not alter your MAC address > + > +The MAC addresses may be "set" as follows by writing as a custom config to > SPI > +(Change the last 3 bytes of MAC addreses as appropriate): > + > +:: > + > + env set serial# STAR64V1-2310-D008E000-00cdef01 > + env set ethaddr 6c:cf:39:ce:64:59 > + env set eth1addr 6c:cf:39:ce:64:5a > + env save > + reset > -- > 2.44.0 > > I understand setting $serial# environment variable to be more illustrative than prescriptive. I think this should be a comment to guide the setting of ethaddr and eth1addr but not itself be set by the user. With that: Reviewed-by: E Shattow
Re: [PATCH 1/2 v5] board: starfive: support Pine64 Star64 board
ler@1700");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1603");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_AONCLK_GMAC0_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_AONCLK_GMAC0_RMII_RTX);
> +
> + /* gmac1 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1302");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1604");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_SYSCLK_GMAC1_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_SYSCLK_GMAC1_RMII_RTX);
> +
> + for (i = 0; i < ARRAY_SIZE(star64_pine64); i++) {
> + offset = fdt_path_offset(fdt, star64_pine64[i].path);
> +
> + if (star64_pine64[i].value)
> + ret = fdt_setprop_u32(fdt, offset,
> star64_pine64[i].name,
> + dectoul(star64_pine64[i].value,
> NULL));
> + else
> + ret = fdt_setprop_empty(fdt, offset,
> star64_pine64[i].name);
> +
> + if (ret) {
> + pr_err("%s set prop %s fail.\n", __func__,
> star64_pine64[i].name);
> + break;
> + }
> + }
> +}
> +
> void spl_perform_fixups(struct spl_image_info *spl_image)
> {
> u8 version;
> @@ -278,6 +365,8 @@ void spl_perform_fixups(struct spl_image_info *spl_image)
> spl_fdt_fixup_version_b(spl_image->fdt_addr);
> break;
> };
> + } else if (!strncmp(product_id, "STAR64", 6)) {
> + spl_fdt_fixup_star64(spl_image->fdt_addr);
> } else {
> pr_err("Unknown product %s\n", product_id);
> };
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> b/board/starfive/visionfive2/starfive_visionfive2.c
> index 6be5348962..f6114602f8 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -27,6 +27,8 @@ DECLARE_GLOBAL_DATA_PTR;
> "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb"
> #define FDTFILE_VISIONFIVE2_1_3B \
> "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb"
> +#define FDTFILE_PINE64_STAR64 \
> + "starfive/jh7110-pine64-star64.dtb"
>
> /* enable U74-mc hart1~hart4 prefetcher */
> static void enable_prefetcher(void)
> @@ -87,6 +89,8 @@ static void set_fdtfile(void)
> fdtfile = FDTFILE_VISIONFIVE2_1_3B;
> break;
> }
> + } else if (!strncmp(product_id, "STAR64", 6)) {
> + fdtfile = FDTFILE_PINE64_STAR64;
> } else {
> log_err("Unknown product\n");
> return;
> --
> 2.44.0
>
>
With that note about phy1 tx-internal-delay-ps (I have no Star64 to test myself)
Reviewed-by: E Shattow
Re: [PATCH 2/2 v4] board: starfive: support Pine64 Star64 board
Hi, On Sun, May 19, 2024 at 9:43 PM H Bell wrote: > > Add documentation files > > Signed-off-by: Henry Bell > Cc: ycli...@andestech.com > Cc: heinrich.schucha...@canonical.com > --- > > Changes since v1 > > - New patch > > Changes since v2 > > - Remove extra params to > - Clarification on boot section > - Add entry on MAC adresses and how to correct them > > Changes since v3 > > - Rebase against d678a59d2d > --- > doc/board/starfive/index.rst | 1 + > doc/board/starfive/pine64_star64.rst | 136 +++ > 2 files changed, 137 insertions(+) > create mode 100644 doc/board/starfive/pine64_star64.rst > > diff --git a/doc/board/starfive/index.rst b/doc/board/starfive/index.rst > index d369b986cc..72ab6ddfbf 100644 > --- a/doc/board/starfive/index.rst > +++ b/doc/board/starfive/index.rst > @@ -8,4 +8,5 @@ StarFive > > milk-v_mars > milk-v_mars_cm > + pine64_star64 > visionfive2 > diff --git a/doc/board/starfive/pine64_star64.rst > b/doc/board/starfive/pine64_star64.rst > new file mode 100644 > index 00..567d1207ba > --- /dev/null > +++ b/doc/board/starfive/pine64_star64.rst > @@ -0,0 +1,136 @@ > +.. SPDX-License-Identifier: GPL-2.0+ > + > +Pine64 Star64 > +=== > + > +U-Boot for the Star64 uses the same U-Boot binaries as the VisionFive 2 > board. > +In U-Boot SPL the actual board is detected and the device-tree patched > +accordingly. > + > +Building > + > + > +1. Add the RISC-V toolchain to your PATH. > +2. Setup ARCH & cross compilation environment variable: > + > +.. code-block:: none > + > + export CROSS_COMPILE= > + > +The M-mode software OpenSBI provides the supervisor binary interface (SBI) > and > +is responsible for the switch to S-Mode. It is a prerequisite to build > U-Boot. > +Support for the JH7110 was introduced in OpenSBI 1.2. It is recommended to > use > +a current release. > + > +.. code-block:: console > + > + git clone https://github.com/riscv/opensbi.git > + cd opensbi > + make PLATFORM=generic FW_TEXT_START=0x4000 > + > +Now build the U-Boot SPL and U-Boot proper. > + > +.. code-block:: console > + > + cd > + make starfive_visionfive2_defconfig > + make > OPENSBI=$(opensbi_dir)/build/platform/generic/firmware/fw_dynamic.bin > + > +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as > well > +as the FIT image (u-boot.itb) with OpenSBI and U-Boot. > + > +Device-tree selection > +~ > + > +U-Boot will set variable $fdtfile to starfive/jh7110-pine64-star64.dtb. > + > +To overrule this selection the variable can be set manually and saved in the > +environment > + > +:: > + > +setenv fdtfile my_device-tree.dtb > +env save Nit: prefer consistent sub-command as: env set fdtfile my_device-tree.dtb > + > +or the configuration variable CONFIG_DEFAULT_FDT_FILE can be used to set to > +provide a default value. > + > +Boot source selection > +~ > + > +The board provides the DIP switches (marked S1804) adjacent to the 40pin GPIO > +Bus for boot selection. The ``L`` (0) and ``H`` (1) silk screening to the > side > +of the should be used to determine state (not the ``ON/ONKE`` markings > +physically on the component). ``GPIO_0`` is channel 2 on the switch, while > +``GPIO_1`` is channel 1. Suggested: "Boot mode is selected by an MSEL-DIP marked S1804 and GPIO_0 position adjacent to the 40pin GPIO header. ON/ONKE and number markings of the MSEL-DIP are misleading; Instead refer to the ``L`` (0) and ``H`` (1) silkscreen for accurate selection." > + > ++ (QSPI) Flash: 00 ``??`` > ++ SD: 01 ``??`` > ++ EMMC: 10 ``??`` > ++ UART: 11 ``??`` > + > +Preparing the SD-Card > +~ > + > +The device firmware loads U-Boot SPL (u-boot-spl.bin.normal.out) from the > +partition with type GUID 2E54B353-1271-4842-806F-E436D6AF6985. You are free > +to choose any partition number. > + > +With the default configuration U-Boot SPL loads the U-Boot FIT image > +(u-boot.itb) from partition 2 > (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=0x2). > +When formatting it is recommended to use GUID > +BC13C2FF-59E6-4262-A352-B275FD6F7172 for this partition. > + > +The FIT image (u-boot.itb) is a combination of OpenSBI's fw_dynamic.bin, > +u-boot-nodtb.bin and the device tree blob. > + > +Format the SD card (make sure the disk has GPT, otherwise use gdisk to > switch) > + > +.. code-block:: bash > + > + sudo sgdisk --clear \ > + --set-alignment=2 \ > + --new=1:4096:8191 --change-name=1:spl > --typecode=1:2E54B353-1271-4842-806F-E436D6AF6985\ > + --new=2:8192:16383 --change-name=2:uboot > --typecode=2:BC13C2FF-59E6-4262-A352-B275FD6F7172 \ > + --new=3:16384:1654784 --change-name=3:system > --typecode=3:EBD0A0A2-B9E5-4433-87C0-68B6B72699C7 \ > + /dev/sdb > + > +Copy U-Boot to the SD card > + > +.. code-block:: bash > + > + sudo dd if=u-boot-spl.bin.normal.out of=/dev/sdb1 >
Re: [PATCH v1 0/7] Add Starfive JH7110 Cadence USB driver
Hi, there is a compile warning. I don't know why. On Sat, May 4, 2024 at 8:04 AM Minda Chen wrote: > > Add Starfive JH7110 Cadence USB driver and related PHY driver. > So the codes can be used in visionfive2 and milkv 7110 board. > > The driver is almost the same with kernel driver. > > patch1: Add set phy mode function in cdns3 core driver > which is used by Starfive. > > patch2-3: USB and PCIe 2.0 (usb 3.0) PHY drivier > patch4: Cadence USB wrapper driver. > patch5-7 dts, config and maintainers update. > > Minda Chen (7): > usb: cdns3: Set USB PHY mode in cdns3_probe() > phy: starfive: Add Starfive JH7110 USB 2.0 PHY driver > phy: starfive: Add Starfive JH7110 PCIe 2.0 PHY driver > usb: cdns: starfive: Add cdns USB driver > configs: starfive: Add visionfive2 cadence USB configuration > dts: starfive: Add JH7110 Cadence USB dts node > MAINTAINERS: Update Starfive visionfive2 maintain files. > > .../dts/jh7110-starfive-visionfive-2.dtsi | 5 + > arch/riscv/dts/jh7110.dtsi| 52 + > board/starfive/visionfive2/MAINTAINERS| 2 + > configs/starfive_visionfive2_defconfig| 9 + > drivers/phy/Kconfig | 1 + > drivers/phy/Makefile | 1 + > drivers/phy/starfive/Kconfig | 19 ++ > drivers/phy/starfive/Makefile | 7 + > drivers/phy/starfive/phy-jh7110-pcie.c| 211 ++ > drivers/phy/starfive/phy-jh7110-usb2.c| 135 +++ > drivers/usb/cdns3/Kconfig | 7 + > drivers/usb/cdns3/Makefile| 2 + > drivers/usb/cdns3/cdns3-starfive.c| 184 +++ > drivers/usb/cdns3/core.c | 17 ++ > 14 files changed, 652 insertions(+) > create mode 100644 drivers/phy/starfive/Kconfig > create mode 100644 drivers/phy/starfive/Makefile > create mode 100644 drivers/phy/starfive/phy-jh7110-pcie.c > create mode 100644 drivers/phy/starfive/phy-jh7110-usb2.c > create mode 100644 drivers/usb/cdns3/cdns3-starfive.c > > > base-commit: 174ac987655c888017c82df1883c0c2ea0dc2495 > -- > 2.17.1 > The compile warning as follows: In file included from /home/user/source/u-boot.git/drivers/usb/cdns3/gadget.c:70: /home/user/source/u-boot.git/include/linux/bitmap.h: In function ‘bitmap_find_next_zero_area’: /home/user/source/u-boot.git/include/linux/bitmap.h:170:17: warning: implicit declaration of function ‘find_next_zero_bit’; did you mean ‘find_next_bit’? [-Wimplicit-function-declaration] 170 | index = find_next_zero_bit(map, size, start); | ^~ | find_next_bit CC drivers/usb/cdns3/ep0.o In file included from /home/user/source/u-boot.git/include/linux/usb/composite.h:26, from /home/user/source/u-boot.git/drivers/usb/cdns3/ep0.c:19: /home/user/source/u-boot.git/include/linux/bitmap.h: In function ‘bitmap_find_next_zero_area’: /home/user/source/u-boot.git/include/linux/bitmap.h:170:17: warning: implicit declaration of function ‘find_next_zero_bit’; did you mean ‘find_next_bit’? [-Wimplicit-function-declaration] 170 | index = find_next_zero_bit(map, size, start); | ^~ | find_next_bit Is this something missing in the patch series? -E
Re: [PATCH 1/2 v2] board: starfive: support Pine64 Star64 board
P.S. some typo in my last message.
On Sun, May 12, 2024 at 4:57 PM E Shattow wrote:
>
> Off-list we reasoned further about the correct PHY values.
>
> As our reference was the pre-loaded SPI NOR content containing dtb data:
>
> ethernet@1603 {
> ...
> ethernet-phy@0 {
> rxc_dly_en = <0x01>;
> tx_delay_sel_fe = <0x05>;
> tx_delay_sel = <0x0a>;
> tx_inverted_10 = <0x01>;
> tx_inverted_100 = <0x01>;
> tx_inverted_1000 = <0x01>;
> phandle = <0x54>;
> };
> ...
> ethernet@1604 {
> ...
> ethernet-phy@1 {
> tx_delay_sel_fe = <0x05>;
> tx_delay_sel = <0x00>;
> rxc_dly_en = <0x00>;
> tx_inverted_10 = <0x01>;
> tx_inverted_100 = <0x01>;
> tx_inverted_1000 = <0x00>;
> phandle = <0x56>;
> };
>
>
> This is identical to ethernet-phy@0 and ethernet-phy@1 sections in
> Linux-riscv series 705018 "Add Ethernet driver for StarFive JH7110 SoC":
>
> https://patchwork.kernel.org/project/linux-riscv/patch/20221216070632.11444-10-yanhong.w...@starfivetech.com/
>
> Defaults are assumed to be from that version of the motorcomm driver.
> More recent versions
> of the driver deprecate setting the Fast Ethernet TX delay
> tx_delay_sel_fe and instead is this
> comment in the source code:
> "Generally, it is not necessary to adjust YT8531_RC1R_FE_TX_DELAY"
>
> On Wed, May 8, 2024 at 9:59 PM H Bell wrote:
> >
> > Similar to the Milk-V Mars, The Star64 board contains few differences to the
> > VisionFive 2 boards, so can be part of the same U-boot build.
> >
> > Signed-off-by: Henry Bell
> > Cc: ycli...@andestech.com
> > Cc: heinrich.schucha...@canonical.com
> > ---
> >
> > Changes since v1
> >
> > - Fix typos on naming
> > - Create pine64_star64 struct to be populated with PHY values once confirmed
> > ---
> > board/starfive/visionfive2/spl.c | 85 +++
> > .../visionfive2/starfive_visionfive2.c| 4 +
> > 2 files changed, 89 insertions(+)
> >
> > diff --git a/board/starfive/visionfive2/spl.c
> > b/board/starfive/visionfive2/spl.c
> > index ca61b5be22..248c6ba01d 100644
> > --- a/board/starfive/visionfive2/spl.c
> > +++ b/board/starfive/visionfive2/spl.c
> > @@ -86,6 +86,39 @@ static const struct starfive_vf2_pro starfive_verb[] = {
> > "tx-internal-delay-ps", "0"},
> > };
> >
> > +static const struct starfive_vf2_pro star64_pine64[] = {
> > + {"/soc/ethernet@1603", "starfive,tx-use-rgmii-clk", NULL},
> > + {"/soc/ethernet@1604", "starfive,tx-use-rgmii-clk", NULL},
> > +
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "motorcomm,tx-clk-adj-enabled", NULL},
>
> Add:
>{"/soc/ethernet@1603/mdio/ethernet-phy@0",
>"motorcomm,tx-clk-10-inverted", NULL},
>
>
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "motorcomm,tx-clk-100-inverted", NULL},
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "motorcomm,tx-clk-1000-inverted", NULL},
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "motorcomm,rx-clk-drv-microamp", "3970"},
>
> This rx-clk-drv-microamp value 3970 is suspect. Delete
> rx-clk-drv-microamp property or set default value 2910.
>
> From drivers/net/phy/motorcomm.c:
> YT8531_RGMII_RX_DS_DEFAULT 0x3
>
> Presumably this default is the same as in the version of the
> motorcomm driver that is included on pre-loaded SPI NOR content.
>
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "motorcomm,rx-data-drv-microamp", "2910"},
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "rx-internal-delay-ps", "1900"},
> > + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> > + "tx-internal-delay-ps", "1500"},
>
> N.b. tx-internal-delay-ps corresponds to tx_delay_sel (1000Mbit GE)
> and there is not
> any property corresponding to tx_delay_sel_fe (10/100Mbit FE). The
> driver does not
> implement tx_delay_sel_fe so no corresponding property on @0, else it would be
> included here.
>
> > +
> &
Re: [PATCH 1/2 v2] board: starfive: support Pine64 Star64 board
Off-list we reasoned further about the correct PHY values.
As our reference was the pre-loaded SPI NOR content containing dtb data:
ethernet@1603 {
...
ethernet-phy@0 {
rxc_dly_en = <0x01>;
tx_delay_sel_fe = <0x05>;
tx_delay_sel = <0x0a>;
tx_inverted_10 = <0x01>;
tx_inverted_100 = <0x01>;
tx_inverted_1000 = <0x01>;
phandle = <0x54>;
};
...
ethernet@1604 {
...
ethernet-phy@1 {
tx_delay_sel_fe = <0x05>;
tx_delay_sel = <0x00>;
rxc_dly_en = <0x00>;
tx_inverted_10 = <0x01>;
tx_inverted_100 = <0x01>;
tx_inverted_1000 = <0x00>;
phandle = <0x56>;
};
This is identical to ethernet-phy@0 and ethernet-phy@1 sections in
Linux-riscv series 705018 "Add Ethernet driver for StarFive JH7110 SoC":
https://patchwork.kernel.org/project/linux-riscv/patch/20221216070632.11444-10-yanhong.w...@starfivetech.com/
Defaults are assumed to be from that version of the motorcomm driver.
More recent versions
of the driver deprecate setting the Fast Ethernet TX delay
tx_delay_sel_fe and instead is this
comment in the source code:
"Generally, it is not necessary to adjust YT8531_RC1R_FE_TX_DELAY"
On Wed, May 8, 2024 at 9:59 PM H Bell wrote:
>
> Similar to the Milk-V Mars, The Star64 board contains few differences to the
> VisionFive 2 boards, so can be part of the same U-boot build.
>
> Signed-off-by: Henry Bell
> Cc: ycli...@andestech.com
> Cc: heinrich.schucha...@canonical.com
> ---
>
> Changes since v1
>
> - Fix typos on naming
> - Create pine64_star64 struct to be populated with PHY values once confirmed
> ---
> board/starfive/visionfive2/spl.c | 85 +++
> .../visionfive2/starfive_visionfive2.c| 4 +
> 2 files changed, 89 insertions(+)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index ca61b5be22..248c6ba01d 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -86,6 +86,39 @@ static const struct starfive_vf2_pro starfive_verb[] = {
> "tx-internal-delay-ps", "0"},
> };
>
> +static const struct starfive_vf2_pro star64_pine64[] = {
> + {"/soc/ethernet@1603", "starfive,tx-use-rgmii-clk", NULL},
> + {"/soc/ethernet@1604", "starfive,tx-use-rgmii-clk", NULL},
> +
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "motorcomm,tx-clk-adj-enabled", NULL},
Add:
{"/soc/ethernet@1603/mdio/ethernet-phy@0",
"motorcomm,tx-clk-10-inverted", NULL},
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "motorcomm,tx-clk-100-inverted", NULL},
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "motorcomm,tx-clk-1000-inverted", NULL},
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "motorcomm,rx-clk-drv-microamp", "3970"},
This rx-clk-drv-microamp value 3970 is suspect. Delete
rx-clk-drv-microamp property or set default value 2910.
>From drivers/net/phy/motorcomm.c:
YT8531_RGMII_RX_DS_DEFAULT 0x3
Presumably this default is the same as in the version of the
motorcomm driver that is included on pre-loaded SPI NOR content.
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "motorcomm,rx-data-drv-microamp", "2910"},
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "rx-internal-delay-ps", "1900"},
> + {"/soc/ethernet@1603/mdio/ethernet-phy@0",
> + "tx-internal-delay-ps", "1500"},
N.b. tx-internal-delay-ps corresponds to tx_delay_sel (1000Mbit GE)
and there is not
any property corresponding to tx_delay_sel_fe (10/100Mbit FE). The
driver does not
implement tx_delay_sel_fe so no corresponding property on @0, else it would be
included here.
> +
> + {"/soc/ethernet@1604/mdio/ethernet-phy@1",
> + "motorcomm,tx-clk-adj-enabled", NULL},
Add:
"motorcomm,tx-clk-10-inverted", NULL},
{"/soc/ethernet@1604/mdio/ethernet-phy@1",
> + { "/soc/ethernet@1604/mdio/ethernet-phy@1",
Nit: delete space between curly bracket and open-quote to match other lines.
> + "motorcomm,tx-clk-100-inverted", NULL},
> + {"/soc/ethernet@1604/mdio/ethernet-phy@1",
> + "motorcomm,rx-clk-drv-microamp", "3970"},
Again suspect 3970 value. Delete or set default 2910.
> + {"/soc/ethernet@1604/mdio/ethernet-phy@1",
> + "motorcomm,rx-data-drv-microamp", "2910"},
> + {"/soc/ethernet@1604/mdio/ethernet-phy@1",
> + "rx-internal-delay-ps", "0"},
> + {"/soc/ethernet@1604/mdio/ethernet-phy@1",
> + "tx-internal-delay-ps", "0"},
N.b. driver does not implement tx_delay_sel_fe so no corresponding
property on @1, else it would be included here.
> +};
> +
> void spl_fdt_fixup_mars(void *fdt)
> {
> static const char compat[] = "milkv,ma
Re: [PATCH 2/2 v2] board: starfive: support Pine64 Star64 board
On Wed, May 8, 2024 at 10:15 PM H Bell wrote: > > > Add documentation files > > Signed-off-by: Henry Bell > Cc: ycli...@andestech.com > Cc: heinrich.schucha...@canonical.com > --- > > Changes since v1 > > - New patch > --- > doc/board/starfive/index.rst | 1 + > doc/board/starfive/pine64_star64.rst | 109 +++ > 2 files changed, 110 insertions(+) > create mode 100644 doc/board/starfive/pine64_star64.rst > > diff --git a/doc/board/starfive/index.rst b/doc/board/starfive/index.rst > index 2762bf74c1..f05f8a0765 100644 > --- a/doc/board/starfive/index.rst > +++ b/doc/board/starfive/index.rst > @@ -7,4 +7,5 @@ StarFive > :maxdepth: 1 > > milk-v_mars.rst > + pine64_star64 > visionfive2 Ack. FYI series 406254 "board: starfive: add Milk-V Mars CM support" corrects the milk-v_mars.rst listing in index.rst > diff --git a/doc/board/starfive/pine64_star64.rst > b/doc/board/starfive/pine64_star64.rst > new file mode 100644 > index 00..047f109028 > --- /dev/null > +++ b/doc/board/starfive/pine64_star64.rst > @@ -0,0 +1,109 @@ > +.. SPDX-License-Identifier: GPL-2.0+ > + > +Pine64 Star64 > +=== > + > +U-Boot for the Star64 uses the same U-Boot binaries as the VisionFive 2 > board. > +In U-Boot SPL the actual board is detected and the device-tree patched > +accordingly. > + > +Building > + > + > +1. Add the RISC-V toolchain to your PATH. > +2. Setup ARCH & cross compilation environment variable: > + > +.. code-block:: none > + > + export CROSS_COMPILE= > + > +The M-mode software OpenSBI provides the supervisor binary interface (SBI) > and > +is responsible for the switch to S-Mode. It is a prerequisite to build > U-Boot. > +Support for the JH7110 was introduced in OpenSBI 1.2. It is recommended to > use > +a current release. > + > +.. code-block:: console > + > + git clone https://github.com/riscv/opensbi.git > + cd opensbi > + make PLATFORM=generic FW_TEXT_START=0x4000 FW_OPTIONS=0 > + Delete FW_OPTIONS. Keep FW_TEXT_START for compatibility. FW_OPTIONS=0 is no-op and FW_TEXT_START is deprecated as of OpenSBI commit d4d2582e (assume ~6mo release schedule 1.5 could be expected 6-7wks). > +Now build the U-Boot SPL and U-Boot proper. > + > +.. code-block:: console > + > + cd > + make starfive_visionfive2_defconfig > + make > OPENSBI=$(opensbi_dir)/build/platform/generic/firmware/fw_dynamic.bin > + > +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as > well > +as the FIT image (u-boot.itb) with OpenSBI and U-Boot. > + > +Device-tree selection > +~ > + > +U-Boot will set variable $fdtfile to starfive/jh7110-pine64-star64.dtb. > + > +To overrule this selection the variable can be set manually and saved in the > +environment > + > +:: > + > +setenv fdtfile my_device-tree.dtb > +env save > + > +or the configuration variable CONFIG_DEFAULT_FDT_FILE can be used to set to > +provide a default value. > + > +Boot source selection > +~ > + > +The board provides the DIP switches MSEL[1:0] to select the boot device out > of > +SPI flash, eMMC, SD-card, UART. To select booting from SD-card set the DIP > +switches MSEL[1:0] to 10. > + Seems unclear to me looking at a picture of the board and reading this description. >From the Pine64 Wiki page about Star64: "" Booting from uSD: Component S1804 is adjacent to the 40pin GPIO Bus; ignore the printed text on S1804 that says "ON" or "ONKE". Do pay attention to the '1' and '2' printed on S1804. Also pay attention to the 'L' and 'H' text on the board next to S1804. The 'L' stand for '0' and the 'H' stands for '1'. You will need to flip switch '1' (GPIO_1) on S1804 to the 'L' position and switch '2' (GPIO_0) on S1804 to the 'H' position. S1804 maps to the table next to S1804 that has text [ [GPIO_1 | GPIO_0], [0|0] Flash, [0|1] SD, [1|0] EMMC, [1|1] UART ]; "" So the table silkscreen on the board is correct, and in the table 0=L 1=H ? Also the JH7110 TRM from StarFive officially cautions that booting from SD is not reliable. > +Preparing the SD-Card > +~ > + > +The device firmware loads U-Boot SPL (u-boot-spl.bin.normal.out) from the > +partition with type GUID 2E54B353-1271-4842-806F-E436D6AF6985. You are free > +to choose any partition number. > + > +With the default configuration U-Boot SPL loads the U-Boot FIT image > +(u-boot.itb) from partition 2 > (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=0x2). > +When formatting it is recommended to use GUID > +BC13C2FF-59E6-4262-A352-B275FD6F7172 for this partition. > + > +The FIT image (u-boot.itb) is a combination of OpenSBI's fw_dynamic.bin, > +u-boot-nodtb.bin and the device tree blob. > + > +Format the SD card (make sure the disk has GPT, otherwise use gdisk to > switch) > + > +.. code-block:: bash > + > + sudo sgdisk --clear \ > + --set-alignment=2 \ > + --new=1:4096:8191 --change-name=1:spl > --ty
Re: [PATCH v4 5/5] starfive: add mac vendor sub-command
On comparing raw bytes from manufacturer data in EEPROM to having done
mac initialize and each of the commands incl. 'mac vendor' to
construct again that data, looks good.
On Thu, May 9, 2024 at 10:52 PM Heinrich Schuchardt
wrote:
>
> As boards from multiple vendors (Milk-V, StarFive, Pine64) use the mac
> command provide a sub-command to set the vendor string.
>
> Reported-by: E. Shattow
> Signed-off-by: Heinrich Schuchardt
> ---
> v4:
> no change
> v3:
> new patch
> ---
> .../visionfive2/visionfive2-i2c-eeprom.c | 25 ++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index 9648a270494..141d3db8667 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -404,6 +404,24 @@ static void set_product_id(char *string)
> update_crc();
> }
>
> +/**
> + * set_vendor() - set vendor name
> + *
> + * Takes a pointer to a string representing the vendor name, e.g.
> + * "StarFive Technology Co., Ltd.", stores it in the vendor field
> + * of the EEPROM local copy, and updates the CRC of the local copy.
> + */
> +static void set_vendor(char *string)
> +{
> + memset(pbuf.eeprom.atom1.data.vstr, 0,
> + sizeof(pbuf.eeprom.atom1.data.vstr));
> +
> + snprintf(pbuf.eeprom.atom1.data.vstr,
> +sizeof(pbuf.eeprom.atom1.data.vstr), string);
> +
> + update_crc();
> +}
> +
> const char *get_product_id_from_eeprom(void)
> {
> if (read_eeprom())
> @@ -463,6 +481,9 @@ int do_mac(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> } else if (!strcmp(cmd, "product_id")) {
> set_product_id(argv[2]);
> return 0;
> + } else if (!strcmp(cmd, "vendor")) {
> + set_vendor(argv[2]);
> + return 0;
> }
>
> return CMD_RET_USAGE;
> @@ -586,7 +607,9 @@ U_BOOT_LONGHELP(mac,
> "mac bom_revision \n"
> "- stores a StarFive BOM revision into the local EEPROM copy\n"
> "mac product_id \n"
> - "- stores a StarFive product ID into the local EEPROM copy\n");
> + "- stores a StarFive product ID into the local EEPROM copy\n"
> + "mac vendor \n"
> + "- set vendor string\n");
>
> U_BOOT_CMD(
> mac, 3, 1, do_mac,
> --
> 2.43.0
>
Tested-by: E Shattow
Re: [PATCH v4 2/5] board: add support for Milk-V Mars CM
Hi Shengyu,
On Fri, May 10, 2024 at 8:39 AM Shengyu Qu wrote:
>
> Sorry, seems this is a false warning as fdt_fixup_ethernet() would do this.
>
> Best regards
>
If you need to set a network mac address in U-Boot for an extra
network port (i.e. PCIe attached) this may be:
env set eth2addr aa:bb:cc:dd:ee:ff
env save
In this example ethaddr is gmac0 from JH7110 CPU via the RPi CM4
connector, and eth1addr is gmac1 from JH7110 CPU but is not accessible
on the RPi CM4 connector so disabled by the fdt fixup. The second
network port on DFRobot mini router is a realtek rtl8168 on the PCIe
bus, so it does not have any mac address. This is how I set its mac
address from U-Boot.
I'm not sure if this needs to be said in the documentation. It is
extra information that may be helpful but we are not documenting here
all the possible carrier boards that can be attached.
-E
> 在 2024/5/10 23:01, Shengyu Qu 写道:
> > Btw I didn't have a code path to pass the MAC address to kernel. So does
> > it actually exist?
> >
> > Best regards,
> > Shengyu
> >
> > 在 2024/5/10 13:52, Heinrich Schuchardt 写道:
> >> We already support the VisionFive 2 and the Milk-V Mars board by
> >> patching the VisionFive 2 device tree. With this patch the same
> >> is done for the Milk-V Mars CM.
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> Tested-by: E. Shattow
> >> Reviewed-by: E. Shattow
> >> ---
> >> v4:
> >> no change
> >> v3:
> >> no change
> >> v2:
> >> rename spl_fdt_fixup_marc() to spl_fdt_fixup_mars_cm()
> >> rename device-trees for Mars CM and Mars CM Lite
> >> change model and compatible properties
> >> ---
> >> board/starfive/visionfive2/spl.c | 28 ++-
> >> .../visionfive2/starfive_visionfive2.c| 11 +++-
> >> 2 files changed, 37 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/board/starfive/visionfive2/spl.c
> >> b/board/starfive/visionfive2/spl.c
> >> index ca61b5be227..b555189556a 100644
> >> --- a/board/starfive/visionfive2/spl.c
> >> +++ b/board/starfive/visionfive2/spl.c
> >> @@ -129,6 +129,30 @@ void spl_fdt_fixup_mars(void *fdt)
> >> }
> >> }
> >> +void spl_fdt_fixup_mars_cm(void *fdt)
> >> +{
> >> +const char *compat;
> >> +const char *model;
> >> +
> >> +spl_fdt_fixup_mars(fdt);
> >> +
> >> +if (!get_mmc_size_from_eeprom()) {
> >> +int offset;
> >> +
> >> +model = "Milk-V Mars CM Lite";
> >> +compat = "milkv,mars-cm-lite\0starfive,jh7110";
> >> +
> >> +offset = fdt_path_offset(fdt,
> >> "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> >> +/* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
> >> +fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> >> +} else {
> >> +model = "Milk-V Mars CM";
> >> +compat = "milkv,mars-cm\0starfive,jh7110";
> >> +}
> >> +fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> >> sizeof(compat));
> >> +fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> >> +}
> >> +
> >> void spl_fdt_fixup_version_a(void *fdt)
> >> {
> >> static const char compat[] =
> >> "starfive,visionfive-2-v1.2a\0starfive,jh7110";
> >> @@ -236,7 +260,9 @@ void spl_perform_fixups(struct spl_image_info
> >> *spl_image)
> >> pr_err("Can't read EEPROM\n");
> >> return;
> >> }
> >> -if (!strncmp(product_id, "MARS", 4)) {
> >> +if (!strncmp(product_id, "MARC", 4)) {
> >> +spl_fdt_fixup_mars_cm(spl_image->fdt_addr);
> >> +} else if (!strncmp(product_id, "MARS", 4)) {
> >> spl_fdt_fixup_mars(spl_image->fdt_addr);
> >> } else if (!strncmp(product_id, "VF7110", 6)) {
> >> version = get_pcb_revision_from_eeprom();
> >> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> >> b/board/starfive/visionfive2/starfive_visionfive2.c
> >> index a86bca533b2..6be53489626 100644
> >> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> >> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> >> @@ -19,6 +19
Re: [PATCH v3 3/5] doc: Milk-V Mars CM and Milk-V Mars CM Lite
Hi, A couple of nits I missed. Good for PR otherwise. On Thu, May 9, 2024 at 3:12 AM Heinrich Schuchardt wrote: > > Provide a man-page describing the usage of U-Boot on > the Milk-V Mars CM and Milk-V Mars CM Lite boards. > > Signed-off-by: Heinrich Schuchardt > --- > v3: > correct device-tree names > suggest not to use mac initialize > refer to XMODEM-1K > v2: > refer to tio as tool for booting via UART > describe how to update serial number > description updates as suggested by E. Shattow > --- > doc/board/starfive/index.rst | 1 + > doc/board/starfive/milk-v_mars_cm.rst | 193 ++ > 2 files changed, 194 insertions(+) > create mode 100644 doc/board/starfive/milk-v_mars_cm.rst > > diff --git a/doc/board/starfive/index.rst b/doc/board/starfive/index.rst > index 2762bf74c11..afa85ad2540 100644 > --- a/doc/board/starfive/index.rst > +++ b/doc/board/starfive/index.rst > @@ -7,4 +7,5 @@ StarFive > :maxdepth: 1 > > milk-v_mars.rst > + milk-v_mars_cm.rst > visionfive2 File extension is omitted for relative toctree references. See: https://documatt.com/restructuredtext-reference/element/toctree.html: "Relative document names are relative to the current document (containing toctree). Relative document names are those which not begin with /. For example, transition is transition.rst in the current folder, or collection/parts is collection/parts.rst, etc." 'milk-v_mars.rst' should also drop the filename extension. > diff --git a/doc/board/starfive/milk-v_mars_cm.rst > b/doc/board/starfive/milk-v_mars_cm.rst > new file mode 100644 > index 000..68561adadfc > --- /dev/null > +++ b/doc/board/starfive/milk-v_mars_cm.rst > @@ -0,0 +1,193 @@ > +.. SPDX-License-Identifier: GPL-2.0+ > + > +Milk-V Mars CM > +== > + > +U-Boot for the Milk-V Mars CM uses the same U-Boot binaries as the > VisionFive 2 > +board. In U-Boot SPL the actual board is detected and the device-tree patched > +accordingly. > + > +The Milk-V Mars CM Lite comes without eMMC and needs a different pin muxing > +than the Milk-V Mars CM. The availability and size of the eMMC shows up in > the > +serial number displayed by the *mac* command, e.g. > +MARC-V10-2340-D002E016-0304. The number after the E is the MMC size. > U-Boot > +takes a value of E000 as an indicator for the Lite version. Unfortunately the > +vendor has not set this value correctly on some Lite boards. > + > +Please, use CONFIG_STARFIVE_NO_EMMC=y if EEPROM data indicates eMMC is > present > +on the Milk-V Mars CM Lite. Otherwise you will not be able to read from the > +SD-card. > + > +The serial number can be corrected using the *mac* command: > + > +.. code-block:: > + > +mac read_eeprom > +mac product_id MARC-V10-2340-D002E000-0304 > +mac write_eeprom > + > +.. note:: > + > + The *mac initialize* command overwrites the vendor string and the MAC > + addresses. This is why it is avoided here. > + > +By default the EEPROM is write protected. The write protection may be > overcome > +by connecting the "GND" and "EN" test pads on top of the module. > + > +Building > + > + > +1. Add the RISC-V toolchain to your PATH. > +2. Setup ARCH & cross compilation environment variable: > + > +.. code-block:: none > + > + export CROSS_COMPILE= > + > +The M-mode software OpenSBI provides the supervisor binary interface (SBI) > and > +is responsible for the switch to S-Mode. It is a prerequisite to build > U-Boot. > +Support for the JH7110 was introduced in OpenSBI 1.2. It is recommended to > use > +a current release. > + > +.. code-block:: console > + > + git clone https://github.com/riscv/opensbi.git > + cd opensbi > + make PLATFORM=generic FW_TEXT_START=0x4000 > + > +(*FW_TEXT_START* is not needed anymore after OpenSBI patch d4d2582eef7a > +"firmware: remove FW_TEXT_START" which should appear in OpenSBI 1.5.) > + > +Now build the U-Boot SPL and U-Boot proper. > + > +.. code-block:: console > + > + cd > + make starfive_visionfive2_defconfig > + make > OPENSBI=$(opensbi_dir)/build/platform/generic/firmware/fw_dynamic.bin > + > +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as > well > +as the FIT image (u-boot.itb) with OpenSBI and U-Boot. > + > +Device-tree selection > +~ > + > +Depending on the board version U-Boot sets variable $fdtfile to either > +starfive/jh7110-milkv-mars-cm.dtb (with eMMC storage) or > +starfive/jh7110-milkv-mars-c
Re: [PATCH] board: starfive: support Pine64 Star64 board
On Pine64 Star64 8GB (May 2023 order date: "Star64 v1.1" silkscreen)
by contributor Tenkawa (contact info withheld so posting on their
behalf and listing myself in the tag). Other than the inconclusive
networking result (for discussion via code review) it seems to have
basic functionality.
# mac
Vendor : PINE64
Product full SN: STAR64V1-2310-D008E000-0005
data version: 0x2
PCB revision: 0xc1
BOM revision: A
Ethernet MAC0 address: 6c:cf:39:00:75:61
Ethernet MAC1 address: 6c:cf:39:00:75:62
# mmc info
Device: mmc@1601
Manufacturer ID: 15
OEM: 0
Name: BJTD4R
Bus Speed: 5200
Mode: MMC High Speed (52MHz)
Rd Block Len: 512
MMC version 5.1
High Capacity: Yes
Capacity: 29.1 GiB
Bus Width: 8-bit
Erase Group Size: 512 KiB
HC WP Group Size: 8 MiB
User Capacity: 29.1 GiB WRREL
Boot Capacity: 4 MiB ENH
RPMB Capacity: 4 MiB ENH
Boot area 0 is not write protected
Boot area 1 is not write protected
# mmc dev 1 ; mmc info
switch to partitions #0, OK
mmc1 is current device
Device: mmc@1602
Manufacturer ID: 1b
OEM: 534d
Name: GD2S5
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 119.4 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes
# # Silkscreen: Upper=A Port
# env erase;env load;env set bootcmd 'dhcp;net list'; env save; reset
ethernet@1603 Waiting for PHY auto negotiation to
complete. TIMEOUT !
phy_startup() failed: -110
FAILED: -110
ethernet@1604 Waiting for PHY auto negotiation to complete... done
BOOTP broadcast 1
BOOTP broadcast 2
DHCP client bound to address...
...
eth0 : ethernet@1603 6c:cf:39:00:75:61 active
eth1 : ethernet@1604 6c:cf:39:00:75:62
# # Silkscreen: Lower=B Port
# env erase;env load;env set bootcmd 'dhcp;net list'; env save; reset
ethernet@1603 Waiting for PHY auto negotiation to
complete. TIMEOUT !
phy_startup() failed: -110
FAILED: -110
ethernet@1604 Waiting for PHY auto negotiation to complete... done
BOOTP broadcast 1
BOOTP broadcast 2
DHCP client bound to address...
...
eth0 : ethernet@1603 6c:cf:39:00:75:61
eth1 : ethernet@1604 6c:cf:39:00:75:62 active
Some dropped network packets were observed with the test setup:
Approx 6% packet loss on Upper=A Port
Approx 71% packet loss on Lower=B Port.
The test network was not known to be reliable and might have
introduced other sources of error. No comparison was available to any
other firmware releases.
Tested-by: E Shattow
On Mon, May 6, 2024 at 8:30 AM H Bell wrote:
>
> Similar to the Milk-V Mars, The Star64 board contains few differences to the
> VisionFive 2 boards, so can be part of the same U-boot build.
>
> Signed-off-by: Henry Bell
> Cc: ycli...@andestech.com
> Cc: heinrich.schucha...@canonical.com
> ---
> board/starfive/visionfive2/spl.c | 52 +++
> .../visionfive2/starfive_visionfive2.c| 4 ++
> 2 files changed, 56 insertions(+)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index ca61b5be22..b8d29a02af 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -226,6 +226,56 @@ void spl_fdt_fixup_version_b(void *fdt)
> }
> }
>
> +void spl_fdt_fixup_star64(void *fdt)
> +{
> + static const char compat[] = "starfive,star64\0starfive,jh7110";
> + u32 phandle;
> + u8 i;
> + int offset;
> + int ret;
> +
> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> sizeof(compat));
> + fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> + "Pine64 Star64");
> +
> + /* gmac0 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1603");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_AONCLK_GMAC0_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_AONCLK_GMAC0_RMII_RTX);
> +
> + /* gmac1 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1302");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1604");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_SYSCLK_GMAC1_TX);
> + fdt_setprop_u32(fdt, off
Re: [PATCH] board: starfive: support Pine64 Star64 board
Hi,
On Mon, May 6, 2024 at 8:30 AM H Bell wrote:
>
> Similar to the Milk-V Mars, The Star64 board contains few differences to the
> VisionFive 2 boards, so can be part of the same U-boot build.
>
> Signed-off-by: Henry Bell
> Cc: ycli...@andestech.com
> Cc: heinrich.schucha...@canonical.com
> ---
> board/starfive/visionfive2/spl.c | 52 +++
> .../visionfive2/starfive_visionfive2.c| 4 ++
> 2 files changed, 56 insertions(+)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index ca61b5be22..b8d29a02af 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -226,6 +226,56 @@ void spl_fdt_fixup_version_b(void *fdt)
> }
> }
>
> +void spl_fdt_fixup_star64(void *fdt)
> +{
> + static const char compat[] = "starfive,star64\0starfive,jh7110";
Should be "pine64,star64\nstarfive,jh7110" ?
> + u32 phandle;
> + u8 i;
> + int offset;
> + int ret;
> +
> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> sizeof(compat));
> + fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> + "Pine64 Star64");
> +
> + /* gmac0 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1700");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1603");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_AONCLK_GMAC0_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_AONCLK_GMAC0_RMII_RTX);
> +
> + /* gmac1 */
> + offset = fdt_path_offset(fdt, "/soc/clock-controller@1302");
> + phandle = fdt_get_phandle(fdt, offset);
> + offset = fdt_path_offset(fdt, "/soc/ethernet@1604");
> +
> + fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clocks",
> JH7110_SYSCLK_GMAC1_TX);
> + fdt_setprop_u32(fdt, offset, "assigned-clock-parents", phandle);
> + fdt_appendprop_u32(fdt, offset, "assigned-clock-parents",
> + JH7110_SYSCLK_GMAC1_RMII_RTX);
> +
> + for (i = 0; i < ARRAY_SIZE(starfive_verb); i++) {
> + offset = fdt_path_offset(fdt, starfive_verb[i].path);
> +
> + if (starfive_verb[i].value)
> + ret = fdt_setprop_u32(fdt, offset,
> starfive_verb[i].name,
> + dectoul(starfive_verb[i].value,
> NULL));
Using the fishwaldo repo Star64_devel branch Linux sources as my reference
https://github.com/Fishwaldo/Star64_linux
the drive strength is 3600 and looking at StarFive repo:
https://github.com/starfive-tech/u-boot/blob/223ac8b1e907924d3891b3be1b2f6620b56bff31/arch/riscv/dts/starfive_visionfive2.dts#L323
from "rgmii_sw_dr_rxc = <0x6>;" (7th enum is 3600). However the dev
dts still refers to 0x7 ( i.e. " motorcomm,rx-clk-drv-microamp =
<3970>;")
https://github.com/starfive-tech/u-boot/blob/223ac8b1e907924d3891b3be1b2f6620b56bff31/arch/riscv/dts/starfive_devkits.dts#L289
Also the delay inverts differ somewhat. Marked-up
arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dtsi from the above
repo:
&gmac0 {
status = "okay";
#address-cells = <1>;
#size-cells = <0>;
phy0: ethernet-phy@0 {
rgmii_sw_dr_2 = <0x0>; switch drive 1.8V operation
rgmii_sw_dr = <0x3>; switch drive 4th enum is 2910
rgmii_sw_dr_rxc = <0x6>; switch clock drive rx current
7th enum is 3600
rxc_dly_en = <0>; 0 (disabled) 1900ps rxc
clock additional delay added to configured rx delay (default is
disabled). Default tx/rx delay value is 1950ps.
rx_delay_sel = <0xa>;1500 at 1000Mbps
tx_delay_sel_fe = <5>;750 at 10/100Mbps
tx_delay_sel = <0xa>;1500 at 1000Mbps
tx_inverted_10 = <0x1>; true motorcomm,tx-clk-10-inverted
tx_inverted_100 = <0x1>; true motorcomm,tx-clk-100-inverted
tx_inverted_1000 = <0x1>; true motorcomm,tx-clk-1000-inverted
};
};
&gmac1 {
#address-cells = <1>;
#size-cells = <0>;
status = "okay";
phy1: ethernet-phy@1 {
rgmii_sw_dr_2 = <0x0>;switch drive 2 1.8V operation
rgmii_sw_dr = <0x3>; switch drive 4th enum is 2910
rgmii_sw_dr_rxc = <0x6>; switch clock drive rx
current 7th enum is 3600
rxc_dly_en = <0>; 0 (disabled) 1900ps rxc
clock additional delay added to configured rx delay (default is
disabled). Default tx/rx delay value is 1950ps.
rx_delay_sel = <0x2>;300 at 10
Re: [PATCH 73/81] usb: Remove and add needed includes
On Wed, May 1, 2024 at 7:00 PM Tom Rini wrote: > > Remove from this driver directory and when needed > add missing include files directly. > > Signed-off-by: Tom Rini > --- > Cc: Marek Vasut > Cc: Tom Rini > Cc: Neil Armstrong > Cc: Lukasz Majewski > Cc: Mattijs Korpershoek > Cc: Eddie Cai > Cc: Patrice Chotard > Cc: Caleb Connolly > Cc: Sumit Garg > Cc: Michal Simek > Cc: Bin Meng > Cc: Ryder Lee > Cc: Weijie Gao > Cc: Chunfeng Yun > Cc: GSS_MTK_Uboot_upstream > Cc: Nobuhiro Iwamatsu > Cc: Stephan Gerhold > Cc: Linus Walleij > Cc: Simon Glass > Cc: Philipp Tomsich > Cc: Kever Yang > Cc: Nishanth Menon > Cc: Igor Prusov > Cc: Roger Quadros > Cc: Svyatoslav Ryhel > Cc: Jonas Karlman > Cc: Jagan Teki > Cc: Venkatesh Yadav Abbarapu > Cc: Peter Korsgaard > Cc: Oleksandr Suvorov > Cc: Alexey Romanov > Cc: Sean Anderson > Cc: Miquel Raynal > Cc: Teik Heng Chong > Cc: Tim Harvey > Cc: Mathieu Othacehe > Cc: Fabio Estevam > Cc: Johan Jonker > Cc: Xavier Drudis Ferran > Cc: Fabrice Gasnier > Cc: Patrick Delaunay > Cc: Sam Edwards > Cc: Andre Przywara > --- > drivers/usb/cdns3/cdns3-ti.c | 1 - > drivers/usb/cdns3/core.c | 1 - > drivers/usb/common/common.c| 1 - > drivers/usb/common/fsl-dt-fixup.c | 1 - > drivers/usb/common/fsl-errata.c| 1 - > drivers/usb/dwc3/core.c| 1 - > drivers/usb/dwc3/dwc3-generic.c| 1 - > drivers/usb/dwc3/dwc3-layerscape.c | 1 - > drivers/usb/dwc3/dwc3-meson-g12a.c | 1 - > drivers/usb/dwc3/dwc3-meson-gxl.c | 1 - > drivers/usb/dwc3/dwc3-omap.c | 1 - > drivers/usb/dwc3/ep0.c | 1 - > drivers/usb/dwc3/gadget.c | 1 - > drivers/usb/dwc3/samsung_usb_phy.c | 2 +- > drivers/usb/dwc3/ti_usb_phy.c | 1 - > drivers/usb/emul/sandbox_flash.c | 1 - > drivers/usb/emul/sandbox_hub.c | 1 - > drivers/usb/emul/sandbox_keyb.c| 1 - > drivers/usb/emul/usb-emul-uclass.c | 1 - > drivers/usb/eth/asix.c | 1 - > drivers/usb/eth/asix88179.c| 1 - > drivers/usb/eth/mcs7830.c | 1 - > drivers/usb/eth/r8152.c| 1 - > drivers/usb/eth/r8152_fw.c | 1 - > drivers/usb/eth/smsc95xx.c | 1 - > drivers/usb/eth/usb_ether.c| 1 - > drivers/usb/gadget/at91_udc.c | 1 - > drivers/usb/gadget/atmel_usba_udc.c| 1 - > drivers/usb/gadget/bcm_udc_otg_phy.c | 1 - > drivers/usb/gadget/ci_udc.c| 1 - > drivers/usb/gadget/config.c| 1 - > drivers/usb/gadget/dwc2_udc_otg.c | 1 - > drivers/usb/gadget/dwc2_udc_otg_phy.c | 1 - > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 1 - > drivers/usb/gadget/ep0.c | 1 - > drivers/usb/gadget/epautoconf.c| 1 - > drivers/usb/gadget/ether.c | 1 - > drivers/usb/gadget/f_acm.c | 1 - > drivers/usb/gadget/f_dfu.c | 1 - > drivers/usb/gadget/f_fastboot.c| 1 - > drivers/usb/gadget/f_mass_storage.c| 1 - > drivers/usb/gadget/f_rockusb.c | 1 - > drivers/usb/gadget/f_sdp.c | 1 - > drivers/usb/gadget/f_thor.c| 1 - > drivers/usb/gadget/g_dnl.c | 1 - > drivers/usb/gadget/max3420_udc.c | 1 - > drivers/usb/gadget/rndis.c | 1 - > drivers/usb/gadget/udc/udc-core.c | 1 - > drivers/usb/gadget/udc/udc-uclass.c| 1 - > drivers/usb/gadget/usbstring.c | 1 - > drivers/usb/host/dwc2.c| 1 - > drivers/usb/host/dwc3-of-simple.c | 1 - > drivers/usb/host/dwc3-sti-glue.c | 1 - > drivers/usb/host/ehci-atmel.c | 1 - > drivers/usb/host/ehci-exynos.c | 1 - > drivers/usb/host/ehci-fsl.c| 1 - > drivers/usb/host/ehci-generic.c| 1 - > drivers/usb/host/ehci-hcd.c| 1 - > drivers/usb/host/ehci-marvell.c| 1 - > drivers/usb/host/ehci-msm.c| 1 - > drivers/usb/host/ehci-mx5.c| 1 - > drivers/usb/host/ehci-mx6.c| 1 - > drivers/usb/host/ehci-mxs.c| 1 - > drivers/usb/host/ehci-npcm.c | 1 - > drivers/usb/host/ehci-omap.c | 2 +- > drivers/usb/host/ehci-pci.c| 1 - > drivers/usb/host/ehci-tegra.c | 1 - > drivers/usb/host/ehci-vf.c | 1 - > drivers/usb/host/ehci-zynq.c | 1 - > drivers/usb/host/ohci-at91.c | 1 - > drivers/usb/host/ohci-da8xx.c | 1 - > drivers/usb/host/ohci-generic.c| 1 - > drivers/usb/host/ohci-hcd.c| 2 +- > drivers/usb/host/ohci-lpc32xx.c| 1 - > drivers/usb/host/ohci-npcm.c
Re: [PATCH v2 4/4] configs: visionfive2: enable SPL_YMODEM_SUPPORT
On Fri, May 3, 2024 at 2:01 AM Heinrich Schuchardt wrote: > > We can use U-Boot for recovering JH7110 based boards via UART > if CONFIG_SPL_YMODEM_SUPPORT=y. > > * Send u-boot-spl.normal.out via XMODEM. > * Send u-boot.itb via YMODEM. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > configs/starfive_visionfive2_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/starfive_visionfive2_defconfig > b/configs/starfive_visionfive2_defconfig > index 3bbd1dbd67c..174ac24dc74 100644 > --- a/configs/starfive_visionfive2_defconfig > +++ b/configs/starfive_visionfive2_defconfig > @@ -62,6 +62,7 @@ CONFIG_SPL_I2C=y > CONFIG_SPL_DM_SPI_FLASH=y > CONFIG_SPL_DM_RESET=y > CONFIG_SPL_SPI_LOAD=y > +CONFIG_SPL_YMODEM_SUPPORT=y > CONFIG_SYS_PROMPT="StarFive # " > CONFIG_CMD_EEPROM=y > CONFIG_SYS_EEPROM_SIZE=512 > -- > 2.43.0 > Reviewed-by: E. Shattow
Re: [PATCH v2 2/4] board: add support for Milk-V Mars CM
On Fri, May 3, 2024 at 2:00 AM Heinrich Schuchardt
wrote:
>
> We already support the VisionFive 2 and the Milk-V Mars board by
> patching the VisionFive 2 device tree. With this patch the same
> is done for the Milk-V Mars CM.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> rename spl_fdt_fixup_marc() to spl_fdt_fixup_mars_cm()
> rename device-trees for Mars CM and Mars CM Lite
> change model and compatible properties
> ---
> board/starfive/visionfive2/spl.c | 28 ++-
> .../visionfive2/starfive_visionfive2.c| 11 +++-
> 2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index ca61b5be227..b555189556a 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -129,6 +129,30 @@ void spl_fdt_fixup_mars(void *fdt)
> }
> }
>
> +void spl_fdt_fixup_mars_cm(void *fdt)
> +{
> + const char *compat;
> + const char *model;
> +
> + spl_fdt_fixup_mars(fdt);
> +
> + if (!get_mmc_size_from_eeprom()) {
> + int offset;
> +
> + model = "Milk-V Mars CM Lite";
> + compat = "milkv,mars-cm-lite\0starfive,jh7110";
> +
> + offset = fdt_path_offset(fdt,
> "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> + /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
> + fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> + } else {
> + model = "Milk-V Mars CM";
> + compat = "milkv,mars-cm\0starfive,jh7110";
> + }
> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> sizeof(compat));
> + fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> +}
> +
> void spl_fdt_fixup_version_a(void *fdt)
> {
> static const char compat[] =
> "starfive,visionfive-2-v1.2a\0starfive,jh7110";
> @@ -236,7 +260,9 @@ void spl_perform_fixups(struct spl_image_info *spl_image)
> pr_err("Can't read EEPROM\n");
> return;
> }
> - if (!strncmp(product_id, "MARS", 4)) {
> + if (!strncmp(product_id, "MARC", 4)) {
> + spl_fdt_fixup_mars_cm(spl_image->fdt_addr);
> + } else if (!strncmp(product_id, "MARS", 4)) {
> spl_fdt_fixup_mars(spl_image->fdt_addr);
> } else if (!strncmp(product_id, "VF7110", 6)) {
> version = get_pcb_revision_from_eeprom();
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> b/board/starfive/visionfive2/starfive_visionfive2.c
> index a86bca533b2..6be53489626 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -19,6 +19,10 @@ DECLARE_GLOBAL_DATA_PTR;
> #define JH7110_L2_PREFETCHER_HART_OFFSET 0x2000
> #define FDTFILE_MILK_V_MARS \
> "starfive/jh7110-milkv-mars.dtb"
> +#define FDTFILE_MILK_V_MARS_CM \
> + "starfive/jh7110-milkv-mars-cm.dtb"
> +#define FDTFILE_MILK_V_MARS_CM_LITE \
> + "starfive/jh7110-milkv-mars-cm-lite.dtb"
> #define FDTFILE_VISIONFIVE2_1_2A \
> "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb"
> #define FDTFILE_VISIONFIVE2_1_3B \
> @@ -61,7 +65,12 @@ static void set_fdtfile(void)
> log_err("Can't read EEPROM\n");
> return;
> }
> - if (!strncmp(product_id, "MARS", 4)) {
> + if (!strncmp(product_id, "MARC", 4)) {
> + if (get_mmc_size_from_eeprom())
> + fdtfile = FDTFILE_MILK_V_MARS_CM;
> + else
> + fdtfile = FDTFILE_MILK_V_MARS_CM_LITE;
> + } else if (!strncmp(product_id, "MARS", 4)) {
> fdtfile = FDTFILE_MILK_V_MARS;
> } else if (!strncmp(product_id, "VF7110", 6)) {
> version = get_pcb_revision_from_eeprom();
> --
> 2.43.0
>
on Mars CM Lite and DFRobot mini router carrier
Tested-by: E. Shattow
Reviewed-by: E. Shattow
Re: [PATCH v2 3/4] doc: Milk-V Mars CM and Milk-V Mars CM Lite
Hi, looking good to me except $fdtfile filename changes were missed to match the code, and then testing inspires a few more things to suggest. On Fri, May 3, 2024 at 2:01 AM Heinrich Schuchardt wrote: > > Provide a man-page describing the usage of U-Boot on > the Milk-V Mars CM and Milk-V Mars CM Lite boards. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > refer to tio as tool for booting via UART > describe how to update serial number > description updates as suggested by E. Shattow > --- > doc/board/starfive/index.rst | 1 + > doc/board/starfive/milk-v_mars_cm.rst | 183 ++ > 2 files changed, 184 insertions(+) > create mode 100644 doc/board/starfive/milk-v_mars_cm.rst > > diff --git a/doc/board/starfive/index.rst b/doc/board/starfive/index.rst > index 2762bf74c11..afa85ad2540 100644 > --- a/doc/board/starfive/index.rst > +++ b/doc/board/starfive/index.rst > @@ -7,4 +7,5 @@ StarFive > :maxdepth: 1 > > milk-v_mars.rst > + milk-v_mars_cm.rst > visionfive2 > diff --git a/doc/board/starfive/milk-v_mars_cm.rst > b/doc/board/starfive/milk-v_mars_cm.rst > new file mode 100644 > index 000..d2be064d94c > --- /dev/null > +++ b/doc/board/starfive/milk-v_mars_cm.rst > @@ -0,0 +1,183 @@ > +.. SPDX-License-Identifier: GPL-2.0+ > + > +Milk-V Mars CM > +== > + > +U-Boot for the Milk-V Mars CM uses the same U-Boot binaries as the > VisionFive 2 > +board. In U-Boot SPL the actual board is detected and the device-tree patched > +accordingly. > + > +The Milk-V Mars CM Lite comes without eMMC and needs a different pin muxing > +than the Milk-V Mars CM. The availability and size of the eMMC shows up in > the > +serial number displayed by the *mac* command, e.g. > +MARC-V10-2340-D002E016-0304. The number after the E is the MMC size. > U-Boot > +takes a value of E000 as an indicator for the Lite version. Unfortunately the > +vendor has not set this value correctly on some Lite boards. > + > +Please, use CONFIG_STARFIVE_NO_EMMC=y if EEPROM data indicates eMMC is > present > +on the Milk-V Mars CM Lite. Otherwise you will not be able to read from the > +SD-card. > + > +The serial number can be corrected using the *mac* command: > + > +.. code-block:: > + > +mac read_eeprom > +mac product_id MARC-V10-2340-D002E000-0304 > +mac write_eeprom > + > +By default the EEPROM is write protected. The write protection may be > overcome > +by connecting the "GND" and "EN" test pads on top of the module. With adding boards based on VisionFive2 and having different vendor info in EEPROM, the mac command should be extended for setting vendor info from user input. Vendor info is currently not changeable back to "MILK-V" after *mac initialize* sets vendor as "StarFive Technology Co., Ltd." If mac command can be extended then the documentation here is sufficient, else some description warning the user not to *mac initialize* on platforms other than VisionFive2 from StarFive. > + > +Building > + > + > +1. Add the RISC-V toolchain to your PATH. > +2. Setup ARCH & cross compilation environment variable: > + > +.. code-block:: none > + > + export CROSS_COMPILE= > + > +The M-mode software OpenSBI provides the supervisor binary interface (SBI) > and > +is responsible for the switch to S-Mode. It is a prerequisite to build > U-Boot. > +Support for the JH7110 was introduced in OpenSBI 1.2. It is recommended to > use > +a current release. > + > +.. code-block:: console > + > + git clone https://github.com/riscv/opensbi.git > + cd opensbi > + make PLATFORM=generic FW_TEXT_START=0x4000 > + > +(*FW_TEXT_START* is not needed anymore after OpenSBI patch d4d2582eef7a > +"firmware: remove FW_TEXT_START" which should appear in OpenSBI 1.5.) > + > +Now build the U-Boot SPL and U-Boot proper. > + > +.. code-block:: console > + > + cd > + make starfive_visionfive2_defconfig > + make > OPENSBI=$(opensbi_dir)/build/platform/generic/firmware/fw_dynamic.bin > + > +This will generate the U-Boot SPL image (spl/u-boot-spl.bin.normal.out) as > well > +as the FIT image (u-boot.itb) with OpenSBI and U-Boot. > + > +Device-tree selection > +~ > + > +Depending on the board version U-Boot sets variable $fdtfile to either > +starfive/jh7110-milkv-mars-cm-emmc.dtb (for the generic version or > +starfive/jh7110-milkv-mars-cm-sdcard.dtb (for the Lite version). "Depending on the board version U-Boot sets variable $fdtfile to either starfive/jh7110-milkv-mars-cm.dtb (with eMMC sto
Re: [PATCH v2 1/4] board: starfive: function to read eMMC size
On Fri, May 3, 2024 at 2:01 AM Heinrich Schuchardt
wrote:
>
> The EEPROM provides information about the size of the eMMC.
> Provide a new function get_mmc_size_from_eeprom() to read it.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> fix typos in get_mmc_size_from_eeprom() description
> ---
> arch/riscv/include/asm/arch-jh7110/eeprom.h| 7 +++
> board/starfive/visionfive2/Kconfig | 9 +
> .../visionfive2/visionfive2-i2c-eeprom.c | 18 ++
> 3 files changed, 34 insertions(+)
>
> diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> index 62d184aeb57..45ad2a5f7bc 100644
> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> @@ -12,6 +12,13 @@
> u8 get_pcb_revision_from_eeprom(void);
> u32 get_ddr_size_from_eeprom(void);
>
> +/**
> + * get_mmc_size_from_eeprom() - read eMMC size from EEPROM
> + *
> + * @return: size in GiB or 0 on error.
> + */
> +u32 get_mmc_size_from_eeprom(void);
> +
> /**
> * get_product_id_from_eeprom - get product ID string
> *
> diff --git a/board/starfive/visionfive2/Kconfig
> b/board/starfive/visionfive2/Kconfig
> index 2186a939646..d7e8a7a7d78 100644
> --- a/board/starfive/visionfive2/Kconfig
> +++ b/board/starfive/visionfive2/Kconfig
> @@ -50,4 +50,13 @@ config BOARD_SPECIFIC_OPTIONS # dummy
> imply PHY_LIB
> imply PHY_MSCC
>
> +config STARFIVE_NO_EMMC
> + bool "Report eMMC size as zero"
> + help
> + The serial number string in the EEPROM is meant to report the
> + size of onboard eMMC. Unfortunately some Milk-V Mars CM Lite
> + modules without eMMC show a non-zero size here.
> +
> + Set to 'Y' if you have a Mars CM Lite module.
> +
> endif
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index 5095a0e9fdb..9648a270494 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -548,6 +548,24 @@ u32 get_ddr_size_from_eeprom(void)
> return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
> }
>
> +u32 get_mmc_size_from_eeprom(void)
> +{
> + u32 size;
> +
> + if (IS_ENABLED(CONFIG_STARFIVE_NO_EMMC))
> + return 0;
> +
> + if (read_eeprom())
> + return 0;
> +
> + size = dectoul(&pbuf.eeprom.atom1.data.pstr[19], NULL);
> +
> + if (pbuf.eeprom.atom1.data.pstr[21] == 'T')
> + size <<= 10;
> +
> + return size;
> +}
> +
> U_BOOT_LONGHELP(mac,
> "\n"
> "- display EEPROM content\n"
> --
> 2.43.0
>
Reviewed-by: E. Shattow
Re: [PATCH 2/4] board: add support for Milk-V Mars CM
On Tue, Apr 30, 2024 at 12:18 AM Heinrich Schuchardt
wrote:
>
> On 30.04.24 00:46, E Shattow wrote:
> > On Sun, Apr 28, 2024 at 9:13 AM Emil Renner Berthing
> > wrote:
> >>
> >> Heinrich Schuchardt wrote:
> >>> We already support the VisionFive 2 and the Milk-V Mars board by
> >>> patching the VisionFive 2 device tree. With this patch the same
> >>> is done for the Milk-V Mars CM.
> >>
> >> Hi Heinrich.
> >>
> >> Thanks for the patch. As far as I can tell the Milk-V documentation[1] is
> >> pretty consistent in calling the version without eMMC "Milk-V Mars CM Lite"
> >> and the version with eMMC just "Milk-V Mars CM". So I'd prefer the model,
> >> compatible and filenames suggested below.
> >>
> >> [1]:
> >> https://milkv.io/docs/mars/compute-module/introduction#design-philosophy
> >>
> >
> > Are there any actual differences that need representation in the dtb
> > file for downstream OS different from Milk-V Mars to Milk-V Mars CM
> > (or CM Lite)?
> >
> > I did find this vendor repo commit "kernel: dts reconfig sdio0 for
> > SDCard version"
> > https://github.com/milkv-mars/mars-buildroot-sdk/commit/042ea06598995db99dd252ee439c42b9c1a9
> > but it does not seem necessary in mainline Linux, SD Card is working
> > without changes.
> >
> > It was necessary for U-Boot and the Mars CM Lite with SD Card to apply
> > s/GPIO62/GPIO22/ as in the vendor commit "u-boot: configure sdio0 as
> > mars-cm sdcard version"
> > https://github.com/milkv-mars/mars-buildroot-sdk/commit/880a249518f72ecf1e2947dfeb2c66e5035fce90
>
> This is what is patched in fdt_fixup_marc().
That's the Mars fixup with a conditional for the s/GPIO62/GPIO22/ of
CM Lite; which Linux seems not to mind either way so long as this was
done at the U-Boot phase, it is functional for eMMC/SD mmc access with
unmodified visionfive2 1.3b dtb (GPIO62 pinmux) from Linux.
It is not clear to me why in vendor U-Boot this is s/GPIO62/GPIO22/
but in vendor Linux kernel this is s/GPIO62/GPIO24/ ? It seems not
needed (in Linux).
>
> >
> > Then also there is some mention about PMIC and renamed i2c reference,
> > already obsolete. That's all, is there more to it? Possibly a dtb for
> > Mars is enough to also be used on Mars CM and Mars Lite?
>
> Looking at the schematics the biggest difference between the Mars and
> the Mars CM (Lite) is on the USB side. The Mars board has USB 3.0 via a
> PCIe lane and a VIA VL805/806 while the Mars CM has USB directly via the
> SoC.
>
> To add USB support for the Mars CM we will need an adapted device-tree.
Mars also needs this direct-to-SoC USB support, as its USB ports are a
mix of VL805 and directly via the SoC. This can be the same for Mars
and Mars CM/Lite.
See this photo from
https://milkv.io/docs/mars/getting-started/bootloader what highlights
this direct SoC USB port of Milk-V Mars:
https://milkv.io/assets/images/mars-usb-port-a-b48fe1ff1003539d42bf5e1dde1725a3.jpg
The over-current errata
https://doc-en.rvspace.org/VisionFive2/DG_USB/JH7110_SDK/usb_overcurrent_debug.html
suggests "please modify the above settings during the U-Boot phase".
-E
>
> Best regards
>
> Heinrich
> >
> >>> Signed-off-by: Heinrich Schuchardt
> >>> ---
> >>> board/starfive/visionfive2/spl.c | 27 ++-
> >>> .../visionfive2/starfive_visionfive2.c| 11 +++-
> >>> 2 files changed, 36 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/board/starfive/visionfive2/spl.c
> >>> b/board/starfive/visionfive2/spl.c
> >>> index 45848db6d8b..bb0f28d7aad 100644
> >>> --- a/board/starfive/visionfive2/spl.c
> >>> +++ b/board/starfive/visionfive2/spl.c
> >>> @@ -129,6 +129,29 @@ void spl_fdt_fixup_mars(void *fdt)
> >>>}
> >>> }
> >>>
> >>> +void spl_fdt_fixup_marc(void *fdt)
> >>> +{
> >>> + const char *compat;
> >>> + const char *model;
> >>> +
> >>> + spl_fdt_fixup_mars(fdt);
> >>> +
> >>> + if (!get_mmc_size_from_eeprom()) {
> >>> + int offset;
> >>> +
> >>> + model = "Milk-V Mars CM SDCard";
> >>
> >> "Milk-V Mars CM Lite"
> >>
> >>> + compat = "milkv,mars-cm-sdcard\0starfive,jh7110";
> >>
> >> "milkv,mars-cm-lite\0starfive,jh7110"
> >>
Re: [PATCH 2/4] board: add support for Milk-V Mars CM
On Sun, Apr 28, 2024 at 9:13 AM Emil Renner Berthing
wrote:
>
> Heinrich Schuchardt wrote:
> > We already support the VisionFive 2 and the Milk-V Mars board by
> > patching the VisionFive 2 device tree. With this patch the same
> > is done for the Milk-V Mars CM.
>
> Hi Heinrich.
>
> Thanks for the patch. As far as I can tell the Milk-V documentation[1] is
> pretty consistent in calling the version without eMMC "Milk-V Mars CM Lite"
> and the version with eMMC just "Milk-V Mars CM". So I'd prefer the model,
> compatible and filenames suggested below.
>
> [1]: https://milkv.io/docs/mars/compute-module/introduction#design-philosophy
>
Are there any actual differences that need representation in the dtb
file for downstream OS different from Milk-V Mars to Milk-V Mars CM
(or CM Lite)?
I did find this vendor repo commit "kernel: dts reconfig sdio0 for
SDCard version"
https://github.com/milkv-mars/mars-buildroot-sdk/commit/042ea06598995db99dd252ee439c42b9c1a9
but it does not seem necessary in mainline Linux, SD Card is working
without changes.
It was necessary for U-Boot and the Mars CM Lite with SD Card to apply
s/GPIO62/GPIO22/ as in the vendor commit "u-boot: configure sdio0 as
mars-cm sdcard version"
https://github.com/milkv-mars/mars-buildroot-sdk/commit/880a249518f72ecf1e2947dfeb2c66e5035fce90
Then also there is some mention about PMIC and renamed i2c reference,
already obsolete. That's all, is there more to it? Possibly a dtb for
Mars is enough to also be used on Mars CM and Mars Lite?
-E
> > Signed-off-by: Heinrich Schuchardt
> > ---
> > board/starfive/visionfive2/spl.c | 27 ++-
> > .../visionfive2/starfive_visionfive2.c| 11 +++-
> > 2 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/board/starfive/visionfive2/spl.c
> > b/board/starfive/visionfive2/spl.c
> > index 45848db6d8b..bb0f28d7aad 100644
> > --- a/board/starfive/visionfive2/spl.c
> > +++ b/board/starfive/visionfive2/spl.c
> > @@ -129,6 +129,29 @@ void spl_fdt_fixup_mars(void *fdt)
> > }
> > }
> >
> > +void spl_fdt_fixup_marc(void *fdt)
> > +{
> > + const char *compat;
> > + const char *model;
> > +
> > + spl_fdt_fixup_mars(fdt);
> > +
> > + if (!get_mmc_size_from_eeprom()) {
> > + int offset;
> > +
> > + model = "Milk-V Mars CM SDCard";
>
> "Milk-V Mars CM Lite"
>
> > + compat = "milkv,mars-cm-sdcard\0starfive,jh7110";
>
> "milkv,mars-cm-lite\0starfive,jh7110"
>
> > +
> > + offset = fdt_path_offset(fdt,
> > "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> > + fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> > + } else {
> > + model = "Milk-V Mars CM eMMC";
>
> "Milk-V Mars CM"
>
> > + compat = "milkv,mars-cm-emmc\0starfive,jh7110";
>
> "milkv,mars-cm\0starfive,jh7110"
>
> > + }
> > + fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> > sizeof(compat));
> > + fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> > +}
> > +
> > void spl_fdt_fixup_version_a(void *fdt)
> > {
> > static const char compat[] =
> > "starfive,visionfive-2-v1.2a\0starfive,jh7110";
> > @@ -236,7 +259,9 @@ void spl_perform_fixups(struct spl_image_info
> > *spl_image)
> > pr_err("Can't read EEPROM\n");
> > return;
> > }
> > - if (!strncmp(product_id, "MARS", 4)) {
> > + if (!strncmp(product_id, "MARC", 4)) {
> > + spl_fdt_fixup_marc(spl_image->fdt_addr);
> > + } else if (!strncmp(product_id, "MARS", 4)) {
> > spl_fdt_fixup_mars(spl_image->fdt_addr);
> > } else if (!strncmp(product_id, "VF7110", 6)) {
> > version = get_pcb_revision_from_eeprom();
> > diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> > b/board/starfive/visionfive2/starfive_visionfive2.c
> > index a86bca533b2..be6ca85b030 100644
> > --- a/board/starfive/visionfive2/starfive_visionfive2.c
> > +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> > @@ -17,6 +17,10 @@
> > DECLARE_GLOBAL_DATA_PTR;
> > #define JH7110_L2_PREFETCHER_BASE_ADDR 0x203
> > #define JH7110_L2_PREFETCHER_HART_OFFSET 0x2000
> > +#define FDTFILE_MILK_V_MARC_SD \
> > + "starfive/jh7110-milkv-mars-cm-sdcard.dtb"
>
> "starfive/jh7110-milkv-mars-cm-lite.dtb"
>
> > +#define FDTFILE_MILK_V_MARC_MMC \
> > + "starfive/jh7110-milkv-mars-cm-emmc.dtb"
>
> "starfive/jh7110-milkv-mars-cm.dtb"
>
> > #define FDTFILE_MILK_V_MARS \
> > "starfive/jh7110-milkv-mars.dtb"
> > #define FDTFILE_VISIONFIVE2_1_2A \
> > @@ -61,7 +65,12 @@ static void set_fdtfile(void)
> > log_err("Can't read EEPROM\n");
> > return;
> > }
> > - if (!strncmp(product_id, "MARS", 4)) {
> > + if (!strncmp(product_id, "MARC", 4)) {
> > + if (get_mmc_size_from_eeprom())
> > + fdtfile = FDTFILE_MILK_V_MARC_MMC;
> > + els
How to improve eficonfig menu for users?
Hi, I suggest to flatten the eficonfig UI menu so it is easier for users and avoid needing efidebug command for most situations. Flatten the eficonfig menu: Replace the UEFI Maintenance Menu with Change Boot Order as the UEFI Maintenance Menu. Add to UEFI Maintenance Menu a keypress to edit. Add to UEFI Maintenance Menu a keypress to delete. Relocate the Add Boot Option menu invocation into UEFI Maintenance Menu above Save. Relocate and replace the Delete Boot Option menu as a contextual action into Edit Boot Option above "Save". Quality of life for global boot order items: Boot order items not represented by the Boot variable could be displayed with some text suffix i.e. "... (EFI global)" to make it clear that trying to edit may be no-op, and action of delete could no-op or deactivate as likely what the user wants for trying to delete an immutable boot option. Avoid need of efidebug command to edit boot variables: Add to UEFI Maintenance Menu a keypress to edit advanced. This would be an alternate version of Edit Boot Option taking user input (as is done for Optional Data) instead of file selection UI. If it is too complicated to try and cross-reference boot order against Boot variable then this can simply go to that existing sub-menu for the same overall menu depth. Importantly the UEFI Maintenance Menu would always show the boot order to the user so it will not be overlooked as it is now. Open for comments. -E
Re: [RFC 04/14] cmd: eficonfig: add support for setting fdt
On Sat, Apr 27, 2024 at 2:25 PM Heinrich Schuchardt
wrote:
>
> On 4/27/24 19:21, E Shattow wrote:
> > On Fri, Apr 26, 2024 at 7:25 AM Heinrich Schuchardt
> > wrote:
> >>
> >> We already support creating a load option where the device-path
> >> field contains the concatenation of the binary device-path and
> >> optionally the device path of the initrd which we expose via the
> >> EFI_LOAD_FILE2_PROTOCOL.
> >>
> >> Allow to append another device-path pointing to the device-tree
> >> identified by the device-tree GUID.
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> cmd/eficonfig.c | 68 +
> >> 1 file changed, 63 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> >> index 1c57e66040b..d314051ee58 100644
> >> --- a/cmd/eficonfig.c
> >> +++ b/cmd/eficonfig.c
> >> @@ -62,6 +62,7 @@ struct eficonfig_filepath_info {
> >> struct eficonfig_boot_option {
> >> struct eficonfig_select_file_info file_info;
> >> struct eficonfig_select_file_info initrd_info;
> >> + struct eficonfig_select_file_info fdt_info;
> >> unsigned int boot_index;
> >> u16 *description;
> >> u16 *optional_data;
> >> @@ -1308,6 +1309,10 @@ static efi_status_t
> >> eficonfig_show_boot_option(struct eficonfig_boot_option *bo,
> >> if (ret != EFI_SUCCESS)
> >> goto out;
> >>
> >> + ret = prepare_file_selection_entry(efi_menu, "Fdt File: ",
> >> &bo->fdt_info);
> >> + if (ret != EFI_SUCCESS)
> >> + goto out;
> >> +
> >> ret = create_boot_option_entry(efi_menu, "Optional Data: ",
> >> bo->optional_data,
> >> eficonfig_boot_add_optional_data,
> >> bo);
> >> if (ret != EFI_SUCCESS)
> >> @@ -1394,21 +1399,39 @@ static efi_status_t eficonfig_edit_boot_option(u16
> >> *varname, struct eficonfig_bo
> >> struct efi_device_path *final_dp = NULL;
> >> struct efi_device_path *device_dp = NULL;
> >> struct efi_device_path *initrd_dp = NULL;
> >> + struct efi_device_path *fdt_dp = NULL;
> >> struct efi_device_path *initrd_device_dp = NULL;
> >> + struct efi_device_path *fdt_device_dp = NULL;
> >>
> >> - const struct efi_initrd_dp id_dp = {
> >> + const struct efi_initrd_dp initrd_prefix = {
> >> .vendor = {
> >> {
> >> DEVICE_PATH_TYPE_MEDIA_DEVICE,
> >> DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> >> - sizeof(id_dp.vendor),
> >> + sizeof(initrd_prefix.vendor),
> >> },
> >> EFI_INITRD_MEDIA_GUID,
> >> },
> >> .end = {
> >> DEVICE_PATH_TYPE_END,
> >> DEVICE_PATH_SUB_TYPE_END,
> >> - sizeof(id_dp.end),
> >> + sizeof(initrd_prefix.end),
> >> + }
> >> + };
> >> +
> >> + const struct efi_initrd_dp fdt_prefix = {
> >> + .vendor = {
> >> + {
> >> + DEVICE_PATH_TYPE_MEDIA_DEVICE,
> >> + DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> >> + sizeof(fdt_prefix.vendor),
> >> + },
> >> + EFI_FDT_GUID,
> >> + },
> >> + .end = {
> >> + DEVICE_PATH_TYPE_END,
> >> + DEVICE_PATH_SUB_TYPE_END,
> >> + sizeof(initrd_prefix.end),
> >> }
> >> };
> >>
> >> @@ -1424,6 +1447,12 @@ static efi_status_t eficonfig_edit_boot_option(u16
> >> *varname, struct eficonfig_bo
> >> goto out;
> >> }
> >>
> >> + bo->fdt_info.current_path = calloc(1,
> >> EFICONFIG_FILE_PATH_BUF_SIZE);
> >> + if (!bo->fdt_info.current_path) {
> >> + ret = E
Re: [RFC 04/14] cmd: eficonfig: add support for setting fdt
t file path (optional) is placed as third instance. */
> + fdt_dp = efi_dp_from_lo(&lo, &efi_guid_fdt);
> + if (fdt_dp) {
> + fill_file_info(fdt_dp, &bo->fdt_info, fdt_device_dp);
> + efi_free_pool(fdt_dp);
> + }
> +
> if (size > 0)
> memcpy(bo->optional_data, lo.optional_data, size);
> }
> @@ -1484,11 +1520,23 @@ static efi_status_t eficonfig_edit_boot_option(u16
> *varname, struct eficonfig_bo
> ret = EFI_OUT_OF_RESOURCES;
> goto out;
> }
> - initrd_dp = efi_dp_concat((const struct efi_device_path
> *)&id_dp,
> + initrd_dp = efi_dp_concat((const struct efi_device_path
> *)&initrd_prefix,
> dp);
> efi_free_pool(dp);
> }
>
> + if (bo->fdt_info.dp_volume) {
> + dp = eficonfig_create_device_path(bo->fdt_info.dp_volume,
> + bo->fdt_info.current_path);
> + if (!dp) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> + fdt_dp = efi_dp_concat((const struct efi_device_path
> *)&fdt_prefix,
> + dp);
> + efi_free_pool(dp);
> + }
> +
> dp = eficonfig_create_device_path(bo->file_info.dp_volume,
> bo->file_info.current_path);
> if (!dp) {
> ret = EFI_OUT_OF_RESOURCES;
> @@ -1505,6 +1553,13 @@ static efi_status_t eficonfig_edit_boot_option(u16
> *varname, struct eficonfig_bo
> goto out;
> }
> }
> + if (fdt_dp) {
> + final_dp = efi_dp_merge(final_dp, &final_dp_size, fdt_dp);
> + if (!final_dp) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> + }
>
> if (utf16_utf8_strlen(bo->optional_data)) {
> len = utf16_utf8_strlen(bo->optional_data) + 1;
> @@ -1522,9 +1577,12 @@ out:
> free(bo->description);
> free(bo->file_info.current_path);
> free(bo->initrd_info.current_path);
> + free(bo->fdt_info.current_path);
> efi_free_pool(device_dp);
> efi_free_pool(initrd_device_dp);
> efi_free_pool(initrd_dp);
> + efi_free_pool(fdt_device_dp);
> + efi_free_pool(fdt_dp);
> efi_free_pool(final_dp);
>
> return ret;
> --
> 2.43.0
>
Would it make sense to skip showing the user partitions that are not
valid (or why does the Linux Swap partition not show here but the
Linux partition with ext4 does? Neither is valid for selecting Fdt
File ?) and/or extend eficonfig for user-entered data? For example if
I was very sure I wanted U-Boot to search a location but I just didn't
have the SD Card that configuration is meant for currently inserted, I
must use efidebug to configure this?
It was always confusing how Edit action hides from view an automatic
(global?) boot option i.e. 'mmc 0' that is configurable from Change
Boot Order. I guess that there would have just been File
EFI\BOOT\BOOTRISCV64.EFI to show and that is not interesting enough
information for the added complexity of a save-disabled Edit entry.
However now or in future there will be useful information to display
so this should become viewable as a save-disabled entry from Edit
(rename? View/Edit) action, where it is convenient to see File
constant EFI\BOOT\BOOTRISCV64.EFI and also the detected values that
will be used i.e. from parsing U-Boot variable $fdtfile. I do not mean
that this should be a method for editing U-Boot environment variables,
only that it would be useful to know for example when
$fdtfile=vendor/boardname.dtb that the U-Boot EFI code heuristic has
decided that this currently resolves to mmc
0:1/dtb\vendor\boardname.dtb value. If the automatic values are not
what the user wants then they may create a new boot entry or leave the
context of eficonfig to update something that affects the detection
logic such as U-Boot env variables, and perhaps again enter eficonfig
to confirm that the heuristic is now doing what they would like.
Without any visibility of this global boot option from the Edit
action, it is not known to need to edit the boot order after adding a
boot entry where before there was apparently none. Some of that could
be addressed with documentation but the best result would be it being
obvious through use. It's not really obvious now that something named
'mmc 0' only appearing in the Boot Order action of eficonfig what
filename it requires or if it is going to use an Initrd and/or Fdt.
Aside these more broad usability concerns there was success in adding
a boot item with Fdt setting and changing the order that it be
preferred entry.
Tested-by: E Shattow
Re: [PATCH v3] mmc: allow use of hardware partition names for mmc partconf
On Sat, Apr 27, 2024 at 3:22 AM Marek Vasut wrote:
>
> On 4/27/24 3:29 AM, E Shattow wrote:
> > Hi Marek,
> >
> > On Fri, Apr 26, 2024 at 5:49 PM Marek Vasut wrote:
> >>
> >> [...]
> >>
> >>> diff --git a/include/mmc.h b/include/mmc.h
> >>> index 4b8327f1f93b..7243bd761202 100644
> >>> --- a/include/mmc.h
> >>> +++ b/include/mmc.h
> >>> @@ -381,6 +381,21 @@ enum mmc_voltage {
> >>>#define MMC_TIMING_MMC_HS2009
> >>>#define MMC_TIMING_MMC_HS40010
> >>>
> >>> +/* emmc hardware partition values */
> >>> +enum emmc_hwpart {
> >>> + EMMC_HWPART_DEFAULT = 0,
> >>> + EMMC_HWPART_BOOT0 = 1,
> >>> + EMMC_HWPART_BOOT1 = 2,
> >>> + EMMC_HWPART_GP1 = 3,
> >>> + EMMC_HWPART_GP2 = 4,
> >>> + EMMC_HWPART_GP3 = 5,
> >>> + EMMC_HWPART_GP4 = 6,
> >>> + EMMC_HWPART_USER = 7,
> >>> +};
> >>> +
> >>> +/* emmc hardware partition names */
> >>> +extern const char *emmc_hwpart_names[];
> >>
> >> Maybe the array should have fixed size here, i.e. 8 ?
> >
> > Is there an ABI reason to do so? Can you explain further why it would
> > be needed to do that?
>
> It has nothing to do with ABI, it is only to let the compiler validate
> that nobody would index the array with index > 7 by accident.
At least GCC knows this without doing its work again ourselves. How
about a const for the upper limit, where currently EMMC_HWPART_USER
substitutes for expressing an upper limit? You may as well be writing
EMMC_HWPART_MAX = 8 in the enum and using that for the initializer
also any iterators with a less-than condition to make it more
expressive.
$ gcc -o testobj test.c -Wall -Wextra -O2
test.c: In function ‘main’:
test.c:16:5: warning: array subscript 8 is above array bounds of
‘const char *[8]’ [-Warray-bounds=]
16 | printf(emmc_hwpart_names[8]);
| ^~~~
test.c:3:13: note: while referencing ‘emmc_hwpart_names’
3 | const char *emmc_hwpart_names[] = {
| ^
#include
const char *emmc_hwpart_names[] = {
"user",
"boot0",
"boot1",
"gp1",
"gp2",
"gp3",
"gp4",
"user",
};
int main(int argc, char** argv) {
(void) argc; (void) argv;
printf(emmc_hwpart_names[8]);
return 0;
}
Sorry to belabor this and I'm not a professional programmer. Best regards, -E
Re: [PATCH v3] mmc: allow use of hardware partition names for mmc partconf
Hi Marek,
On Fri, Apr 26, 2024 at 5:49 PM Marek Vasut wrote:
>
> [...]
>
> > diff --git a/include/mmc.h b/include/mmc.h
> > index 4b8327f1f93b..7243bd761202 100644
> > --- a/include/mmc.h
> > +++ b/include/mmc.h
> > @@ -381,6 +381,21 @@ enum mmc_voltage {
> > #define MMC_TIMING_MMC_HS2009
> > #define MMC_TIMING_MMC_HS40010
> >
> > +/* emmc hardware partition values */
> > +enum emmc_hwpart {
> > + EMMC_HWPART_DEFAULT = 0,
> > + EMMC_HWPART_BOOT0 = 1,
> > + EMMC_HWPART_BOOT1 = 2,
> > + EMMC_HWPART_GP1 = 3,
> > + EMMC_HWPART_GP2 = 4,
> > + EMMC_HWPART_GP3 = 5,
> > + EMMC_HWPART_GP4 = 6,
> > + EMMC_HWPART_USER = 7,
> > +};
> > +
> > +/* emmc hardware partition names */
> > +extern const char *emmc_hwpart_names[];
>
> Maybe the array should have fixed size here, i.e. 8 ?
Is there an ABI reason to do so? Can you explain further why it would
be needed to do that?
Re: [PATCH v1 2/3] board: sifive: Call spl_dram_init() for DRAM initialization
On Mon, Apr 22, 2024 at 6:16 AM wrote:
>
> From: Lukas Funke
>
> Call spl_dram_init() since this is commonly used for dram initialization
> in u-boot.
>
> Signed-off-by: Lukas Funke
> ---
>
> board/sifive/unleashed/spl.c | 4 ++--
> board/sifive/unmatched/spl.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/board/sifive/unleashed/spl.c b/board/sifive/unleashed/spl.c
> index fe27316b2d..d6725a0e0e 100644
> --- a/board/sifive/unleashed/spl.c
> +++ b/board/sifive/unleashed/spl.c
> @@ -27,9 +27,9 @@ int spl_board_init_f(void)
> {
> int ret;
>
> - ret = spl_soc_init();
> + ret = spl_dram_init();
> if (ret) {
> - debug("FU540 SPL init failed: %d\n", ret);
> + debug("FU540 dram init failed: %d\n", ret);
> return ret;
> }
>
> diff --git a/board/sifive/unmatched/spl.c b/board/sifive/unmatched/spl.c
> index e69bed9d99..500f484844 100644
> --- a/board/sifive/unmatched/spl.c
> +++ b/board/sifive/unmatched/spl.c
> @@ -134,9 +134,9 @@ int spl_board_init_f(void)
> {
> int ret;
>
> - ret = spl_soc_init();
> + ret = spl_dram_init();
> if (ret) {
> - debug("HiFive Unmatched FU740 SPL init failed: %d\n", ret);
> + debug("HiFive Unmatched FU740 dram init failed: %d\n", ret);
> goto end;
> }
>
> --
> 2.30.2
>
Acronym DRAM should be all capitalized letters in strings.
-E
Re: [PATCH] riscv: dts: jh7110: Enable PLL node in SPL
On Fri, Apr 19, 2024 at 5:51 PM Bo Gan wrote:
>
...snip...
>
> If without the change (reverted), can you read/write the same SD media in
> U-boot
> proper? (U-boot proper will switch BUS_ROOT to PLL2).
I tested again this change in commit e6b7aeef, before this change in
parent commit e6b7aeef~, af04f37a HEAD from today 19th Apr 2024 (which
due to not matching EEPROM product_id will be in the fall-through case
of board/starfive/visionfive2/spl.c), af04f37a with applied patchset
"board: starfive: add Milk-V Mars CM support" from 15th Apr 2024, and
af04f37a reverting changes from e6b7aeef also with applied patchset
"board: starfive: add Milk-V Mars CM support" from 15th Apr 2024.
In all builds is OpenSBI at commit d4d2582e HEAD from today 19 Apr 2024.
For each build tested per vendor Milk-V the Mars CM Lite (SD Card only
non-eMMC) has pinmux of GPIO22 instead of GPIO62:
-- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
@@ -233,7 +233,7 @@
mmc0_pins: mmc0-pins {
mmc0-pins-rest {
- pinmux = ;
bias-pull-up;
drive-strength = <12>;
U-Boot config is simply starfive_visionfive2_defconfig.
Results are as follows.
StarFive # mac
EEPROM INFO
Vendor : MILK-V
Product full SN: MARC-V10-2340-D004E000-06DF
data version: 0x2
PCB revision: 0xc1
BOM revision: A
Ethernet MAC0 address: 6c:cf:39:00:83:11
Ethernet MAC1 address: 6c:cf:39:00:83:12
EEPROM INFO
e6b7aeef: 2GB microSD (no speed class markings)
af04f37a: 2GB microSD (no speed class markings)
af04f37a with Mars CM patchset: 2GB microSD (no speed class markings)
StarFive # mmc rescan ; mmc info
unable to select a mode
unable to select a mode
e6b7aeef~: 2GB microSD (no speed class markings)
af04f37a revert e6b7aeef with Mars CM patchset: 2GB microSD (no speed
class markings)
StarFive # mmc rescan ; mmc info
Device: mmc@1601
Manufacturer ID: 1c
OEM: 5356
Name: USD
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 2.0
High Capacity: No
Capacity: 1.9 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes
e6b7aeef: 8GB microSD Class 4
e6b7aeef~: 8GB microSD Class 4
af04f37a: 8GB microSD Class 4
af04f37a with Mars CM patchset: 8GB microSD Class 4
af04f37a revert e6b7aeef with Mars CM patchset: 8GB microSD Class 4
StarFive # mmc rescan ; mmc info
Device: mmc@1601
Manufacturer ID: 2
OEM: 544d
Name: SA08G
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 7.4 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes
e6b7aeef: 8GB microSD Class 10
e6b7aeef~: 8GB microSD Class 10
af04f37a: 8GB microSD Class 10
af04f37a with Mars CM patchset: 8GB microSD Class 10
af04f37a revert e6b7aeef with Mars CM patchset: 8GB microSD Class 10
StarFive # mmc rescan ; mmc info
Device: mmc@1601
Manufacturer ID: 74
OEM: 4a60
Name: USD
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 7.5 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes
e6b7aeef: 32GB microSD Class 10 A1 U1 HC1
e6b7aeef~: 32GB microSD Class 10 A1 U1 HC1
af04f37a: 32GB microSD Class 10 A1 U1 HC1
af04f37a with Mars CM patchset: 32GB microSD Class 10 A1 U1 HC1
af04f37a revert e6b7aeef with Mars CM patchset: 32GB microSD Class 10 A1 U1 HC1
StarFive # mmc rescan ; mmc info
Device: mmc@1601
Manufacturer ID: 3
OEM: 5344
Name: SC32G
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 29.7 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes
e6b7aeef: 200GB microSD Class 10 A1 U1 XC1
e6b7aeef~: 200GB microSD Class 10 A1 U1 XC1
af04f37a: 200GB microSD Class 10 A1 U1 XC1
af04f37a with Mars CM patchset: 200GB microSD Class 10 A1 U1 XC1
af04f37a revert e6b7aeef with Mars CM patchset: 200GB microSD Class 10 A1 U1 XC1
StarFive # mmc rescan ; mmc info
Device: mmc@1601
Manufacturer ID: 3
OEM: 5344
Name: SC200
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 183.3 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes
e6b7aeef: 256GB microSD Class U3 XC1
e6b7aeef~: 256GB microSD Class U3 XC1
af04f37a: 256GB microSD Class U3 XC1
af04f37a with Mars CM patchset: 256GB microSD Class U3 XC1
af04f37a revert e6b7aeef with Mars CM patchset: 256GB microSD Class U3 XC1
StarFive # mmc rescan ; mmc info
Device: mmc@1601
Manufacturer ID: 1b
OEM: 534d
Name: GE4S5
Bus Speed: 5000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 238.8 GiB
Bus Width: 1-bit
Erase Group Size: 512 Bytes
> One potential problem I
> could think of is perhaps the SPL built is without SPL_PINCTRL_STARFIVE/JH7110
> or the u-boot dts is missing the pinctrl that properly sets drive-strength and
> other properties of the mmc0/1 pins. What dtb are you using? I tested this
> with
> visionfi
Re: [PATCH 4/4] configs: visionfive2: enable SPL_YMODEM_SUPPORT
P.S. mis-spoke should have written "Having 'env load' allows to skip that reboot step in-between On Wed, Apr 17, 2024 at 12:28 PM E Shattow wrote: > > On Wed, Apr 17, 2024 at 1:22 AM Heinrich Schuchardt > wrote: > > > > On 17.04.24 03:41, E Shattow wrote: > > > Successful in use w/ 'tio' serial tool and Adafruit CP2102N Friend > > > adapter on Mars CM Lite board in DFRobot mini router carrier. > > > > > > While SPL and u-boot.itb payload are sourced via UART the environment > > > variables are from the environment variable storage as-is. This makes > > > sense in the use case for development but may have side-effects in the > > > case of U-Boot as a JH7110 recovery tool. There is now 'env default -f > > > -a' which does not purge non-default variables and retains the > > > non-default variables when migrating from vendor firmware. Consider to > > > also build for U-Boot the commands that can aid in cleaning the stored > > > environment variable state CONFIG_CMD_ERASEENV=y and in-memory state > > > CONFIG_CMD_NVEDIT_LOAD=y. With these it can be done easily with: 'env > > > erase; env load; env save'. > > > > Thank you for testing. > > > > After erasing there is no need to save the environment. If there is no > > environment on flash, the default will be loaded anyway: > > > >*** Warning - bad CRC, using default environment > > > > Instead of 'env erase' you could use 'sf erase 0xf 0x1000' which is > > already available. As adding CONFIG_CMD_ERASEENV=y is not necessary in > > the scope of this patch series I would prefer leaving it to future > > discussion. > > > > Best regards > > > > Heinrich > > > > The 'env erase; env load; env save' can be done at the same session > just before recovery write to provide a sanitized environment with the > defaults of the present U-Boot payload loaded via UART, so is not the > same as 'sf erase ...' waiting for a reboot (5 minutes more to > transfer via UART at 115200 baud) and needing to interrupt the boot to > further 'env save'. Having 'env load' allows to allow that reboot step > in-between. Point taken though, it's helpful for recovery but not > required. -E
Re: [PATCH 4/4] configs: visionfive2: enable SPL_YMODEM_SUPPORT
On Wed, Apr 17, 2024 at 1:22 AM Heinrich Schuchardt wrote: > > On 17.04.24 03:41, E Shattow wrote: > > Successful in use w/ 'tio' serial tool and Adafruit CP2102N Friend > > adapter on Mars CM Lite board in DFRobot mini router carrier. > > > > While SPL and u-boot.itb payload are sourced via UART the environment > > variables are from the environment variable storage as-is. This makes > > sense in the use case for development but may have side-effects in the > > case of U-Boot as a JH7110 recovery tool. There is now 'env default -f > > -a' which does not purge non-default variables and retains the > > non-default variables when migrating from vendor firmware. Consider to > > also build for U-Boot the commands that can aid in cleaning the stored > > environment variable state CONFIG_CMD_ERASEENV=y and in-memory state > > CONFIG_CMD_NVEDIT_LOAD=y. With these it can be done easily with: 'env > > erase; env load; env save'. > > Thank you for testing. > > After erasing there is no need to save the environment. If there is no > environment on flash, the default will be loaded anyway: > >*** Warning - bad CRC, using default environment > > Instead of 'env erase' you could use 'sf erase 0xf 0x1000' which is > already available. As adding CONFIG_CMD_ERASEENV=y is not necessary in > the scope of this patch series I would prefer leaving it to future > discussion. > > Best regards > > Heinrich > The 'env erase; env load; env save' can be done at the same session just before recovery write to provide a sanitized environment with the defaults of the present U-Boot payload loaded via UART, so is not the same as 'sf erase ...' waiting for a reboot (5 minutes more to transfer via UART at 115200 baud) and needing to interrupt the boot to further 'env save'. Having 'env load' allows to allow that reboot step in-between. Point taken though, it's helpful for recovery but not required. -E
Re: [PATCH] riscv: dts: jh7110: Enable PLL node in SPL
On Tue, Apr 9, 2024 at 11:44 PM Bo Gan wrote: > > On 4/9/24 6:55 PM, E Shattow wrote: > > Original speed class SD cards fail with this change "unable to change mode". > > > > The BUS_ROOT clock will have to be switched to PLL2 anyway in U-Boot proper or > in Linux, because it's the parent or grandparent clock for *lots* of devices, > including PCIe, i2c, spi, qspi... If there's an issue with this change, then > I suspect there's something wrong with the dw_mmc driver. > > Bo I've bisected and can confirm this change is what breaks original speed SD function on Milk-V Mars CM Lite (DFRobot mini router carrier). Class 10 speed SD media does not seem to be affected. Reverting the change allows the original speed SD media access to function again. SD access is functional in Linux with and without the change. How to troubleshoot this? Thanks, E
Re: [PATCH 4/4] configs: visionfive2: enable SPL_YMODEM_SUPPORT
Successful in use w/ 'tio' serial tool and Adafruit CP2102N Friend adapter on Mars CM Lite board in DFRobot mini router carrier. While SPL and u-boot.itb payload are sourced via UART the environment variables are from the environment variable storage as-is. This makes sense in the use case for development but may have side-effects in the case of U-Boot as a JH7110 recovery tool. There is now 'env default -f -a' which does not purge non-default variables and retains the non-default variables when migrating from vendor firmware. Consider to also build for U-Boot the commands that can aid in cleaning the stored environment variable state CONFIG_CMD_ERASEENV=y and in-memory state CONFIG_CMD_NVEDIT_LOAD=y. With these it can be done easily with: 'env erase; env load; env save'. On Mon, Apr 15, 2024 at 4:51 AM Heinrich Schuchardt wrote: > > We can use U-Boot for recovering JH7110 based boards via UART > if CONFIG_SPL_YMODEM_SUPPORT=y. > > * Send u-boot-spl.normal.out via XMODEM. > * Send u-boot.itb via YMODEM. > > Signed-off-by: Heinrich Schuchardt > --- > configs/starfive_visionfive2_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/starfive_visionfive2_defconfig > b/configs/starfive_visionfive2_defconfig > index fa80d489f5e..e2d83c62b28 100644 > --- a/configs/starfive_visionfive2_defconfig > +++ b/configs/starfive_visionfive2_defconfig > @@ -62,6 +62,7 @@ CONFIG_SPL_I2C=y > CONFIG_SPL_DM_SPI_FLASH=y > CONFIG_SPL_DM_RESET=y > CONFIG_SPL_SPI_LOAD=y > +CONFIG_SPL_YMODEM_SUPPORT=y > CONFIG_SYS_PROMPT="StarFive # " > CONFIG_CMD_EEPROM=y > CONFIG_SYS_EEPROM_SIZE=512 > -- > 2.43.0 > Tested-by: E Shattow
Re: [PATCH 2/4] board: add support for Milk-V Mars CM
On Mon, Apr 15, 2024 at 4:50 AM Heinrich Schuchardt
wrote:
>
> We already support the VisionFive 2 and the Milk-V Mars board by
> patching the VisionFive 2 device tree. With this patch the same
> is done for the Milk-V Mars CM.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> board/starfive/visionfive2/spl.c | 27 ++-
> .../visionfive2/starfive_visionfive2.c| 11 +++-
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/board/starfive/visionfive2/spl.c
> b/board/starfive/visionfive2/spl.c
> index 45848db6d8b..bb0f28d7aad 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -129,6 +129,29 @@ void spl_fdt_fixup_mars(void *fdt)
> }
> }
>
> +void spl_fdt_fixup_marc(void *fdt)
> +{
> + const char *compat;
> + const char *model;
> +
> + spl_fdt_fixup_mars(fdt);
> +
> + if (!get_mmc_size_from_eeprom()) {
> + int offset;
> +
> + model = "Milk-V Mars CM SDCard";
> + compat = "milkv,mars-cm-sdcard\0starfive,jh7110";
> +
> + offset = fdt_path_offset(fdt,
> "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> + fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
Note this is:
- pinmux = https://github.com/milkv-mars/mars-buildroot-sdk/commit/880a249518f72ecf1e2947dfeb2c66e5035fce90
But the GPIOMUX macro here (for code readability) would bring in
another header to include. Add a comment of what the magic number is
for.
Does anyone have a better explanation of why this is needed than the
vendor "u-boot: configure sdio0 as mars-cm sdcard version" commit
message?
> + } else {
> + model = "Milk-V Mars CM eMMC";
> + compat = "milkv,mars-cm-emmc\0starfive,jh7110";
> + }
> + fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat,
> sizeof(compat));
> + fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> +}
> +
> void spl_fdt_fixup_version_a(void *fdt)
> {
> static const char compat[] =
> "starfive,visionfive-2-v1.2a\0starfive,jh7110";
> @@ -236,7 +259,9 @@ void spl_perform_fixups(struct spl_image_info *spl_image)
> pr_err("Can't read EEPROM\n");
> return;
> }
> - if (!strncmp(product_id, "MARS", 4)) {
> + if (!strncmp(product_id, "MARC", 4)) {
> + spl_fdt_fixup_marc(spl_image->fdt_addr);
> + } else if (!strncmp(product_id, "MARS", 4)) {
> spl_fdt_fixup_mars(spl_image->fdt_addr);
> } else if (!strncmp(product_id, "VF7110", 6)) {
> version = get_pcb_revision_from_eeprom();
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> b/board/starfive/visionfive2/starfive_visionfive2.c
> index a86bca533b2..be6ca85b030 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -17,6 +17,10 @@
> DECLARE_GLOBAL_DATA_PTR;
> #define JH7110_L2_PREFETCHER_BASE_ADDR 0x203
> #define JH7110_L2_PREFETCHER_HART_OFFSET 0x2000
> +#define FDTFILE_MILK_V_MARC_SD \
> + "starfive/jh7110-milkv-mars-cm-sdcard.dtb"
> +#define FDTFILE_MILK_V_MARC_MMC \
> + "starfive/jh7110-milkv-mars-cm-emmc.dtb"
> #define FDTFILE_MILK_V_MARS \
> "starfive/jh7110-milkv-mars.dtb"
> #define FDTFILE_VISIONFIVE2_1_2A \
> @@ -61,7 +65,12 @@ static void set_fdtfile(void)
> log_err("Can't read EEPROM\n");
> return;
> }
> - if (!strncmp(product_id, "MARS", 4)) {
> + if (!strncmp(product_id, "MARC", 4)) {
> + if (get_mmc_size_from_eeprom())
> + fdtfile = FDTFILE_MILK_V_MARC_MMC;
> + else
> + fdtfile = FDTFILE_MILK_V_MARC_SD;
> + } else if (!strncmp(product_id, "MARS", 4)) {
> fdtfile = FDTFILE_MILK_V_MARS;
> } else if (!strncmp(product_id, "VF7110", 6)) {
> version = get_pcb_revision_from_eeprom();
> --
> 2.43.0
>
Re: [PATCH 3/4] doc: Milk-V Mars CM and Milk-V Mars CM Lite
On Mon, Apr 15, 2024 at 4:50 AM Heinrich Schuchardt wrote: > > Provide a man-page describing the usage of U-Boot on > the Milk-V Mars CM and Milk-V Mars CM Lite boards. > > Signed-off-by: Heinrich Schuchardt > --- > doc/board/starfive/index.rst | 1 + > doc/board/starfive/milk-v_mars_cm.rst | 125 ++ > 2 files changed, 126 insertions(+) > create mode 100644 doc/board/starfive/milk-v_mars_cm.rst > > diff --git a/doc/board/starfive/index.rst b/doc/board/starfive/index.rst > index 2762bf74c11..afa85ad2540 100644 > --- a/doc/board/starfive/index.rst > +++ b/doc/board/starfive/index.rst > @@ -7,4 +7,5 @@ StarFive > :maxdepth: 1 > > milk-v_mars.rst > + milk-v_mars_cm.rst > visionfive2 > diff --git a/doc/board/starfive/milk-v_mars_cm.rst > b/doc/board/starfive/milk-v_mars_cm.rst > new file mode 100644 > index 000..4cd6034281c > --- /dev/null > +++ b/doc/board/starfive/milk-v_mars_cm.rst > @@ -0,0 +1,125 @@ > +.. SPDX-License-Identifier: GPL-2.0+ > + > +Milk-V Mars CM > +== > + > +U-Boot for the Milk-V Mars CM uses the same U-Boot binaries as the > VisionFive 2 > +board. In U-Boot SPL the actual board is detected and the device-tree patched > +accordingly. > + > +The Milk-V Mars CM Lite comes without eMMC it needs a different pin muxing. JH7110 Technical Reference Manual does list some sdio related mux function with this GPIO62/GPIO22 substitution. I did not hear back yet from Milk-V developer support per my request for clarifying why this is needed and relevant schematics. I want this explained more clearly in documentation the purpose for this different pin muxing or why it is needed in the U-Boot phase of the boot process for specifically Milk-V Mars CM Lite (SD Card non-eMMC version). Previously I thought it was something about the power regulator not having a complete device driver in U-Boot but I don't know. Is it a bodge or is it intentional for easier manufacture? > +The size of the eMMC shows up in the serial number shown by the *mac* > command, > +e.g. MARC-V10-2340-D002E016-0304. The number after the E is the MMC size The number may have 'T' suffix making this description wrong. Describing as unit-less "size." is enough, so delete "in GB." > +in GB. U-Boot takes a value of E000 as an indicator for the Lite version. None of the Lite versions have eMMC so that can be inferred without naming the specific product. Maybe in future there is some other need for this config option so keep it generic. Suggest "U-Boot takes a value of E000 as an indicator for not having an eMMC." > +Unfortunately the vendor has not set this value correctly on some Lite > boards. Milk-V developer support acknowledged 14th Apr 2024 my e-mail inquiry about wrong EEPROM data, but did not comment (yet) on instructions for a fix for boards that have wrong EEPROM data. As for documentation we should have something that describes how to confirm the data is correct and if it is not then what can be done, in the case of fixing the EEPROM data and not. Suggest to add: "In default operation EEPROM Write Protect is enabled, and may be disabled connecting the "GND" and "EN" test pads on top of the module." and an example of `mac product_id` also `mac write_eeprom` commands. You can do this with a steady hand and a paperclip, as the pads are accessible near the board edge even with the heatsink in place. Lest we be blamed for bad decisions users make with a paperclip though, I don't know if you want to include this information. I think it was easy to do and future production runs should have the correct EEPROM data value for eMMC so it would make sense to want to update this. > +Please, use CONFIG_STARFIVE_NO_EMMC=y to indicate a Milk-V Mars CM Lite in > this > +case. Otherwise you will not be able to read from eMMC Suggest: "Please, use CONFIG_STARFIVE_NO_EMMC=y when EEPROM data indicates eMMC is present on Milk-V Mars CM Lite. Otherwise you will not be able to read from SD Card." > + > +Building > + > + > +1. Add the RISC-V toolchain to your PATH. > +2. Setup ARCH & cross compilation environment variable: > + > +.. code-block:: none > + > + export CROSS_COMPILE= > + > +The M-mode software OpenSBI provides the supervisor binary interface (SBI) > and > +is responsible for the switch to S-Mode. It is a prerequisite to build > U-Boot. > +Support for the JH7110 was introduced in OpenSBI 1.2. It is recommended to > use > +a current release. > + > +.. code-block:: console > + > + git clone https://github.com/riscv/opensbi.git > + cd opensbi > + make PLATFORM=generic FW_TEXT_START=0x4000 FW_OPTIONS=0 Is it needed to pass FW_OPTIONS ? FW_TEXT_START is deprecated in opensbi commit d4d2582e (8th Apr 2024). Keep for compatibility but noted here in review. > + > +Now build the U-Boot SPL and U-Boot proper. > + > +.. code-block:: console > + > + cd > + make starfive_visionfive2_defconfig > + make > OPENSB
Re: [PATCH 1/4] board: starfive: function to read eMMC size
On Mon, Apr 15, 2024 at 4:50 AM Heinrich Schuchardt
wrote:
>
> The EEPROM provides information about the size of the EEPROM.
"The EEPROM provides information about the size of the eMMC."
> Provide a new function get_mmc_size_from_eeprom() to read it.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> arch/riscv/include/asm/arch-jh7110/eeprom.h| 7 +++
> board/starfive/visionfive2/Kconfig | 9 +
> .../visionfive2/visionfive2-i2c-eeprom.c | 18 ++
> 3 files changed, 34 insertions(+)
>
> diff --git a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> index 62d184aeb57..17395d4269e 100644
> --- a/arch/riscv/include/asm/arch-jh7110/eeprom.h
> +++ b/arch/riscv/include/asm/arch-jh7110/eeprom.h
> @@ -12,6 +12,13 @@
> u8 get_pcb_revision_from_eeprom(void);
> u32 get_ddr_size_from_eeprom(void);
>
> +/**
> + * get_mmc_size_from_eeprom() - read MMC size form EEPROM
> + *
> + * @return: size in GiB or 0 on error.
> + */
> +u32 get_mmc_size_from_eeprom(void);
> +
> /**
> * get_product_id_from_eeprom - get product ID string
> *
> diff --git a/board/starfive/visionfive2/Kconfig
> b/board/starfive/visionfive2/Kconfig
> index 2186a939646..d7e8a7a7d78 100644
> --- a/board/starfive/visionfive2/Kconfig
> +++ b/board/starfive/visionfive2/Kconfig
> @@ -50,4 +50,13 @@ config BOARD_SPECIFIC_OPTIONS # dummy
> imply PHY_LIB
> imply PHY_MSCC
>
> +config STARFIVE_NO_EMMC
> + bool "Report eMMC size as zero"
> + help
> + The serial number string in the EEPROM is meant to report the
> + size of onboard eMMC. Unfortunately some Milk-V Mars CM Lite
> + modules without eMMC show a non-zero size here.
> +
> + Set to 'Y' if you have a Mars CM Lite module.
> +
> endif
> diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> index ddef7d61235..cd3d8bd51a6 100644
> --- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> +++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
> @@ -548,6 +548,24 @@ u32 get_ddr_size_from_eeprom(void)
> return hextoul(&pbuf.eeprom.atom1.data.pstr[14], NULL);
> }
>
> +u32 get_mmc_size_from_eeprom(void)
> +{
> + u32 size;
> +
> + if (IS_ENABLED(CONFIG_STARFIVE_NO_EMMC))
> + return 0;
> +
> + if (read_eeprom())
> + return 0;
> +
> + size = dectoul(&pbuf.eeprom.atom1.data.pstr[19], NULL);
> +
> + if (pbuf.eeprom.atom1.data.pstr[21] == 'T')
> + size <<= 10;
> +
> + return size;
> +}
> +
> U_BOOT_LONGHELP(mac,
> "\n"
> "- display EEPROM content\n"
> --
> 2.43.0
>
Fixed-position parsing on a data format of ordered variable length
hyphen-delimited fields. Notable is that some Pine64 Star64 and Milk-V
Mars CM Lite boards shipped with uninitialized or wrong EEPROM data;
further the EEPROM Write Protect can be trivially disabled and
arbitrary data written i.e. with a paperclip then `mac` command. Could
this code be generalized to split fields on hyphen character better
expressing the expected data format or is that unwanted complexity and
code size?
Re: [PATCH] riscv: dts: jh7110: Enable PLL node in SPL
Original speed class SD cards fail with this change "unable to change mode". On Tue, Mar 12, 2024 at 4:12 AM Hal Feng wrote: > > > On 06.03.24 11:00, Bo Gan wrote: > > > > Previously PLL node was missing from SPL dts. This caused BUS_ROOT to stay > > on > > OSC clock (24Mhz). As a result, all peripherals have to run at a much lower > > frequency, and loading from sdcard/emmc is slow. > > Thus, enabling PLL node in dts to fix this. > > > > Signed-off-by: Bo Gan > > Reviewed-by: Hal Feng >
Re: [PATCH] starfive: visionfive2: switch to standard boot
Hi, does this standard boot no longer try to boot from the configured EFI list? I have a boot listing (for Debian's grub EFI loader) configured with u-boot eficonfig but it does not seem to be attempting EFI boot anymore, there is a message about card select error but I think that must be trying the eMMC (mmc 1) and not the SD Card (mmc 0). StarFive # mmc list mmc@1601: 0 (SD) mmc@1602: 1 StarFive # help bootflow bootflow - Boot flows Usage: bootflow scan - boot first available bootflow StarFive # bootflow scan Card did not respond to voltage select! : -110 No working controllers found ethernet@1603 Waiting for PHY auto negotiation to complete... done BOOTP broadcast 1 StarFive # bootefi bootmgr Card did not respond to voltage select! : -110 Booting: Debian bootloader GRUB error: no suitable video mode found. "GNU GRUB version 2.12-1+b1"... The documentation for bootflow command seems to have a lot more options than what I'm seeing now?
Re: [PATCH v2 4/6] usb: Add environment based device blocklist
Should be usb_denylist, since "block devices" is ambiguous meaning if
it is a list of data chunks (blocks) or if it blocks (deny). There is
no question what a denylist is.
On Sun, Mar 17, 2024 at 4:07 AM Janne Grunau via B4 Relay
wrote:
>
> From: Janne Grunau
>
> Add the environment variable "usb_blocklist" to prevent USB devices
> listed in it from being used. This allows to ignore devices which
> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
> Devices emulating keyboards are one example. U-boot currently supports
> only one USB keyboard device. Most commonly, people run into this with
> Yubikeys, so let's ignore those in the default environment.
>
> Based on previous USB keyboard specific patches for the same purpose.
>
> Link:
> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
> Signed-off-by: Janne Grunau
> ---
> common/usb.c | 56
> +++
> doc/usage/environment.rst | 12 ++
> include/env_default.h | 11 ++
> 3 files changed, 79 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 836506dcd9..73af5be066 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev,
> int addr, bool do_read,
> return 0;
> }
>
> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> +{
> + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> + blocklist);
> + return 0;
> +}
> +
> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
> +{
> + ulong vid, pid;
> + char *end;
> + const char *block_str = env_get("usb_blocklist");
> + const char *cur = block_str;
> +
> + /* parse "usb_blocklist" strictly */
> + while (cur && cur[0] != '\0') {
> + vid = simple_strtoul(cur, &end, 0);
> + if (cur == end || end[0] != ':')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + cur = end + 1;
> + pid = simple_strtoul(cur, &end, 0);
> + if (cur == end && end[0] != '*')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + if (end[0] == '*') {
> + /* use out of range idProduct as wildcard indication
> */
> + pid = U16_MAX + 1;
> + end++;
> + }
> + if (end[0] != ',' && end[0] != '\0')
> + return usb_blocklist_parse_error(block_str,
> +cur - block_str);
> +
> + if (id_vendor == vid && (pid > U16_MAX || id_product == pid))
> {
> + printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
> + debug("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> + id_product);
> + return 1;
> + }
> + if (end[0] == '\0')
> + break;
> + cur = end + 1;
> + }
> +
> + return 0;
> +}
> +
> int usb_select_config(struct usb_device *dev)
> {
> unsigned char *tmpbuf = NULL;
> @@ -1099,6 +1150,11 @@ int usb_select_config(struct usb_device *dev)
> le16_to_cpus(&dev->descriptor.idProduct);
> le16_to_cpus(&dev->descriptor.bcdDevice);
>
> + /* ignore devices from usb_blocklist */
> + if (usb_device_is_blocked(dev->descriptor.idVendor,
> + dev->descriptor.idProduct))
> + return -ENODEV;
> +
> /*
> * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
> * about this first Get Descriptor request. If there are any other
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index ebf75fa948..e42702adb2 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -366,6 +366,18 @@ tftpwindowsize
> This means the count of blocks we can receive before
> sending ack to server.
>
> +usb_blocklist
> +Block USB devices from being bound to an USB device driver. This can be
> used
> +to ignore devices which causes crashes in u-boot's USB stack or are
> +undesirable for other reasons.
> +The default environment blocks Yubico devices since they emulate USB
> +keyboards. U-boot currently supports only a single USB keyboard device
> and
> +it is undesirable that a Yubikey is used as keyboard.
> +Devices are matched by idVendor and idProduct. The variable contains a
> comma
> +separated list of idVendor:idProduct pairs as hexadecimal numbers joined
> +by a colon. '*' functions as a wildcar
Re: [RFC 2/5] riscv: set fdtfile on Milk-V Mars
On Fri, Mar 8, 2024 at 2:57 PM Heinrich Schuchardt <
heinrich.schucha...@canonical.com> wrote:
...
> > I do not like this practice of force setting fdtfile environment
> > variable. If I set fdtfile from u-boot prompt and saveenv, I expect that
> > my action as the user will persist over a reboot but instead it gets
> > ignored and overwritten by this strategy. Is this necessary to happen
> > before the boot scripts?
>
>
We could check if $fdtfile is already set before the update.
>
> This would require that the default environment does not include it. Cf.
>
> include/configs/starfive-visionfive2.h:51:
> "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
>
...
The default environment is expected to have a sensible default, and
implicitly is only used when there is a checksum error (so no existing
valid fdtfile environment variable). It is not nice to remove capability
from userspace i.e. Linux mtd interface access to update u-boot environment
variables; setting fdtfile as such has no effect as it will always be force
set by this routine. The workaround now is to set fdtfile from a boot.scr
but that adds a dependency to reside on storage (which is not always true
for all boot situations).
What of "fdtfile=${fdtfile_board}" for the default environment and this
routine instead updates $fdtfile_board (or similar) ? I'm not sure what to
suggest, or if this is a common practice for u-boot board support. Thanks -
E
Re: [RFC 2/5] riscv: set fdtfile on Milk-V Mars
Sharing the info from Mars CM Lite 4GB:
EEPROM INFO
Vendor : MILK-V
Product full SN: MARC-V10-2340-D004E016-06DF
data version: 0x2
PCB revision: 0xc1
BOM revision: A
Ethernet MAC0 address: 6c:cf:39:00:83:11
Ethernet MAC1 address: 6c:cf:39:00:83:12
EEPROM INFO
I do not like this practice of force setting fdtfile environment variable.
If I set fdtfile from u-boot prompt and saveenv, I expect that my action as
the user will persist over a reboot but instead it gets ignored and
overwritten by this strategy. Is this necessary to happen before the boot
scripts?
On Sun, Mar 3, 2024 at 5:02 AM Heinrich Schuchardt <
heinrich.schucha...@canonical.com> wrote:
> Set environment variable fdtfile to the correct value for the Milk-V Mars
> board.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> .../visionfive2/starfive_visionfive2.c| 43 +--
> 1 file changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> b/board/starfive/visionfive2/starfive_visionfive2.c
> index 78e118d5a05..9970e309690 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -17,6 +18,8 @@
> DECLARE_GLOBAL_DATA_PTR;
> #define JH7110_L2_PREFETCHER_BASE_ADDR 0x203
> #define JH7110_L2_PREFETCHER_HART_OFFSET 0x2000
> +#define FDTFILE_MILK_V_MARS \
> + "starfive/jh7110-milkv-mars.dtb"
> #define FDTFILE_VISIONFIVE2_1_2A \
> "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb"
> #define FDTFILE_VISIONFIVE2_1_3B \
> @@ -48,20 +51,34 @@ static void set_fdtfile(void)
> {
> u8 version;
> const char *fdtfile;
> + const char *product_id;
>
> - version = get_pcb_revision_from_eeprom();
> - switch (version) {
> - case 'a':
> - case 'A':
> - fdtfile = FDTFILE_VISIONFIVE2_1_2A;
> - break;
> -
> - case 'b':
> - case 'B':
> - default:
> - fdtfile = FDTFILE_VISIONFIVE2_1_3B;
> - break;
> - };
> + product_id = get_product_id_from_eeprom();
> + if (!product_id) {
> + log_err("Can't read EEPROM\n");
> + return;
> + }
> + if (!strncmp(product_id, "MARS", 4)) {
> + fdtfile = FDTFILE_MILK_V_MARS;
> + } else if (!strncmp(product_id, "VF7110", 6)) {
> + version = get_pcb_revision_from_eeprom();
> +
> + switch (version) {
> + case 'a':
> + case 'A':
> + fdtfile = FDTFILE_VISIONFIVE2_1_2A;
> + break;
> +
> + case 'b':
> + case 'B':
> + default:
> + fdtfile = FDTFILE_VISIONFIVE2_1_3B;
> + break;
> + }
> + } else {
> + log_err("Unknown product\n");
> + return;
> + }
>
> env_set("fdtfile", fdtfile);
> }
> --
> 2.43.0
>
>
Re: [RFC 5/5] doc: describe Milk-V Mars board
Yes this reference to GPIO high/low states is clearer to understand. Have you tested the DIP switch functionality to confirm?; It is not shown in the MilkV documentation or outdated schematics and I don't have a Mars to test. I did find function descriptions from what is likely cut-and-paste of VisionFive2 board reference. Ref: https://github.com/milkv-mars/mars-files/blob/main/Mars_hardware_schematics/Mars_V1.11_20230821.pdf Sheet 7 of 22 JH7110 GPIOs There is a schematic for SW2 (bootloader button?) that lists an inset table: * - GPIO_1 - GPIO_0 - Boot * - 0 - 0 - Flash * - 0 - 1 - SD * - 1 - 0 - eMMC * - 1 - 1 - UART That circuit on SW2 appears to pull high both RGPIO_1 and RGPIO_0 with transistors. Again, no DIP switch as this is an earlier revision. Ref: https://doc-en.rvspace.org/VisionFive2/Developer_Guide/JH7110_Boot_UG.pdf page 9 table 1-4: RGPIO1=0x0 RGPIO0=0x0 Boot Source: Quad SPI NOR flash memory, Read SPL from Sector 0. RGPIO1=0x1 RGPIO0=0x1 Boot Source: UART0, (description of UART Xmodem function). Following in the same document on page 13 figure 2-1 Boot Flow: JH7110 supports the following boot devices. QSPI Flash (For SPL + OpenSBI + U-Boot) + NVMe/SD Card/eMMC (For Kernel + Fi le System and later) Note: System will detect in sequence whether it can boot from the following device sequence: NVMe > SD > eMMC. For example, if the boot program is found on the SD, eMMC will be ignored. Again in this document Figure 4-2 on page 17 is a visual listing of the DIP switch positions for QSPI, SDIO, eMMC, and UART boot modes, of the VisionFive2 board. The only consistent physical interface over VisionFive2, Mars, Star64, Mars CM all is RGPIO1=L RGPIO0=L SPI and RGPIO1=H RGPIO1=H UART; either by DIP switch or pushbutton attached circuit. So, I question our assumptions about what the actual behavior is for RGPIO1=H RGPIO0=L pairing and RGPIO1=L RGPIO0=H, and in what circumstance would there be followed a device sequence as suggested by the JH7110 reference. Why does the StarFive documentation list a JH7110 boot device sequence if there is also these H+L or L+H pairings to choose the device? On Thu, Mar 7, 2024 at 6:37 PM Heinrich Schuchardt < heinrich.schucha...@canonical.com> wrote: > On 3/8/24 00:20, E Shattow wrote: > > > > On Sun, Mar 3, 2024 at 5:02 AM Heinrich Schuchardt > > > <mailto:heinrich.schucha...@canonical.com>> wrote: > > ... > > > > +The board provides the DIP switches MSEL[1:0] to select the boot > > device out of > > +SPI flash, eMMC, SD-card, UART. To select booting from SD-card set > > the DIP > > +switches MSEL[1:0] to 10. > > > > > > This does not match the [Milk-V Mars vendor > > documentation](https://milkv.io/docs/mars/getting-started/bootloader > > <https://milkv.io/docs/mars/getting-started/bootloader>). Maybe you > have > > a different board revision? > > Thank you for reviewing. > > My board revision is V1.21 according to the silk screen. > > The Milk-V Mars has DIP switches for the boot selection as shown in > https://gist.github.com/xypron/e28f95b1ed6911aeb9699ba63ae1a885 > > If you look at the photo > > https://milkv.io/assets/images/mars-icon-04-e8814f18158a0e9d4387f4fa330693f1.webp > in the https://milkv.io/mars page, it also shows DIP switches (in the > SPI-Flash position) on a rev 1.2 board. > > Did you see a board without DIP switches being sold? > > The silk screen markings on the board and the switch don't match. > The same confusion exists on the VisionFive2. > > So maybe I should better write in a table: > > SPI-Flash: > GPIO0=L > GPIO1=L > > SD-Card: > GPIO0=H > GPIO1=L > > eMMC: > GPIO0=L > GPIO1=H > > UART: > GPIO0=H > GPIO1=H > > Best regards > > Heinrich >
Re: [RFC 3/5] board: starfive: support Milk-V Mars board
P.S. Found the longer description at https://github.com/milkv-mars/mars-buildroot-sdk/commit/d381610c92827de01b25843786012351b3f35519 as follows: "if configured as 24c04, its address will occupy 0x51, conflicting with the RTC chip pcf85063 on the IO-Board. Refer to: Documentation/misc-devices/eeprom.rst". What does that even mean, it is for Raspberry Pi Compute IO carrier only? On Thu, Mar 7, 2024 at 3:03 PM E Shattow wrote: > > On Sun, Mar 3, 2024 at 5:02 AM Heinrich Schuchardt < > heinrich.schucha...@canonical.com> wrote: > ... > >> * The EEPROM is atmel,24c02 according to the vendor U-Boot. >> > ... > > The same vendor (Milk-V) U-Boot change is present on their branch for Mars > CM Lite (which despite the naming has more in common with Pine64 Star64 > than Mars); However on visual inspection the silkscreen markings are for > 24c04. See: [Photo flatbed scans of Mars CM Lite 4GB rev 1.01]( > https://github.com/geerlingguy/sbc-reviews/issues/22#issuecomment-1872478605). > Can this be determined at runtime, or I must trust that this is not a > wrongly marked chip? >
Re: [RFC 5/5] doc: describe Milk-V Mars board
On Sun, Mar 3, 2024 at 5:02 AM Heinrich Schuchardt < heinrich.schucha...@canonical.com> wrote: ... > +The board provides the DIP switches MSEL[1:0] to select the boot device > out of > +SPI flash, eMMC, SD-card, UART. To select booting from SD-card set the DIP > +switches MSEL[1:0] to 10. > This does not match the [Milk-V Mars vendor documentation]( https://milkv.io/docs/mars/getting-started/bootloader). Maybe you have a different board revision?
Re: [RFC 3/5] board: starfive: support Milk-V Mars board
On Sun, Mar 3, 2024 at 5:02 AM Heinrich Schuchardt < heinrich.schucha...@canonical.com> wrote: ... > * The EEPROM is atmel,24c02 according to the vendor U-Boot. > ... The same vendor (Milk-V) U-Boot change is present on their branch for Mars CM Lite (which despite the naming has more in common with Pine64 Star64 than Mars); However on visual inspection the silkscreen markings are for 24c04. See: [Photo flatbed scans of Mars CM Lite 4GB rev 1.01]( https://github.com/geerlingguy/sbc-reviews/issues/22#issuecomment-1872478605). Can this be determined at runtime, or I must trust that this is not a wrongly marked chip?
Re: riscv defconfig starfive visionfive2 env is nowhere, missing axp15060 regulator, misc vendor hacks for Milk-V Mars CM Lite
Aside: Mailing list archive mangles devicetree node names in the patch as if it were an email address to obscure. related linux dts axp15060 node naming updates: https://github.com/MichaIng/linux/commit/1419a6eeb6a8b834ce9631fc558a57fb634f6949 GMAC and eMMC functions depend on power regulation. The sus commit for ENV_IS_NOWHERE=y in the starfive_visionfive2 defconfig: https://github.com/u-boot/u-boot/commit/7d79bed00c9e02187a09e74e90c5bd5a927a6a61 Why should ENV_IS_NOWHERE=y and CONFIG_ENV_IS_IN_SPI_FLASH=y both survive a make oldconfig, I would think they will be mutually exclusive. Local testing make starfive_visionfive2, disable ENV_IS_NOWHERE, compile and flash to the board and saveenv works as expected. On Fri, Jan 26, 2024 at 4:19 AM Nam Cao wrote: > On Fri, 26 Jan 2024 02:53:45 -0800 E Shattow wrote: > > There is missing the AXP15060 power regulation in u-boot needed for > SDCard > > and networking. As it is happens to work for some variants and not > others, > > and not always. The AXP15060 driver exists in mainline Linux as a variant > > of AXP20x and there are corresponding devicetree entries of the StarFive > > JH7110-based VisionFive2 variants queued up (ref: *1 Patchwork series > > 806238). > > I sent a patch to add AXP15060 power regulation device tree node: > https://lists.denx.de/pipermail/u-boot/2024-January/543614.html > > However, I sent it because it is needed by OpenSBI to reboot/shutdown. I > was not aware that U-Boot also needs this. > > Best regards, > Nam > >
riscv defconfig starfive visionfive2 env is nowhere, missing axp15060 regulator, misc vendor hacks for Milk-V Mars CM Lite
Hi, ENV_IS_NOWHERE=y when it should not be, on make starfive_visionfive2_defconfig. I guess that riscv is forgotten in some of the logic there. TL;DR the env is supposed to save in SPI flash as the defconfig provides (but then also ENV_IS_NOWHERE=y appears ? strange). There is missing the AXP15060 power regulation in u-boot needed for SDCard and networking. As it is happens to work for some variants and not others, and not always. The AXP15060 driver exists in mainline Linux as a variant of AXP20x and there are corresponding devicetree entries of the StarFive JH7110-based VisionFive2 variants queued up (ref: *1 Patchwork series 806238). Milk-V Mars CM Lite variant ships with a hack substituting GPIO22 MIPI_PWR_EN in dts (what would be GPIO62 SDIO0_RSTN_GPIO62) as a trick to get mmc0 doing something. I don't understand the details of why it works just that it was there in the SDK git repo change log and I see results. There's a possibility this was actually changed in how it is routed on the Mars CM Lite PCB but I find it would be more likely this is just some kind of hack. ^1: https://patchwork.kernel.org/project/linux-riscv/list/?series=806238&state=%2A&archive=both The other notable vendor Milk-V Mars hacks are to specify an eeprom 24c02 as compatible on the Mars CM Lite board instead of the 24c04 what is on the StarFive VisionFive 2. I can't find references to this anymore in mainline Linux, was it abstracted in some way that should be also done for u-boot? Finally there is possibly a 2GB RAM version of the Milk-V Mars which conflicts the StarFive VisionFive2 dts assumptions of at least 4GB RAM. I note some adjustments to the linux,cma dts node accordingly from vendor SDK. I failed to reproduce the vendor SDK build and this is my best guess from that SDK git repo commit log notes.
Re: booti usage offset requirements for compressed kernel and initrd with dtb
Hi Simon, "If this [fdt_high environment variable] is set to the special value 0x (32-bit machines) or 0x (64-bit machines) then the fdt will not be copied at all on boot. For this to work it must reside in writable memory, have sufficient padding on the end of it for U-Boot to add the information it needs into it, and the memory must be accessible by the kernel. This usage is strongly discouraged however as it also stops U-Boot from ensuring the device tree starting address is properly aligned and a misaligned tree will cause OS failures." https://docs.u-boot.org/en/latest/usage/environment.html "The device tree blob (dtb) must be placed on an 8-byte boundary" https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arch/arm64/booting.rst No mention of dtb alignment requirement at the similar documentation for riscv64, but there is this mention: "The RISC-V kernel expects to be placed at a PMD boundary (2MB aligned for rv64 and 4MB aligned for rv32)." https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arch/riscv/boot.rst Perhaps then it is both a dtb alignment issue (not mentioned in the documentation?) and the spurious extra requirement after the transferred data length? If yes, how much is enough to pad after the end of devicetree blob? Thanks! E Shattow On Thu, Jan 4, 2024 at 8:02 AM Simon Glass wrote: > Hi, > > On Thu, Jan 4, 2024 at 5:19 AM E Shattow wrote: > > > > Hello, > > > > There is data corruption when using booti on a JH7110 (riscv64) board to > > load (over tftp or ymodem) a compressed kernel, compressed initrd, and > > device tree blob, when the data are too close together (even though not > > overlapping). In the adjustments I have tried it seems to be consistent > but > > not clear "why?" by how much for each adjustment is needed to observe the > > failure versus success. > > > > My question is simple enough, what are the intended offset requirements > for > > loading these data into memory for use with booti ? > > > > For example: > > > > setenv ip 192.168.2.216 > > setenv loadaddr 0x6000 # default from Milk-V vendor for Mars CM > product > > setenv kernel_uncomp_size 24193024 # uncompressed size of > > vmlinuz-5.15.0-gpu117 from file command information > > setenv kernel_comp_addr_r $loadaddr > > setenv kernel_addr_r 0x61712800 # $kernel_comp_addr_r + > $kernel_uncomp_size > > dhcp $kernel_addr_r > > $ip:mars-cm_debian-desktop_sdk-boot/vmlinuz-5.15.0-gpu117 > > setenv kernel_comp_size 0x$filesize > > setenv dtb_addr 0x62e25000 # $kernel_addr_r + $kernel_uncomp_size > > dhcp $dtb_addr > > > $ip:mars-cm_debian-desktop_sdk-boot/dtbs/starfive/jh7110-visionfive-v2.dtb > > setenv initrd_addr 0x62e31d7f # $dtb_addr + 0x$filesize > > dhcp $initrd_addr $ip:debian-installer-netboot/initrd.gz; setenv > > initrd_size 0x$filesize > > booti $kernel_addr_r $initrd_addr:$initrd_size $dtb_addr > > > > Waiting for root device ... > > > > (and nothing else, this is a failure) > > > > If 0x62e31d7f is replaced by 0x62e31ec6 then the boot is successful and > > dmesg from debian-installer shows the additional information: > > > > Freeing unused kernel image (initmem) memory: 2196K > > Run /init as init process > >with arguments: > > /init > >with environment: > > HOME=/ > > TERM=linux > > > > > > So in this the kernel boots and all works normally. Even if that address > is > > 0x62e31ec5 one byte earlier it is a failure. > > > > In other attempts I would change the ordering i.e. so the initrd is first > > at $loadaddr and then kernel and DTB, or DTB and kernel. If I learned > > anything it is that compressed initrd must be loaded to a span of memory > at > > least the size of its uncompressed length, and this is also true of the > > compressed kernel vmlinuz even though booti documentation vaguely infers > > that there is an area of memory specified for decompression - so a > > compressed kernel needs two times its uncompressed size is that correct? > > Also there's no hint anywhere if the device tree blob has special memory > > constraints but if it is not starting on some 16byte aligned address at > > least my superstition from observations in testing is that it is not > > successful... and now, where it ends (in the above example) and initrd > > begins seems arbitrary. > > > > Sure I could pad things out, they tend to work that way, but I would like > > to know "why?" > > I don't know why, but I imagine that Linux finds the initrd in one > case and not inthe other...does the kernel log give you any clues? > > Also, can you use U-Boot's 'md5sum' command or similar to check that > the initrd and DT are the same in each case? > > Also, you could use a FIT [1] rather than booti. > > Regards, > Simon > > [1] https://docs.u-boot.org/en/latest/usage/fit/index.html >
booti usage offset requirements for compressed kernel and initrd with dtb
Hello, There is data corruption when using booti on a JH7110 (riscv64) board to load (over tftp or ymodem) a compressed kernel, compressed initrd, and device tree blob, when the data are too close together (even though not overlapping). In the adjustments I have tried it seems to be consistent but not clear "why?" by how much for each adjustment is needed to observe the failure versus success. My question is simple enough, what are the intended offset requirements for loading these data into memory for use with booti ? For example: setenv ip 192.168.2.216 setenv loadaddr 0x6000 # default from Milk-V vendor for Mars CM product setenv kernel_uncomp_size 24193024 # uncompressed size of vmlinuz-5.15.0-gpu117 from file command information setenv kernel_comp_addr_r $loadaddr setenv kernel_addr_r 0x61712800 # $kernel_comp_addr_r + $kernel_uncomp_size dhcp $kernel_addr_r $ip:mars-cm_debian-desktop_sdk-boot/vmlinuz-5.15.0-gpu117 setenv kernel_comp_size 0x$filesize setenv dtb_addr 0x62e25000 # $kernel_addr_r + $kernel_uncomp_size dhcp $dtb_addr $ip:mars-cm_debian-desktop_sdk-boot/dtbs/starfive/jh7110-visionfive-v2.dtb setenv initrd_addr 0x62e31d7f # $dtb_addr + 0x$filesize dhcp $initrd_addr $ip:debian-installer-netboot/initrd.gz; setenv initrd_size 0x$filesize booti $kernel_addr_r $initrd_addr:$initrd_size $dtb_addr Waiting for root device ... (and nothing else, this is a failure) If 0x62e31d7f is replaced by 0x62e31ec6 then the boot is successful and dmesg from debian-installer shows the additional information: Freeing unused kernel image (initmem) memory: 2196K Run /init as init process with arguments: /init with environment: HOME=/ TERM=linux So in this the kernel boots and all works normally. Even if that address is 0x62e31ec5 one byte earlier it is a failure. In other attempts I would change the ordering i.e. so the initrd is first at $loadaddr and then kernel and DTB, or DTB and kernel. If I learned anything it is that compressed initrd must be loaded to a span of memory at least the size of its uncompressed length, and this is also true of the compressed kernel vmlinuz even though booti documentation vaguely infers that there is an area of memory specified for decompression - so a compressed kernel needs two times its uncompressed size is that correct? Also there's no hint anywhere if the device tree blob has special memory constraints but if it is not starting on some 16byte aligned address at least my superstition from observations in testing is that it is not successful... and now, where it ends (in the above example) and initrd begins seems arbitrary. Sure I could pad things out, they tend to work that way, but I would like to know "why?" E Shattow (kindly reply-all off-list I am not subscriber)
