[PATCH v4 1/4] imx9: Add support for saving DDR training data to NVM

[Kumar, Udit]

Hi Simona,

On 6/2/2026 4:29 PM, Simona Toaca wrote:
> Hello Kumar,
> 
> On Thu, May 28, 2026 at 10:55:23AM +0530, Kumar, Udit wrote:
>>
>>
>> On 4/30/2026 2:03 PM, Simona Toaca (OSS) wrote:
>>> From: Simona Toaca <simona.toaca at nxp.com>
>>>
>>> DDR training data can be saved to NVM and be available
>>> to OEI at boot time, which will trigger QuickBoot flow.
>>>
>>> U-Boot only checks for data integrity (CRC32), while
>>> OEI is in charge of authentication when it tries to
>>> load the data from NVM.
>>>
>>>  
> [...]
>>> +#define DDRPHY_QB_PSTATES	0
>>> +#define DDRPHY_QB_PST_SIZE	(DDRPHY_QB_PSTATES * 4 * 1024)
>>
>> [..]
>> Sorry for basic question, in case OEI layer changes format due to one of
>> other reason then how you are making sure, struct ddrphy_qb_state is
>> aligned
>>
>> + * This structure needs to be aligned with the one in OEI.
>>
> For now, the structure is stable. If anything changes, I will take care
> to update U-Boot accordingly.

Hmm, you might break backward compatibility in case of change in struct.
Or you can think of some abstracted struct, which will take care of
changes in OEI.
Something like
struct {
   qb_version;
   qb_data_pointer;
   qb_data_size; 	
}
 >>
>> [..]
>>> +
>>> +bool imx_qb_check(void)
>>> +{
>>> +	struct ddrphy_qb_state *qb_state;
>>> +	u32 size, crc;
>>> +
>>> +	/**
>>> +	 * Ensure CRC is not empty, the reason is that
>>> +	 * the data is invalidated after first save run
>>> +	 * or after it is overwritten.
>>> +	 */
>>
>> In case OEI layer using saved DDR training data, then will
>> passed CRC will be zero ?
>>
> No, the data in DRAM isn't guaranteed to be 0 after 'qb save' and
> then reset. It is 0 only immediately after a successful 'qb save'.

Ok then then you are making sure, if boot is from qb firmware then
u-boot has a way to detect this ?
> 
>>> +	qb_state = (struct ddrphy_qb_state *)CONFIG_QB_SAVED_STATE_BASE;
>>
>> If yes then you can return from here, if no CRC found instead of doing
>> crc calculation.
> Agreed, but the CRC32 doesn't add significant overhead and, more often than
> not, the data will not be 0, but something else.

you can think to remove this, it may save few nS or uS in boot.

>>
>>> +	size = sizeof(struct ddrphy_qb_state) - sizeof(qb_state->crc);
>>> +	crc = crc32(0, (u8 *)qb_state->mac, size);
>>> +
>>> +	if (!qb_state->crc || crc != qb_state->crc)
>>> +		return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>> [..]
>>> +
>>> +void spl_imx_qb_save(void)
>>> +{
>>> +	/* Save QB data on current boot device */
>>> +	if (imx_qb("auto", "", true))
>>> +		printf("QB save failed\n");
>>
>> In case, of 3rd boot or so, there is no need to save training data,
>> whereas you will return error, in case this feature is auto enabled.
>>
> There is no way to tell from U-Boot if Quickboot flow was run or not,
> so that is why 'qb save' is tried everytime. The data in DRAM can be
> invalid for 2 reasons:
> - it was saved by Training flow, but it got corrupted/got saved wrong
> - Quickboot flow was run so no data was saved - this doesn't mean that the
> data in DRAM will be 0
> 

No further comments on this, As long as, you are making sure, above
print will come only if QB save really failed.

>>> +}
>> [..]
> 
> Best regards,
> Simona