| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 14 Feb 2019 07:00:50 -0800
From: Angus Ainslie <angus@...ea.ca>
To: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc: mazzisaccount@...il.com, Lee Jones <lee.jones@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, heikki.haikola@...rohmeurope.com,
mikko.mutanen@...rohmeurope.com, Robin Gong <yibin.gong@....com>,
Elven Wang <elven.wang@....com>,
Anson Huang <anson.huang@....com>,
Angus Ainslie <angus.ainslie@...i.sm>,
linux-kernel-owner@...r.kernel.org
Subject: Re: [PATCH v3 3/3] regulator: bd718x7: Support SNVS low power state
On 2019-02-14 01:39, Matti Vaittinen wrote:
> read ROHM BD71837 / BD71847 specific device tree bindings for
> controlling the PMIC shutdown/reset states and voltages for
> different HW states. The PMIC was designed to be used with NXP
> i.MX8 SoC and it supports SNVS low power state which seems to
> be typical for NXP i.MX SoCs. However, when SNVS is used we must
> not allow SW to control enabling/disabling those regulators which
> are crucial for system to boot as there is a HW limitation which
> causes SW controlled regulators to be kept shut down after SNVS
> reset.
>
> Allow setting the SNVS to be used as reset target state and allow
> marking those regulators which are critical for boot.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Tested-by: Angus Ainslie <angus@...ea.ca>
Reviewed-by: Angus Ainslie <angus@...ea.ca>
> ---
>
> Angus, I would be grateful if you have a chance to test this
> on your system :)
>
> drivers/regulator/bd718x7-regulator.c | 201
> +++++++++++++++++++++++++++++-----
> 1 file changed, 176 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/regulator/bd718x7-regulator.c
> b/drivers/regulator/bd718x7-regulator.c
> index ccea133778c8..b2191be49670 100644
> --- a/drivers/regulator/bd718x7-regulator.c
> +++ b/drivers/regulator/bd718x7-regulator.c
> @@ -350,6 +350,135 @@ static const struct reg_init bd71837_ldo6_inits[]
> = {
> },
> };
>
> +#define NUM_DVS_BUCKS 4
> +
> +struct of_dvs_setting {
> + const char *prop;
> + unsigned int reg;
> +};
> +
> +static int set_dvs_levels(const struct of_dvs_setting *dvs,
> + struct device_node *np,
> + const struct regulator_desc *desc,
> + struct regmap *regmap)
> +{
> + int ret, i;
> + unsigned int uv;
> +
> + ret = of_property_read_u32(np, dvs->prop, &uv);
> + if (ret) {
> + if (ret != -EINVAL)
> + return ret;
> + return 0;
> + }
> +
> + for (i = 0; i < desc->n_voltages; i++) {
> + ret = regulator_desc_list_voltage_linear_range(desc, i);
> + if (ret < 0)
> + continue;
> + if (ret == uv) {
> + i <<= ffs(desc->vsel_mask) - 1;
> + ret = regmap_update_bits(regmap, dvs->reg,
> + DVS_BUCK_RUN_MASK, i);
> + break;
> + }
> + }
> + return ret;
> +}
> +
> +static int buck4_set_hw_dvs_levels(struct device_node *np,
> + const struct regulator_desc *desc,
> + struct regulator_config *cfg)
> +{
> + int ret, i;
> + const struct of_dvs_setting dvs[] = {
> + {
> + .prop = "rohm,dvs-run-voltage",
> + .reg = BD71837_REG_BUCK4_VOLT_RUN,
> + },
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> + ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +static int buck3_set_hw_dvs_levels(struct device_node *np,
> + const struct regulator_desc *desc,
> + struct regulator_config *cfg)
> +{
> + int ret, i;
> + const struct of_dvs_setting dvs[] = {
> + {
> + .prop = "rohm,dvs-run-voltage",
> + .reg = BD71837_REG_BUCK3_VOLT_RUN,
> + },
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> + ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +
> +static int buck2_set_hw_dvs_levels(struct device_node *np,
> + const struct regulator_desc *desc,
> + struct regulator_config *cfg)
> +{
> + int ret, i;
> + const struct of_dvs_setting dvs[] = {
> + {
> + .prop = "rohm,dvs-run-voltage",
> + .reg = BD718XX_REG_BUCK2_VOLT_RUN,
> + },
> + {
> + .prop = "rohm,dvs-idle-voltage",
> + .reg = BD718XX_REG_BUCK2_VOLT_IDLE,
> + },
> + };
> +
> +
> +
> + for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> + ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +
> +static int buck1_set_hw_dvs_levels(struct device_node *np,
> + const struct regulator_desc *desc,
> + struct regulator_config *cfg)
> +{
> + int ret, i;
> + const struct of_dvs_setting dvs[] = {
> + {
> + .prop = "rohm,dvs-run-voltage",
> + .reg = BD718XX_REG_BUCK1_VOLT_RUN,
> + },
> + {
> + .prop = "rohm,dvs-idle-voltage",
> + .reg = BD718XX_REG_BUCK1_VOLT_IDLE,
> + },
> + {
> + .prop = "rohm,dvs-suspend-voltage",
> + .reg = BD718XX_REG_BUCK1_VOLT_SUSP,
> + },
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(dvs); i++) {
> + ret = set_dvs_levels(&dvs[i], np, desc, cfg->regmap);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +
> static const struct bd718xx_regulator_data bd71847_regulators[] = {
> {
> .desc = {
> @@ -368,6 +497,7 @@ static const struct bd718xx_regulator_data
> bd71847_regulators[] = {
> .enable_reg = BD718XX_REG_BUCK1_CTRL,
> .enable_mask = BD718XX_BUCK_EN,
> .owner = THIS_MODULE,
> + .of_parse_cb = buck1_set_hw_dvs_levels,
> },
> .init = {
> .reg = BD718XX_REG_BUCK1_CTRL,
> @@ -391,6 +521,7 @@ static const struct bd718xx_regulator_data
> bd71847_regulators[] = {
> .enable_reg = BD718XX_REG_BUCK2_CTRL,
> .enable_mask = BD718XX_BUCK_EN,
> .owner = THIS_MODULE,
> + .of_parse_cb = buck2_set_hw_dvs_levels,
> },
> .init = {
> .reg = BD718XX_REG_BUCK2_CTRL,
> @@ -662,6 +793,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
> .enable_reg = BD718XX_REG_BUCK1_CTRL,
> .enable_mask = BD718XX_BUCK_EN,
> .owner = THIS_MODULE,
> + .of_parse_cb = buck1_set_hw_dvs_levels,
> },
> .init = {
> .reg = BD718XX_REG_BUCK1_CTRL,
> @@ -685,6 +817,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
> .enable_reg = BD718XX_REG_BUCK2_CTRL,
> .enable_mask = BD718XX_BUCK_EN,
> .owner = THIS_MODULE,
> + .of_parse_cb = buck2_set_hw_dvs_levels,
> },
> .init = {
> .reg = BD718XX_REG_BUCK2_CTRL,
> @@ -708,6 +841,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
> .enable_reg = BD71837_REG_BUCK3_CTRL,
> .enable_mask = BD718XX_BUCK_EN,
> .owner = THIS_MODULE,
> + .of_parse_cb = buck3_set_hw_dvs_levels,
> },
> .init = {
> .reg = BD71837_REG_BUCK3_CTRL,
> @@ -731,6 +865,7 @@ static const struct bd718xx_regulator_data
> bd71837_regulators[] = {
> .enable_reg = BD71837_REG_BUCK4_CTRL,
> .enable_mask = BD718XX_BUCK_EN,
> .owner = THIS_MODULE,
> + .of_parse_cb = buck4_set_hw_dvs_levels,
> },
> .init = {
> .reg = BD71837_REG_BUCK4_CTRL,
> @@ -1029,6 +1164,7 @@ static int bd718xx_probe(struct platform_device
> *pdev)
> };
>
> int i, j, err;
> + bool use_snvs;
>
> mfd = dev_get_drvdata(pdev->dev.parent);
> if (!mfd) {
> @@ -1055,27 +1191,28 @@ static int bd718xx_probe(struct platform_device
> *pdev)
> BD718XX_REG_REGLOCK);
> }
>
> - /* At poweroff transition PMIC HW disables EN bit for regulators but
> - * leaves SEL bit untouched. So if state transition from POWEROFF
> - * is done to SNVS - then all power rails controlled by SW (having
> - * SEL bit set) stay disabled as EN is cleared. This may result boot
> - * failure if any crucial systems are powered by these rails.
> - *
> + use_snvs = of_property_read_bool(pdev->dev.parent->of_node,
> + "rohm,reset-snvs-powered");
> +
> + /*
> * Change the next stage from poweroff to be READY instead of SNVS
> * for all reset types because OTP loading at READY will clear SEL
> * bit allowing HW defaults for power rails to be used
> */
> - err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1,
> - BD718XX_ON_REQ_POWEROFF_MASK |
> - BD718XX_SWRESET_POWEROFF_MASK |
> - BD718XX_WDOG_POWEROFF_MASK |
> - BD718XX_KEY_L_POWEROFF_MASK,
> - BD718XX_POWOFF_TO_RDY);
> - if (err) {
> - dev_err(&pdev->dev, "Failed to change reset target\n");
> - goto err;
> - } else {
> - dev_dbg(&pdev->dev, "Changed all resets from SVNS to READY\n");
> + if (!use_snvs) {
> + err = regmap_update_bits(mfd->regmap, BD718XX_REG_TRANS_COND1,
> + BD718XX_ON_REQ_POWEROFF_MASK |
> + BD718XX_SWRESET_POWEROFF_MASK |
> + BD718XX_WDOG_POWEROFF_MASK |
> + BD718XX_KEY_L_POWEROFF_MASK,
> + BD718XX_POWOFF_TO_RDY);
> + if (err) {
> + dev_err(&pdev->dev, "Failed to change reset target\n");
> + goto err;
> + } else {
> + dev_dbg(&pdev->dev,
> + "Changed all resets from SVNS to READY\n");
> + }
> }
>
> for (i = 0; i < pmic_regulators[mfd->chip_type].r_amount; i++) {
> @@ -1098,19 +1235,33 @@ static int bd718xx_probe(struct platform_device
> *pdev)
> err = PTR_ERR(rdev);
> goto err;
> }
> - /* Regulator register gets the regulator constraints and
> +
> + /*
> + * Regulator register gets the regulator constraints and
> * applies them (set_machine_constraints). This should have
> * turned the control register(s) to correct values and we
> * can now switch the control from PMIC state machine to the
> * register interface
> + *
> + * At poweroff transition PMIC HW disables EN bit for
> + * regulators but leaves SEL bit untouched. So if state
> + * transition from POWEROFF is done to SNVS - then all power
> + * rails controlled by SW (having SEL bit set) stay disabled
> + * as EN is cleared. This will result boot failure if any
> + * crucial systems are powered by these rails. We don't
> + * enable SW control for crucial regulators if snvs state is
> + * used
> */
> - err = regmap_update_bits(mfd->regmap, r->init.reg,
> - r->init.mask, r->init.val);
> - if (err) {
> - dev_err(&pdev->dev,
> - "Failed to write BUCK/LDO SEL bit for (%s)\n",
> - desc->name);
> - goto err;
> + if (!use_snvs || !rdev->constraints->always_on ||
> + !rdev->constraints->boot_on) {
> + err = regmap_update_bits(mfd->regmap, r->init.reg,
> + r->init.mask, r->init.val);
> + if (err) {
> + dev_err(&pdev->dev,
> + "Failed to take control for (%s)\n",
> + desc->name);
> + goto err;
> + }
> }
> for (j = 0; j < r->additional_init_amnt; j++) {
> err = regmap_update_bits(mfd->regmap,
> --
> 2.14.3
Powered by blists - more mailing lists