Hi Marek,
looks good, as small comment below...
On 12.05.2017 16:10, Marek Behún wrote:
> The DDR3 training code for Marvell A38X currently computes 1t timing
> when given board topology map of the Turris Omnia, but Omnia needs 2t.
>
> This patch adds support for enforcing the 2t timing in struct
> hws_topology_map, through a new enum hws_timing, which can assume
> following values:
> HWS_TIM_DEFAULT - default behaviour, compute whether to enable 2t
> from the number of CSs
> HWS_TIM_1T - enforce 1t
> HWS_TIM_2T - enforce 2t
>
> This patch also sets all the board topology maps (db-88f6820-amc,
> db-88f6820-gp, controlcenterdc and clearfog) to have timing set to
> HWS_TIM_DEFAULT.
>
> Signed-off-by: Marek Behun <marek.be...@nic.cz>
> ---
> board/Marvell/db-88f6820-amc/db-88f6820-amc.c | 3 ++-
> board/Marvell/db-88f6820-gp/db-88f6820-gp.c | 3 ++-
> board/gdsys/a38x/controlcenterdc.c | 3 ++-
> board/solidrun/clearfog/clearfog.c | 3 ++-
> drivers/ddr/marvell/a38x/ddr3_training.c | 5 +++++
> drivers/ddr/marvell/a38x/ddr_topology_def.h | 9 +++++++++
> 6 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/board/Marvell/db-88f6820-amc/db-88f6820-amc.c
> b/board/Marvell/db-88f6820-amc/db-88f6820-amc.c
> index cade99c..40fa599 100644
> --- a/board/Marvell/db-88f6820-amc/db-88f6820-amc.c
> +++ b/board/Marvell/db-88f6820-amc/db-88f6820-amc.c
> @@ -69,7 +69,8 @@ static struct hws_topology_map board_topology_map = {
> MEM_4G, /* mem_size */
> DDR_FREQ_800, /* frequency */
> 0, 0, /* cas_l cas_wl */
> - HWS_TEMP_LOW} }, /* temperature */
> + HWS_TEMP_LOW, /* temperature */
> + HWS_TIM_DEFAULT} }, /* timing */
> 5, /* Num Of Bus Per Interface*/
> BUS_MASK_32BIT /* Busses mask */
> };
> diff --git a/board/Marvell/db-88f6820-gp/db-88f6820-gp.c
> b/board/Marvell/db-88f6820-gp/db-88f6820-gp.c
> index e700781..a1974cb 100644
> --- a/board/Marvell/db-88f6820-gp/db-88f6820-gp.c
> +++ b/board/Marvell/db-88f6820-gp/db-88f6820-gp.c
> @@ -90,7 +90,8 @@ static struct hws_topology_map board_topology_map = {
> MEM_4G, /* mem_size */
> DDR_FREQ_800, /* frequency */
> 0, 0, /* cas_l cas_wl */
> - HWS_TEMP_LOW} }, /* temperature */
> + HWS_TEMP_LOW, /* temperature */
> + HWS_TIM_DEFAULT} }, /* timing */
> 5, /* Num Of Bus Per Interface*/
> BUS_MASK_32BIT /* Busses mask */
> };
> diff --git a/board/gdsys/a38x/controlcenterdc.c
> b/board/gdsys/a38x/controlcenterdc.c
> index f0efb53..32168d3 100644
> --- a/board/gdsys/a38x/controlcenterdc.c
> +++ b/board/gdsys/a38x/controlcenterdc.c
> @@ -53,7 +53,8 @@ static struct hws_topology_map ddr_topology_map = {
> MEM_4G, /* mem_size */
> DDR_FREQ_533, /* frequency */
> 0, 0, /* cas_l cas_wl */
> - HWS_TEMP_LOW} }, /* temperature */
> + HWS_TEMP_LOW, /* temperature */
> + HWS_TIM_DEFAULT} }, /* timing */
> 5, /* Num Of Bus Per Interface*/
> BUS_MASK_32BIT /* Busses mask */
> };
> diff --git a/board/solidrun/clearfog/clearfog.c
> b/board/solidrun/clearfog/clearfog.c
> index 2773f59..036e183 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -83,7 +83,8 @@ static struct hws_topology_map board_topology_map = {
> MEM_4G, /* mem_size */
> DDR_FREQ_800, /* frequency */
> 0, 0, /* cas_l cas_wl */
> - HWS_TEMP_LOW} }, /* temperature */
> + HWS_TEMP_LOW, /* temperature */
> + HWS_TIM_DEFAULT} }, /* timing */
> 5, /* Num Of Bus Per Interface*/
> BUS_MASK_32BIT /* Busses mask */
> };
> diff --git a/drivers/ddr/marvell/a38x/ddr3_training.c
> b/drivers/ddr/marvell/a38x/ddr3_training.c
> index 7e0749f..c79db6a 100644
> --- a/drivers/ddr/marvell/a38x/ddr3_training.c
> +++ b/drivers/ddr/marvell/a38x/ddr3_training.c
> @@ -571,6 +571,11 @@ int hws_ddr3_tip_init_controller(u32 dev_num, struct
> init_cntr_param *init_cntr_
>
> if (mode2_t != 0xff) {
> t2t = mode2_t;
> + } else if (tm->interface_params[if_id].
> + timing != HWS_TIM_DEFAULT) {
> + /* Board topology map is forcing timing */
> + t2t = (tm->interface_params[if_id].
> + timing == HWS_TIM_2T) ? 1 : 0;
Perhaps its better to create a local variable for this
"tm->interface_params[if_id].timing" value. It makes the lines somewhat
shorter and easier to read and you don't need to address it twice this
way. Something like this instead:
+ timing = tm->interface_params[if_id].timing;
...
+ } else if (timing != HWS_TIM_DEFAULT) {
+ /* Board topology map is forcing timing */
+ t2t = timing == HWS_TIM_2T ? 1 : 0;
Other than this, you can add my:
Reviewed-by: Stefan Roese <s...@denx.de>
to the next patch version.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
