| Message ID | 20190209225411.32756-2-krzk@kernel.org |
|---|---|
| State | RFC |
| Delegated to: | Minkyu Kang |
| Headers | show |
| Series | exynos: Fix reboot on Odroid HC1 | expand |
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
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
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
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
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.
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
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(-)