[PATCH 2/2] ARM: mvebu: x530: Add option for ECC

[Chris Packham]

On Mon, Jan 10, 2022 at 9:10 PM Chris Packham <judge.packham at gmail.com> wrote:
>
>
>
> On Mon, 10 Jan 2022, 8:37 PM Stefan Roese, <sr at denx.de> wrote:
>>
>> On 1/6/22 04:08, 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.packham at gmail.com>
>>
>> Just checking: Most likely you don't have the possibility to detect the
>> board version at runtime?
>
>
> Not in a way that can be done without working RAM. For our internal fork we maintain 2 defconfigs one with ECC one without. We can manage via the BOM which boards get which bootloader.
>
>>
>> If this is not possible:
>>
>> Reviewed-by: Stefan Roese <sr at denx.de>
>>
>> Thanks,
>> Stefan
>>
>> > ---
>> >
>> >   arch/arm/mach-mvebu/Kconfig      |  1 +
>> >   board/alliedtelesis/x530/Kconfig | 20 ++++++++++++++++++++
>> >   board/alliedtelesis/x530/x530.c  |  8 +++++++-
>> >   3 files changed, 28 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..5c1ae36aebaa
>> > --- /dev/null
>> > +++ b/board/alliedtelesis/x530/Kconfig
>> > @@ -0,0 +1,20 @@
>> > +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.
>> > +
>> > +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
>> > +

Hmm this doesn't actually appear to be working as intended. When the
SPL is built I end up with CONFIG_IS_ENABLED(X530_ECC) == 0 even when
I set it to y. Do I need to define a SPL version of this Kconfig
symbol?

>> >   /*
>> >    * 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: sr at denx.de