diff mbox series

[v3,7/8] dts: starfive: Add JH7110 Cadence USB dts node

Message ID 20240719013822.101374-8-minda.chen@starfivetech.com
State Superseded
Delegated to: Marek Vasut
Headers show
Series Add Starfive JH7110 Cadence USB driver | expand

Commit Message

Minda Chen July 19, 2024, 1:38 a.m. UTC
Add Jh7110 Cadence USB dts node, Visionfive2 default setting
is USB 2.0 device.

Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
---
 .../dts/jh7110-starfive-visionfive-2.dtsi     |  5 ++
 arch/riscv/dts/jh7110.dtsi                    | 52 +++++++++++++++++++
 2 files changed, 57 insertions(+)

Comments

E Shattow July 21, 2024, 1:47 a.m. UTC | #1
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 <minda.chen@starfivetech.com> wrote:
>
> Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> is USB 2.0 device.
>
> Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> ---
>  .../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@10100000 {
> +                       compatible = "starfive,jh7110-usb";
> +                       ranges = <0x0 0x0 0x10100000 0x100000>;
> +                       #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 0x10000>,
> +                                     <0x10000 0x10000>,
> +                                     <0x20000 0x10000>;
> +                               reg-names = "otg", "xhci", "dev";
> +                               interrupts = <100>, <108>, <110>;
> +                               interrupt-names = "host", "peripheral", "otg";
> +                               phys = <&usbphy0>;
> +                               phy-names = "cdns3,usb2-phy";
> +                       };
> +               };
> +
> +               usbphy0: phy@10200000 {
> +                       compatible = "starfive,jh7110-usb-phy";
> +                       reg = <0x0 0x10200000 0x0 0x10000>;
> +                       clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
> +                                <&stgcrg JH7110_STGCLK_USB_APP_125>;
> +                       clock-names = "125m", "app_125m";
> +                       #phy-cells = <0>;
> +               };
> +
> +               pciephy0: phy@10210000 {
> +                       compatible = "starfive,jh7110-pcie-phy";
> +                       reg = <0x0 0x10210000 0x0 0x10000>;
> +                       #phy-cells = <0>;
> +               };
> +
> +               pciephy1: phy@10220000 {
> +                       compatible = "starfive,jh7110-pcie-phy";
> +                       reg = <0x0 0x10220000 0x0 0x10000>;
> +                       #phy-cells = <0>;
> +               };
> +
>                 stgcrg: clock-controller@10230000 {
>                         compatible = "starfive,jh7110-stgcrg";
>                         reg = <0x0 0x10230000 0x0 0x10000>;
> --
> 2.17.1
>

Access fault

        starting USB...
        Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0
        scanning bus usb@0 for devices... Unhandled exception: Load access fault
        EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL: 0000000000000004
        EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /* &sysgpio */
        fdt_add_subnode(fdt, offset, "usb0-0");
        fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
"usb_pins", "/soc/pinctrl@13040000/usb0-0");
        offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@10100000"); /* &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@13040000/usb0-0")));
        fdt_setprop_string(fdt, offset, "status", "okay");

        offset = fdt_path_offset(fdt, "/soc/usb@10100000/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@13040000"); /* &sysgpio */
        fdt_add_subnode(fdt, offset, "usb0-0");
        fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
"usb_pins", "/soc/pinctrl@13040000/usb0-0");
        offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0 */
        fdt_setprop_string(fdt, offset, "status", "disabled");

        offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /* &pciephy0 */
        fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
        fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18);
/* append <magic number> */
        fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
        fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148);
/* append <magic number> */
        fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4);
/* append <magic number> */
        fdt_setprop_string(fdt, offset, "status", "okay");

        offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &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@13040000/usb0-0")));
        fdt_setprop_string(fdt, offset, "status", "okay");

        offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
