mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Joacim Zetterling <joaze@westermo.com>, barebox@lists.infradead.org
Subject: Re: [PATCH] ARM: imx: Fix problem with imx_ddrc_sdram_size
Date: Thu, 28 Oct 2021 14:19:18 +0200	[thread overview]
Message-ID: <7eda470f-1d70-3893-62e0-42eed28f0a55@pengutronix.de> (raw)
In-Reply-To: <20211028084655.GA252698@wsevst-c0022>

Hello Joacim,

On 28.10.21 10:46, Joacim Zetterling wrote:
> The imx8mn has a 16-bit SDRAM bus width access but the calculation
> of the memory size treat it as a 32-bit width bus which makes the
> memory calculation to be wrong (meminfo wrong and memtest fails).

I missed this because of the /memory node in the device tree filling
up the remainder. I removed /memory and tried your patch on an
i.MX8MN DDR EVK, but it doesn't change iomem output:

  0x0000000040000000 - 0x000000007fffffff (size 0x0000000040000000) ram0

The 8MN DDR4 EVK reports FIELD_GET(DDRC_MSTR_DEVICE_CONFIG, mstr) == 0b10
and FIELD_GET(DDRC_MSTR_DATA_BUS_WIDTH, mstr) == 0b00.

> There is a difference between the imx7 and the imx8 familys.
> The imx8 family has a device config field in the master register of
> the DDRC controller which the imx7 family doesn't have (the bus width
> is 32-bit as default).
> 
> The device config field together with the DQ configuration tells us
> the actual bus width of the device for a correct mem size calculaton.
> 
> From the imx8mn reference manual:
> +----------------------------------------------------+
> |    Field      |     Function                       |
> |----------------------------------------------------|
> |    31-30      | Indicates the configuration of the |
> |               | device used in the system.         |
> | device_config |     00b - x4 device                |
> |               |     01b - x8 device                |
> |               |     10b - x16 device               |
> |               |     11b - x32 device               |
> +----------------------------------------------------+
> ...
> ...
> 
> Tested on the IMX8MN Evk with 2GB DDR4 and on a IMX8MN custom board
> with 2GB LPDDR4, checked size and made memory test.

What's the device_config for each?

> Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
> ---
>  arch/arm/mach-imx/esdctl.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
> index e56da3cb76d4..f80d94f2fca0 100644
> --- a/arch/arm/mach-imx/esdctl.c
> +++ b/arch/arm/mach-imx/esdctl.c
> @@ -320,6 +320,7 @@ static int vf610_ddrmc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
>  #define DDRC_MSTR_LPDDR4			BIT(5)
>  #define DDRC_MSTR_DATA_BUS_WIDTH		GENMASK(13, 12)
>  #define DDRC_MSTR_ACTIVE_RANKS			GENMASK(27, 24)
> +#define DDRC_MSTR_DEVICE_CONFIG		GENMASK(31, 30)
>  
>  #define DDRC_ADDRMAP0_CS_BIT1			GENMASK(12,  8)
>  
> @@ -361,7 +362,7 @@ static resource_size_t
>  imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[],
>  		    u8 col_max, const u8 col_b[], unsigned int col_b_num,
>  		    u8 row_max, const u8 row_b[], unsigned int row_b_num,
> -		    bool reduced_adress_space)
> +		    bool reduced_adress_space, bool is_imx8)
>  {
>  	const u32 mstr = readl(ddrc + DDRC_MSTR);
>  	unsigned int banks, ranks, columns, rows, active_ranks, width;
> @@ -384,15 +385,20 @@ imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[],
>  		BUG();
>  	}
>  
> +	if (is_imx8)
> +		width = (1 << FIELD_GET(DDRC_MSTR_DEVICE_CONFIG, mstr)) >> 1;

for device_config == 0, size should be halved, but instead you would get a
width of zero here.

> +	else
> +		width = 4;
> +
>  	switch (FIELD_GET(DDRC_MSTR_DATA_BUS_WIDTH, mstr)) {
>  	case 0b00:	/* Full DQ bus  */
>  		width = 4;

I can't really follow. Shouldn't this be dropped to take the
value of width above?

>  		break;
>  	case 0b01:      /* Half DQ bus  */
> -		width = 2;
> +		width >>= 1;
>  		break;
>  	case 0b10:	/* Quarter DQ bus  */
> -		width = 1;
> +		width >>= 2;
>  		break;
>  	default:
>  		BUG();
> @@ -466,7 +472,7 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc)
>  	return imx_ddrc_sdram_size(ddrc, addrmap,
>  				   12, ARRAY_AND_SIZE(col_b),
>  				   16, ARRAY_AND_SIZE(row_b),
> -				   reduced_adress_space);
> +				   reduced_adress_space, true);
>  }
>  
>  static int imx8m_ddrc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
> @@ -508,7 +514,7 @@ static resource_size_t imx7d_ddrc_sdram_size(void __iomem *ddrc)
>  	return imx_ddrc_sdram_size(ddrc, addrmap,
>  				   11, ARRAY_AND_SIZE(col_b),
>  				   15, ARRAY_AND_SIZE(row_b),
> -				   reduced_adress_space);
> +				   reduced_adress_space, false);
>  }
>  
>  static int imx7d_ddrc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
> 


-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


  reply	other threads:[~2021-10-28 12:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-28  8:46 Joacim Zetterling
2021-10-28 12:19 ` Ahmad Fatoum [this message]
2021-10-28 13:06   ` Joacim Zetterling
2021-10-29 12:15     ` Ahmad Fatoum
2021-10-29 13:41       ` Joacim Zetterling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7eda470f-1d70-3893-62e0-42eed28f0a55@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=joaze@westermo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox