Messages in this thread | | | Subject | Re: [PATCH v2 3/7] regulator: qcom-labibb: Implement pull-down, softstart, active discharge | From | AngeloGioacchino Del Regno <> | Date | Sun, 17 Jan 2021 19:15:58 +0100 |
| |
Il 15/01/21 05:53, Bjorn Andersson ha scritto: > On Wed 13 Jan 13:42 CST 2021, AngeloGioacchino Del Regno wrote: > >> Soft start is required to avoid inrush current during LAB ramp-up and >> IBB ramp-down, protecting connected hardware to which we supply voltage. >> >> Since soft start is configurable on both LAB and IBB regulators, it >> was necessary to add two DT properties, respectively "qcom,soft-start-us" >> to control LAB ramp-up and "qcom,discharge-resistor-kohms" to control >> the discharge resistor for IBB ramp-down, which obviously brought the >> need of implementing a of_parse callback for both regulators. >> >> Finally, also implement pull-down mode in order to avoid unpredictable >> behavior when the regulators are disabled (random voltage spikes etc). >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> --- >> drivers/regulator/qcom-labibb-regulator.c | 94 +++++++++++++++++++++++ >> 1 file changed, 94 insertions(+) >> >> diff --git a/drivers/regulator/qcom-labibb-regulator.c b/drivers/regulator/qcom-labibb-regulator.c >> index d364f54ad294..38ab1eba1c59 100644 >> --- a/drivers/regulator/qcom-labibb-regulator.c >> +++ b/drivers/regulator/qcom-labibb-regulator.c >> @@ -29,12 +29,23 @@ >> #define LABIBB_STATUS1_VREG_OK_BIT BIT(7) >> #define LABIBB_CONTROL_ENABLE BIT(7) >> >> +#define REG_LABIBB_PD_CTL 0x47 >> + #define LAB_PD_CTL_MASK GENMASK(1, 0) >> + #define IBB_PD_CTL_MASK (BIT(0) | BIT(7)) >> + #define LAB_PD_CTL_STRONG_PULL BIT(0) >> + #define IBB_PD_CTL_HALF_STRENGTH BIT(0) >> + #define IBB_PD_CTL_EN BIT(7) >> + >> #define REG_LABIBB_CURRENT_LIMIT 0x4b >> #define LAB_CURRENT_LIMIT_MASK GENMASK(2, 0) >> #define IBB_CURRENT_LIMIT_MASK GENMASK(4, 0) >> #define LAB_CURRENT_LIMIT_OVERRIDE_EN BIT(3) >> #define LABIBB_CURRENT_LIMIT_EN BIT(7) >> >> +#define REG_IBB_PWRUP_PWRDN_CTL_1 0x58 >> + #define IBB_CTL_1_DISCHARGE_EN BIT(2) >> + >> +#define REG_LABIBB_SOFT_START_CTL 0x5f >> #define REG_LABIBB_SEC_ACCESS 0xd0 >> #define LABIBB_SEC_UNLOCK_CODE 0xa5 >> >> @@ -60,6 +71,8 @@ struct labibb_regulator { >> struct labibb_current_limits uA_limits; >> u16 base; >> u8 type; >> + u8 dischg_sel; >> + u8 soft_start_sel; >> }; >> >> struct labibb_regulator_data { >> @@ -120,6 +133,70 @@ static int qcom_labibb_get_current_limit(struct regulator_dev *rdev) >> return (cur_step * lim->uA_step) + lim->uA_min; >> } >> >> +static int qcom_labibb_set_soft_start(struct regulator_dev *rdev) >> +{ >> + struct labibb_regulator *vreg = rdev_get_drvdata(rdev); >> + u32 val = 0; >> + >> + if (vreg->type == QCOM_IBB_TYPE) >> + val = vreg->dischg_sel; >> + else >> + val = vreg->soft_start_sel; >> + >> + return regmap_write(rdev->regmap, rdev->desc->soft_start_reg, val); >> +} >> + >> +static int qcom_labibb_get_table_sel(const int *table, int sz, u32 value) >> +{ >> + int i; >> + >> + for (i = 0; i < sz; i++) >> + if (table[i] == value) >> + return i; >> + return -EINVAL; >> +} >> + >> +/* IBB discharge resistor values in KOhms */ >> +static const int dischg_resistor_values[] = { 300, 64, 32, 16 }; >> + >> +/* Soft start time in microseconds */ >> +static const int soft_start_values[] = { 200, 400, 600, 800 }; >> + >> +static int qcom_labibb_of_parse_cb(struct device_node *np, >> + const struct regulator_desc *desc, >> + struct regulator_config *config) >> +{ >> + struct labibb_regulator *vreg = config->driver_data; >> + u32 dischg_kohms, soft_start_time; >> + int ret; >> + >> + ret = of_property_read_u32(np, "qcom,discharge-resistor-kohms", >> + &dischg_kohms); >> + if (ret) >> + dischg_kohms = 300; > > Nit, if you just initialize dischg_kohms to 300 during definition you > can rely on of_property_read_u32() not updating the value on failure... >
I can change it if that's really necessary, I did it like this in an attempt of spoon-feed the logic to the reader, but perhaps just initializing it during definition would achieve the same, anyway.
Should I?
> That said, I think this patch looks good. > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> >
Thank you!
> Regards, > Bjorn >
| |