&usb_cdns3 */
        fdt_setprop_u32(fdt,    offset, "phys",
fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10200000"))); /* =
<&usbphy0> */
        fdt_appendprop_u32(fdt, offset, "phys",
fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10210000"))); /*
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
E Shattow July 21, 2024, 6:27 p.m. UTC | #2
On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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 <minda.chen@starfivetech.com> wrote:
> >
> > Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> > is USB 2.0 device.
> >
> > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > ---
> >  .../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@10100000 {
> > +                       compatible = "starfive,jh7110-usb";
> > +                       ranges = <0x0 0x0 0x10100000 0x100000>;
> > +                       #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 0x10000>,
> > +                                     <0x10000 0x10000>,
> > +                                     <0x20000 0x10000>;
> > +                               reg-names = "otg", "xhci", "dev";
> > +                               interrupts = <100>, <108>, <110>;
> > +                               interrupt-names = "host", "peripheral", "otg";
> > +                               phys = <&usbphy0>;
> > +                               phy-names = "cdns3,usb2-phy";
> > +                       };
> > +               };
> > +
> > +               usbphy0: phy@10200000 {
> > +                       compatible = "starfive,jh7110-usb-phy";
> > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > +                       clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
> > +                                <&stgcrg JH7110_STGCLK_USB_APP_125>;
> > +                       clock-names = "125m", "app_125m";
> > +                       #phy-cells = <0>;
> > +               };
> > +
> > +               pciephy0: phy@10210000 {
> > +                       compatible = "starfive,jh7110-pcie-phy";
> > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > +                       #phy-cells = <0>;
> > +               };
> > +
> > +               pciephy1: phy@10220000 {
> > +                       compatible = "starfive,jh7110-pcie-phy";
> > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > +                       #phy-cells = <0>;
> > +               };
> > +
> >                 stgcrg: clock-controller@10230000 {
> >                         compatible = "starfive,jh7110-stgcrg";
> >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > --
> > 2.17.1
> >
>
> Access fault
>
>         starting USB...
>         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0
>         scanning bus usb@0 for devices... Unhandled exception: Load access fault
>         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL: 0000000000000004
>         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /* &sysgpio */
>         fdt_add_subnode(fdt, offset, "usb0-0");
>         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> "usb_pins", "/soc/pinctrl@13040000/usb0-0");
>         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@10100000"); /* &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@13040000/usb0-0")));
>         fdt_setprop_string(fdt, offset, "status", "okay");
>
>         offset = fdt_path_offset(fdt, "/soc/usb@10100000/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@13040000"); /* &sysgpio */
>         fdt_add_subnode(fdt, offset, "usb0-0");
>         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> "usb_pins", "/soc/pinctrl@13040000/usb0-0");
>         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0 */
>         fdt_setprop_string(fdt, offset, "status", "disabled");
>
>         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /* &pciephy0 */
>         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
>         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18);
> /* append <magic number> */
>         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
>         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148);
> /* append <magic number> */
>         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4);
> /* append <magic number> */
>         fdt_setprop_string(fdt, offset, "status", "okay");
>
>         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &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@13040000/usb0-0")));
>         fdt_setprop_string(fdt, offset, "status", "okay");
>
>         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
> &usb_cdns3 */
>         fdt_setprop_u32(fdt,    offset, "phys",
> fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10200000"))); /* =
> <&usbphy0> */
>         fdt_appendprop_u32(fdt, offset, "phys",
> fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10210000"))); /*
> 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

P.S. here is with debug logging:

drivers/core/ofnode.c:517-    ofnode_read_prop() ofnode_read_prop:
assigned-clock-rates: <not found>
drivers/core/uclass.c:537-uclass_get_device_by_ofnode() Looking for phy@10200000
drivers/core/uclass.c:388-uclass_find_device_by_ofnode() Looking for
phy@10200000
drivers/core/uclass.c:407-uclass_find_device_by_ofnode()    - result
for phy@10200000: phy@10200000 (ret=0)
drivers/core/uclass.c:540-uclass_get_device_by_ofnode()    - result
for phy@10200000: phy@10200000 (ret=0)
drivers/core/ofnode.c:517-    ofnode_read_prop() ofnode_read_prop:
assigned-clock-rates: <not found>
drivers/core/uclass.c:537-uclass_get_device_by_ofnode() Looking for
clock-controller@13020000
drivers/core/uclass.c:388-uclass_find_device_by_ofnode() Looking for
clock-controller@13020000
drivers/core/uclass.c:407-uclass_find_device_by_ofnode()    - result
for clock-controller@13020000: clock-controller@13020000 (ret=0)
drivers/core/uclass.c:540-uclass_get_device_by_ofnode()    - result
for clock-controller@13020000: clock-controller@13020000 (ret=0)
drivers/core/uclass.c:537-uclass_get_device_by_ofnode() Looking for
clock-controller@10230000
drivers/core/uclass.c:388-uclass_find_device_by_ofnode() Looking for
clock-controller@10230000
drivers/core/uclass.c:407-uclass_find_device_by_ofnode()    - result
for clock-controller@10230000: clock-controller@10230000 (ret=0)
drivers/core/uclass.c:540-uclass_get_device_by_ofnode()    - result
for clock-controller@10230000: clock-controller@10230000 (ret=0)
common/event.c:113-      notify_dynamic() Sending event 5/(unknown) to
spy 'efi_disk add'
drivers/usb/cdns3/drd.c:287-      cdns3_drd_init() cdns-usb3-host
usb@0: DRD version v1 (ID: 0004024e, rev: 00000200)
drivers/usb/cdns3/drd.c:297-      cdns3_drd_init() cdns-usb3-host
usb@0: Controller strapped to HOST
drivers/core/ofnode.c:517-    ofnode_read_prop() ofnode_read_prop: dr_mode: host
drivers/phy/starfive/phy-jh7110-usb2.c:58-   usb2_phy_set_mode()
jh7110_usb2_phy phy@10200000: Changing phy to 1
drivers/usb/cdns3/core.c:304-cdns3_hw_role_switch() cdns-usb3-host
usb@0: Switching role 0 -> 1cdns-usb3-host usb@0: Waiting till Host
mode is turned on
drivers/usb/cdns3/core.c:309-cdns3_hw_role_switch() cdns-usb3-host
usb@0: set 1 has failed, back to 0
drivers/usb/cdns3/core.c:375-         cdns3_probe() cdns-usb3-host
usb@0: Cadence USB3 core: probe succeed
common/event.c:113-      notify_dynamic() Sending event 5/(unknown) to
spy 'efi_disk add'
drivers/core/ofnode.c:394-ofnode_read_u32_index()
ofnode_read_u32_index: companion: (not found)
scanning bus usb@0 for devices... Unhandled exception: Load access fault
EPC: 00000000fff7f50e RA: 00000000fff7f508 TVAL: 0000000000000004
EPC: 000000004024d50e RA: 000000004024d508 reloc adjusted

Code: 9863 3ee7 8526 f0ef c37f 651c 3a03 0105 (43dc)


resetting ...

-E
Minda Chen July 23, 2024, 1:29 a.m. UTC | #3
> 
> On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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 <minda.chen@starfivetech.com>
> wrote:
> > >
> > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting is USB
> > > 2.0 device.
> > >
> > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > > ---
> > >  .../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@10100000 {
> > > +                       compatible = "starfive,jh7110-usb";
> > > +                       ranges = <0x0 0x0 0x10100000 0x100000>;
> > > +                       #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 0x10000>,
> > > +                                     <0x10000 0x10000>,
> > > +                                     <0x20000 0x10000>;
> > > +                               reg-names = "otg", "xhci", "dev";
> > > +                               interrupts = <100>, <108>, <110>;
> > > +                               interrupt-names = "host",
> "peripheral", "otg";
> > > +                               phys = <&usbphy0>;
> > > +                               phy-names = "cdns3,usb2-phy";
> > > +                       };
> > > +               };
> > > +
> > > +               usbphy0: phy@10200000 {
> > > +                       compatible = "starfive,jh7110-usb-phy";
> > > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > > +                       clocks = <&syscrg
> JH7110_SYSCLK_USB_125M>,
> > > +                                <&stgcrg
> JH7110_STGCLK_USB_APP_125>;
> > > +                       clock-names = "125m", "app_125m";
> > > +                       #phy-cells = <0>;
> > > +               };
> > > +
> > > +               pciephy0: phy@10210000 {
> > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > > +                       #phy-cells = <0>;
> > > +               };
> > > +
> > > +               pciephy1: phy@10220000 {
> > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > > +                       #phy-cells = <0>;
> > > +               };
> > > +
> > >                 stgcrg: clock-controller@10230000 {
> > >                         compatible = "starfive,jh7110-stgcrg";
> > >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > > --
> > > 2.17.1
> > >
> >
> > Access fault
> >
> >         starting USB...
> >         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0
> >         scanning bus usb@0 for devices... Unhandled exception: Load
> access fault
> >         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL:
> 0000000000000004
> >         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /*
> &sysgpio */
> >         fdt_add_subnode(fdt, offset, "usb0-0");
> >         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> > "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> >         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@10100000"); /* &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@13040000/usb0-0")));
> >         fdt_setprop_string(fdt, offset, "status", "okay");
> >
> >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/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:
> >
I checked t Milk-V CM board do not contain USB3.0 host
So I think the USB 3.0 configuration is not required. 

> >         int offset;
> >
> >         offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /*
> &sysgpio */
> >         fdt_add_subnode(fdt, offset, "usb0-0");
> >         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> > "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> >         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0 */
> >         fdt_setprop_string(fdt, offset, "status", "disabled");
> >
> >         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /* &pciephy0
> */
> >         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
> >         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18);
> > /* append <magic number> */
> >         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
> >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148);
> > /* append <magic number> */
> >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4);
> > /* append <magic number> */
> >         fdt_setprop_string(fdt, offset, "status", "okay");
> >
> >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &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@13040000/usb0-0")));
> >         fdt_setprop_string(fdt, offset, "status", "okay");
> >
> >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
> > &usb_cdns3 */
> >         fdt_setprop_u32(fdt,    offset, "phys",
> > fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10200000"))); /* =
> > <&usbphy0> */
> >         fdt_appendprop_u32(fdt, offset, "phys", fdt_get_phandle(fdt,
> > fdt_path_offset(fdt, "/soc/phy@10210000"))); /* 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
> 
I don’t have a Milk-V CM board. So I just can test this code to Star64.
Thanks. I will test this and add the code to board_fixup_star64().

> P.S. here is with debug logging:
> 
> drivers/core/ofnode.c:517-    ofnode_read_prop() ofnode_read_prop:
> assigned-clock-rates: <not found>
> drivers/core/uclass.c:537-uclass_get_device_by_ofnode() Looking for
> phy@10200000
> drivers/core/uclass.c:388-uclass_find_device_by_ofnode() Looking for
> phy@10200000
> drivers/core/uclass.c:407-uclass_find_device_by_ofnode()    - result
> for phy@10200000: phy@10200000 (ret=0)
> drivers/core/uclass.c:540-uclass_get_device_by_ofnode()    - result
> for phy@10200000: phy@10200000 (ret=0)
> drivers/core/ofnode.c:517-    ofnode_read_prop() ofnode_read_prop:
> assigned-clock-rates: <not found>
> drivers/core/uclass.c:537-uclass_get_device_by_ofnode() Looking for
> clock-controller@13020000
> drivers/core/uclass.c:388-uclass_find_device_by_ofnode() Looking for
> clock-controller@13020000
> drivers/core/uclass.c:407-uclass_find_device_by_ofnode()    - result
> for clock-controller@13020000: clock-controller@13020000 (ret=0)
> drivers/core/uclass.c:540-uclass_get_device_by_ofnode()    - result
> for clock-controller@13020000: clock-controller@13020000 (ret=0)
> drivers/core/uclass.c:537-uclass_get_device_by_ofnode() Looking for
> clock-controller@10230000
> drivers/core/uclass.c:388-uclass_find_device_by_ofnode() Looking for
> clock-controller@10230000
> drivers/core/uclass.c:407-uclass_find_device_by_ofnode()    - result
> for clock-controller@10230000: clock-controller@10230000 (ret=0)
> drivers/core/uclass.c:540-uclass_get_device_by_ofnode()    - result
> for clock-controller@10230000: clock-controller@10230000 (ret=0)
> common/event.c:113-      notify_dynamic() Sending event 5/(unknown) to
> spy 'efi_disk add'
> drivers/usb/cdns3/drd.c:287-      cdns3_drd_init() cdns-usb3-host
> usb@0: DRD version v1 (ID: 0004024e, rev: 00000200)
> drivers/usb/cdns3/drd.c:297-      cdns3_drd_init() cdns-usb3-host
> usb@0: Controller strapped to HOST
> drivers/core/ofnode.c:517-    ofnode_read_prop() ofnode_read_prop:
> dr_mode: host
> drivers/phy/starfive/phy-jh7110-usb2.c:58-   usb2_phy_set_mode()
> jh7110_usb2_phy phy@10200000: Changing phy to 1
> drivers/usb/cdns3/core.c:304-cdns3_hw_role_switch() cdns-usb3-host
> usb@0: Switching role 0 -> 1cdns-usb3-host usb@0: Waiting till Host mode is
> turned on
> drivers/usb/cdns3/core.c:309-cdns3_hw_role_switch() cdns-usb3-host
> usb@0: set 1 has failed, back to 0
> drivers/usb/cdns3/core.c:375-         cdns3_probe() cdns-usb3-host
> usb@0: Cadence USB3 core: probe succeed
> common/event.c:113-      notify_dynamic() Sending event 5/(unknown) to
> spy 'efi_disk add'
> drivers/core/ofnode.c:394-ofnode_read_u32_index()
> ofnode_read_u32_index: companion: (not found) scanning bus usb@0 for
> devices... Unhandled exception: Load access fault
> EPC: 00000000fff7f50e RA: 00000000fff7f508 TVAL: 0000000000000004
> EPC: 000000004024d50e RA: 000000004024d508 reloc adjusted
> 
> Code: 9863 3ee7 8526 f0ef c37f 651c 3a03 0105 (43dc)
> 
> 
> resetting ...
> 
> -E
E Shattow July 23, 2024, 3:53 a.m. UTC | #4
On Mon, Jul 22, 2024 at 6:29 PM Minda Chen <minda.chen@starfivetech.com> wrote:
>
>
>
> >
> > On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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 <minda.chen@starfivetech.com>
> > wrote:
> > > >
> > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting is USB
> > > > 2.0 device.
> > > >
> > > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > > > ---
> > > >  .../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@10100000 {
> > > > +                       compatible = "starfive,jh7110-usb";
> > > > +                       ranges = <0x0 0x0 0x10100000 0x100000>;
> > > > +                       #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 0x10000>,
> > > > +                                     <0x10000 0x10000>,
> > > > +                                     <0x20000 0x10000>;
> > > > +                               reg-names = "otg", "xhci", "dev";
> > > > +                               interrupts = <100>, <108>, <110>;
> > > > +                               interrupt-names = "host",
> > "peripheral", "otg";
> > > > +                               phys = <&usbphy0>;
> > > > +                               phy-names = "cdns3,usb2-phy";
> > > > +                       };
> > > > +               };
> > > > +
> > > > +               usbphy0: phy@10200000 {
> > > > +                       compatible = "starfive,jh7110-usb-phy";
> > > > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > > > +                       clocks = <&syscrg
> > JH7110_SYSCLK_USB_125M>,
> > > > +                                <&stgcrg
> > JH7110_STGCLK_USB_APP_125>;
> > > > +                       clock-names = "125m", "app_125m";
> > > > +                       #phy-cells = <0>;
> > > > +               };
> > > > +
> > > > +               pciephy0: phy@10210000 {
> > > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > > > +                       #phy-cells = <0>;
> > > > +               };
> > > > +
> > > > +               pciephy1: phy@10220000 {
> > > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > > > +                       #phy-cells = <0>;
> > > > +               };
> > > > +
> > > >                 stgcrg: clock-controller@10230000 {
> > > >                         compatible = "starfive,jh7110-stgcrg";
> > > >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > > > --
> > > > 2.17.1
> > > >
> > >
> > > Access fault
> > >
> > >         starting USB...
> > >         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0
> > >         scanning bus usb@0 for devices... Unhandled exception: Load
> > access fault
> > >         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL:
> > 0000000000000004
> > >         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /*
> > &sysgpio */
> > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > >         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> > > "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > >         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@10100000"); /* &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@13040000/usb0-0")));
> > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > >
> > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/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:
> > >

> I checked t Milk-V CM board do not contain USB3.0 host
> So I think the USB 3.0 configuration is not required.

I agree it should be USB 2.0, but with your patch this is the only
configuration that does anything successful with USB2.0 or 3.0


>
> > >         int offset;
> > >
> > >         offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /*
> > &sysgpio */
> > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > >         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> > > "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > >         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0 */
> > >         fdt_setprop_string(fdt, offset, "status", "disabled");
> > >
> > >         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /* &pciephy0
> > */
> > >         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
> > >         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18);
> > > /* append <magic number> */
> > >         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
> > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148);
> > > /* append <magic number> */
> > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4);
> > > /* append <magic number> */
> > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > >
> > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &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@13040000/usb0-0")));
> > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > >
> > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
> > > &usb_cdns3 */
> > >         fdt_setprop_u32(fdt,    offset, "phys",
> > > fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10200000"))); /* =
> > > <&usbphy0> */
> > >         fdt_appendprop_u32(fdt, offset, "phys", fdt_get_phandle(fdt,
> > > fdt_path_offset(fdt, "/soc/phy@10210000"))); /* 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
> >

> I don’t have a Milk-V CM board. So I just can test this code to Star64.
> Thanks. I will test this and add the code to board_fixup_star64().

Can you try to configure in USB2.0-only mode on Star64?  Do you see
the same problem with "load access fault"?

...snip...

Same devicetree fixup code but separated some of the statements to
multiple lines:

https://paste.debian.net/1324036/

```
diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
index b794b73b6b..b4b0930e75 100644
--- a/board/starfive/visionfive2/spl.c
+++ b/board/starfive/visionfive2/spl.c
@@ -123,6 +123,53 @@ static const struct starfive_vf2_pro star64_pine64[] = {
  "tx-internal-delay-ps", "300"},
 };

+void spl_fdt_fixup_jh7110_cadence_usb3_host(void *fdt)
+{
+ int offset;
+ u32 phandle;
+
+ offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /* &sysgpio */
+ fdt_add_subnode(fdt, offset, "usb0-0");
+ fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
"usb_pins", "/soc/pinctrl@13040000/usb0-0");
+ offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0 */
+ fdt_setprop_string(fdt, offset, "status", "disabled");
+
+ offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /* &pciephy0 */
+ phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/sys_syscon@13030000")); /* = <&sys_syscon> */
+ fdt_setprop_u32(fdt, offset, "starfive,sys-syscon", phandle);
+ fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18); /*
append <magic number> */
+ phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/stg_syscon@10240000")); /* = <&stg_syscon> */
+ fdt_setprop_u32(fdt, offset, "starfive,stg-syscon", phandle);
+ fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148); /*
append <magic number> */
+ fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4); /*
append <magic number> */
+ fdt_setprop_string(fdt, offset, "status", "okay");
+
+ offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &usb0 */
+ fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
+ phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/pinctrl@13040000/usb0-0")); /* <&usb_pins> */
+ fdt_setprop_u32(fdt, offset, "pinctrl-0", phandle);
+ fdt_setprop_string(fdt, offset, "status", "okay");
+
+ offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /* &usb_cdns3 */
+ phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/phy@10200000")); /* = <&usbphy0> */
+ fdt_setprop_u32(fdt, offset, "phys", phandle);
+ phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/phy@10210000")); /* append <&pciephy0> */
+ fdt_appendprop_u32(fdt, offset, "phys", phandle);
+ fdt_setprop_string(fdt, offset, "phy-names", "cdns3,usb2-phy");
+ fdt_appendprop_string(fdt, offset, "phy-names", "cdns3,usb3-phy");
+ fdt_setprop_string(fdt, offset, "dr_mode", "host");
+}
+
 void spl_fdt_fixup_mars(void *fdt)
 {
  static const char compat[] = "milkv,mars\0starfive,jh7110";
@@ -335,6 +382,8 @@ void spl_fdt_fixup_star64(void *fdt)
  break;
  }
  }
