Hi Tien Fong,
Thanks for the review.
Agreed on those points — addressed in v2 as below.
On 9/22/2026 1:51 pm, Tien Fong Chee wrote:
>> + u32 length = MEMCLK_HANDOFF_SIZE - 4;
>> + u32 offset = length - 8;
>> + u32 handoff_table[length];
>
> Even though MEMCLK_HANDOFF_SIZE is a macro, `length` is a local variable
> so this is a Variable Length Array (VLA). U-Boot SPL style discourages
> VLAs.
>
> Use a fixed-size array of MEMCLK_HANDOFF_SIZE (or MEMCLK_HANDOFF_SIZE-4)
> elements.
Changed to a fixed-size array:
u32 handoff_table[MEMCLK_HANDOFF_SIZE - 4], with length from ARRAY_SIZE().
>> + if (ret) {
>> + debug("%s: Memclks config Magic Number miss match. ret : %d\n",
>> + __func__, ret);
>> + return -EFAULT;
>> + }
>
> is_ddrconfig_hash_match() is bool but returns -EFAULT on handoff read
> failure.
>
> A bool returning -EFAULT (-1) is truthy in C.
Return false on handoff read failure instead of -EFAULT.
Will include these in the v2 series.
Best regards,
Lok
