[PATCH v2] ddr: marvell: a38x: Add support for DDR4 from Marvell mv-ddr-marvell repository

Pali Rohár pali at kernel.org
Tue Jan 17 09:35:24 CET 2023


Hello! Thank you for update. It is much better.

On Monday 16 January 2023 21:34:39 Tony Dinh wrote:
>     This syncs drivers/ddr/marvell/a38x/ with the master branch of repository
>     https://github.com/MarvellEmbeddedProcessors/mv-ddr-marvell.git
> 
>     up to the commit "mv_ddr: a3700: Use the right size for memset to not overflow"
>     d5acc10c287e40cc2feeb28710b92e45c93c702c
> 
>     This patch was created by following steps:
> 
>     1. Replace all a38x files in U-Boot tree by files from upstream github
>        Marvell mv-ddr-marvell repository.
> 
>     2. Run following command to omit portions not relevant for a38x, ddr3, and ddr4:
> 
>         files=drivers/ddr/marvell/a38x/*
>         sed 's/#if defined(CONFIG_ARMADA_38X) || defined(CONFIG_ARMADA_39X)/#ifdef TRUE/' -i $files
>         unifdef -m -UMV_DDR -UMV_DDR_ATF -UCONFIG_APN806 \
>             -UCONFIG_MC_STATIC -UCONFIG_MC_STATIC_PRINT -UCONFIG_PHY_STATIC \
>             -UCONFIG_PHY_STATIC_PRINT -UCONFIG_CUSTOMER_BOARD_SUPPORT \
>             -UCONFIG_A3700 -UA3900 -UA80X0 -UA70X0 -DTRUE $files

Do not forget to also update commit message.

And btw, commit messages has on each line some leading spaces which is
not probably intended.

>     3. Manually change license to SPDX-License-Identifier
>        (upstream license in  upstream github repository contains long license
>        texts and U-Boot is using just SPDX-License-Identifier.
> 
>     After applying this patch, a38x ddr3 ddr4 code in upstream Marvell github
>     repository and in U-Boot would be fully identical. So in future applying
>     above steps could be used to sync code again.
> 
>     The only change in this patch are:
>     - Removal of common board_topology_map code using ifdefs in mv_ddr_brd.c
>     - Some fixes with include files.
>     - Some basic type defines (original from ATF headers) in mv_ddr_plat.c
> 
>     Reference:
>     "ddr: marvell: a38x: Sync code with Marvell mv-ddr-marvell repository"
>     https://source.denx.de/u-boot/u-boot/-/commit/107c3391b95bcc2ba09a876da4fa0c31b6c1e460
> 
> Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> ---
> 
> Changes in v2:
> - Modified the filter scrip to explicitly include ARMADA_38X code
> and exclude ARMADA_39X code; also remove 64BIT code. Reran it on
> drivers/ddr/marvell/a38x/
> - Updated script
> files=drivers/ddr/marvell/a38x/*
> sed 's/#if defined(CONFIG_ARMADA_38X) || defined(CONFIG_ARMADA_39X)/#ifdef TRUE/' -i $files

You do not need this sed anymore. CONFIG_ARMADA_39X is explicitly
removed and CONFIG_ARMADA_38X already handled by unifdef.

> unifdef -m -UMV_DDR -UMV_DDR_ATF -UCONFIG_APN806 \
>                 -UCONFIG_MC_STATIC -UCONFIG_MC_STATIC_PRINT -UCONFIG_PHY_STATIC \
>                 -UCONFIG_PHY_STATIC_PRINT -UCONFIG_CUSTOMER_BOARD_SUPPORT \
>                 -UCONFIG_A3700 -UA3900 -UA80X0 -UA70X0 -DCONFIG_ARMADA_38X -UCONFIG_ARMADA_39X \
>                                 -UCONFIG_64BIT $files
> - Remove more dead code files
> - Correct SPDX license header
> 
>  drivers/ddr/marvell/a38x/Makefile             |    8 +
>  drivers/ddr/marvell/a38x/ddr3_debug.c         |  120 +
>  drivers/ddr/marvell/a38x/ddr3_init.c          |   25 +
>  drivers/ddr/marvell/a38x/ddr3_init.h          |   14 +
>  drivers/ddr/marvell/a38x/ddr3_logging_def.h   |   27 +
>  drivers/ddr/marvell/a38x/ddr3_training.c      |  131 +
>  drivers/ddr/marvell/a38x/ddr3_training_bist.c |   12 +
>  .../a38x/ddr3_training_centralization.c       |    4 +
>  drivers/ddr/marvell/a38x/ddr3_training_db.c   |  212 ++
>  drivers/ddr/marvell/a38x/ddr3_training_ip.h   |   17 +
>  .../ddr/marvell/a38x/ddr3_training_ip_db.h    |   61 +
>  .../marvell/a38x/ddr3_training_ip_engine.c    |  145 +
>  .../ddr/marvell/a38x/ddr3_training_ip_flow.h  |    5 +
>  .../ddr/marvell/a38x/ddr3_training_leveling.c |  135 +
>  drivers/ddr/marvell/a38x/dram_if.h            |   13 -
>  drivers/ddr/marvell/a38x/mv_ddr4_mpr_pda_if.c |  674 +++++
>  drivers/ddr/marvell/a38x/mv_ddr4_mpr_pda_if.h |   59 +
>  drivers/ddr/marvell/a38x/mv_ddr4_training.c   |  565 ++++
>  drivers/ddr/marvell/a38x/mv_ddr4_training.h   |   32 +
>  .../a38x/mv_ddr4_training_calibration.c       | 2336 +++++++++++++++++
>  .../a38x/mv_ddr4_training_calibration.h       |   26 +
>  .../ddr/marvell/a38x/mv_ddr4_training_db.c    |  545 ++++
>  .../marvell/a38x/mv_ddr4_training_leveling.c  |  441 ++++
>  .../marvell/a38x/mv_ddr4_training_leveling.h  |   11 +
>  drivers/ddr/marvell/a38x/mv_ddr_plat.c        |  249 ++
>  drivers/ddr/marvell/a38x/mv_ddr_plat.h        |   11 +
>  drivers/ddr/marvell/a38x/mv_ddr_regs.h        |   59 +
>  drivers/ddr/marvell/a38x/mv_ddr_topology.h    |   72 +
>  28 files changed, 5996 insertions(+), 13 deletions(-)
>  delete mode 100644 drivers/ddr/marvell/a38x/dram_if.h

I see that you are removing some existing file. If it is not needed
neither for DDR3 nor for DDR4 then please remove it in separate commit
or patch. So we do not mix different things into one commit.

>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_mpr_pda_if.c
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_mpr_pda_if.h
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training.c
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training.h
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.h
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training_db.c
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training_leveling.c
>  create mode 100644 drivers/ddr/marvell/a38x/mv_ddr4_training_leveling.h
...
> diff --git a/drivers/ddr/marvell/a38x/mv_ddr_plat.c b/drivers/ddr/marvell/a38x/mv_ddr_plat.c
> index 7c7bce73a3..16d177b42f 100644
> --- a/drivers/ddr/marvell/a38x/mv_ddr_plat.c
> +++ b/drivers/ddr/marvell/a38x/mv_ddr_plat.c
> @@ -12,6 +12,11 @@
>  #define DDR_INTERFACES_NUM		1
>  #define DDR_INTERFACE_OCTETS_NUM	5
>  
> +/* These were defined in ATF area that was stripped out */
> +#define MV_STATUS	int
> +#define MV_U32		u32
> +#define MV_U8		u8
> +

This is something new which you added? Because I do not see it in
Marvell code.


More information about the U-Boot mailing list