+
+ spl_fdt_fixup_jh7110_cadence_usb3_host(fdt);
 }

 void spl_perform_fixups(struct spl_image_info *spl_image)
```

I think there is missing some of the USB2.0-only logic in your patch.
Can you try some more testing to understand why this fault happens
here?

Thanks,

-E
Minda Chen July 23, 2024, 10:16 a.m. UTC | #5
> 
> On Mon, Jul 22, 2024 at 6:29 PM Minda Chen <minda.chen@starfivetech.com>
> wrote:
> >
> >
> >
> > >
> > > On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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
> > > > <minda.chen@starfivetech.com>
> > > wrote:
> > > > >
> > > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting is
> > > > > USB
> > > > > 2.0 device.
> > > > >
> > > > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > > > > ---
> > > > >  .../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@10100000 {
> > > > > +                       compatible = "starfive,jh7110-usb";
> > > > > +                       ranges = <0x0 0x0 0x10100000 0x100000>;
> > > > > +                       #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 0x10000>,
> > > > > +                                     <0x10000 0x10000>,
> > > > > +                                     <0x20000 0x10000>;
> > > > > +                               reg-names = "otg", "xhci",
> "dev";
> > > > > +                               interrupts = <100>, <108>,
> <110>;
> > > > > +                               interrupt-names = "host",
> > > "peripheral", "otg";
> > > > > +                               phys = <&usbphy0>;
> > > > > +                               phy-names = "cdns3,usb2-phy";
> > > > > +                       };
> > > > > +               };
> > > > > +
> > > > > +               usbphy0: phy@10200000 {
> > > > > +                       compatible = "starfive,jh7110-usb-phy";
> > > > > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > > > > +                       clocks = <&syscrg
> > > JH7110_SYSCLK_USB_125M>,
> > > > > +                                <&stgcrg
> > > JH7110_STGCLK_USB_APP_125>;
> > > > > +                       clock-names = "125m", "app_125m";
> > > > > +                       #phy-cells = <0>;
> > > > > +               };
> > > > > +
> > > > > +               pciephy0: phy@10210000 {
> > > > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > > > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > > > > +                       #phy-cells = <0>;
> > > > > +               };
> > > > > +
> > > > > +               pciephy1: phy@10220000 {
> > > > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > > > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > > > > +                       #phy-cells = <0>;
> > > > > +               };
> > > > > +
> > > > >                 stgcrg: clock-controller@10230000 {
> > > > >                         compatible = "starfive,jh7110-stgcrg";
> > > > >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > > > Access fault
> > > >
> > > >         starting USB...
> > > >         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0
> > > >         scanning bus usb@0 for devices... Unhandled exception:
> > > > Load
> > > access fault
> > > >         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL:
> > > 0000000000000004
> > > >         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /*
> > > &sysgpio */
> > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > >         offset = fdt_path_offset(fdt,
> > > > "/soc/pinctrl@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@13040000/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@10100000"); /* &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@13040000/usb0-0")));
> > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > >
> > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/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:
> > > >
> 
> > I checked t Milk-V CM board do not contain USB3.0 host So I think the
> > USB 3.0 configuration is not required.
> 
> I agree it should be USB 2.0, but with your patch this is the only configuration
> that does anything successful with USB2.0 or 3.0
> 
> 
> >
> > > >         int offset;
> > > >
> > > >         offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /*
> > > &sysgpio */
> > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > >         offset = fdt_path_offset(fdt,
> > > > "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0
> */
> > > >         fdt_setprop_string(fdt, offset, "status", "disabled");
> > > >
> > > >         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /*
> > > > &pciephy0
> > > */
> > > >         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
> > > >         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > 0x18);
> > > > /* append <magic number> */
> > > >         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
> > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > 0x148);
> > > > /* append <magic number> */
> > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > 0x1f4);
> > > > /* append <magic number> */
> > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > >
> > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &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@13040000/usb0-0")));
> > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > >
> > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0");
> > > > /*
> > > > &usb_cdns3 */
> > > >         fdt_setprop_u32(fdt,    offset, "phys",
> > > > fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10200000")));
> > > > /* = <&usbphy0> */
> > > >         fdt_appendprop_u32(fdt, offset, "phys",
> > > > fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10210000"))); /*
> 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
> > >
> 
> > I don’t have a Milk-V CM board. So I just can test this code to Star64.
> > Thanks. I will test this and add the code to board_fixup_star64().
> 
> Can you try to configure in USB2.0-only mode on Star64?  Do you see the same
> problem with "load access fault"?
> 
Yes ,USB 2.0 only can work.
> ...snip...
> 
> Same devicetree fixup code but separated some of the statements to multiple
> lines:
> 
> https://paste.debian.net/1324036/
> 
> ```
Now I can see the issue,Usb 2.0 host can work in Milk-V CM, but PCIe can not work.
Is that correct? 
The PCIe in CM board is PCIe0, The PCIe1 is not used. So I think
disable PCIe1 node and just set USB 2.0 to host is OK. 
milkV CM PCIe0 setting is the same with VisionFive v2.
The USB 3.0 setting should not be added. 

