[PATCH v4 8/9] spl: starfive: star64: Setup USB fdt fixup function

[E Shattow]

Hi Minda,

On Wed, Aug 28, 2024 at 6:31 PM Minda Chen <minda.chen at starfivetech.com> wrote:
>
> Setup star64 USB fdt fixup function. Set dr_mode to host,
> and add vbus pin (GPIO25), and set USB 3.0 mode.
> the functions can be used by other 7110 board like Milk-V
> board.
>
> Signed-off-by: Minda Chen <minda.chen at starfivetech.com>
> ---
>  board/starfive/visionfive2/spl.c | 66 ++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>
> diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> index 388a06e4d9..b3034b19a3 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -123,6 +123,69 @@ static const struct starfive_vf2_pro star64_pine64[] = {
>                 "tx-internal-delay-ps", "300"},
>  };
>
> +static void spl_fdt_fixup_usb_vbus_pin(void *fdt, int pin)
> +{
> +       int offset, pin_offset;
> +
> +       offset = fdt_path_offset(fdt, "/soc/pinctrl at 13040000"); /* &sysgpio */
> +       fdt_add_subnode(fdt, offset, "usb0-0");
> +       fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> +                          "usb_pins", "/soc/pinctrl at 13040000/usb0-0");
> +       offset = fdt_path_offset(fdt, "/soc/pinctrl at 13040000/usb0-0");
> +
> +       /* usb_pins */
> +       fdt_create_phandle(fdt, offset);
> +       fdt_add_subnode(fdt, offset, "driver-vbus-pin");
> +       offset = fdt_path_offset(fdt, "/soc/pinctrl at 13040000/usb0-0/driver-vbus-pin");
> +       /* GPIOMUX(25, GPOUT_SYS_USB_DRIVE_VBUS, GPOEN_ENABLE, GPI_NONE) */

This comment may now be updated:

/* GPIOMUX(pin, GPOUT_SYS_USB_DRIVE_VBUS, GPOEN_ENABLE, GPI_NONE) */

Aside I am confused why we do not use the GPIOMUX macro directly here.
Can anyone say if that would be a problem to just use the macro
GPIOMUX what we are pretending to be here?

I think it will not be a problem here anymore when OF_UPSTREAM is
implemented to jh7110 boards in U-Boot so it is enough now to update
the comment about GPIOMUX.

> +       fdt_setprop_u32(fdt, offset, "pinmux", (0xff07 << 16) | pin);
> +       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 at 10100000"); /* &usb0 */
> +       fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
> +       pin_offset = fdt_path_offset(fdt, "/soc/pinctrl at 13040000/usb0-0");
> +       fdt_setprop_u32(fdt, offset, "pinctrl-0",
> +                       fdt_get_phandle(fdt, pin_offset));
> +}
> +
> +static void spl_fdt_fixup_usb_host(void *fdt)
> +{
> +       int offset;
> +
> +       offset = fdt_path_offset(fdt, "/soc/usb at 10100000/usb at 0"); /*&usb_cdns3 */
> +       fdt_setprop_string(fdt, offset, "dr_mode", "host");
> +}
> +
> +static void spl_fdt_fixup_set_usb3(void *fdt)
> +{
> +       int offset, phy_offset;
> +
> +       /* disable pcie0 */
> +       offset = fdt_path_offset(fdt, "/soc/pcie at 2b000000"); /* &pcie0 */
> +       fdt_setprop_string(fdt, offset, "status", "disabled");
> +
> +       offset = fdt_path_offset(fdt, "/soc/phy at 10210000"); /* &pciephy0 */
> +       fdt_setprop_u32(fdt, offset, "starfive,sys-syscon", /* syscon */
> +                       fdt_get_phandle(fdt,
> +                                       fdt_path_offset(fdt, "/soc/sys_syscon at 13030000")));
> +       fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18); /* append reg offset */
> +       fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> +                       fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/stg_syscon at 10240000")));
> +       /* append reg offset */
> +       fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148);
> +       fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4);
> +
> +       offset = fdt_path_offset(fdt, "/soc/usb at 10100000/usb at 0"); /* usb_cdns3 */
> +       phy_offset = fdt_path_offset(fdt, "/soc/phy at 10210000"); /* <&pciephy0> */
> +       /* append <&pciephy0> */
> +       fdt_appendprop_u32(fdt, offset, "phys", fdt_get_phandle(fdt, phy_offset));
> +       fdt_setprop(fdt, offset, "phy-names", "cdns3,usb2-phy\0cdns3,usb3-phy",
> +                   sizeof("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 13010000.spi: couldn't determine
trigger-address
[   11.730479] pcie-starfive 2c000000.pcie: error -ENODEV: failed to
get valid pcie domain
[   11.734112] cadence-qspi 13010000.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 <lucent at gmail.com>
Tested-by: E Shattow <lucent at gmail.com>