diff mbox series

[v2,3/7] regulator: qcom-labibb: Implement pull-down, softstart, active discharge

Message ID 20210113194214.522238-4-angelogioacchino.delregno@somainline.org (mailing list archive)
State Superseded
Headers show
Series Really implement Qualcomm LAB/IBB regulators | expand

Commit Message

AngeloGioacchino Del Regno Jan. 13, 2021, 7:42 p.m. UTC 
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(+)

Comments

Bjorn Andersson Jan. 15, 2021, 4:53 a.m. UTC | #1 
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...

That said, I think this patch looks good.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn
AngeloGioacchino Del Regno Jan. 17, 2021, 6:15 p.m. UTC | #2 
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
>
diff mbox series

Patch

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;
+
+	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,
+	.set_active_discharge	= regulator_set_active_discharge_regmap,
+	.set_pull_down		= regulator_set_pull_down_regmap,
 	.set_current_limit	= qcom_labibb_set_current_limit,
 	.get_current_limit	= qcom_labibb_get_current_limit,
+	.set_soft_start		= qcom_labibb_set_soft_start,
 };
 
 static const struct regulator_desc pmi8998_lab_desc = {
@@ -138,6 +218,10 @@  static const struct regulator_desc pmi8998_lab_desc = {
 	.enable_val		= LABIBB_CONTROL_ENABLE,
 	.enable_time		= LAB_ENABLE_TIME,
 	.poll_enabled_time	= LABIBB_POLL_ENABLED_TIME,
+	.soft_start_reg		= (PMI8998_LAB_REG_BASE + REG_LABIBB_SOFT_START_CTL),
+	.pull_down_reg		= (PMI8998_LAB_REG_BASE + REG_LABIBB_PD_CTL),
+	.pull_down_mask		= LAB_PD_CTL_MASK,
+	.pull_down_val_on	= LAB_PD_CTL_STRONG_PULL,
 	.vsel_reg		= (PMI8998_LAB_REG_BASE + REG_LABIBB_VOLTAGE),
 	.vsel_mask		= LAB_VOLTAGE_SET_MASK,
 	.apply_reg		= (PMI8998_LAB_REG_BASE + REG_LABIBB_VOLTAGE),
@@ -154,6 +238,7 @@  static const struct regulator_desc pmi8998_lab_desc = {
 	.n_linear_ranges	= 1,
 	.n_voltages		= 16,
 	.ops			= &qcom_labibb_ops,
+	.of_parse_cb		= qcom_labibb_of_parse_cb,
 };
 
 static const struct regulator_desc pmi8998_ibb_desc = {
@@ -162,6 +247,14 @@  static const struct regulator_desc pmi8998_ibb_desc = {
 	.enable_val		= LABIBB_CONTROL_ENABLE,
 	.enable_time		= IBB_ENABLE_TIME,
 	.poll_enabled_time	= LABIBB_POLL_ENABLED_TIME,
+	.soft_start_reg		= (PMI8998_IBB_REG_BASE + REG_LABIBB_SOFT_START_CTL),
+	.active_discharge_off	= 0,
+	.active_discharge_on	= IBB_CTL_1_DISCHARGE_EN,
+	.active_discharge_mask	= IBB_CTL_1_DISCHARGE_EN,
+	.active_discharge_reg	= (PMI8998_IBB_REG_BASE + REG_IBB_PWRUP_PWRDN_CTL_1),
+	.pull_down_reg		= (PMI8998_IBB_REG_BASE + REG_LABIBB_PD_CTL),
+	.pull_down_mask		= IBB_PD_CTL_MASK,
+	.pull_down_val_on	= IBB_PD_CTL_HALF_STRENGTH | IBB_PD_CTL_EN,
 	.vsel_reg		= (PMI8998_IBB_REG_BASE + REG_LABIBB_VOLTAGE),
 	.vsel_mask		= IBB_VOLTAGE_SET_MASK,
 	.apply_reg		= (PMI8998_IBB_REG_BASE + REG_LABIBB_VOLTAGE),
@@ -178,6 +271,7 @@  static const struct regulator_desc pmi8998_ibb_desc = {
 	.n_linear_ranges	= 1,
 	.n_voltages		= 64,
 	.ops			= &qcom_labibb_ops,
+	.of_parse_cb		= qcom_labibb_of_parse_cb,
 };
 
 static const struct labibb_regulator_data pmi8998_labibb_data[] = {