On 30/07/25 18:03, Peng Fan wrote:
>> Subject: Re: [PATCH 8/9] ddr: imx9: Implement Synopsys LPDDR PHY
>> QuickBoot flow
>>
>> On Sun, Jul 27, 2025 at 11:14 PM Peng Fan (OSS)
>> <peng....@oss.nxp.com> wrote:
>>>
>>> From: Viorel Suman <viorel.su...@nxp.com>
>>>
>>> Implement Synopsys LPDDR PHY QuickBoot flow.
>>
>> What does "QuickBoot flow" mean? What are the
>> motivations/advantages of adding support for it?
>
> Save the training data, then next time, the boot
> will be quick and no need ddr phy training step
> which would consume 4~5 seconds.
>
>>
>> Please expand the commit message to explain why you are doing this.
>>
>>> +config IMX_SNPS_DDR_PHY_QB
>>> + bool "i.MX Synopsys DDR PHY QuickBoot mode"
>>> + depends on IMX_SNPS_DDR_PHY
>> && !IMX_SNPS_DDR_PHY_QB_GEN
>>> + help
>>> + Select the DDR PHY QuickBoot mode on i.MX9 SOC.
>>
>> Nothing selects IMX_SNPS_DDR_PHY_QB in this series, so this patch is
>> only adding dead code.
>
> I could add user in V2.
>
>>
>>> +static int ddrphy_qb_restore(struct dram_timing_info *info, int
>>> +fsp_id) {
>>> + int i;
>>> + u32 to_addr;
>>> + struct dram_fsp_msg *fsp_msg;
>>> + struct dram_cfg_param *dram_cfg;
>>> +
>>> + fsp_msg = &info->fsp_msg[fsp_id];
>>> + for (i = 0; i < fsp_msg->fsp_cfg_num; i++) {
>>> + dram_cfg = &fsp_msg->fsp_cfg[i];
>>> + dwc_ddrphy_apb_wr(dram_cfg->reg, dram_cfg->val);
>>> + }
>>> +
>>> + /* enable the ddrphy apb */
>>> + dwc_ddrphy_apb_wr(0xd0000, 0x00);
>>> + dwc_ddrphy_apb_wr(0x54008, 0x01); /* SequenceCtrl = 0x1
>> (DevInit Only)*/
>>> + ddrphy_w(0x5400c, 0x19, 0x1); /* Lp4Quickboot = 0x1 */
>>> +
>>> + /* Adjust MR14_xy if pstate=0 and 2D training data collected
>> during training phase */
>>> + if (fsp_id == 0 && (qb_state.flags & DDRPHY_QB_FLAG_2D)) {
>>> + ddrphy_w(0x5401c, 0x39, qb_state.fsp[0] >> 8); /*
>> TrainedVREFDQ_A0 -> MR14_A0 */
>>> + ddrphy_w(0x54022, 0x45, qb_state.fsp[1] & 0xFF); /*
>> TrainedVREFDQ_A1 -> MR14_A1 */
>>> + ddrphy_w(0x54036, 0x6c, qb_state.fsp[2] & 0xFF); /*
>> TrainedVREFDQ_B0 -> MR14_B0 */
>>> + ddrphy_w(0x5403c, 0x78, qb_state.fsp[2] >> 8); /*
>> TrainedVREFDQ_B1 -> MR14_B1 */
>>> + }
>>> +
>>> + /* save CSRs to address starting with 0x54800 */
>>> + for (i = 0, to_addr = 0x54800; i < DDRPHY_QB_CSR_SIZE; i++,
>> to_addr++)
>>> + dwc_ddrphy_apb_wr(to_addr, qb_state.csr[i]);
>>> +
>>> + /* disable the ddrphy apb */
>>> + dwc_ddrphy_apb_wr(0xd0000, 0x01);
>>
>> Lots of magic numbers in this function.
>
> We could not do much on this. Synopsys release
> reference code to NXP. Using macros to replace
> the numbers will make upgrading much harder.
>
>>
>>> +
>>> + return 0;
>>
>> No need to return 0. Make the function 'void'.
>
> ok.
>
>>
>>> +int ddr_cfg_phy_qb(struct dram_timing_info *dram_timing, int
>> fsp_id)
>>> +{
>>> + int ret;
>>> +
>>> + /* MemReset Toggle */
>>> + dwc_ddrphy_apb_wr(0xd0000, 0x0);
>>> + dwc_ddrphy_apb_wr(0x20060, 0x3);
>>> + dwc_ddrphy_apb_wr(0x2008F, 0x1);
>>> +
>>> + /* load the dram quick boot firmware image */
>>> + dwc_ddrphy_apb_wr(0xd0000, 0x0);
>>> + ddr_load_train_firmware(FW_1D_IMAGE);
>>> + ddrphy_qb_restore(dram_timing, fsp_id);
>>> +
>>> + /* excute the firmware */
>>> + dwc_ddrphy_apb_wr(0xd0000, 0x0);
>>> + dwc_ddrphy_apb_wr(0xc0080, 0x3);
>>> + dwc_ddrphy_apb_wr(0xd0031, 0x1);
>>> + dwc_ddrphy_apb_wr(0xd0000, 0x1);
>>> + dwc_ddrphy_apb_wr(0xd0099, 0x9);
>>> + dwc_ddrphy_apb_wr(0xd0099, 0x1);
>>> + dwc_ddrphy_apb_wr(0xd0099, 0x0);
>>> +
>>> + /* Wait for the quick boot firmware to complete */
>>
>> Is there a special firmware for this quick boot firmware?
>
> Yes.
>
>>
>> Please explain the details. It is not clear why or how someone should
>> enable this QuickBoot feature.
>
> I think needs to use B0 chip to use this feature. I could
> add more information in V2.
Also please add more information on the flow, I see you use the binman
symbol from the image to grab the offset of this Quick Boot f/w. Please
add the flow as to what is the expectation of your boot image as well.
>
> Regards
> Peng.
--
Thanking You
Neha Malcom Francis