This USB 3.0 setting. Should NOT be added to Milk-V CM board.
&pciephy0 {
	starfive,sys-syscon = <&sys_syscon 0x18>;
	starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
	status = "okay";
};

&usb_cdns3 {
	phys = <&usbphy0>, <&pciephy0>;
	phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
	dr_mode = "host";
	status = "okay";
};

> diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> index b794b73b6b..b4b0930e75 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -123,6 +123,53 @@ static const struct starfive_vf2_pro star64_pine64[] = {
>   "tx-internal-delay-ps", "300"},
>  };
> 
> +void spl_fdt_fixup_jh7110_cadence_usb3_host(void *fdt) {  int offset;
> + u32 phandle;
> +
> + offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /* &sysgpio */
> + fdt_add_subnode(fdt, offset, "usb0-0"); fdt_setprop_string(fdt,
> + fdt_path_offset(fdt, "/__symbols__"),
> "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> + offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0 */
> + fdt_setprop_string(fdt, offset, "status", "disabled");
> +
> + offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /* &pciephy0 */
> + phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/sys_syscon@13030000")); /* = <&sys_syscon> */
> + fdt_setprop_u32(fdt, offset, "starfive,sys-syscon", phandle);
> + fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon", 0x18); /*
> append <magic number> */
> + phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/stg_syscon@10240000")); /* = <&stg_syscon> */
> + fdt_setprop_u32(fdt, offset, "starfive,stg-syscon", phandle);
> + fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x148); /*
> append <magic number> */
> + fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon", 0x1f4); /*
> append <magic number> */
> + fdt_setprop_string(fdt, offset, "status", "okay");
> +
> + offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &usb0 */
> + fdt_setprop_string(fdt, offset, "pinctrl-names", "default"); phandle =
> + fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/pinctrl@13040000/usb0-0")); /* <&usb_pins> */
> + fdt_setprop_u32(fdt, offset, "pinctrl-0", phandle);
> + fdt_setprop_string(fdt, offset, "status", "okay");
> +
> + offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
> + &usb_cdns3 */ phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/phy@10200000")); /* = <&usbphy0> */
> + fdt_setprop_u32(fdt, offset, "phys", phandle); phandle =
> + fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/phy@10210000")); /* append <&pciephy0> */
> + fdt_appendprop_u32(fdt, offset, "phys", phandle);
> +fdt_setprop_string(fdt, offset, "phy-names", "cdns3,usb2-phy");
> +fdt_appendprop_string(fdt, offset, "phy-names", "cdns3,usb3-phy");
> +fdt_setprop_string(fdt, offset, "dr_mode", "host"); }
> +
>  void spl_fdt_fixup_mars(void *fdt)
>  {
>   static const char compat[] = "milkv,mars\0starfive,jh7110"; @@ -335,6
> +382,8 @@ void spl_fdt_fixup_star64(void *fdt)
>   break;
>   }
>   }
> +
> + spl_fdt_fixup_jh7110_cadence_usb3_host(fdt);
>  }
> 
>  void spl_perform_fixups(struct spl_image_info *spl_image) ```
> 
> I think there is missing some of the USB2.0-only logic in your patch.
> Can you try some more testing to understand why this fault happens here?
> 
> Thanks,
> 
> -E
E Shattow July 23, 2024, 1:05 p.m. UTC | #6
On Tue, Jul 23, 2024 at 3:16 AM Minda Chen <minda.chen@starfivetech.com> wrote:
>
>
>
> >
> > On Mon, Jul 22, 2024 at 6:29 PM Minda Chen <minda.chen@starfivetech.com>
> > wrote:
> > >
> > >
> > >
> > > >
> > > > On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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
> > > > > <minda.chen@starfivetech.com>
> > > > wrote:
> > > > > >
> > > > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting is
> > > > > > USB
> > > > > > 2.0 device.
> > > > > >
> > > > > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > > > > > ---
> > > > > >  .../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@10100000 {
> > > > > > +                       compatible = "starfive,jh7110-usb";
> > > > > > +                       ranges = <0x0 0x0 0x10100000 0x100000>;
> > > > > > +                       #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 0x10000>,
> > > > > > +                                     <0x10000 0x10000>,
> > > > > > +                                     <0x20000 0x10000>;
> > > > > > +                               reg-names = "otg", "xhci",
> > "dev";
> > > > > > +                               interrupts = <100>, <108>,
> > <110>;
> > > > > > +                               interrupt-names = "host",
> > > > "peripheral", "otg";
> > > > > > +                               phys = <&usbphy0>;
> > > > > > +                               phy-names = "cdns3,usb2-phy";
> > > > > > +                       };
> > > > > > +               };
> > > > > > +
> > > > > > +               usbphy0: phy@10200000 {
> > > > > > +                       compatible = "starfive,jh7110-usb-phy";
> > > > > > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > > > > > +                       clocks = <&syscrg
> > > > JH7110_SYSCLK_USB_125M>,
> > > > > > +                                <&stgcrg
> > > > JH7110_STGCLK_USB_APP_125>;
> > > > > > +                       clock-names = "125m", "app_125m";
> > > > > > +                       #phy-cells = <0>;
> > > > > > +               };
> > > > > > +
> > > > > > +               pciephy0: phy@10210000 {
> > > > > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > > > > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > > > > > +                       #phy-cells = <0>;
> > > > > > +               };
> > > > > > +
> > > > > > +               pciephy1: phy@10220000 {
> > > > > > +                       compatible = "starfive,jh7110-pcie-phy";
> > > > > > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > > > > > +                       #phy-cells = <0>;
> > > > > > +               };
> > > > > > +
> > > > > >                 stgcrg: clock-controller@10230000 {
> > > > > >                         compatible = "starfive,jh7110-stgcrg";
> > > > > >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > > > > > --
> > > > > > 2.17.1
> > > > > >
> > > > >
> > > > > Access fault
> > > > >
> > > > >         starting USB...
> > > > >         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0
> > > > >         scanning bus usb@0 for devices... Unhandled exception:
> > > > > Load
> > > > access fault
> > > > >         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL:
> > > > 0000000000000004
> > > > >         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /*
> > > > &sysgpio */
> > > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > > >         offset = fdt_path_offset(fdt,
> > > > > "/soc/pinctrl@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@13040000/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@10100000"); /* &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@13040000/usb0-0")));
> > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > >
> > > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/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:
> > > > >
> >
> > > I checked t Milk-V CM board do not contain USB3.0 host So I think the
> > > USB 3.0 configuration is not required.
> >
> > I agree it should be USB 2.0, but with your patch this is the only configuration
> > that does anything successful with USB2.0 or 3.0
> >
> >
> > >
> > > > >         int offset;
> > > > >
> > > > >         offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /*
> > > > &sysgpio */
> > > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > > >         offset = fdt_path_offset(fdt,
> > > > > "/soc/pinctrl@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@13040000/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@2b000000"); /* &pcie0
> > */
> > > > >         fdt_setprop_string(fdt, offset, "status", "disabled");
> > > > >
> > > > >         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /*
> > > > > &pciephy0
> > > > */
> > > > >         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
> > > > >         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > > 0x18);
> > > > > /* append <magic number> */
> > > > >         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
> > > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > 0x148);
> > > > > /* append <magic number> */
> > > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > 0x1f4);
> > > > > /* append <magic number> */
> > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > >
> > > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &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@13040000/usb0-0")));
> > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > >
> > > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0");
> > > > > /*
> > > > > &usb_cdns3 */
> > > > >         fdt_setprop_u32(fdt,    offset, "phys",
> > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10200000")));
> > > > > /* = <&usbphy0> */
> > > > >         fdt_appendprop_u32(fdt, offset, "phys",
> > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt, "/soc/phy@10210000"))); /*
> > 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
> > > >
> >
> > > I don’t have a Milk-V CM board. So I just can test this code to Star64.
> > > Thanks. I will test this and add the code to board_fixup_star64().
> >
> > Can you try to configure in USB2.0-only mode on Star64?  Do you see the same
> > problem with "load access fault"?
> >
> Yes ,USB 2.0 only can work.
> > ...snip...
> >
> > Same devicetree fixup code but separated some of the statements to multiple
> > lines:
> >
> > https://paste.debian.net/1324036/
> >
> > ```
> Now I can see the issue,Usb 2.0 host can work in Milk-V CM, but PCIe can not work.
> Is that correct?
> The PCIe in CM board is PCIe0, The PCIe1 is not used. So I think
> disable PCIe1 node and just set USB 2.0 to host is OK.
> milkV CM PCIe0 setting is the same with VisionFive v2.
> The USB 3.0 setting should not be added.
>
> This USB 3.0 setting. Should NOT be added to Milk-V CM board.
> &pciephy0 {
>         starfive,sys-syscon = <&sys_syscon 0x18>;
>         starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
>         status = "okay";
> };
>
> &usb_cdns3 {
>         phys = <&usbphy0>, <&pciephy0>;
>         phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
>         dr_mode = "host";
>         status = "okay";
> };
>

