[U-Boot] [PATCH v2 01/10] ram: Add driver for MPC83xx

[Mario Six]

Hi Simon,

On Thu, May 3, 2018 at 9:01 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mario,
>
> On 27 April 2018 at 06:52, Mario Six <mario.six at gdsys.cc> wrote:
>> Add a RAM driver for the MPC83xx architecture.
>>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> No changes
>>
>> ---
>>  arch/powerpc/cpu/mpc83xx/spd_sdram.c       |   4 +
>>  drivers/ram/Kconfig                        |   8 +
>>  drivers/ram/Makefile                       |   1 +
>>  drivers/ram/mpc83xx_sdram.c                | 948 +++++++++++++++++++++++++++++
>>  include/dt-bindings/memory/mpc83xx-sdram.h | 143 +++++
>>  include/mpc83xx.h                          |   6 +
>>  6 files changed, 1110 insertions(+)
>>  create mode 100644 drivers/ram/mpc83xx_sdram.c
>>  create mode 100644 include/dt-bindings/memory/mpc83xx-sdram.h
>>
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Question below
>
> [..]
>
>> new file mode 100644
>> index 0000000000..1a73f7b3da
>> --- /dev/null
>> +++ b/drivers/ram/mpc83xx_sdram.c
>> @@ -0,0 +1,948 @@
>> +#define DEBUG
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <ram.h>
>> +#include <asm/io.h>
>> +#include <dt-bindings/memory/mpc83xx-sdram.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define CSCONFIG_ENABLE                        0x80000000
>> +
>> +#define BANK_BITS_2            0x00000000
>> +#define BANK_BITS_3            0x00004000
>> +
>> +#define ROW_BITS_12            0x00000000
>> +#define ROW_BITS_13            0x00000100
>> +#define ROW_BITS_14            0x00000200
>> +
>> +#define COL_BITS_8             0x00000000
>> +#define COL_BITS_9             0x00000001
>> +#define COL_BITS_10            0x00000002
>> +#define COL_BITS_11            0x00000003
>> +
>> +#define TIMING_CFG3_EXT_REFREC_SHIFT   16
>> +
>> +#define TIMING_CFG0_RWT_SHIFT          30
>> +#define TIMING_CFG0_WRT_SHIFT          28
>> +#define TIMING_CFG0_RRT_SHIFT          26
>> +#define TIMING_CFG0_WWT_SHIFT          24
>> +#define TIMING_CFG0_ACT_PD_EXIT_SHIFT  20
>> +#define TIMING_CFG0_PRE_PD_EXIT_SHIFT  16
>> +#define TIMING_CFG0_ODT_PD_EXIT_SHIFT  8
>> +#define TIMING_CFG0_MRS_CYC_SHIFT      0
>> +
>> +#define TIMING_CFG1_PRETOACT_SHIFT     28
>> +#define TIMING_CFG1_ACTTOPRE_SHIFT     24
>> +#define TIMING_CFG1_ACTTORW_SHIFT      20
>> +#define TIMING_CFG1_CASLAT_SHIFT       16
>> +#define TIMING_CFG1_REFREC_SHIFT       12
>> +#define TIMING_CFG1_WRREC_SHIFT                8
>> +#define TIMING_CFG1_ACTTOACT_SHIFT     4
>> +#define TIMING_CFG1_WRTORD_SHIFT       0
>> +
>> +#define TIMING_CFG2_CPO_SHIFT          23
>> +#define TIMING_CFG2_WR_DATA_DELAY_SHIFT        10
>> +#define TIMING_CFG2_ADD_LAT_SHIFT      28
>> +#define TIMING_CFG2_WR_LAT_DELAY_SHIFT 19
>> +#define TIMING_CFG2_RD_TO_PRE_SHIFT    13
>> +#define TIMING_CFG2_CKE_PLS_SHIFT      6
>> +#define TIMING_CFG2_FOUR_ACT_SHIFT     0
>> +
>> +#define SDRAM_CFG_SREN_SHIFT           (31 - 1)
>> +#define SDRAM_CFG_ECC_EN_SHIFT         (31 - 2)
>> +#define SDRAM_CFG_RD_EN_SHIFT          (31 - 3)
>> +#define SDRAM_CFG_SDRAM_TYPE_SHIFT     (31 - 7)
>> +#define SDRAM_CFG_DYN_PWR_SHIFT                (31 - 10)
>> +#define SDRAM_CFG_DBW_SHIFT            (31 - 12)
>> +#define SDRAM_CFG_NCAP_SHIFT           (31 - 14)
>> +#define SDRAM_CFG_2T_EN_SHIFT          (31 - 16)
>> +#define SDRAM_CFG_BA_INTLV_CTL_SHIFT   (31 - 23)
>> +#define SDRAM_CFG_PCHB8_SHIFT          (31 - 27)
>> +#define SDRAM_CFG_HSE_SHIFT            (31 - 28)
>> +#define SDRAM_CFG_BI_SHIFT             (31 - 31)
>> +
>> +#define SDRAM_CFG2_FRC_SR_SHIFT        (31 - 0)
>> +#define SDRAM_CFG2_DLL_RST_DIS (31 - 2)
>> +#define SDRAM_CFG2_DQS_CFG     (31 - 5)
>> +#define SDRAM_CFG2_ODT_CFG     (31 - 10)
>> +#define SDRAM_CFG2_NUM_PR      (31 - 19)
>> +
>> +#define SDRAM_MODE_ESD_SHIFT           16
>> +#define SDRAM_MODE_SD_SHIFT            0
>> +
>> +#define SDRAM_MODE2_ESD2_SHIFT         (31 - 15)
>> +#define SDRAM_MODE2_ESD3_SHIFT         (31 - 31)
>> +
>> +#define SDRAM_INTERVAL_REFINT_SHIFT    16
>> +#define SDRAM_INTERVAL_BSTOPRE_SHIFT   0
>> +
>> +#define SDRAM_CFG_MEM_EN              0x80000000
>> +
>> +int dram_init(void)
>> +{
>> +       struct udevice *ram_ctrl;
>> +       int ret;
>> +
>> +       /* Current assumption: There is only one RAM controller */
>> +       ret = uclass_first_device_err(UCLASS_RAM, &ram_ctrl);
>> +
>> +       if (ret) {
>> +               debug("uclass_first_device_err failed: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       /* Set gd->ram_size? */
>> +
>> +       return 0;
>> +}
>> +
>> +phys_size_t get_effective_memsize(void)
>> +{
>> +#ifndef CONFIG_VERY_BIG_RAM
>
> Can this (and the #ifdefs below in this file) be converted to
>
> if (IS_ENABLED(CONFIG_...))
>
> instead, to increase build coverage?
>

Yes, no problem, will convert for v3.

By the way, is this a practice that's generally encouraged, or is it just
useful in special cases such as this one?

> Regards,
> Simon

Best regards,
Mario