Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 23.06.22 13:47, Teresa Remmet wrote:
> Am Donnerstag, dem 23.06.2022 um 13:26 +0200 schrieb Teresa Remmet:
>> Am Donnerstag, dem 23.06.2022 um 13:14 +0200 schrieb Ahmad Fatoum:
>>>> I have just played around also setting ADDRMAP7 right before 
>>>> ddr_cfg_umctl2() without fixing the timings and the register was
>>>> 0
>>>> again after startup and so the RAM size wrong.
>>>> So it seems to me that the value is overwritten/resetted at some
>>>> other
>>>> point again ... or maybe I just did something wrong.
>>>
>>> Strange. What SoC/board is this on?
>>
>> phyCORE-i.MX8MQ but with a not upstreamed 2GB RAM configuration.
>>
>> I applied this patch now with the same result. Wrong RAM size. 
> 
> I now have also tried some out of tree phyCORE-i.MX8MM implementation
> without and with this patch and the patch works for our MM. too. RAM
> size is corrected.
> So maybe the MQ is somehow different here?

The PhyCORE with i.MX8MQ upstream doesn't use the generic DRAM register
setup in drivers/ddr/imx8m, so you wouldn't get a warning there and we
can't add one easily, because there is no suitable hook point.

Could this be your issue?

For i.MX8MM, the generic setup is used, so you get the fixup and the
warning.


> 
> Teresa
> 
> 
> 
> 
>> Regards,
>> Teresa
>>
>>> Cheers,
>>> Ahmad
>>>
>>>> Regards,
>>>> Teresa
>>>>
>>>>> Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit
>>>>> SDRAM
>>>>> row
>>>>> size handle")
>>>>> Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for
>>>>> imx8m")
>>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>>> ---
>>>>>  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
>>>>>  drivers/ddr/imx8m/helper.c   |  6 ++++++
>>>>>  include/soc/imx8m/ddr.h      |  1 +
>>>>>  3 files changed, 25 insertions(+)
>>>>>
>>>>> diff --git a/drivers/ddr/imx8m/ddr_init.c
>>>>> b/drivers/ddr/imx8m/ddr_init.c
>>>>> index ae05b136229c..9a4b4e2ca88a 100644
>>>>> --- a/drivers/ddr/imx8m/ddr_init.c
>>>>> +++ b/drivers/ddr/imx8m/ddr_init.c
>>>>> @@ -13,14 +13,32 @@
>>>>>  #include <mach/imx8m-regs.h>
>>>>>  #include <mach/imx8m-ccm-regs.h>
>>>>>  
>>>>> +bool imx8m_ddr_old_spreadsheet = true;
>>>>> +
>>>>>  static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg,
>>>>> int
>>>>> num)
>>>>>  {
>>>>>  	int i = 0;
>>>>>  
>>>>>  	for (i = 0; i < num; i++) {
>>>>> +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
>>>>> +		    imx8m_ddr_old_spreadsheet = false;
>>>>>  		reg32_write((unsigned long)ddrc_cfg->reg,
>>>>> ddrc_cfg-
>>>>>> val);
>>>>>  		ddrc_cfg++;
>>>>>  	}
>>>>> +
>>>>> +	/*
>>>>> +	 * Older NXP DDR configuration spreadsheets don't
>>>>> initialize
>>>>> ADDRMAP7,
>>>>> +	 * which falsifies the memory size read back from the
>>>>> controller
>>>>> +	 * in barebox proper.
>>>>> +	 */
>>>>> +	if (imx8m_ddr_old_spreadsheet) {
>>>>> +		pr_warn("Working around old spreadsheet. Please
>>>>> regenerate\n");
>>>>> +		/*
>>>>> +		 * Alternatively, stick { DDRC_ADDRMAP7(0),
>>>>> 0xf0f }
>>>>> into
>>>>> +		 * struct dram_timing_info::ddrc_cfg of your
>>>>> old timing
>>>>> file
>>>>> +		 */
>>>>> +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
>>>>> +	}
>>>>>  }
>>>>>  
>>>>>  /*
>>>>> diff --git a/drivers/ddr/imx8m/helper.c
>>>>> b/drivers/ddr/imx8m/helper.c
>>>>> index 94bbb811576d..98e40849584b 100644
>>>>> --- a/drivers/ddr/imx8m/helper.c
>>>>> +++ b/drivers/ddr/imx8m/helper.c
>>>>> @@ -62,6 +62,12 @@ void dram_config_save(struct
>>>>> dram_timing_info
>>>>> *timing_info,
>>>>>  		cfg++;
>>>>>  	}
>>>>>  
>>>>> +	if (imx8m_ddr_old_spreadsheet) {
>>>>> +		cfg->reg = DDRC_ADDRMAP7(0);
>>>>> +		cfg->val = 0xf0f;
>>>>> +		cfg++;
>>>>> +	}
>>>>> +
>>>>>  	/* save ddrphy config */
>>>>>  	saved_timing->ddrphy_cfg = cfg;
>>>>>  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
>>>>> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
>>>>> index 9ae7cb877686..147a7d499aaf 100644
>>>>> --- a/include/soc/imx8m/ddr.h
>>>>> +++ b/include/soc/imx8m/ddr.h
>>>>> @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned
>>>>> long
>>>>> addr, u32 bit)
>>>>>  #define dwc_ddrphy_apb_rd(addr) \
>>>>>  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 *
>>>>> (addr))
>>>>>  
>>>>> +extern bool imx8m_ddr_old_spreadsheet;
>>>>>  extern struct dram_cfg_param ddrphy_trained_csr[];
>>>>>  extern uint32_t ddrphy_trained_csr_num;
>>>>>  


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |