[U-Boot] [PATCH] kmp204x: set the ZL clk chips reset to power up only

[U-Boot] [PATCH] kmp204x: set the ZL clk chips reset to power up only

[Valentin Longchamp]

There is the requirement on the chassis's backplane that when the clocks
are enabled, they should not disappear. Resetting theses chips at unit
reset also has the side effect that their clocks disappear.

To avoid this side effect, both the ZL chips are reset only in the event
of a power cycle.

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

 board/keymile/kmp204x/kmp204x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
index 6bc8eb8..638b2a3 100644
--- a/board/keymile/kmp204x/kmp204x.c
+++ b/board/keymile/kmp204x/kmp204x.c
@@ -94,7 +94,7 @@ int board_early_init_f(void)
 	qrio_wdmask(BFTIC4_RST, true);
 
 	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
-	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
+	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
 	/* and take it out of reset as soon as possible (needed for Hooper) */
 	qrio_prst(ZL30158_RST, false, false);
 
@@ -144,7 +144,7 @@ int misc_init_f(void)
 	qrio_prst(ETH_FRONT_PHY_RST, false, false);
 
 	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
-	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
+	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
 	/* and enable the WD on it */
 	qrio_wdmask(ZL30343_RST, true);
 
-- 
1.8.0.1

[U-Boot] [PATCH] kmp204x: set the ZL clk chips reset to power up only

[York Sun]

On 07/17/2014 05:22 AM, Valentin Longchamp wrote:
> There is the requirement on the chassis's backplane that when the clocks
> are enabled, they should not disappear. Resetting theses chips at unit

I guess here "they" means the "clocks".

> reset also has the side effect that their clocks disappear.

What does "their" point to? Whose clocks?

> 
> To avoid this side effect, both the ZL chips are reset only in the event
> of a power cycle.
> 

Do you mean "power up" here?

I suggest you rephrase your commit message. It sounds twisting.

> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> ---
> 
>  board/keymile/kmp204x/kmp204x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
> index 6bc8eb8..638b2a3 100644
> --- a/board/keymile/kmp204x/kmp204x.c
> +++ b/board/keymile/kmp204x/kmp204x.c
> @@ -94,7 +94,7 @@ int board_early_init_f(void)
>  	qrio_wdmask(BFTIC4_RST, true);
>  
>  	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
> -	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
> +	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
>  	/* and take it out of reset as soon as possible (needed for Hooper) */
>  	qrio_prst(ZL30158_RST, false, false);
>  
> @@ -144,7 +144,7 @@ int misc_init_f(void)
>  	qrio_prst(ETH_FRONT_PHY_RST, false, false);
>  
>  	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
> -	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
> +	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
>  	/* and enable the WD on it */
>  	qrio_wdmask(ZL30343_RST, true);

You changed the macros here but not the comments.

York

[U-Boot] [PATCH] kmp204x: set the ZL clk chips reset to power up only

[Valentin Longchamp]

On 08/12/2014 07:54 PM, York Sun wrote:
> On 07/17/2014 05:22 AM, Valentin Longchamp wrote:
>> There is the requirement on the chassis's backplane that when the clocks
>> are enabled, they should not disappear. Resetting theses chips at unit
> 
> I guess here "they" means the "clocks".


Yes, correct.
> 
>> reset also has the side effect that their clocks disappear.
> 
> What does "their" point to? Whose clocks?

This refers to the "chips", that provide the clocks, the ZL clocking chips.

> 
>>
>> To avoid this side effect, both the ZL chips are reset only in the event
>> of a power cycle.
>>
> 
> Do you mean "power up" here?

Yes, correct

> 
> I suggest you rephrase your commit message. It sounds twisting.

OK