Here is Mars CM Lite:

StarFive # pci
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
Minda Chen July 24, 2024, 3:24 a.m. UTC | #7
> -----邮件原件-----
> 发件人: E Shattow <lucent@gmail.com>
> 发送时间: 2024年7月23日 21:06
> 收件人: Minda Chen <minda.chen@starfivetech.com>
> 抄送: Marek Vasut <marex@denx.de>; Tom Rini <trini@konsulko.com>; Roger
> Quadros <rogerq@kernel.org>; Neil Armstrong <neil.armstrong@linaro.org>;
> Alexey Romanov <avromanov@salutedevices.com>; Sumit Garg
> <sumit.garg@linaro.org>; Mark Kettenis <kettenis@openbsd.org>; Nishanth
> Menon <nm@ti.com>; Rick Chen <rick@andestech.com>; Leo Yu-Chi Liang
> <ycliang@andestech.com>; u-boot@lists.denx.de; Heinrich Schuchardt
> <xypron.glpk@gmx.de>; Simon Glass <sjg@chromium.org>
> 主题: Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
> 
> On Tue, Jul 23, 2024 at 3:16 AM Minda Chen <minda.chen@starfivetech.com>
> wrote:
> >
> >
> >
> > >
> > > On Mon, Jul 22, 2024 at 6:29 PM Minda Chen
> > > <minda.chen@starfivetech.com>
> > > wrote:
> > > >
> > > >
> > > >
> > > > >
> > > > > On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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
> > > > > > <minda.chen@starfivetech.com>
> > > > > wrote:
> > > > > > >
> > > > > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> > > > > > > is USB
> > > > > > > 2.0 device.
> > > > > > >
> > > > > > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > > > > > > ---
> > > > > > >  .../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@10100000 {
> > > > > > > +                       compatible = "starfive,jh7110-usb";
> > > > > > > +                       ranges = <0x0 0x0 0x10100000
> 0x100000>;
> > > > > > > +                       #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 0x10000>,
> > > > > > > +                                     <0x10000 0x10000>,
> > > > > > > +                                     <0x20000 0x10000>;
> > > > > > > +                               reg-names = "otg", "xhci",
> > > "dev";
> > > > > > > +                               interrupts = <100>, <108>,
> > > <110>;
> > > > > > > +                               interrupt-names = "host",
> > > > > "peripheral", "otg";
> > > > > > > +                               phys = <&usbphy0>;
> > > > > > > +                               phy-names =
> "cdns3,usb2-phy";
> > > > > > > +                       };
> > > > > > > +               };
> > > > > > > +
> > > > > > > +               usbphy0: phy@10200000 {
> > > > > > > +                       compatible =
> "starfive,jh7110-usb-phy";
> > > > > > > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > > > > > > +                       clocks = <&syscrg
> > > > > JH7110_SYSCLK_USB_125M>,
> > > > > > > +                                <&stgcrg
> > > > > JH7110_STGCLK_USB_APP_125>;
> > > > > > > +                       clock-names = "125m", "app_125m";
> > > > > > > +                       #phy-cells = <0>;
> > > > > > > +               };
> > > > > > > +
> > > > > > > +               pciephy0: phy@10210000 {
> > > > > > > +                       compatible =
> "starfive,jh7110-pcie-phy";
> > > > > > > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > > > > > > +                       #phy-cells = <0>;
> > > > > > > +               };
> > > > > > > +
> > > > > > > +               pciephy1: phy@10220000 {
> > > > > > > +                       compatible =
> "starfive,jh7110-pcie-phy";
> > > > > > > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > > > > > > +                       #phy-cells = <0>;
> > > > > > > +               };
> > > > > > > +
> > > > > > >                 stgcrg: clock-controller@10230000 {
> > > > > > >                         compatible = "starfive,jh7110-stgcrg";
> > > > > > >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > > > > > > --
> > > > > > > 2.17.1
> > > > > > >
> > > > > >
> > > > > > Access fault
> > > > > >
> > > > > >         starting USB...
> > > > > >         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to
> 0
> > > > > >         scanning bus usb@0 for devices... Unhandled exception:
> > > > > > Load
> > > > > access fault
> > > > > >         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL:
> > > > > 0000000000000004
> > > > > >         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /*
> > > > > &sysgpio */
> > > > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > > > >         offset = fdt_path_offset(fdt,
> > > > > > "/soc/pinctrl@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@13040000/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@10100000"); /*
> > > > > > &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@13040000/usb0-0")));
> > > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > > >
> > > > > >         offset = fdt_path_offset(fdt,
> > > > > > "/soc/usb@10100000/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:
> > > > > >
> > >
> > > > I checked t Milk-V CM board do not contain USB3.0 host So I think
> > > > the USB 3.0 configuration is not required.
> > >
> > > I agree it should be USB 2.0, but with your patch this is the only
> > > configuration that does anything successful with USB2.0 or 3.0
> > >
> > >
> > > >
> > > > > >         int offset;
> > > > > >
> > > > > >         offset = fdt_path_offset(fdt,
> > > > > > "/soc/pinctrl@13040000"); /*
> > > > > &sysgpio */
> > > > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > > > >         offset = fdt_path_offset(fdt,
> > > > > > "/soc/pinctrl@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@13040000/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@2b000000");
> > > > > > /* &pcie0
> > > */
> > > > > >         fdt_setprop_string(fdt, offset, "status", "disabled");
> > > > > >
> > > > > >         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /*
> > > > > > &pciephy0
> > > > > */
> > > > > >         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
> > > > > >         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > > > 0x18);
> > > > > > /* append <magic number> */
> > > > > >         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
> > > > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > > 0x148);
> > > > > > /* append <magic number> */
> > > > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > > 0x1f4);
> > > > > > /* append <magic number> */
> > > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > > >
> > > > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /*
> > > > > > &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@13040000/usb0-0")));
> > > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > > >
> > > > > >         offset = fdt_path_offset(fdt,
> > > > > > "/soc/usb@10100000/usb@0");
> > > > > > /*
> > > > > > &usb_cdns3 */
> > > > > >         fdt_setprop_u32(fdt,    offset, "phys",
> > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > "/soc/phy@10200000")));
> > > > > > /* = <&usbphy0> */
> > > > > >         fdt_appendprop_u32(fdt, offset, "phys",
> > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > "/soc/phy@10210000"))); /*
> > > 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
> > > > >
> > >
> > > > I don’t have a Milk-V CM board. So I just can test this code to Star64.
> > > > Thanks. I will test this and add the code to board_fixup_star64().
> > >
> > > Can you try to configure in USB2.0-only mode on Star64?  Do you see
> > > the same problem with "load access fault"?
> > >
> > Yes ,USB 2.0 only can work.
> > > ...snip...
> > >
> > > Same devicetree fixup code but separated some of the statements to
> > > multiple
> > > lines:
> > >
> > > https://paste.debian.net/1324036/
> > >
> > > ```
> > Now I can see the issue,Usb 2.0 host can work in Milk-V CM, but PCIe can not
> work.
> > Is that correct?
> > The PCIe in CM board is PCIe0, The PCIe1 is not used. So I think
> > disable PCIe1 node and just set USB 2.0 to host is OK.
> > milkV CM PCIe0 setting is the same with VisionFive v2.
> > The USB 3.0 setting should not be added.
> >
> > This USB 3.0 setting. Should NOT be added to Milk-V CM board.
> > &pciephy0 {
> >         starfive,sys-syscon = <&sys_syscon 0x18>;
> >         starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> >         status = "okay";
> > };
> >
> > &usb_cdns3 {
> >         phys = <&usbphy0>, <&pciephy0>;
> >         phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
> >         dr_mode = "host";
> >         status = "okay";
> > };
> >
> 
> Here is Mars CM Lite:
> 
> StarFive # pci
> BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> _____________________________________________________________
> 00.00.00   0x1556     0x1111     Bridge device           0x04
> 01.00.00   0x10ec     0x8168     Network controller      0x00
> 02.00.00   0x1556     0x1111     Bridge device           0x04
> 
> This is the DFRobot Mini Router carrier and the Mars CM Lite.  There is a
> network interface connected to PCIe.
> 
> When I apply this fixup:
> 
> void spl_fdt_fixup_jh7110_cadence_usb2_host(void *fdt) {
>         int offset;
>         u32 phandle;
> 
>         offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /* &sysgpio
> */
>         fdt_add_subnode(fdt, offset, "usb0-0");
>         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> "usb_pins", "/soc/pinctrl@13040000/usb0-0");
>         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2c000000"); /* &pcie1 */
>         fdt_setprop_string(fdt, offset, "status", "disabled");
> 
>         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &usb0 */
>         fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
>         phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
> "/soc/pinctrl@13040000/usb0-0")); /* <&usb_pins> */
>         fdt_setprop_u32(fdt, offset, "pinctrl-0", phandle);
>         fdt_setprop_string(fdt, offset, "status", "okay");
> 
>         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
> &usb_cdns3 */
>         fdt_setprop_string(fdt, offset, "dr_mode", "host"); }
> 
> at the last instruction of spl_fdt_fixup_mars_cm() then there is this error I am
> telling you about:
> 
> U-Boot 2024.07-00971-g8eed381de5ef-dirty (Jul 23 2024 - 05:49:09 -0700)
> 
> CPU:   sifive,u74-mc
> Model: Milk-V Mars CM Lite
> DRAM:  4 GiB
> Core:  139 devices, 29 uclasses, devicetree: board
> WDT:   Not starting watchdog@13070000
> MMC:   mmc@16010000: 0, mmc@16020000: 1
> Loading Environment from SPIFlash... SF: Detected gd25lq128 with page size
> 256 Bytes, erase size 4 KiB, total 16 MiB OK StarFive EEPROM format v2
> 
> --------EEPROM INFO--------
> Vendor : MILK-V
> Product full SN: MARC-V10-2340-D004E000-000006DF 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--------
> 
> starfive_7110_pcie pcie@2b000000: Starfive PCIe bus probed.
> PCI: Failed autoconfig bar 10
> In:    serial@10000000
> Out:   serial@10000000
> Err:   serial@10000000
> Net:   eth0: ethernet@16030000
> Error: eth_rtl8169 No valid MAC address found.
> 
> starting USB...
> Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0 scanning bus
> usb@0 for devices... Unhandled exception: Load access fault
> EPC: 00000000fff85b2e RA: 00000000fff85b28 TVAL: 0000000000000004
> EPC: 0000000040246b2e RA: 0000000040246b28 reloc adjusted
> 
> Code: 9863 3ee7 8526 f0ef c37f 651c 3a03 0105 (43dc)
> 
> 
> resetting ...
> 
> (C)StarFive
> CCC
I can reproduce USB 2.0 only on Star64 now. I know this.
Next version I will fix this.

