[1/2,v2] board: starfive: support Pine64 Star64 board

Similar to the Milk-V Mars, The Star64 board contains few differences to the
VisionFive 2 boards, so can be part of the same U-boot build.

Signed-off-by: Henry Bell <dmoo_dv@protonmail.com>
Cc: ycliang@andestech.com
Cc: heinrich.schuchardt@canonical.com
---

Changes since v1

- Fix typos on naming
- Create pine64_star64 struct to be populated with PHY values once confirmed
---
 board/starfive/visionfive2/spl.c              | 85 +++++++++++++++++++
 .../visionfive2/starfive_visionfive2.c        |  4 +
 2 files changed, 89 insertions(+)

Off-list we reasoned further about the correct PHY values.

As our reference was the pre-loaded SPI NOR content containing dtb data:

ethernet@16030000 {
...
    ethernet-phy@0 {
        rxc_dly_en = <0x01>;
        tx_delay_sel_fe = <0x05>;
        tx_delay_sel = <0x0a>;
        tx_inverted_10 = <0x01>;
        tx_inverted_100 = <0x01>;
        tx_inverted_1000 = <0x01>;
        phandle = <0x54>;
    };
...
ethernet@16040000 {
...
    ethernet-phy@1 {
        tx_delay_sel_fe = <0x05>;
        tx_delay_sel = <0x00>;
        rxc_dly_en = <0x00>;
        tx_inverted_10 = <0x01>;
        tx_inverted_100 = <0x01>;
        tx_inverted_1000 = <0x00>;
        phandle = <0x56>;
    };


This is identical to ethernet-phy@0 and ethernet-phy@1 sections in
Linux-riscv series 705018 "Add Ethernet driver for StarFive JH7110 SoC":

https://patchwork.kernel.org/project/linux-riscv/patch/20221216070632.11444-10-yanhong.wang@starfivetech.com/

Defaults are assumed to be from that version of the motorcomm driver.
More recent versions
of the driver deprecate setting the Fast Ethernet TX delay
tx_delay_sel_fe and instead is this
comment in the source code:
"Generally, it is not necessary to adjust YT8531_RC1R_FE_TX_DELAY"

On Wed, May 8, 2024 at 9:59 PM H Bell <dmoo_dv@protonmail.com> wrote:
>
> Similar to the Milk-V Mars, The Star64 board contains few differences to the
> VisionFive 2 boards, so can be part of the same U-boot build.
>
> Signed-off-by: Henry Bell <dmoo_dv@protonmail.com>
> Cc: ycliang@andestech.com
> Cc: heinrich.schuchardt@canonical.com
> ---
>
> Changes since v1
>
> - Fix typos on naming
> - Create pine64_star64 struct to be populated with PHY values once confirmed
> ---
>  board/starfive/visionfive2/spl.c              | 85 +++++++++++++++++++
>  .../visionfive2/starfive_visionfive2.c        |  4 +
>  2 files changed, 89 insertions(+)
>
> diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> index ca61b5be22..248c6ba01d 100644
> --- a/board/starfive/visionfive2/spl.c
> +++ b/board/starfive/visionfive2/spl.c
> @@ -86,6 +86,39 @@ static const struct starfive_vf2_pro starfive_verb[] = {
>                 "tx-internal-delay-ps", "0"},
>  };
>
> +static const struct starfive_vf2_pro star64_pine64[] = {
> +       {"/soc/ethernet@16030000", "starfive,tx-use-rgmii-clk", NULL},
> +       {"/soc/ethernet@16040000", "starfive,tx-use-rgmii-clk", NULL},
> +
> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "motorcomm,tx-clk-adj-enabled", NULL},

Add:
       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
               "motorcomm,tx-clk-10-inverted", NULL},


> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "motorcomm,tx-clk-100-inverted", NULL},
> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "motorcomm,tx-clk-1000-inverted", NULL},
> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "motorcomm,rx-clk-drv-microamp", "3970"},

This rx-clk-drv-microamp value 3970 is suspect. Delete
rx-clk-drv-microamp property or set default value 2910.