> 
>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>> ---
>>
>>  board/keymile/kmp204x/kmp204x.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
>> index 6bc8eb8..638b2a3 100644
>> --- a/board/keymile/kmp204x/kmp204x.c
>> +++ b/board/keymile/kmp204x/kmp204x.c
>> @@ -94,7 +94,7 @@ int board_early_init_f(void)
>>  	qrio_wdmask(BFTIC4_RST, true);
>>  
>>  	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
>> -	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
>> +	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
>>  	/* and take it out of reset as soon as possible (needed for Hooper) */
>>  	qrio_prst(ZL30158_RST, false, false);
>>  
>> @@ -144,7 +144,7 @@ int misc_init_f(void)
>>  	qrio_prst(ETH_FRONT_PHY_RST, false, false);
>>  
>>  	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
>> -	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
>> +	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
>>  	/* and enable the WD on it */
>>  	qrio_wdmask(ZL30343_RST, true);
> 
> You changed the macros here but not the comments.
> 
> York
> 

[U-Boot] [PATCH v2] kmp204x: reset the Zarlink clocking chips at power up only

[Valentin Longchamp]

There is the requirement on the chassis's backplane that when the clocks
have been enabled, they then should not disappear.

Resetting the Zarlink clocking chips at unit reset violates this
requirement because the backplane clocks are not supplied during the
reset time.

To avoid this side effect, both the Zarlink clocking chips are reset
only at power up.

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

---

Changes in v2:
- Improve the commit message for more clarity

 board/keymile/kmp204x/kmp204x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
index 6bc8eb8..638b2a3 100644
--- a/board/keymile/kmp204x/kmp204x.c
+++ b/board/keymile/kmp204x/kmp204x.c
@@ -94,7 +94,7 @@ int board_early_init_f(void)
 	qrio_wdmask(BFTIC4_RST, true);
 
 	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
-	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
+	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
 	/* and take it out of reset as soon as possible (needed for Hooper) */
 	qrio_prst(ZL30158_RST, false, false);
 
@@ -144,7 +144,7 @@ int misc_init_f(void)
 	qrio_prst(ETH_FRONT_PHY_RST, false, false);
 
 	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
-	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
+	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
 	/* and enable the WD on it */
 	qrio_wdmask(ZL30343_RST, true);
 
-- 
1.8.0.1

[U-Boot] [PATCH v2] kmp204x: reset the Zarlink clocking chips at power up only

[York Sun]

On 08/13/2014 01:17 AM, Valentin Longchamp wrote:
> There is the requirement on the chassis's backplane that when the clocks
> have been enabled, they then should not disappear.
> 
> Resetting the Zarlink clocking chips at unit reset violates this
> requirement because the backplane clocks are not supplied during the
> reset time.
> 
> To avoid this side effect, both the Zarlink clocking chips are reset
> only at power up.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> 
> ---
> 
> Changes in v2:
> - Improve the commit message for more clarity
> 
>  board/keymile/kmp204x/kmp204x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
> index 6bc8eb8..638b2a3 100644
> --- a/board/keymile/kmp204x/kmp204x.c
> +++ b/board/keymile/kmp204x/kmp204x.c
> @@ -94,7 +94,7 @@ int board_early_init_f(void)
>  	qrio_wdmask(BFTIC4_RST, true);
>  
>  	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
> -	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
> +	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
>  	/* and take it out of reset as soon as possible (needed for Hooper) */
>  	qrio_prst(ZL30158_RST, false, false);
>  
> @@ -144,7 +144,7 @@ int misc_init_f(void)
>  	qrio_prst(ETH_FRONT_PHY_RST, false, false);
>  
>  	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
> -	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
> +	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
>  	/* and enable the WD on it */
>  	qrio_wdmask(ZL30343_RST, true);
>  
> 
Did you miss the comments, or they are still correct after the change?

York

[U-Boot] [PATCH v2] kmp204x: reset the Zarlink clocking chips at power up only

[Valentin Longchamp]