You can test with this patch to check whether can fix this.

diff --git a/drivers/phy/starfive/phy-jh7110-usb2.c b/drivers/phy/starfive/phy-jh7110-usb2.c
index d48c9f8a74..a09fb2efea 100644
--- a/drivers/phy/starfive/phy-jh7110-usb2.c
+++ b/drivers/phy/starfive/phy-jh7110-usb2.c
@@ -116,7 +116,7 @@ int jh7110_usb2_phy_probe(struct udevice *dev)
                dev_err(dev, "Failed to get app 125m clock\n");
                return PTR_ERR(phy->app_125m);
        }
-
+       writel(BIT(17), 0x13030018);
        return 0;
 }
E Shattow July 24, 2024, 6:05 p.m. UTC | #8
On Tue, Jul 23, 2024 at 8:24 PM Minda Chen <minda.chen@starfivetech.com> wrote:
>
>
>
> > -----邮件原件-----
> > 发件人: E Shattow <lucent@gmail.com>
> > 发送时间: 2024年7月23日 21:06
> > 收件人: Minda Chen <minda.chen@starfivetech.com>
> > 抄送: Marek Vasut <marex@denx.de>; Tom Rini <trini@konsulko.com>; Roger
> > Quadros <rogerq@kernel.org>; Neil Armstrong <neil.armstrong@linaro.org>;
> > Alexey Romanov <avromanov@salutedevices.com>; Sumit Garg
> > <sumit.garg@linaro.org>; Mark Kettenis <kettenis@openbsd.org>; Nishanth
> > Menon <nm@ti.com>; Rick Chen <rick@andestech.com>; Leo Yu-Chi Liang
> > <ycliang@andestech.com>; u-boot@lists.denx.de; Heinrich Schuchardt
> > <xypron.glpk@gmx.de>; Simon Glass <sjg@chromium.org>
> > 主题: Re: [PATCH v3 7/8] dts: starfive: Add JH7110 Cadence USB dts node
> >
> > On Tue, Jul 23, 2024 at 3:16 AM Minda Chen <minda.chen@starfivetech.com>
> > wrote:
> > >
> > >
> > >
> > > >
> > > > On Mon, Jul 22, 2024 at 6:29 PM Minda Chen
> > > > <minda.chen@starfivetech.com>
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > On Sat, Jul 20, 2024 at 6:47 PM E Shattow <lucent@gmail.com> 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
> > > > > > > <minda.chen@starfivetech.com>
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Add Jh7110 Cadence USB dts node, Visionfive2 default setting
> > > > > > > > is USB
> > > > > > > > 2.0 device.
> > > > > > > >
> > > > > > > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com>
> > > > > > > > ---
> > > > > > > >  .../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@10100000 {
> > > > > > > > +                       compatible = "starfive,jh7110-usb";
> > > > > > > > +                       ranges = <0x0 0x0 0x10100000
> > 0x100000>;
> > > > > > > > +                       #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 0x10000>,
> > > > > > > > +                                     <0x10000 0x10000>,
> > > > > > > > +                                     <0x20000 0x10000>;
> > > > > > > > +                               reg-names = "otg", "xhci",
> > > > "dev";
> > > > > > > > +                               interrupts = <100>, <108>,
> > > > <110>;
> > > > > > > > +                               interrupt-names = "host",
> > > > > > "peripheral", "otg";
> > > > > > > > +                               phys = <&usbphy0>;
> > > > > > > > +                               phy-names =
> > "cdns3,usb2-phy";
> > > > > > > > +                       };
> > > > > > > > +               };
> > > > > > > > +
> > > > > > > > +               usbphy0: phy@10200000 {
> > > > > > > > +                       compatible =
> > "starfive,jh7110-usb-phy";
> > > > > > > > +                       reg = <0x0 0x10200000 0x0 0x10000>;
> > > > > > > > +                       clocks = <&syscrg
> > > > > > JH7110_SYSCLK_USB_125M>,
> > > > > > > > +                                <&stgcrg
> > > > > > JH7110_STGCLK_USB_APP_125>;
> > > > > > > > +                       clock-names = "125m", "app_125m";
> > > > > > > > +                       #phy-cells = <0>;
> > > > > > > > +               };
> > > > > > > > +
> > > > > > > > +               pciephy0: phy@10210000 {
> > > > > > > > +                       compatible =
> > "starfive,jh7110-pcie-phy";
> > > > > > > > +                       reg = <0x0 0x10210000 0x0 0x10000>;
> > > > > > > > +                       #phy-cells = <0>;
> > > > > > > > +               };
> > > > > > > > +
> > > > > > > > +               pciephy1: phy@10220000 {
> > > > > > > > +                       compatible =
> > "starfive,jh7110-pcie-phy";
> > > > > > > > +                       reg = <0x0 0x10220000 0x0 0x10000>;
> > > > > > > > +                       #phy-cells = <0>;
> > > > > > > > +               };
> > > > > > > > +
> > > > > > > >                 stgcrg: clock-controller@10230000 {
> > > > > > > >                         compatible = "starfive,jh7110-stgcrg";
> > > > > > > >                         reg = <0x0 0x10230000 0x0 0x10000>;
> > > > > > > > --
> > > > > > > > 2.17.1
> > > > > > > >
> > > > > > >
> > > > > > > Access fault
> > > > > > >
> > > > > > >         starting USB...
> > > > > > >         Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to
> > 0
> > > > > > >         scanning bus usb@0 for devices... Unhandled exception:
> > > > > > > Load
> > > > > > access fault
> > > > > > >         EPC: 00000000fff85ce2 RA: 00000000fff85cdc TVAL:
> > > > > > 0000000000000004
> > > > > > >         EPC: 0000000040246ce2 RA: 0000000040246cdc 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@13040000"); /*
> > > > > > &sysgpio */
> > > > > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > > > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > > > > >         offset = fdt_path_offset(fdt,
> > > > > > > "/soc/pinctrl@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@13040000/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@10100000"); /*
> > > > > > > &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@13040000/usb0-0")));
> > > > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > > > >
> > > > > > >         offset = fdt_path_offset(fdt,
> > > > > > > "/soc/usb@10100000/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:
> > > > > > >
> > > >
> > > > > I checked t Milk-V CM board do not contain USB3.0 host So I think
> > > > > the USB 3.0 configuration is not required.
> > > >
> > > > I agree it should be USB 2.0, but with your patch this is the only
> > > > configuration that does anything successful with USB2.0 or 3.0
> > > >
> > > >
> > > > >
> > > > > > >         int offset;
> > > > > > >
> > > > > > >         offset = fdt_path_offset(fdt,
> > > > > > > "/soc/pinctrl@13040000"); /*
> > > > > > &sysgpio */
> > > > > > >         fdt_add_subnode(fdt, offset, "usb0-0");
> > > > > > >         fdt_setprop_string(fdt, fdt_path_offset(fdt,
> > > > > > > "/__symbols__"), "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> > > > > > >         offset = fdt_path_offset(fdt,
> > > > > > > "/soc/pinctrl@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@13040000/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@2b000000");
> > > > > > > /* &pcie0
> > > > */
> > > > > > >         fdt_setprop_string(fdt, offset, "status", "disabled");
> > > > > > >
> > > > > > >         offset = fdt_path_offset(fdt, "/soc/phy@10210000"); /*
> > > > > > > &pciephy0
> > > > > > */
> > > > > > >         fdt_setprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > > "/soc/sys_syscon@13030000"))); /* = <&sys_syscon> */
> > > > > > >         fdt_appendprop_u32(fdt, offset, "starfive,sys-syscon",
> > > > > > > 0x18);
> > > > > > > /* append <magic number> */
> > > > > > >         fdt_setprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > > "/soc/stg_syscon@10240000"))); /* = <&stg_syscon> */
> > > > > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > > > 0x148);
> > > > > > > /* append <magic number> */
> > > > > > >         fdt_appendprop_u32(fdt, offset, "starfive,stg-syscon",
> > > > > > > 0x1f4);
> > > > > > > /* append <magic number> */
> > > > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > > > >
> > > > > > >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /*
> > > > > > > &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@13040000/usb0-0")));
> > > > > > >         fdt_setprop_string(fdt, offset, "status", "okay");
> > > > > > >
> > > > > > >         offset = fdt_path_offset(fdt,
> > > > > > > "/soc/usb@10100000/usb@0");
> > > > > > > /*
> > > > > > > &usb_cdns3 */
> > > > > > >         fdt_setprop_u32(fdt,    offset, "phys",
> > > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > > "/soc/phy@10200000")));
> > > > > > > /* = <&usbphy0> */
> > > > > > >         fdt_appendprop_u32(fdt, offset, "phys",
> > > > > > > fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > > > > > > "/soc/phy@10210000"))); /*
> > > > 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
> > > > > >
> > > >
> > > > > I don’t have a Milk-V CM board. So I just can test this code to Star64.
> > > > > Thanks. I will test this and add the code to board_fixup_star64().
> > > >
> > > > Can you try to configure in USB2.0-only mode on Star64?  Do you see
> > > > the same problem with "load access fault"?
> > > >
> > > Yes ,USB 2.0 only can work.
> > > > ...snip...
> > > >
> > > > Same devicetree fixup code but separated some of the statements to
> > > > multiple
> > > > lines:
> > > >
> > > > https://paste.debian.net/1324036/
> > > >
> > > > ```
> > > Now I can see the issue,Usb 2.0 host can work in Milk-V CM, but PCIe can not
> > work.
> > > Is that correct?
> > > The PCIe in CM board is PCIe0, The PCIe1 is not used. So I think
> > > disable PCIe1 node and just set USB 2.0 to host is OK.
> > > milkV CM PCIe0 setting is the same with VisionFive v2.
> > > The USB 3.0 setting should not be added.
> > >
> > > This USB 3.0 setting. Should NOT be added to Milk-V CM board.
> > > &pciephy0 {
> > >         starfive,sys-syscon = <&sys_syscon 0x18>;
> > >         starfive,stg-syscon = <&stg_syscon 0x148 0x1f4>;
> > >         status = "okay";
> > > };
> > >
> > > &usb_cdns3 {
> > >         phys = <&usbphy0>, <&pciephy0>;
> > >         phy-names = "cdns3,usb2-phy", "cdns3,usb3-phy";
> > >         dr_mode = "host";
> > >         status = "okay";
> > > };
> > >
> >
> > Here is Mars CM Lite:
> >
> > StarFive # pci
> > BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
> > _____________________________________________________________
> > 00.00.00   0x1556     0x1111     Bridge device           0x04
> > 01.00.00   0x10ec     0x8168     Network controller      0x00
> > 02.00.00   0x1556     0x1111     Bridge device           0x04
> >
> > This is the DFRobot Mini Router carrier and the Mars CM Lite.  There is a
> > network interface connected to PCIe.
> >
> > When I apply this fixup:
> >
> > void spl_fdt_fixup_jh7110_cadence_usb2_host(void *fdt) {
> >         int offset;
> >         u32 phandle;
> >
> >         offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /* &sysgpio
> > */
> >         fdt_add_subnode(fdt, offset, "usb0-0");
> >         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
> > "usb_pins", "/soc/pinctrl@13040000/usb0-0");
> >         offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@2c000000"); /* &pcie1 */
> >         fdt_setprop_string(fdt, offset, "status", "disabled");
> >
> >         offset = fdt_path_offset(fdt, "/soc/usb@10100000"); /* &usb0 */
> >         fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
> >         phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
> > "/soc/pinctrl@13040000/usb0-0")); /* <&usb_pins> */
> >         fdt_setprop_u32(fdt, offset, "pinctrl-0", phandle);
> >         fdt_setprop_string(fdt, offset, "status", "okay");
> >
> >         offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
> > &usb_cdns3 */
> >         fdt_setprop_string(fdt, offset, "dr_mode", "host"); }
> >
> > at the last instruction of spl_fdt_fixup_mars_cm() then there is this error I am
> > telling you about:
> >
> > U-Boot 2024.07-00971-g8eed381de5ef-dirty (Jul 23 2024 - 05:49:09 -0700)
> >
> > CPU:   sifive,u74-mc
> > Model: Milk-V Mars CM Lite
> > DRAM:  4 GiB
> > Core:  139 devices, 29 uclasses, devicetree: board
> > WDT:   Not starting watchdog@13070000
> > MMC:   mmc@16010000: 0, mmc@16020000: 1
> > Loading Environment from SPIFlash... SF: Detected gd25lq128 with page size
> > 256 Bytes, erase size 4 KiB, total 16 MiB OK StarFive EEPROM format v2
> >
> > --------EEPROM INFO--------
> > Vendor : MILK-V
> > Product full SN: MARC-V10-2340-D004E000-000006DF 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--------
> >
> > starfive_7110_pcie pcie@2b000000: Starfive PCIe bus probed.
> > PCI: Failed autoconfig bar 10
> > In:    serial@10000000
> > Out:   serial@10000000
> > Err:   serial@10000000
> > Net:   eth0: ethernet@16030000
> > Error: eth_rtl8169 No valid MAC address found.
> >
> > starting USB...
> > Bus usb@0: cdns-usb3-host usb@0: set 1 has failed, back to 0 scanning bus
> > usb@0 for devices... Unhandled exception: Load access fault
> > EPC: 00000000fff85b2e RA: 00000000fff85b28 TVAL: 0000000000000004
> > EPC: 0000000040246b2e RA: 0000000040246b28 reloc adjusted
> >
> > Code: 9863 3ee7 8526 f0ef c37f 651c 3a03 0105 (43dc)
> >
> >
> > resetting ...
> >
> > (C)StarFive
> > CCC
> I can reproduce USB 2.0 only on Star64 now. I know this.
> Next version I will fix this.

