On 1/18/22 09:57, Chris Packham wrote:
On Tue, 18 Jan 2022, 8:11 PM Stefan Roese, <s...@denx.de
<mailto:s...@denx.de>> wrote:
Hi Chris,
On 1/11/22 02:49, Chris Packham wrote:
> Some older x530 boards have layout issues that cause problems for
DDR.
> These are usually seen as training failures but can also cause
problems
> after training has completed. Add an option to enable ECC leaving the
> default as N which will work with both old and new boards.
>
> Signed-off-by: Chris Packham <judge.pack...@gmail.com
<mailto:judge.pack...@gmail.com>>
> Reviewed-by: Stefan Roese <s...@denx.de <mailto:s...@denx.de>>
> ---
>
> Changes in v2:
> - Define Kconfig symbol for SPL.
>
> arch/arm/mach-mvebu/Kconfig | 1 +
> board/alliedtelesis/x530/Kconfig | 25 +++++++++++++++++++++++++
> board/alliedtelesis/x530/x530.c | 8 +++++++-
> 3 files changed, 33 insertions(+), 1 deletion(-)
> create mode 100644 board/alliedtelesis/x530/Kconfig
>
> diff --git a/arch/arm/mach-mvebu/Kconfig
b/arch/arm/mach-mvebu/Kconfig
> index d23cc0c760f1..7388ade98d52 100644
> --- a/arch/arm/mach-mvebu/Kconfig
> +++ b/arch/arm/mach-mvebu/Kconfig
> @@ -341,5 +341,6 @@ config SECURED_MODE_CSK_INDEX
>
> source "board/solidrun/clearfog/Kconfig"
> source "board/kobol/helios4/Kconfig"
> +source "board/alliedtelesis/x530/Kconfig"
>
> endif
> diff --git a/board/alliedtelesis/x530/Kconfig
b/board/alliedtelesis/x530/Kconfig
> new file mode 100644
> index 000000000000..9e676f17f39c
> --- /dev/null
> +++ b/board/alliedtelesis/x530/Kconfig
> @@ -0,0 +1,25 @@
> +menu "x530 configuration"
> + depends on TARGET_X530
> +
> +config X530_ECC
> + bool "Enable DDR3 ECC"
> + help
> + Some of the older x530 board have layout issues which
cause problems
> + for the DDR which usually exhibit as DDR training failures or
> + problems accessing DDR after training.
> +
> + The known affected boards are:
> +
> + * 844-001897-00 (x530-28GTXm, x530-28GPXm, GS980MX/28PSm)
> + * 844-001948-00 (GS980MX/28)
> + * 844-002008-00 (x530L-52GTX, x530L-52GPX)
> + * 844-001974-00 (x530-52GTXm, x530-52GPXm, GS980MX/52PSm)
> +
> + If you have a newer board you can set Y here, otherwise
say N.
> +
> +config SPL_X530_ECC
> + bool
> + depends on X530_ECC
> + default X530_ECC
> +
This seems a bit superfluous, as you don't really need to differentiate
between SPL and non-SPL, right?
Did you take a look at my comment about this in v1 of this patch? Here
again:
I think your comment and v2 crossed in cyberspace.
;)
AFAIK, IS_ENABLED(CONFIG_X530_ECC) would be a solution here. There is
no need for a new SPL Kconfig option this way. Please give it a try.
I saw a few IS_ENABLED(CONFIG_FOO) and figured the were typos, but your
comment suggests otherwise. I'll give it a go at some point (I'm away
right now so it may miss this merge window).
No hurry here. It does not seem to be super urgent. I'll just defer this
patch for a few weeks.
Thanks,
Stefan
In the meantime Marek's latest patch seems to work on the boards I've
tried with ECC enabled. I suspect we will still want to have this
available as it has been used on the earlier boards running our u-boot fork.
Thanks,
Stefan
> +endmenu
> diff --git a/board/alliedtelesis/x530/x530.c
b/board/alliedtelesis/x530/x530.c
> index 866b6e68cc16..de20684f4353 100644
> --- a/board/alliedtelesis/x530/x530.c
> +++ b/board/alliedtelesis/x530/x530.c
> @@ -45,6 +45,12 @@ int hws_board_topology_load(struct serdes_map
**serdes_map_array, u8 *count)
> return 0; }
>
> +#if CONFIG_IS_ENABLED(X530_ECC)
> + #define BUS_MASK BUS_MASK_32BIT_ECC
> +#else
> + #define BUS_MASK BUS_MASK_32BIT
> +#endif
> +
> /*
> * Define the DDR layout / topology here in the board file.
This will
> * be used by the DDR3 init code in the SPL U-Boot version to
configure
> @@ -66,7 +72,7 @@ static struct mv_ddr_topology_map
board_topology_map = {
> 0, 0, /* cas_l cas_wl */
> MV_DDR_TEMP_LOW, /* temperature */
> MV_DDR_TIM_2T} }, /* timing */
> - BUS_MASK_32BIT_ECC, /* subphys mask */
> + BUS_MASK, /* subphys mask */
> MV_DDR_CFG_DEFAULT, /* ddr configuration data
source */
> NOT_COMBINED, /* ddr twin-die combined */
> { {0} }, /* raw spd data */
>
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email:
s...@denx.de <mailto:s...@denx.de>
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