From drivers/net/phy/motorcomm.c:
YT8531_RGMII_RX_DS_DEFAULT 0x3

Presumably this default is the same as in the version of the
motorcomm driver that is included on pre-loaded SPI NOR content.

> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "motorcomm,rx-data-drv-microamp", "2910"},
> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "rx-internal-delay-ps", "1900"},
> +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> +               "tx-internal-delay-ps", "1500"},

N.b. tx-internal-delay-ps corresponds to tx_delay_sel (1000Mbit GE)
and there is not
any property corresponding to tx_delay_sel_fe (10/100Mbit FE). The
driver does not
implement tx_delay_sel_fe so no corresponding property on @0, else it would be
included here.

> +
> +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> +               "motorcomm,tx-clk-adj-enabled", NULL},

Add:
               "motorcomm,tx-clk-10-inverted", NULL},
       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",

> +       { "/soc/ethernet@16040000/mdio/ethernet-phy@1",

Nit: delete space between curly bracket and open-quote to match other lines.

> +               "motorcomm,tx-clk-100-inverted", NULL},
> +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> +               "motorcomm,rx-clk-drv-microamp", "3970"},

Again suspect 3970 value. Delete or set default 2910.

> +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> +               "motorcomm,rx-data-drv-microamp", "2910"},
> +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> +               "rx-internal-delay-ps", "0"},
> +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> +               "tx-internal-delay-ps", "0"},

N.b. driver does not implement tx_delay_sel_fe so no corresponding
property on @1, else it would be included here.

