[U-Boot] [PATCH 4/4] rockchip: puma-rk3399: Enable vdd-log during bootup.
Philipp Tomsich
philipp.tomsich at theobroma-systems.com
Thu Dec 6 13:40:58 UTC 2018
> On 06.12.2018, at 12:25, Christoph Muellner <christoph.muellner at theobroma-systems.com> wrote:
>
> On the RK3399-Q7 "Puma" module VDD_LOG is generated by an external
> regulator, which sets the voltage level to 900 mV, which is within
> the allowed range and which works quite fine.
>
> However, in specific use-cases we need to adjust VDD_LOG to
> maintain stable operation or to reduce power consumption.
> This adjustment can be done via pwm2.
>
> This patch allows to tune the VDD_LOG voltage level
> via DTS. To do so the following things are done:
>
> * Setup pin muxing for PWM2 (based on DTS settings)
> * Turn on the vdd_log regulator (based on DTS settings)
>
> Reported-by: Assaf Agmon <assaf at r-go.io>
> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> Signed-off-by: Christoph Muellner <christoph.muellner at theobroma-systems.com>
Reviewed-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
See below for requested changes.
> ---
> board/theobroma-systems/puma_rk3399/puma-rk3399.c | 43 +++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> index 573e691457f..e2915fcca50 100644
> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> @@ -23,10 +23,34 @@
> #include <power/regulator.h>
> #include <u-boot/sha256.h>
>
> +static int setup_pinctrl(void)
I’d prefer a forward-declaration and this further down, as I want to keep board_init()
up front. Just a personal preference, but still ...
> +{
> + int ret;
> + struct udevice *pinctrl;
> +
> + ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl);
> + if (ret) {
> + debug("%s: Cannot find pinctrl device\n", __func__);
Please use pr_debug.
> + goto out;
> + }
> +
> + /* Enable pwm2 for vdd_log regulator. */
> + ret = pinctrl_request_noflags(pinctrl, PERIPH_ID_PWM2);
> + if (ret) {
> + printf("%s PWM2 pinctrl init fail!\n", __func__);
Don’t use printf here — either this is a pr_err or a pr_debug.
> + goto out;
> + }
> +
> +out:
> + return 0;
> +}
> +
> int board_init(void)
> {
> int ret;
>
> + setup_pinctrl();
> +
> /*
> * We need to call into regulators_enable_boot_on() again, as the call
> * during SPL may have not included all regulators.
> @@ -35,6 +59,25 @@ int board_init(void)
> if (ret)
> debug("%s: Cannot enable boot on regulator\n", __func__);
>
> + /*
> + * vdd_log is ignored by regulators_enable_boot_on(), because
> + * regulator-min-microvolt != regulator-max-microvolt.
> + * Therefore we explicitly enable it here.
> + */
> + struct udevice *regulator;
> + ret = regulator_get_by_platname("vdd_log", ®ulator);
> + if (ret) {
> + debug("%s Looking up regulator vdd-log failed!\n", __func__);
> + goto out;
> + }
> +
> + ret = regulator_set_enable(regulator, true);
> + if (ret) {
> + debug("%s Enabling vdd-log failed!\n", __func__);
> + goto out;
> + }
Generally speaking: I would prefer a
if (ret == 0)
ret = regulator_set_enable(regulator, true);
if (ret)
pr_debug(…)
However, this should be irrelevant anyway: why doesn’t
ret = regulators_enable_boot_on(false);
in board_init() suffice?
> +
> +out:
> return 0;
> }
>
> --
> 2.11.0
>
More information about the U-Boot
mailing list