On 9/8/22 03:31, Sebastian Reichel wrote:
Add rk806 support to the existing rk808 regulator
driver.
This has been implemented using shengfei Xu's rk806
specific driver from the vendor tree as reference.
Co-Developed-by: shengfei Xu <xsf@xxxxxxxxxxxxxx>
Signed-off-by: shengfei Xu <xsf@xxxxxxxxxxxxxx>
Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
---
drivers/regulator/rk808-regulator.c | 482 ++++++++++++++++++++++++++++
1 file changed, 482 insertions(+)
diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index fa9fc1aa1ae3..cd1a2cff4a37 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
Thanks for upstreaming the downstream stuff! :)
I wonder if we could drop some code by using the existing helpers? Or
maybe I am missreading some code. Wouldn't be the first (and probably
not the last) time...
//snip
struct rk808_regulator_data {
struct gpio_desc *dvs_gpio[2];
};
@@ -216,6 +271,223 @@ static const unsigned int rk817_buck1_4_ramp_table[] = {
3000, 6300, 12500, 25000
};
+static int rk806_get_voltage_sel_regmap(struct regulator_dev *rdev)
+{
+ unsigned int val;
+ int vsel_reg, ret;
+
+ vsel_reg = rdev->desc->vsel_reg;
+
+ ret = regmap_read(rdev->regmap, vsel_reg, &val);
+ if (ret != 0)
+ return ret;
+
+ val &= rdev->desc->vsel_mask;
+ val >>= ffs(rdev->desc->vsel_mask) - 1;
+
+ return val;
+}
Could we just use the regulator_get_voltage_sel_regmap()?
+
+static int rk806_set_voltage(struct regulator_dev *rdev,
+ int req_min_uV, int req_max_uV,
+ unsigned int *selector)
+{
+ int vsel_reg, ret, sel;
+
+ ret = regulator_map_voltage_linear_range(rdev, req_min_uV, req_max_uV);
+ if (ret >= 0) {
+ *selector = ret;
+ sel = ret;
+ } else {
+ return -EINVAL;
+ }
+
+ vsel_reg = rdev->desc->vsel_reg;
+
+ sel <<= ffs(rdev->desc->vsel_mask) - 1;
+
+ ret = regmap_update_bits(rdev->regmap, vsel_reg,
+ rdev->desc->vsel_mask, sel);
+
+ return ret;
+}
Hmm. Maybe this is not necessary? I wonder if we could get away just
with the .map and .set_voltage_sel (regulator_get_voltage_sel_regmap()
and regulator_map_voltage_linear_range()).
// snip
+
+static int rk806_set_ramp_delay_ldo(struct regulator_dev *rdev, int ramp_delay)
+{
+ unsigned int ramp_value = RK806_RAMP_RATE_2LSB_PER_1CLK;
+ int regval;
+
+ switch (ramp_delay) {
+ case 1 ... 780:
+ ramp_value = RK806_RAMP_RATE_1LSB_PER_32CLK;
+ break;
+ case 781 ... 1900:
+ ramp_value = RK806_RAMP_RATE_1LSB_PER_13CLK;
+ break;
+ case 1901 ... 3120:
+ ramp_value = RK806_RAMP_RATE_1LSB_PER_8CLK;
+ break;
+ case 3121 ... 6280:
+ ramp_value = RK806_RAMP_RATE_1LSB_PER_4CLK;
+ break;
+ case 6281 ... 12500:
+ ramp_value = RK806_RAMP_RATE_1LSB_PER_2CLK;
+ break;
+ case 12501 ... 25000:
+ ramp_value = RK806_RAMP_RATE_1LSB_PER_1CLK;
+ break;
+ case 25001 ... 50000:
+ ramp_value = RK806_RAMP_RATE_2LSB_PER_1CLK;
+ break;
+ case 50001 ... 100000:
+ ramp_value = RK806_RAMP_RATE_4LSB_PER_1CLK;
+ break;
+ default:
+ pr_warn("%s ramp_delay: %d not supported, setting 10000\n",
+ rdev->desc->name, ramp_delay);
+ }
+
+ regval = ramp_value << (ffs(rdev->desc->ramp_mask) - 1);
+ return regmap_update_bits(rdev->regmap, rdev->desc->ramp_reg,
+ rdev->desc->ramp_mask, regval);
+}
Do you think we could get rid of this function by populating a
ramp-delay table and using regulator_set_ramp_delay_regmap()?
Best Regards
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