On 08/13/2014 06:51 PM, York Sun wrote:
> On 08/13/2014 01:17 AM, Valentin Longchamp wrote:
>> There is the requirement on the chassis's backplane that when the clocks
>> have been enabled, they then should not disappear.
>>
>> Resetting the Zarlink clocking chips at unit reset violates this
>> requirement because the backplane clocks are not supplied during the
>> reset time.
>>
>> To avoid this side effect, both the Zarlink clocking chips are reset
>> only at power up.
>>
>> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
>>
>> ---
>>
>> Changes in v2:
>> - Improve the commit message for more clarity
>>
>>  board/keymile/kmp204x/kmp204x.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
>> index 6bc8eb8..638b2a3 100644
>> --- a/board/keymile/kmp204x/kmp204x.c
>> +++ b/board/keymile/kmp204x/kmp204x.c
>> @@ -94,7 +94,7 @@ int board_early_init_f(void)
>>  	qrio_wdmask(BFTIC4_RST, true);
>>  
>>  	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
>> -	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
>> +	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
>>  	/* and take it out of reset as soon as possible (needed for Hooper) */
>>  	qrio_prst(ZL30158_RST, false, false);
>>  
>> @@ -144,7 +144,7 @@ int misc_init_f(void)
>>  	qrio_prst(ETH_FRONT_PHY_RST, false, false);
>>  
>>  	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
>> -	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
>> +	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
>>  	/* and enable the WD on it */
>>  	qrio_wdmask(ZL30343_RST, true);
>>  
>>
> Did you miss the comments, or they are still correct after the change?
> 

Yeah I missed them, sorry, sending V3.

Valentin

[U-Boot] [PATCH v3] kmp204x: reset the Zarlink clocking chips at power up only

[Valentin Longchamp]

There is the requirement on the chassis's backplane that when the clocks
have been enabled, they then should not disappear.

Resetting the Zarlink clocking chips at unit reset violates this
requirement because the backplane clocks are not supplied during the
reset time.

To avoid this side effect, both the Zarlink clocking chips are reset
only at power up.

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

---

Changes in v3:
- fix the commentaries that also had to be changed

Changes in v2:
- Improve the commit message for more clarity

 board/keymile/kmp204x/kmp204x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/board/keymile/kmp204x/kmp204x.c b/board/keymile/kmp204x/kmp204x.c
index 6bc8eb8..1c3b066 100644
--- a/board/keymile/kmp204x/kmp204x.c
+++ b/board/keymile/kmp204x/kmp204x.c
@@ -93,8 +93,8 @@ int board_early_init_f(void)
 	/* and enable WD on it */
 	qrio_wdmask(BFTIC4_RST, true);
 
-	/* set the ZL30138's prstcfg to reset at power-up and unit reset only */
-	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_UNIT_RST);
+	/* set the ZL30138's prstcfg to reset at power-up only */
+	qrio_prstcfg(ZL30158_RST, PRSTCFG_POWUP_RST);
 	/* and take it out of reset as soon as possible (needed for Hooper) */
 	qrio_prst(ZL30158_RST, false, false);
 
@@ -143,8 +143,8 @@ int misc_init_f(void)
 	qrio_prstcfg(ETH_FRONT_PHY_RST, PRSTCFG_POWUP_UNIT_CORE_RST);
 	qrio_prst(ETH_FRONT_PHY_RST, false, false);
 
-	/* set the ZL30343 prstcfg to reset at power-up and unit reset only */
-	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_UNIT_RST);
+	/* set the ZL30343 prstcfg to reset at power-up only */
+	qrio_prstcfg(ZL30343_RST, PRSTCFG_POWUP_RST);
 	/* and enable the WD on it */
 	qrio_wdmask(ZL30343_RST, true);
 
-- 
1.8.0.1

[U-Boot] [PATCH v3] kmp204x: reset the Zarlink clocking chips at power up only

[York Sun]

On 08/19/2014 06:40 AM, Valentin Longchamp wrote:
> There is the requirement on the chassis's backplane that when the clocks
> have been enabled, they then should not disappear.
> 
> Resetting the Zarlink clocking chips at unit reset violates this
> requirement because the backplane clocks are not supplied during the
> reset time.
> 
> To avoid this side effect, both the Zarlink clocking chips are reset
> only at power up.
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> 
> ---
> 
> Changes in v3:
> - fix the commentaries that also had to be changed
> 

Applied to u-boot-mpc85xx master branch, awaiting for upstream.

York