[PATCH 1/1] board: fix compatible property Milk-V Mars CM

[E Shattow]

P.S. my suggestion below

On Fri, Jul 19, 2024 at 5:06 PM E Shattow <lucent at gmail.com> wrote:
>
> On Fri, Jul 19, 2024 at 4:12 PM Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
> >
> > For the Milk-V Mars CM (lite) we have only been copying sizeof(void *)
> > bytes to the compatible property instead of the whole string list.
>
> This const char array must be so that we may get an accurate data
> length with sizeof() but it highlights there are helper functions for
> get of fdt stringlist and not for set of fdt stringlist.
>
> >
> > Fixes: de3229599d4f ("board: add support for Milk-V Mars CM")
> > Reported-by: E Shattow <lucent at gmail.com>
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> > ---
> >  board/starfive/visionfive2/spl.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/board/starfive/visionfive2/spl.c b/board/starfive/visionfive2/spl.c
> > index b794b73b6bd..f55c6b5d34c 100644
> > --- a/board/starfive/visionfive2/spl.c
> > +++ b/board/starfive/visionfive2/spl.c
> > @@ -170,23 +170,32 @@ void spl_fdt_fixup_mars_cm(void *fdt)
> >  {
> >         const char *compat;
> >         const char *model;
> > +       int compat_size;
> >
> >         spl_fdt_fixup_mars(fdt);
> >
> >         if (!get_mmc_size_from_eeprom()) {
> >                 int offset;
> > +               static const char
> > +               compat_cm_lite[] = "milkv,mars-cm-lite\0starfive,jh7110";
> >
> >                 model = "Milk-V Mars CM Lite";
> > -               compat = "milkv,mars-cm-lite\0starfive,jh7110";
> > +               compat = compat_cm_lite;
> > +               compat_size = sizeof(compat_cm_lite);
> >
> >                 offset = fdt_path_offset(fdt, "/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
> >                 /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
> >                 fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
> >         } else {
> > +               static const char
> > +               compat_cm[] = "milkv,mars-cm\0starfive,jh7110";
> > +
> >                 model = "Milk-V Mars CM";
> > -               compat = "milkv,mars-cm\0starfive,jh7110";
> > +               compat = compat_cm;
> > +               compat_size = sizeof(compat_cm);
> >         }
> > -       fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible", compat, sizeof(compat));
> > +       fdt_setprop(fdt, fdt_path_offset(fdt, "/"),
> > +                   "compatible", compat, compat_size);
> >         fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
> >  }
> >
> > --
> > 2.45.2
> >
>
> -E

What about something like as follows?

 void spl_fdt_fixup_mars(void *fdt)
 {
-       static const char compat[] = "milkv,mars\0starfive,jh7110";
        u32 phandle;
        u8 i;
        int offset;
        int ret;

-       fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible",
compat, sizeof(compat));
-       fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model",
-                          "Milk-V Mars");
+       offset = fdt_path_offset(fdt, "/");
+       fdt_setprop_string(fdt,    offset, "compatible", "milkv,mars");
+       fdt_appendprop_string(fdt, offset, "compatible", "starfive,jh7110");
+       fdt_setprop_string(fdt,    offset, "model",      "Milk-V Mars");

        /* gmac0 */
        offset = fdt_path_offset(fdt, "/soc/clock-controller at 17000000");
@@ -164,30 +226,31 @@ void spl_fdt_fixup_mars(void *fdt)
                                break;
                }
        }
 }

 void spl_fdt_fixup_mars_cm(void *fdt)
 {
-       const char *compat;
-       const char *model;
+       int offset;

        spl_fdt_fixup_mars(fdt);

        if (!get_mmc_size_from_eeprom()) {
-               int offset;
-
-               model = "Milk-V Mars CM Lite";
-               compat = "milkv,mars-cm-lite\0starfive,jh7110";
+               offset = fdt_path_offset(fdt, "/");
+               fdt_setprop_string(fdt, offset, "compatible",
"milkv,mars-cm-lite");
+               fdt_appendprop_string(fdt, offset, "compatible",
"starfive,jh7110");
+               fdt_setprop_string(fdt, offset, "model", "Milk-V Mars CM Lite");

                offset = fdt_path_offset(fdt,
"/soc/pinctrl/mmc0-pins/mmc0-pins-rest");
                /* GPIOMUX(22, GPOUT_SYS_SDIO0_RST, GPOEN_ENABLE, GPI_NONE) */
                fdt_setprop_u32(fdt, offset, "pinmux", 0xff130016);
        } else {
-               model = "Milk-V Mars CM";
-               compat = "milkv,mars-cm\0starfive,jh7110";
+               offset = fdt_path_offset(fdt, "/");
+               fdt_setprop_string(fdt, offset, "compatible", "milkv,mars-cm");
+               fdt_appendprop_string(fdt, offset, "compatible",
"starfive,jh7110");
+               fdt_setprop_string(fdt, offset, "model", "Milk-V Mars CM");
        }
-       fdt_setprop(fdt, fdt_path_offset(fdt, "/"), "compatible",
compat, sizeof(compat));
-       fdt_setprop_string(fdt, fdt_path_offset(fdt, "/"), "model", model);
 }

 void spl_fdt_fixup_version_a(void *fdt)
@@ -235,6 +298,7 @@ void spl_fdt_fixup_version_a(void *fdt)
                                break;
                }
        }
+
 }


And similar changes to the other "string1\0string2" settings in the
same source file.

-E