Re: [PATCH 1/1] regulators: bd718x7: Add enable times

[Vaittinen, Matti]

Hello Guido,

Thanks for looking at this!

On Wed, 2020-12-16 at 12:05 +0100, Guido Günther wrote:
> Use the typical startup times from the data sheet so boards get a
> reasonable default. Not setting any enable time can lead to board
> hangs
> when e.g. clocks are enabled too soon afterwards.
> 
> This fixes gpu power domain resume on the Librem 5.
> 
> Signed-off-by: Guido Günther <agx@xxxxxxxxxxx>
> ---
>  drivers/regulator/bd718x7-regulator.c | 27
> +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/regulator/bd718x7-regulator.c
> b/drivers/regulator/bd718x7-regulator.c
> index e6d5d98c3cea..d6d34aa4ee2e 100644
> --- a/drivers/regulator/bd718x7-regulator.c
> +++ b/drivers/regulator/bd718x7-regulator.c
> @@ -613,6 +613,7 @@ static struct bd718xx_regulator_data
> bd71847_regulators[] = {
>  			.vsel_mask = DVS_BUCK_RUN_MASK,
>  			.enable_reg = BD718XX_REG_BUCK1_CTRL,
>  			.enable_mask = BD718XX_BUCK_EN,
> +			.enable_time = 144,

Where are these values obtained from? I have a feeling they might be
board / load specific. If this is the case - can the "regulator-enable-
ramp-delay" from device-tree be used instead to avoid hard-coding board
specific values in the driver? Although, sane defaults would probably
not be a bad idea - if I read code correctly then the constrains from
DT can be used to override these values.

I'd prefer well named defines over raw numeric values though.

Best Regards
    Matti Vaittinen