> +};
> +
>  void spl_fdt_fixup_mars(void *fdt)
>  {
>         static const char compat[] = "milkv,mars\0starfive,jh7110";
> @@ -226,6 +259,56 @@ void spl_fdt_fixup_version_b(void *fdt)
>         }
>  }
>
> +void spl_fdt_fixup_star64(void *fdt)
> +{
> +       static const char compat[] = "pine64,star64\0starfive,jh7110";
> +       u32 phandle;
> +       u8 i;
> +       int offset;
> +       int ret;
> +
> +       fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat, sizeof(compat));
> +       fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> +                          "Pine64 Star64");
> +
> +       /* gmac0 */
> +       offset = fdt_path_offset(fdt, "/soc/clock-controller@17000000");
> +       phandle = fdt_get_phandle(fdt, offset);
> +       offset = fdt_path_offset(fdt, "/soc/ethernet@16030000");
> +
> +       fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> +       fdt_appendprop_u32(fdt, offset, "assigned-clocks", JH7110_AONCLK_GMAC0_TX);
> +       fdt_setprop_u32(fdt, offset,  "assigned-clock-parents", phandle);
> +       fdt_appendprop_u32(fdt, offset,  "assigned-clock-parents",
> +                          JH7110_AONCLK_GMAC0_RMII_RTX);
> +
> +       /* gmac1 */
> +       offset = fdt_path_offset(fdt, "/soc/clock-controller@13020000");
> +       phandle = fdt_get_phandle(fdt, offset);
> +       offset = fdt_path_offset(fdt, "/soc/ethernet@16040000");
> +
> +       fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> +       fdt_appendprop_u32(fdt, offset, "assigned-clocks", JH7110_SYSCLK_GMAC1_TX);
> +       fdt_setprop_u32(fdt, offset,  "assigned-clock-parents", phandle);
> +       fdt_appendprop_u32(fdt, offset,  "assigned-clock-parents",
> +                          JH7110_SYSCLK_GMAC1_RMII_RTX);
> +
> +       for (i = 0; i < ARRAY_SIZE(star64_pine64); i++) {
> +               offset = fdt_path_offset(fdt, star64_pine64[i].path);
> +
> +               if (star64_pine64[i].value)
> +                       ret = fdt_setprop_u32(fdt, offset,  star64_pine64[i].name,
> +                                             dectoul(star64_pine64[i].value, NULL));
> +               else
> +                       ret = fdt_setprop_empty(fdt, offset, star64_pine64[i].name);
> +
> +               if (ret) {
> +                       pr_err("%s set prop %s fail.\n", __func__, star64_pine64[i].name);
> +                               break;
> +               }
> +       }
> +}
> +
>  void spl_perform_fixups(struct spl_image_info *spl_image)
>  {
>         u8 version;
> @@ -252,6 +335,8 @@ void spl_perform_fixups(struct spl_image_info *spl_image)
>                         spl_fdt_fixup_version_b(spl_image->fdt_addr);
>                 break;
>                 };
> +       } else if (!strncmp(product_id, "STAR64", 6)) {
> +               spl_fdt_fixup_star64(spl_image->fdt_addr);
>         } else {
>                 pr_err("Unknown product %s\n", product_id);
>         };
> diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
> index a86bca533b..1e7c41f01b 100644
> --- a/board/starfive/visionfive2/starfive_visionfive2.c
> +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> @@ -23,6 +23,8 @@ DECLARE_GLOBAL_DATA_PTR;
>         "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb"
>  #define FDTFILE_VISIONFIVE2_1_3B \
>         "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb"
> +#define FDTFILE_PINE64_STAR64 \
> +       "starfive/jh7110-pine64-star64.dtb"
>
>  /* enable U74-mc hart1~hart4 prefetcher */
>  static void enable_prefetcher(void)
> @@ -78,6 +80,8 @@ static void set_fdtfile(void)
>                         fdtfile = FDTFILE_VISIONFIVE2_1_3B;
>                         break;
>                 }
> +       } else if (!strncmp(product_id, "STAR64", 6)) {
> +               fdtfile = FDTFILE_PINE64_STAR64;
>         } else {
>                 log_err("Unknown product\n");
>                 return;
> --
> 2.44.0
>
>

I've posted builds to people interested in testing where I add a
tx-internal-delay-ps property and register setting in motorcomm.c
which would be some later patch if it has a useful result.

P.S. some typo in my last message.

On Sun, May 12, 2024 at 4:57 PM E Shattow <lucent@gmail.com> wrote:
>
> Off-list we reasoned further about the correct PHY values.
>
> As our reference was the pre-loaded SPI NOR content containing dtb data:
>
> ethernet@16030000 {
> ...
>     ethernet-phy@0 {
>         rxc_dly_en = <0x01>;
>         tx_delay_sel_fe = <0x05>;
>         tx_delay_sel = <0x0a>;
>         tx_inverted_10 = <0x01>;
>         tx_inverted_100 = <0x01>;
>         tx_inverted_1000 = <0x01>;
>         phandle = <0x54>;
>     };
> ...
> ethernet@16040000 {
> ...
>     ethernet-phy@1 {
>         tx_delay_sel_fe = <0x05>;
>         tx_delay_sel = <0x00>;
>         rxc_dly_en = <0x00>;
>         tx_inverted_10 = <0x01>;
>         tx_inverted_100 = <0x01>;
>         tx_inverted_1000 = <0x00>;
>         phandle = <0x56>;
>     };
>
>
> This is identical to ethernet-phy@0 and ethernet-phy@1 sections in
> Linux-riscv series 705018 "Add Ethernet driver for StarFive JH7110 SoC":
>
> https://patchwork.kernel.org/project/linux-riscv/patch/20221216070632.11444-10-yanhong.wang@starfivetech.com/
>
> Defaults are assumed to be from that version of the motorcomm driver.
> More recent versions
> of the driver deprecate setting the Fast Ethernet TX delay
> tx_delay_sel_fe and instead is this
> comment in the source code:
> "Generally, it is not necessary to adjust YT8531_RC1R_FE_TX_DELAY"
>
> On Wed, May 8, 2024 at 9:59 PM H Bell <dmoo_dv@protonmail.com> wrote:
> >
> > Similar to the Milk-V Mars, The Star64 board contains few differences to the
> > VisionFive 2 boards, so can be part of the same U-boot build.
> >
> > Signed-off-by: Henry Bell <dmoo_dv@protonmail.com>
> > Cc: ycliang@andestech.com
> > Cc: heinrich.schuchardt@canonical.com
> > ---
> >
> > Changes since v1
> >
> > - Fix typos on naming
> > - Create pine64_star64 struct to be populated with PHY values once confirmed
> > ---
> >  board/starfive/visionfive2/spl.c              | 85 +++++++++++++++++++
> >  .../visionfive2/starfive_visionfive2.c        |  4 +
> >  2 files changed, 89 insertions(+)
> >
> > diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> > index ca61b5be22..248c6ba01d 100644
> > --- a/board/starfive/visionfive2/spl.c
> > +++ b/board/starfive/visionfive2/spl.c
> > @@ -86,6 +86,39 @@ static const struct starfive_vf2_pro starfive_verb[] = {
> >                 "tx-internal-delay-ps", "0"},
> >  };
> >
> > +static const struct starfive_vf2_pro star64_pine64[] = {
> > +       {"/soc/ethernet@16030000", "starfive,tx-use-rgmii-clk", NULL},
> > +       {"/soc/ethernet@16040000", "starfive,tx-use-rgmii-clk", NULL},
> > +
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "motorcomm,tx-clk-adj-enabled", NULL},
>
> Add:
>        {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
>                "motorcomm,tx-clk-10-inverted", NULL},
>
>
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "motorcomm,tx-clk-100-inverted", NULL},
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "motorcomm,tx-clk-1000-inverted", NULL},
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "motorcomm,rx-clk-drv-microamp", "3970"},
>
> This rx-clk-drv-microamp value 3970 is suspect. Delete
> rx-clk-drv-microamp property or set default value 2910.
>
> From drivers/net/phy/motorcomm.c:
> YT8531_RGMII_RX_DS_DEFAULT 0x3
>
> Presumably this default is the same as in the version of the
> motorcomm driver that is included on pre-loaded SPI NOR content.
>
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "motorcomm,rx-data-drv-microamp", "2910"},
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "rx-internal-delay-ps", "1900"},
> > +       {"/soc/ethernet@16030000/mdio/ethernet-phy@0",
> > +               "tx-internal-delay-ps", "1500"},
>
> N.b. tx-internal-delay-ps corresponds to tx_delay_sel (1000Mbit GE)
> and there is not
> any property corresponding to tx_delay_sel_fe (10/100Mbit FE). The
> driver does not
> implement tx_delay_sel_fe so no corresponding property on @0, else it would be
> included here.
>
> > +
> > +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> > +               "motorcomm,tx-clk-adj-enabled", NULL},
>
> Add:
>                "motorcomm,tx-clk-10-inverted", NULL},
>        {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
>
> > +       { "/soc/ethernet@16040000/mdio/ethernet-phy@1",
>
> Nit: delete space between curly bracket and open-quote to match other lines.
>
> > +               "motorcomm,tx-clk-100-inverted", NULL},
> > +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> > +               "motorcomm,rx-clk-drv-microamp", "3970"},
>
> Again suspect 3970 value. Delete or set default 2910.
>
> > +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> > +               "motorcomm,rx-data-drv-microamp", "2910"},
> > +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> > +               "rx-internal-delay-ps", "0"},
> > +       {"/soc/ethernet@16040000/mdio/ethernet-phy@1",
> > +               "tx-internal-delay-ps", "0"},
>
> N.b. driver does not implement tx_delay_sel_fe so no corresponding
> property on @1, else it would be included here.
>
> > +};
> > +
> >  void spl_fdt_fixup_mars(void *fdt)
> >  {
> >         static const char compat[] = "milkv,mars\0starfive,jh7110";
> > @@ -226,6 +259,56 @@ void spl_fdt_fixup_version_b(void *fdt)
> >         }
> >  }
> >
> > +void spl_fdt_fixup_star64(void *fdt)
> > +{
> > +       static const char compat[] = "pine64,star64\0starfive,jh7110";
> > +       u32 phandle;
> > +       u8 i;
> > +       int offset;
> > +       int ret;
> > +
> > +       fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat, sizeof(compat));
> > +       fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
> > +                          "Pine64 Star64");
> > +
> > +       /* gmac0 */
> > +       offset = fdt_path_offset(fdt, "/soc/clock-controller@17000000");
> > +       phandle = fdt_get_phandle(fdt, offset);
> > +       offset = fdt_path_offset(fdt, "/soc/ethernet@16030000");
> > +
> > +       fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> > +       fdt_appendprop_u32(fdt, offset, "assigned-clocks", JH7110_AONCLK_GMAC0_TX);
> > +       fdt_setprop_u32(fdt, offset,  "assigned-clock-parents", phandle);
> > +       fdt_appendprop_u32(fdt, offset,  "assigned-clock-parents",
> > +                          JH7110_AONCLK_GMAC0_RMII_RTX);
> > +
> > +       /* gmac1 */
> > +       offset = fdt_path_offset(fdt, "/soc/clock-controller@13020000");
> > +       phandle = fdt_get_phandle(fdt, offset);
> > +       offset = fdt_path_offset(fdt, "/soc/ethernet@16040000");
> > +
> > +       fdt_setprop_u32(fdt, offset, "assigned-clocks", phandle);
> > +       fdt_appendprop_u32(fdt, offset, "assigned-clocks", JH7110_SYSCLK_GMAC1_TX);
> > +       fdt_setprop_u32(fdt, offset,  "assigned-clock-parents", phandle);
> > +       fdt_appendprop_u32(fdt, offset,  "assigned-clock-parents",
> > +                          JH7110_SYSCLK_GMAC1_RMII_RTX);
> > +
> > +       for (i = 0; i < ARRAY_SIZE(star64_pine64); i++) {
> > +               offset = fdt_path_offset(fdt, star64_pine64[i].path);
> > +
> > +               if (star64_pine64[i].value)
> > +                       ret = fdt_setprop_u32(fdt, offset,  star64_pine64[i].name,
> > +                                             dectoul(star64_pine64[i].value, NULL));
> > +               else
> > +                       ret = fdt_setprop_empty(fdt, offset, star64_pine64[i].name);
> > +
> > +               if (ret) {
> > +                       pr_err("%s set prop %s fail.\n", __func__, star64_pine64[i].name);
> > +                               break;
> > +               }
> > +       }
> > +}
> > +
> >  void spl_perform_fixups(struct spl_image_info *spl_image)
> >  {
> >         u8 version;
> > @@ -252,6 +335,8 @@ void spl_perform_fixups(struct spl_image_info *spl_image)
> >                         spl_fdt_fixup_version_b(spl_image->fdt_addr);
> >                 break;
> >                 };
> > +       } else if (!strncmp(product_id, "STAR64", 6)) {
> > +               spl_fdt_fixup_star64(spl_image->fdt_addr);
> >         } else {
> >                 pr_err("Unknown product %s\n", product_id);
> >         };
> > diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
> > index a86bca533b..1e7c41f01b 100644
> > --- a/board/starfive/visionfive2/starfive_visionfive2.c
> > +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> > @@ -23,6 +23,8 @@ DECLARE_GLOBAL_DATA_PTR;
> >         "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb"
> >  #define FDTFILE_VISIONFIVE2_1_3B \
> >         "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb"
> > +#define FDTFILE_PINE64_STAR64 \
> > +       "starfive/jh7110-pine64-star64.dtb"
> >
> >  /* enable U74-mc hart1~hart4 prefetcher */
> >  static void enable_prefetcher(void)
> > @@ -78,6 +80,8 @@ static void set_fdtfile(void)
> >                         fdtfile = FDTFILE_VISIONFIVE2_1_3B;
> >                         break;
> >                 }
> > +       } else if (!strncmp(product_id, "STAR64", 6)) {
> > +               fdtfile = FDTFILE_PINE64_STAR64;
> >         } else {
> >                 log_err("Unknown product\n");
> >                 return;
> > --
> > 2.44.0
> >
> >
>
> I've posted builds to people interested in testing where I add a
> tx-internal-delay-ps property and register setting in motorcomm.c
> which would be some later patch if it has a useful result.

Fixing my typo: The future testing is tx-internal-delay-fe-ps
corresponding to tx_delay_sel_fe.

- E