Hi Tien Fong,
Thanks for the review.
Agreed on those points — addressed in v2 as below.
On 9/22/2026 3:28 pm, Tien Fong Chee wrote:
>> +#define BAC_CAL_MMC_RAW_OFFSET (CONFIG_ENV_SIZE + CONFIG_ENV_OFFSET)
>
> BAC_CAL_MMC_RAW_OFFSET is defined but never used.
>
> Remove the define or wire it if MMC raw backup is still planned.
Removed the unused BAC_CAL_MMC_RAW_OFFSET define.
>> + debug("FPGA: Failed to read DDR bak cal data from flash.\n");
>
> Use a DDR/retention-prefixed message instead of FPGA
Changed to use __func__ like the other bak-cal debug messages.
>> + ddr_offset = simple_strtoul(offset, &endptr, 16);
>> + if (!(offset == endptr || *endptr != '\0'))
>> + memcpy(..., SZ_4K);
>
> Missing intel,ddrcal-ddr-offset / qspi-offset yields NULL →
> simple_strtoul/fs path undefined behaviour.
>
> Even when present, copying SZ_4K ignores cal->header.data_len (can
> truncate a larger backup or copy past valid data).
>
> Recommendation:
>
> NULL-check both DT strings and fail closed.
>
> copy header.data_len + sizeof(header) (already overflow-checked
> elsewhere), not a hard-coded SZ_4K.
Going with your suggestion here.
NULL-check intel,ddrcal-qspi-offset and intel,ddrcal-ddr-offset and fail
closed. Copy sizeof(cal_header_t) + cal->header.data_len after the
existing overflow check, instead of SZ_4K.
Will include these in the v2 series.
Best regards,
Lok
