Re: [PATCH v2 11/13] regulator: hi6421v600-regulator: move it from staging
From: Mark Brown <broonie@kernel.org>
Date: 2021-01-18 13:58:08
Also in:
lkml
On Mon, Jan 18, 2021 at 02:28:12PM +0100, Mauro Carvalho Chehab wrote:
quoted hunk ↗ jump to hunk
index f385146d2bd1..3b23ad56b31a 100644--- a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml +++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml@@ -60,6 +60,8 @@ required: - reg - regulators +additionalProperties: false + examples: - | /* pmic properties */
Why is this part of this patch?
quoted hunk ↗ jump to hunk
+// SPDX-License-Identifier: GPL-2.0 +// +// Device driver for regulators in Hisi IC +// +// Copyright (c) 2013 Linaro Ltd. +// Copyright (c) 2011 Hisilicon. +//
This looks like it needs an update.
quoted hunk ↗ jump to hunk
+// This program is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 2 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details.
This boilerplate can be removed.
quoted hunk ↗ jump to hunk
+static int hi6421_spmi_regulator_enable(struct regulator_dev *rdev) +{ + struct hi6421_spmi_reg_info *sreg = rdev_get_drvdata(rdev); + struct hi6421_spmi_pmic *pmic = sreg->pmic; + int ret; + + /* cannot enable more than one regulator at one time */ + mutex_lock(&sreg->enable_mutex); + usleep_range(HISI_REGS_ENA_PROTECT_TIME, + HISI_REGS_ENA_PROTECT_TIME + 1000); + + /* set enable register */ + ret = hi6421_spmi_pmic_rmw(pmic, rdev->desc->enable_reg, + rdev->desc->enable_mask, + rdev->desc->enable_mask);
If for some reason the PMIC is sufficiently fragile to need a delay between enables it's not clear why the driver is doing it before enabling rather than after, presumably there's issues with the regulator ramping up and stabalising its output
quoted hunk ↗ jump to hunk
+ /* set enable register to 0 */ + return hi6421_spmi_pmic_rmw(pmic, rdev->desc->enable_reg, + rdev->desc->enable_mask, 0);
I'm not sure all these comments are adding anything.
+ if (unlikely(selector >= rdev->desc->n_voltages)) + return -EINVAL;
This should not be a hot path that needs an unlikely() annotation.
quoted hunk ↗ jump to hunk
+static unsigned int +hi6421_spmi_regulator_get_optimum_mode(struct regulator_dev *rdev, + int input_uV, int output_uV, + int load_uA) +{ + struct hi6421_spmi_reg_info *sreg = rdev_get_drvdata(rdev); + + if (load_uA || ((unsigned int)load_uA > sreg->eco_uA)) + return REGULATOR_MODE_NORMAL;
This means that for *any* load at all we select NORMAL - I'm not convinced this is intentional?
Attachments
- signature.asc [application/pgp-signature] 488 bytes