I follow the "load access fault" problem to
drivers/usb/host/usb-uclass.c: err = ops->control(bus, udev, pipe,
buffer, length, setup);

ops->control has the value set from drivers/usb/host/xhci.c:

struct dm_usb_ops xhci_usb_ops = {
      .control = xhci_submit_control_msg,
...

I verify this by assigning a unique value to xhci_usb_ops.control and
printf from submit_control_msg() in drivers/usb/host/usb-uclass.c and
the value is the same.

So why does this cause a load access fault when calling this function
pointer of &xhci_submit_control_msg, but just when we are having a
problem in the Cadence USB wrapper with USB2.0-only, and not USB3.0?

The load access fault is a problem because it is a crash / halt and
nothing else can be done, and so after then a different U-Boot must be
flashed with a recovery method.

I think the USB2.0-only problem is different, but anyway it should not
be possible to crash U-Boot this way. How is this possible?

>
> You can test with this patch to check whether can fix this.
>
> diff --git a/drivers/phy/starfive/phy-jh7110-usb2.c b/drivers/phy/starfive/phy-jh7110-usb2.c
> index d48c9f8a74..a09fb2efea 100644
> --- a/drivers/phy/starfive/phy-jh7110-usb2.c
> +++ b/drivers/phy/starfive/phy-jh7110-usb2.c
> @@ -116,7 +116,7 @@ int jh7110_usb2_phy_probe(struct udevice *dev)
>                 dev_err(dev, "Failed to get app 125m clock\n");
>                 return PTR_ERR(phy->app_125m);
>         }
> -
> +       writel(BIT(17), 0x13030018);
>         return 0;
>  }
>

