| Message ID | 20241124-b4-modernise-smem-v1-14-b7852c11b67c@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qcom: smem: modernize SMEM in U-Boot | expand |
Hi Caleb, On Sun, 24 Nov 2024 at 12:18, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > If available, otherwise fall back to cmdline. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > arch/arm/mach-snapdragon/board.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass <sjg@chromium.org> Thoughts below > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c > index 269d39e4f6e1..dbac8aa2709a 100644 > --- a/arch/arm/mach-snapdragon/board.c > +++ b/arch/arm/mach-snapdragon/board.c > @@ -30,8 +30,9 @@ > #include <malloc.h> > #include <fdt_support.h> > #include <usb.h> > #include <sort.h> > +#include <soc/qcom/smem.h> > #include <time.h> > > #include "qcom-priv.h" > > @@ -198,11 +199,16 @@ static const char *get_cmdline(void) > } > > void qcom_set_serialno(void) > { > - const char *cmdline = get_cmdline(); > + const char *cmdline; > char serial[32]; > > + if (!qcom_socinfo_init()) Hmmm that function should return an error code, i.e. 0 on success. Does Linux use a boot interface here? > + return; Shouldn't this return an error? > + > + cmdline = get_cmdline(); > + drop blank line > if (!cmdline) { > log_debug("Failed to get bootargs\n"); Shouldn't this be an error? If not, please add a comment. > return; > } > @@ -353,8 +359,11 @@ int board_late_init(void) > > /* By default copy U-Boots FDT, it will be used as a fallback */ > memcpy((void *)addr, (void *)gd->fdt_blob, fdt32_to_cpu(fdt_blob->totalsize)); > > + /* Initialise SMEM if it wasn't done already */ > + qcom_smem_init(); > + > configure_env(); > qcom_late_init(); > > /* Configure the dfu_string for capsule updates */ > > -- > 2.47.0 > Regards, Simon
On 07/01/2025 16:32, Simon Glass wrote: > Hi Caleb, > > On Sun, 24 Nov 2024 at 12:18, Caleb Connolly <caleb.connolly@linaro.org> wrote: >> >> If available, otherwise fall back to cmdline. >> >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >> --- >> arch/arm/mach-snapdragon/board.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Thoughts below > >> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c >> index 269d39e4f6e1..dbac8aa2709a 100644 >> --- a/arch/arm/mach-snapdragon/board.c >> +++ b/arch/arm/mach-snapdragon/board.c >> @@ -30,8 +30,9 @@ >> #include <malloc.h> >> #include <fdt_support.h> >> #include <usb.h> >> #include <sort.h> >> +#include <soc/qcom/smem.h> >> #include <time.h> >> >> #include "qcom-priv.h" >> >> @@ -198,11 +199,16 @@ static const char *get_cmdline(void) >> } >> >> void qcom_set_serialno(void) >> { >> - const char *cmdline = get_cmdline(); >> + const char *cmdline; >> char serial[32]; >> >> + if (!qcom_socinfo_init()) > > Hmmm that function should return an error code, i.e. 0 on success. > Does Linux use a boot interface here? the name of this function isn't great, what it actually does it retrieve the serial number from smem and set serial#, returning 0 on success (meaning the serial number is set, so we have nothing else to do). > >> + return; > > Shouldn't this return an error? > >> + >> + cmdline = get_cmdline(); >> + > > drop blank line > >> if (!cmdline) { >> log_debug("Failed to get bootargs\n"); > > Shouldn't this be an error? If not, please add a comment. bootargs are only available when we're chainloading from ABL (something we can't easily detect at runtime). I would eventually like to drop this and rely on smem only ideally. But that's quite unrelated to this series. > >> return; >> } >> @@ -353,8 +359,11 @@ int board_late_init(void) >> >> /* By default copy U-Boots FDT, it will be used as a fallback */ >> memcpy((void *)addr, (void *)gd->fdt_blob, fdt32_to_cpu(fdt_blob->totalsize)); >> >> + /* Initialise SMEM if it wasn't done already */ >> + qcom_smem_init(); >> + >> configure_env(); >> qcom_late_init(); >> >> /* Configure the dfu_string for capsule updates */ >> >> -- >> 2.47.0 >> > > Regards, > Simon
diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 269d39e4f6e1..dbac8aa2709a 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -30,8 +30,9 @@ #include <malloc.h> #include <fdt_support.h> #include <usb.h> #include <sort.h> +#include <soc/qcom/smem.h> #include <time.h> #include "qcom-priv.h" @@ -198,11 +199,16 @@ static const char *get_cmdline(void) } void qcom_set_serialno(void) { - const char *cmdline = get_cmdline(); + const char *cmdline; char serial[32]; + if (!qcom_socinfo_init()) + return; + + cmdline = get_cmdline(); + if (!cmdline) { log_debug("Failed to get bootargs\n"); return; } @@ -353,8 +359,11 @@ int board_late_init(void) /* By default copy U-Boots FDT, it will be used as a fallback */ memcpy((void *)addr, (void *)gd->fdt_blob, fdt32_to_cpu(fdt_blob->totalsize)); + /* Initialise SMEM if it wasn't done already */ + qcom_smem_init(); + configure_env(); qcom_late_init(); /* Configure the dfu_string for capsule updates */
If available, otherwise fall back to cmdline. Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- arch/arm/mach-snapdragon/board.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)