Re: [PATCH v2 3/7] regulator: qcom-labibb: Implement pull-down, softstart, active discharge
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
---
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_BITBIT(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_PULLBIT(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_ENBIT(3)
#define LABIBB_CURRENT_LIMIT_EN BIT(7)
+#define REG_IBB_PWRUP_PWRDN_CTL_1 0x58
+ #define IBB_CTL_1_DISCHARGE_ENBIT(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_limitsuA_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
Thank you!
Regards,
Bjorn
Re: [PATCH v2 3/7] regulator: qcom-labibb: Implement pull-down, softstart, active discharge
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
>
> ---
> 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_ENABLEBIT(7)
>
> +#define REG_LABIBB_PD_CTL0x47
> + #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_STRENGTHBIT(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_10x58
> + #define IBB_CTL_1_DISCHARGE_EN BIT(2)
> +
> +#define REG_LABIBB_SOFT_START_CTL0x5f
> #define REG_LABIBB_SEC_ACCESS0xd0
> #define LABIBB_SEC_UNLOCK_CODE 0xa5
>
> @@ -60,6 +71,8 @@ struct labibb_regulator {
> struct labibb_current_limitsuA_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...
That said, I think this patch looks good.
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
[PATCH v2 3/7] regulator: qcom-labibb: Implement pull-down, softstart, active discharge
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
---
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_PULLBIT(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_MASKGENMASK(2, 0)
#define IBB_CURRENT_LIMIT_MASKGENMASK(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_ENBIT(2)
+
+#define REG_LABIBB_SOFT_START_CTL 0x5f
#define REG_LABIBB_SEC_ACCESS 0xd0
#define LABIBB_SEC_UNLOCK_CODE0xa5
@@ -60,6 +71,8 @@ struct labibb_regulator {
struct labibb_current_limitsuA_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;
+
+ ret = qcom_labibb_get_table_sel(dischg_resistor_values,
+ ARRAY_SIZE(dischg_resistor_values),
+ dischg_kohms);
+ if (ret < 0)
+ return ret;
+ vreg->dischg_sel = (u8)ret;
+
+ ret = of_property_read_u32(np, "qcom,soft-start-us",
+ &soft_start_time);
+ if (ret)
+ soft_start_time = 200;
+
+ ret = qcom_labibb_get_table_sel(soft_start_values,
+ ARRAY_SIZE(soft_start_values),
+ soft_start_time);
+ if (ret < 0)
+ return ret;
+ vreg->soft_start_sel = (u8)ret;
+
+ return 0;
+}
+
static const struct regulator_ops qcom_labibb_ops = {
.enable = regulator_enable_regmap,
.disable= regulator_disable_regmap,
@@ -128,8 +205,11 @@ static const struct regulator_ops qcom_labibb_ops = {
.get_voltage_sel= regulator_get_voltage_sel_regmap,
.list_voltage = regulator_list_voltage_linear_range,
.map_voltage= regulator_map_voltage_linear_range,
+
