diff mbox

[U-Boot,10/17] powerpc/km8321: set the DDRCDR impedance settings back to half strength

Message ID 1447426768-23226-11-git-send-email-valentin.longchamp@keymile.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Valentin Longchamp Nov. 13, 2015, 2:59 p.m. UTC
The impedance settings have been changed with commit
2ea8ae99595ca11dd228726e854ebc6268208601 (whose goal was to set
the internal voltage level to the DDR2 value - and not DDR1).

There was no other good reason to set them to nominal strength than
"the others do it like that" according to Ludger. The others however
very often use DIMM modules where the nominal strength makes sense.

In our case where the DRAM chips are soldered on board and the routing
for these signals under control, half-strength is sufficient as a few
measurements done in the lasts have shown. Since all the hardware
qualification tests have been performed with half strength, the nominal
strength settings are removed in favor of the default reset half
strength settings.

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---

 include/configs/km/km8321-common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Heiko Schocher Nov. 16, 2015, 11:02 a.m. UTC | #1
Hello Valentin,

Am 13.11.2015 um 15:59 schrieb Valentin Longchamp:
> The impedance settings have been changed with commit
> 2ea8ae99595ca11dd228726e854ebc6268208601 (whose goal was to set
> the internal voltage level to the DDR2 value - and not DDR1).
>
> There was no other good reason to set them to nominal strength than
> "the others do it like that" according to Ludger. The others however
> very often use DIMM modules where the nominal strength makes sense.
>
> In our case where the DRAM chips are soldered on board and the routing
> for these signals under control, half-strength is sufficient as a few
> measurements done in the lasts have shown. Since all the hardware
> qualification tests have been performed with half strength, the nominal
> strength settings are removed in favor of the default reset half
> strength settings.
>
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> ---
>
>   include/configs/km/km8321-common.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
>
> diff --git a/include/configs/km/km8321-common.h b/include/configs/km/km8321-common.h
> index 6f21c05..b2e68e3 100644
> --- a/include/configs/km/km8321-common.h
> +++ b/include/configs/km/km8321-common.h
> @@ -67,8 +67,8 @@
>
>   #define CONFIG_SYS_DDRCDR (\
>   	DDRCDR_EN | \
> -	DDRCDR_PZ_NOMZ | \
> -	DDRCDR_NZ_NOMZ | \
> +	DDRCDR_PZ_MAXZ | \
> +	DDRCDR_NZ_MAXZ | \
>   	DDRCDR_M_ODR)
>
>   #define CONFIG_SYS_DDR_CS0_BNDS		0x0000007f
>
Holger Brunck Nov. 16, 2015, 12:26 p.m. UTC | #2
Hi Valentin,

On 13/11/15 15:59, Valentin Longchamp wrote:
> The impedance settings have been changed with commit
> 2ea8ae99595ca11dd228726e854ebc6268208601 (whose goal was to set
> the internal voltage level to the DDR2 value - and not DDR1).
> 

you are referencing the previous patch 09/17 here, shouldn't these two then not
be squashed?

Regards
Holger
Tom Rini Nov. 16, 2015, 1:34 p.m. UTC | #3
On Mon, Nov 16, 2015 at 01:26:53PM +0100, Holger Brunck wrote:
> Hi Valentin,
> 
> On 13/11/15 15:59, Valentin Longchamp wrote:
> > The impedance settings have been changed with commit
> > 2ea8ae99595ca11dd228726e854ebc6268208601 (whose goal was to set
> > the internal voltage level to the DDR2 value - and not DDR1).
> > 
> 
> you are referencing the previous patch 09/17 here, shouldn't these two then not
> be squashed?

It's bad form to reference commit IDs of previous parts of a series as
they will not match what happens when commited to the tree so please say
the commit subject instead.  And as Holger suggests, if this reference
patch 9/17 here, just squash the two together and v2 just that patch
please (assuming no further feedback on the series), thanks!
Valentin Longchamp Nov. 16, 2015, 3:41 p.m. UTC | #4
On 16/11/2015 14:34, Tom Rini wrote:
> On Mon, Nov 16, 2015 at 01:26:53PM +0100, Holger Brunck wrote:
>> Hi Valentin,
>>
>> On 13/11/15 15:59, Valentin Longchamp wrote:
>>> The impedance settings have been changed with commit
>>> 2ea8ae99595ca11dd228726e854ebc6268208601 (whose goal was to set
>>> the internal voltage level to the DDR2 value - and not DDR1).
>>>
>>
>> you are referencing the previous patch 09/17 here, shouldn't these two then not
>> be squashed?
> 
> It's bad form to reference commit IDs of previous parts of a series as
> they will not match what happens when commited to the tree so please say
> the commit subject instead.  And as Holger suggests, if this reference
> patch 9/17 here, just squash the two together and v2 just that patch
> please (assuming no further feedback on the series), thanks!
> 

Sure, I will squash the 2 commits together and avoid all the internal commit
references.

Thanks

Valentin
diff mbox

Patch

diff --git a/include/configs/km/km8321-common.h b/include/configs/km/km8321-common.h
index 6f21c05..b2e68e3 100644
--- a/include/configs/km/km8321-common.h
+++ b/include/configs/km/km8321-common.h
@@ -67,8 +67,8 @@ 
 
 #define CONFIG_SYS_DDRCDR (\
 	DDRCDR_EN | \
-	DDRCDR_PZ_NOMZ | \
-	DDRCDR_NZ_NOMZ | \
+	DDRCDR_PZ_MAXZ | \
+	DDRCDR_NZ_MAXZ | \
 	DDRCDR_M_ODR)
 
 #define CONFIG_SYS_DDR_CS0_BNDS		0x0000007f