[U-Boot,RFT,1/8] exynos: Redo detection of revision when all resources are ready

Message ID 20190209225411.32756-2-krzk@kernel.org
State RFC
Delegated to: Minkyu Kang
Headers show
Series
  • exynos: Fix reboot on Odroid HC1
Related show

Commit Message

Krzysztof Kozlowski Feb. 9, 2019, 10:54 p.m.
Detection of board type is done early - before power setup.  In case of
Odroid XU3/XU4/HC1 family, the detection is done using ADC which
is supplied by LDO4/VDD_ADC regulator.  This regulator could be turned
off (e.g. by kernel before reboot);  If ADC is used early, the
regulators are not yet available and the detection won't work.

Try to detect the revision again, once power is brought up.

This is necessary to fix the detection of Odroid HC1 after reboot, if
kernel turned off the LDO4 regulator.  Otherwise the board is not
detected....

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 board/samsung/common/board.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Lukasz Majewski Feb. 11, 2019, 7:20 a.m. | #1
Hi Krzysztof,

> Detection of board type is done early - before power setup.  In case
> of Odroid XU3/XU4/HC1 family, the detection is done using ADC which
> is supplied by LDO4/VDD_ADC regulator.  This regulator could be turned
> off (e.g. by kernel before reboot);  If ADC is used early, the
> regulators are not yet available and the detection won't work.
> 
> Try to detect the revision again, once power is brought up.
> 
> This is necessary to fix the detection of Odroid HC1 after reboot, if
> kernel turned off the LDO4 regulator.  Otherwise the board is not
> detected....

But such approach seems not to be the optimal one (as we perform
detection twice - with default LDO4 enabled after power on and after
soft reset).

I would expect to enable the LDO4 regulator in the early code (I2C
would be probably necessary) and then read ADC value properly once.

(I also guess that the "work-by-chance" approach is caused by default
settings of PMIC after power on).

As fair as I remember, TI is able to read the EEPROM via I2C in the
very early u-boot (MLO to be precise) code and then make the decision
regarding the platform.

Maybe it would be possible to do the same with Samsung?

And another thought - if the set_board_type() can be called latter and
it works - why cannot we move it to this latter point and execute
exactly once?

> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  board/samsung/common/board.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/board/samsung/common/board.c
> b/board/samsung/common/board.c index 6fd26a3a9198..1e2dabe68d11 100644
> --- a/board/samsung/common/board.c
> +++ b/board/samsung/common/board.c
> @@ -147,6 +147,11 @@ int board_early_init_f(void)
>  {
>  	int err;
>  #ifdef CONFIG_BOARD_TYPES
> +	/*
> +	 * It is done early so power might not be set up yet.  In
> such case
> +	 * specific revision detection with ADC might not work and
> need to me
> +	 * redone later.
> +	 */
>  	set_board_type();
>  #endif
>  	err = board_uart_init();
> @@ -166,9 +171,21 @@ int board_early_init_f(void)
>  #if defined(CONFIG_POWER) || defined(CONFIG_DM_PMIC)
>  int power_init_board(void)
>  {
> +	int ret;
> +
>  	set_ps_hold_ctrl();
>  
> -	return exynos_power_init();
> +	ret = exynos_power_init();
> +
> +#ifdef CONFIG_BOARD_TYPES
> +	/*
> +	 * Since power is on, redo the board detection (external
> peripherals
> +	 * are on).
> +	 */
> +	set_board_type();
> +#endif
> +
> +	return ret;
>  }
>  #endif
>  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Krzysztof Kozlowski Feb. 11, 2019, 8:02 a.m. | #2
On Mon, 11 Feb 2019 at 08:20, Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Krzysztof,
>
> > Detection of board type is done early - before power setup.  In case
> > of Odroid XU3/XU4/HC1 family, the detection is done using ADC which
> > is supplied by LDO4/VDD_ADC regulator.  This regulator could be turned
> > off (e.g. by kernel before reboot);  If ADC is used early, the
> > regulators are not yet available and the detection won't work.
> >
> > Try to detect the revision again, once power is brought up.
> >
> > This is necessary to fix the detection of Odroid HC1 after reboot, if
> > kernel turned off the LDO4 regulator.  Otherwise the board is not
> > detected....
>
> But such approach seems not to be the optimal one (as we perform
> detection twice - with default LDO4 enabled after power on and after
> soft reset).
>
> I would expect to enable the LDO4 regulator in the early code (I2C
> would be probably necessary) and then read ADC value properly once.
>
> (I also guess that the "work-by-chance" approach is caused by default
> settings of PMIC after power on).

So basically you want to move the board detection after the
exynos_power_init()... maybe it is possible. The other way is to split
set_board_type() into OF part (for compatible and main board
difference) and revision detection (requiring ADC). Maybe it is
possible, but isn't it used before?

> As fair as I remember, TI is able to read the EEPROM via I2C in the
> very early u-boot (MLO to be precise) code and then make the decision
> regarding the platform.
>
> Maybe it would be possible to do the same with Samsung?
>
> And another thought - if the set_board_type() can be called latter and
> it works - why cannot we move it to this latter point and execute
> exactly once?

It is the same as previous idea... or I do not see the difference...

Best regards,
Krzysztof
Lukasz Majewski Feb. 11, 2019, 8:14 a.m. | #3
Hi Krzysztof,

> On Mon, 11 Feb 2019 at 08:20, Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Krzysztof,
> >  
> > > Detection of board type is done early - before power setup.  In
> > > case of Odroid XU3/XU4/HC1 family, the detection is done using
> > > ADC which is supplied by LDO4/VDD_ADC regulator.  This regulator
> > > could be turned off (e.g. by kernel before reboot);  If ADC is
> > > used early, the regulators are not yet available and the
> > > detection won't work.
> > >
> > > Try to detect the revision again, once power is brought up.
> > >
> > > This is necessary to fix the detection of Odroid HC1 after
> > > reboot, if kernel turned off the LDO4 regulator.  Otherwise the
> > > board is not detected....  
> >
> > But such approach seems not to be the optimal one (as we perform
> > detection twice - with default LDO4 enabled after power on and after
> > soft reset).
> >
> > I would expect to enable the LDO4 regulator in the early code (I2C
> > would be probably necessary) and then read ADC value properly once.
> >
> > (I also guess that the "work-by-chance" approach is caused by
> > default settings of PMIC after power on).  
> 
> So basically you want to move the board detection after the
> exynos_power_init()... maybe it is possible. The other way is to split
> set_board_type() into OF part (for compatible and main board
> difference) and revision detection (requiring ADC). Maybe it is
> possible, but isn't it used before?

I do want to avoid making the detection twice;

First time when we have the LDO4 enabled by default and the second time
when it may happen that we do a soft reset from Linux (which disabled
LDO4).

> 
> > As fair as I remember, TI is able to read the EEPROM via I2C in the
> > very early u-boot (MLO to be precise) code and then make the
> > decision regarding the platform.
> >
> > Maybe it would be possible to do the same with Samsung?
> >
> > And another thought - if the set_board_type() can be called latter
> > and it works - why cannot we move it to this latter point and
> > execute exactly once?  
> 
> It is the same as previous idea... or I do not see the difference...
> 
> Best regards,
> Krzysztof




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Krzysztof Kozlowski Feb. 11, 2019, 8:17 a.m. | #4
On Mon, 11 Feb 2019 at 09:14, Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Krzysztof,
>
> > On Mon, 11 Feb 2019 at 08:20, Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > Hi Krzysztof,
> > >
> > > > Detection of board type is done early - before power setup.  In
> > > > case of Odroid XU3/XU4/HC1 family, the detection is done using
> > > > ADC which is supplied by LDO4/VDD_ADC regulator.  This regulator
> > > > could be turned off (e.g. by kernel before reboot);  If ADC is
> > > > used early, the regulators are not yet available and the
> > > > detection won't work.
> > > >
> > > > Try to detect the revision again, once power is brought up.
> > > >
> > > > This is necessary to fix the detection of Odroid HC1 after
> > > > reboot, if kernel turned off the LDO4 regulator.  Otherwise the
> > > > board is not detected....
> > >
> > > But such approach seems not to be the optimal one (as we perform
> > > detection twice - with default LDO4 enabled after power on and after
> > > soft reset).
> > >
> > > I would expect to enable the LDO4 regulator in the early code (I2C
> > > would be probably necessary) and then read ADC value properly once.
> > >
> > > (I also guess that the "work-by-chance" approach is caused by
> > > default settings of PMIC after power on).
> >
> > So basically you want to move the board detection after the
> > exynos_power_init()... maybe it is possible. The other way is to split
> > set_board_type() into OF part (for compatible and main board
> > difference) and revision detection (requiring ADC). Maybe it is
> > possible, but isn't it used before?
>
> I do want to avoid making the detection twice;
>
> First time when we have the LDO4 enabled by default and the second time
> when it may happen that we do a soft reset from Linux (which disabled
> LDO4).

This I understand but isn't the board type used BEFORE the power init?
Power init cannot be moved early as it depends on having proper
resources (as I wrote in commit msg)... so only board detection can be
moved later. But if setting up resources (e.g. regulators) requires
board type then it is circular dependency... so I asked - isn't board
type used before power init?

Best regards,
Krzysztof
Minkyu Kang Feb. 11, 2019, 11:06 a.m. | #5
On 11/02/2019 17:17, Krzysztof Kozlowski wrote:
> On Mon, 11 Feb 2019 at 09:14, Lukasz Majewski <lukma@denx.de> wrote:
>>
>> Hi Krzysztof,
>>
>>> On Mon, 11 Feb 2019 at 08:20, Lukasz Majewski <lukma@denx.de> wrote:
>>>>
>>>> Hi Krzysztof,
>>>>
>>>>> Detection of board type is done early - before power setup.  In
>>>>> case of Odroid XU3/XU4/HC1 family, the detection is done using
>>>>> ADC which is supplied by LDO4/VDD_ADC regulator.  This regulator
>>>>> could be turned off (e.g. by kernel before reboot);  If ADC is
>>>>> used early, the regulators are not yet available and the
>>>>> detection won't work.
>>>>>
>>>>> Try to detect the revision again, once power is brought up.
>>>>>
>>>>> This is necessary to fix the detection of Odroid HC1 after
>>>>> reboot, if kernel turned off the LDO4 regulator.  Otherwise the
>>>>> board is not detected....
>>>>
>>>> But such approach seems not to be the optimal one (as we perform
>>>> detection twice - with default LDO4 enabled after power on and after
>>>> soft reset).
>>>>
>>>> I would expect to enable the LDO4 regulator in the early code (I2C
>>>> would be probably necessary) and then read ADC value properly once.
>>>>
>>>> (I also guess that the "work-by-chance" approach is caused by
>>>> default settings of PMIC after power on).
>>>
>>> So basically you want to move the board detection after the
>>> exynos_power_init()... maybe it is possible. The other way is to split
>>> set_board_type() into OF part (for compatible and main board
>>> difference) and revision detection (requiring ADC). Maybe it is
>>> possible, but isn't it used before?
>>
>> I do want to avoid making the detection twice;

I agreed.
And if you want to move set_board_type, then please use proper function such as a board_late_init or a misc_init. set_board_type doesn't have any relation with power_init

>>
>> First time when we have the LDO4 enabled by default and the second time
>> when it may happen that we do a soft reset from Linux (which disabled
>> LDO4).
> 
> This I understand but isn't the board type used BEFORE the power init?
> Power init cannot be moved early as it depends on having proper
> resources (as I wrote in commit msg)... so only board detection can be
> moved later. But if setting up resources (e.g. regulators) requires
> board type then it is circular dependency... so I asked - isn't board
> type used before power init?
> 

I'm not sure but, it looks that can be moved to after power_init.
But, please verify carefully before you re-post.

Thanks,
Minkyu Kang.

Patch

diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c
index 6fd26a3a9198..1e2dabe68d11 100644
--- a/board/samsung/common/board.c
+++ b/board/samsung/common/board.c
@@ -147,6 +147,11 @@  int board_early_init_f(void)
 {
 	int err;
 #ifdef CONFIG_BOARD_TYPES
+	/*
+	 * It is done early so power might not be set up yet.  In such case
+	 * specific revision detection with ADC might not work and need to me
+	 * redone later.
+	 */
 	set_board_type();
 #endif
 	err = board_uart_init();
@@ -166,9 +171,21 @@  int board_early_init_f(void)
 #if defined(CONFIG_POWER) || defined(CONFIG_DM_PMIC)
 int power_init_board(void)
 {
+	int ret;
+
 	set_ps_hold_ctrl();
 
-	return exynos_power_init();
+	ret = exynos_power_init();
+
+#ifdef CONFIG_BOARD_TYPES
+	/*
+	 * Since power is on, redo the board detection (external peripherals
+	 * are on).
+	 */
+	set_board_type();
+#endif
+
+	return ret;
 }
 #endif