On 07/01/2025 16:32, Simon Glass wrote:
> Hi Caleb,
>
> On Sun, 24 Nov 2024 at 12:18, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>
>> If available, otherwise fall back to cmdline.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>> arch/arm/mach-snapdragon/board.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>
> Reviewed-by: Simon Glass <sjg at 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
--
// Caleb (they/them)