Yes, adding the writel() this avoids the load access fault for Mars CM
Lite, and USB with also PCIe are active:

void spl_fdt_fixup_jh7110_cadence_usb2_host(void *fdt)
{
        int offset;
        u32 phandle;

        offset = fdt_path_offset(fdt, "/soc/pinctrl@13040000"); /* &sysgpio */
        fdt_add_subnode(fdt, offset, "usb0-0");
        fdt_setprop_string(fdt, fdt_path_offset(fdt, "/__symbols__"),
"usb_pins", "/soc/pinctrl@13040000/usb0-0");
        offset = fdt_path_offset(fdt, "/soc/pinctrl@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@13040000/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@10100000"); /* &usb0 */
        fdt_setprop_string(fdt, offset, "pinctrl-names", "default");
        phandle = fdt_get_phandle(fdt, fdt_path_offset(fdt,
"/soc/pinctrl@13040000/usb0-0")); /* <&usb_pins> */
        fdt_setprop_u32(fdt, offset, "pinctrl-0", phandle);
        fdt_setprop_string(fdt, offset, "status", "okay");

        offset = fdt_path_offset(fdt, "/soc/usb@10100000/usb@0"); /*
&usb_cdns3 */
        fdt_setprop_string(fdt, offset, "dr_mode", "host");
}

StarFive # pci;usb tree
BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
diff mbox series

Patch

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@10100000 {
+			compatible = "starfive,jh7110-usb";
+			ranges = <0x0 0x0 0x10100000 0x100000>;
+			#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 0x10000>,
+				      <0x10000 0x10000>,
+				      <0x20000 0x10000>;
+				reg-names = "otg", "xhci", "dev";
+				interrupts = <100>, <108>, <110>;
+				interrupt-names = "host", "peripheral", "otg";
+				phys = <&usbphy0>;
+				phy-names = "cdns3,usb2-phy";
+			};
+		};
+
+		usbphy0: phy@10200000 {
+			compatible = "starfive,jh7110-usb-phy";
+			reg = <0x0 0x10200000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_USB_125M>,
+				 <&stgcrg JH7110_STGCLK_USB_APP_125>;
+			clock-names = "125m", "app_125m";
+			#phy-cells = <0>;
+		};
+
+		pciephy0: phy@10210000 {
+			compatible = "starfive,jh7110-pcie-phy";
+			reg = <0x0 0x10210000 0x0 0x10000>;
+			#phy-cells = <0>;
+		};
+
+		pciephy1: phy@10220000 {
+			compatible = "starfive,jh7110-pcie-phy";
+			reg = <0x0 0x10220000 0x0 0x10000>;
+			#phy-cells = <0>;
+		};
+
 		stgcrg: clock-controller@10230000 {
 			compatible = "starfive,jh7110-stgcrg";
 			reg = <0x0 0x10230000 0x0 0x10000>;