Linux-Devicetree Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix rtq2208 buck ramp_delay and ldo discharge and dvs setting
@ 2024-04-29 10:16 Alina Yu
  2024-04-29 10:16 ` [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not Alina Yu
  2024-04-29 10:16 ` [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put_matches is called Alina Yu
  0 siblings, 2 replies; 6+ messages in thread
From: Alina Yu @ 2024-04-29 10:16 UTC (permalink / raw
  To: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree, alina_yu, johnny_lai, cy_huang

Hi,

This series patches is for hardware modification of RTQ2208.
ramp_delay range of bucks is changed.
The maximum ramp up and down range is shorten from 64mVstep/us tor 16mVstep/us/.
The LDO's Vout is adjustable if the haardware setting allow it, and it can be set either 1800mv or 3300mv.
Additionally, the discharge register is chaned to other position.
The discharge register has been moved to another position.
In this version, a software bug has been fixed. rtq2208_ldo_match is no longer a local variable,
which prevents invalid memory access when devm_of_regulator_put_matches is called.

Thank you,
Alina
---
Alina Yu (2):
  regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC
  regulator: rtq2208: Add Richtek RTQ2208 SubPMIC driver

 .../bindings/regulator/richtek,rtq2208.yaml        |  10 ++
 drivers/regulator/rtq2208-regulator.c              | 169 +++++++++++++--------
 2 files changed, 119 insertions(+), 60 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not
  2024-04-29 10:16 [PATCH 0/2] Fix rtq2208 buck ramp_delay and ldo discharge and dvs setting Alina Yu
@ 2024-04-29 10:16 ` Alina Yu
  2024-04-29 10:37   ` Krzysztof Kozlowski
  2024-04-29 16:09   ` Mark Brown
  2024-04-29 10:16 ` [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put_matches is called Alina Yu
  1 sibling, 2 replies; 6+ messages in thread
From: Alina Yu @ 2024-04-29 10:16 UTC (permalink / raw
  To: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree, alina_yu, johnny_lai, cy_huang

Since there is no way to check is ldo is adjustable or not.
'richtek,use-fix-dvs' is added for that. user is supposed to know whether vout of ldo is adjustable.

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
 .../devicetree/bindings/regulator/richtek,rtq2208.yaml         | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
index 609c066..3951679 100644
--- a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
+++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
@@ -75,6 +75,14 @@ properties:
         description:
           regulator description for ldo[1-2].
 
+        properties:
+          richtek,use-fix-dvs:
+            type: boolean
+            description: |
+              ldo vout ability is determined by this setting. If it's set, the voltage is unadjustable.
+              There is no risk-free method for software to determine whether the ldo vout is fixed or not.
+              Therefore, it can only be done in this way.
+
 required:
   - compatible
   - reg
@@ -180,6 +188,7 @@ examples:
             regulator-min-microvolt = <1200000>;
             regulator-max-microvolt = <1200000>;
             regulator-always-on;
+            richtek,use-fix-dvs;
             regulator-state-mem {
               regulator-on-in-suspend;
             };
@@ -188,6 +197,7 @@ examples:
             regulator-min-microvolt = <3300000>;
             regulator-max-microvolt = <3300000>;
             regulator-always-on;
+            richtek,use-fix-dvs;
             regulator-state-mem {
               regulator-on-in-suspend;
             };
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put_matches is called
  2024-04-29 10:16 [PATCH 0/2] Fix rtq2208 buck ramp_delay and ldo discharge and dvs setting Alina Yu
  2024-04-29 10:16 ` [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not Alina Yu
@ 2024-04-29 10:16 ` Alina Yu
  2024-04-29 16:13   ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Alina Yu @ 2024-04-29 10:16 UTC (permalink / raw
  To: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-kernel, devicetree, alina_yu, johnny_lai, cy_huang

ramp_delay range of bucks is changed.
The maximum ramp up and down range is shorten from 64mVstep/us tor 16mVstep/us/.
The LDO's Vout is adjustable if the haardware setting allow it, and it can be set either 1800mv or 3300mv.
Additionally, the discharge register is chaned to other position.
The discharge register has been moved to another position.
In this version, a software bug has been fixed. rtq2208_ldo_match is no longer a local variable,
which prevents invalid memory access when devm_of_regulator_put_matches is called.

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
 drivers/regulator/rtq2208-regulator.c | 169 ++++++++++++++++++++++------------
 1 file changed, 109 insertions(+), 60 deletions(-)

diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
index 2d54844..95612cc 100644
--- a/drivers/regulator/rtq2208-regulator.c
+++ b/drivers/regulator/rtq2208-regulator.c
@@ -26,6 +26,7 @@
 #define RTQ2208_REG_BUCK_H_CFG0			0xA2
 #define RTQ2208_REG_LDO1_CFG			0xB1
 #define RTQ2208_REG_LDO2_CFG			0xC1
+#define RTQ2208_REG_LDO_DVS_CTRL		0xD0
 
 /* Mask */
 #define RTQ2208_BUCK_NR_MTP_SEL_MASK		GENMASK(7, 0)
@@ -40,6 +41,10 @@
 #define RTQ2208_EN_DIS_MASK			BIT(0)
 #define RTQ2208_BUCK_RAMP_SEL_MASK		GENMASK(2, 0)
 #define RTQ2208_HD_INT_MASK			BIT(0)
+#define RTQ2208_LDO1_DISCHG_EN_MASK		BIT(4)
+#define RTQ2208_LDO1_VOSEL_SD_MASK		BIT(5)
+#define RTQ2208_LDO2_DISCHG_EN_MASK		BIT(6)
+#define RTQ2208_LDO2_VOSEL_SD_MASK		BIT(7)
 
 /* Size */
 #define RTQ2208_VOUT_MAXNUM			256
@@ -48,7 +53,7 @@
 
 /* Value */
 #define RTQ2208_RAMP_VALUE_MIN_uV		500
-#define RTQ2208_RAMP_VALUE_MAX_uV		64000
+#define RTQ2208_RAMP_VALUE_MAX_uV		16000
 
 #define RTQ2208_BUCK_MASK(uv_irq, ov_irq)	(1 << ((uv_irq) % 8) | 1 << ((ov_irq) % 8))
 
@@ -142,12 +147,11 @@ static int rtq2208_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
 	 * Because the relation of seleltion and value is like that
 	 *
 	 * seletion: value
-	 * 000: 64mv
-	 * 001: 32mv
+	 * 010: 16mv
 	 * ...
 	 * 111: 0.5mv
 	 *
-	 * For example, if I would like to select 64mv, the fls(ramp_delay) - 1 will be 0b111,
+	 * For example, if I would like to select 16mv, the fls(ramp_delay) - 1 will be 0b010,
 	 * and I need to use 0b111 - sel to do the shifting
 	 */
 
@@ -215,7 +219,7 @@ static const struct regulator_ops rtq2208_regulator_buck_ops = {
 	.set_suspend_mode = rtq2208_set_suspend_mode,
 };
 
-static const struct regulator_ops rtq2208_regulator_ldo_ops = {
+static const struct regulator_ops rtq2208_regulator_ldo_fix_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
@@ -224,6 +228,28 @@ static const struct regulator_ops rtq2208_regulator_ldo_ops = {
 	.set_suspend_disable = rtq2208_set_suspend_disable,
 };
 
+static const struct regulator_ops rtq2208_regulator_ldo_adj_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_active_discharge = regulator_set_active_discharge_regmap,
+	.set_suspend_enable = rtq2208_set_suspend_enable,
+	.set_suspend_disable = rtq2208_set_suspend_disable,
+};
+
+static const unsigned int rtq2208_ldo_volt_table[] = {
+	1800000,
+	3300000,
+};
+
+static struct of_regulator_match rtq2208_ldo_match[] = {
+	{.name = "ldo2", },
+	{.name = "ldo1", },
+};
+
 static unsigned int rtq2208_of_map_mode(unsigned int mode)
 {
 	switch (mode) {
@@ -318,30 +344,13 @@ static irqreturn_t rtq2208_irq_handler(int irqno, void *devid)
 	return IRQ_HANDLED;
 }
 
-#define RTQ2208_REGULATOR_INFO(_name, _base) \
-{ \
-	.name = #_name, \
-	.base = _base, \
-}
-#define BUCK_RG_BASE(_id)	RTQ2208_REG_BUCK_##_id##_CFG0
-#define BUCK_RG_SHIFT(_base, _shift)	(_base + _shift)
-#define LDO_RG_BASE(_id)	RTQ2208_REG_LDO##_id##_CFG
-#define LDO_RG_SHIFT(_base, _shift)	(_base + _shift)
-#define	VSEL_SHIFT(_sel)	(_sel ? 3 : 1)
-#define MTP_SEL_MASK(_sel)	RTQ2208_BUCK_EN_NR_MTP_SEL##_sel##_MASK
-
-static const struct linear_range rtq2208_vout_range[] = {
-	REGULATOR_LINEAR_RANGE(400000, 0, 180, 5000),
-	REGULATOR_LINEAR_RANGE(1310000, 181, 255, 10000),
-};
-
-static int rtq2208_of_get_fixed_voltage(struct device *dev,
-					struct of_regulator_match *rtq2208_ldo_match, int n_fixed)
+static int rtq2208_of_get_ldo_dvs_ability(struct device *dev)
 {
 	struct device_node *np;
 	struct of_regulator_match *match;
-	struct rtq2208_regulator_desc *rdesc;
+	struct regulator_desc *desc;
 	struct regulator_init_data *init_data;
+	bool fixed_uV;
 	int ret, i;
 
 	if (!dev->of_node)
@@ -351,46 +360,87 @@ static int rtq2208_of_get_fixed_voltage(struct device *dev,
 	if (!np)
 		np = dev->of_node;
 
-	ret = of_regulator_match(dev, np, rtq2208_ldo_match, n_fixed);
+	ret = of_regulator_match(dev, np, rtq2208_ldo_match, ARRAY_SIZE(rtq2208_ldo_match));
 
 	of_node_put(np);
 
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < n_fixed; i++) {
+	for (i = 0; i < ARRAY_SIZE(rtq2208_ldo_match); i++) {
 		match = rtq2208_ldo_match + i;
 		init_data = match->init_data;
-		rdesc = (struct rtq2208_regulator_desc *)match->driver_data;
+		desc = (struct regulator_desc *)match->desc;
 
-		if (!init_data || !rdesc)
+		if (!init_data || !desc)
 			continue;
 
-		if (init_data->constraints.min_uV == init_data->constraints.max_uV)
-			rdesc->desc.fixed_uV = init_data->constraints.min_uV;
+		fixed_uV = of_property_read_bool(match->of_node, "richtek,use-fix-dvs");
+
+		if (fixed_uV) {
+			desc->n_voltages = 1;
+			desc->fixed_uV = init_data->constraints.min_uV;
+			desc->ops = &rtq2208_regulator_ldo_fix_ops;
+		} else {
+			desc->n_voltages = ARRAY_SIZE(rtq2208_ldo_volt_table);
+			desc->volt_table = rtq2208_ldo_volt_table;
+			desc->ops = &rtq2208_regulator_ldo_adj_ops;
+		}
 	}
 
 	return 0;
 }
 
-static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, int mtp_sel,
-					int idx, struct of_regulator_match *rtq2208_ldo_match, int *ldo_idx)
+
+#define BUCK_INFO(_name, _id)						\
+{									\
+	.name = _name,							\
+	.base = RTQ2208_REG_BUCK_##_id##_CFG0,				\
+	.enable_reg = BUCK_RG_SHIFT(RTQ2208_REG_BUCK_##_id##_CFG0, 2),	\
+	.dis_reg = RTQ2208_REG_BUCK_##_id##_CFG0,			\
+}
+
+#define LDO_INFO(_name, _id)						\
+{									\
+	.name = _name,							\
+	.base = RTQ2208_REG_LDO##_id##_CFG,				\
+	.enable_reg = RTQ2208_REG_LDO##_id##_CFG,			\
+	.dis_mask = RTQ2208_LDO##_id##_DISCHG_EN_MASK,			\
+	.dis_on = RTQ2208_LDO##_id##_DISCHG_EN_MASK,			\
+	.vsel_mask = RTQ2208_LDO##_id##_VOSEL_SD_MASK,			\
+}
+
+#define BUCK_RG_SHIFT(_base, _shift)	(_base + _shift)
+#define	VSEL_SHIFT(_sel)	(_sel ? 3 : 1)
+#define MTP_SEL_MASK(_sel)	RTQ2208_BUCK_EN_NR_MTP_SEL##_sel##_MASK
+
+static const struct linear_range rtq2208_vout_range[] = {
+	REGULATOR_LINEAR_RANGE(400000, 0, 180, 5000),
+	REGULATOR_LINEAR_RANGE(1310000, 181, 255, 10000),
+};
+
+static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, int mtp_sel, int idx)
 {
 	struct regulator_desc *desc;
 	static const struct {
 		char *name;
 		int base;
+		int enable_reg;
+		int dis_reg;
+		int dis_mask;
+		int dis_on;
+		int vsel_mask;
 	} regulator_info[] = {
-		RTQ2208_REGULATOR_INFO(buck-b, BUCK_RG_BASE(B)),
-		RTQ2208_REGULATOR_INFO(buck-c, BUCK_RG_BASE(C)),
-		RTQ2208_REGULATOR_INFO(buck-d, BUCK_RG_BASE(D)),
-		RTQ2208_REGULATOR_INFO(buck-a, BUCK_RG_BASE(A)),
-		RTQ2208_REGULATOR_INFO(buck-f, BUCK_RG_BASE(F)),
-		RTQ2208_REGULATOR_INFO(buck-g, BUCK_RG_BASE(G)),
-		RTQ2208_REGULATOR_INFO(buck-h, BUCK_RG_BASE(H)),
-		RTQ2208_REGULATOR_INFO(buck-e, BUCK_RG_BASE(E)),
-		RTQ2208_REGULATOR_INFO(ldo2, LDO_RG_BASE(2)),
-		RTQ2208_REGULATOR_INFO(ldo1, LDO_RG_BASE(1)),
+		BUCK_INFO("buck-b", B),
+		BUCK_INFO("buck-c", C),
+		BUCK_INFO("buck-d", D),
+		BUCK_INFO("buck-a", A),
+		BUCK_INFO("buck-f", F),
+		BUCK_INFO("buck-g", G),
+		BUCK_INFO("buck-h", H),
+		BUCK_INFO("buck-e", E),
+		LDO_INFO("ldo2", 2),
+		LDO_INFO("ldo1", 1),
 	}, *curr_info;
 
 	curr_info = regulator_info + idx;
@@ -402,15 +452,13 @@ static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, in
 	desc->owner = THIS_MODULE;
 	desc->type = REGULATOR_VOLTAGE;
 	desc->enable_mask = mtp_sel ? MTP_SEL_MASK(1) : MTP_SEL_MASK(0);
-	desc->active_discharge_on = RTQ2208_EN_DIS_MASK;
+	desc->enable_reg = curr_info->enable_reg;
 	desc->active_discharge_off = 0;
-	desc->active_discharge_mask = RTQ2208_EN_DIS_MASK;
 
 	rdesc->mode_mask = RTQ2208_BUCK_NRMODE_MASK;
 
 	if (idx >= RTQ2208_BUCK_B && idx <= RTQ2208_BUCK_E) {
 		/* init buck desc */
-		desc->enable_reg = BUCK_RG_SHIFT(curr_info->base, 2);
 		desc->ops = &rtq2208_regulator_buck_ops;
 		desc->vsel_reg = curr_info->base + VSEL_SHIFT(mtp_sel);
 		desc->vsel_mask = RTQ2208_BUCK_NR_MTP_SEL_MASK;
@@ -418,8 +466,10 @@ static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, in
 		desc->linear_ranges = rtq2208_vout_range;
 		desc->n_linear_ranges = ARRAY_SIZE(rtq2208_vout_range);
 		desc->ramp_reg = BUCK_RG_SHIFT(curr_info->base, 5);
-		desc->active_discharge_reg = curr_info->base;
 		desc->of_map_mode = rtq2208_of_map_mode;
+		desc->active_discharge_reg = curr_info->dis_reg;
+		desc->active_discharge_on = RTQ2208_EN_DIS_MASK;
+		desc->active_discharge_mask = RTQ2208_EN_DIS_MASK;
 
 		rdesc->mode_reg = BUCK_RG_SHIFT(curr_info->base, 2);
 		rdesc->suspend_config_reg = BUCK_RG_SHIFT(curr_info->base, 4);
@@ -427,14 +477,11 @@ static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, in
 		rdesc->suspend_mode_mask = RTQ2208_BUCK_STRMODE_MASK;
 	} else {
 		/* init ldo desc */
-		desc->enable_reg = curr_info->base;
-		desc->ops = &rtq2208_regulator_ldo_ops;
-		desc->n_voltages = 1;
-		desc->active_discharge_reg = LDO_RG_SHIFT(curr_info->base, 2);
-
-		rtq2208_ldo_match[*ldo_idx].name = desc->name;
-		rtq2208_ldo_match[*ldo_idx].driver_data = rdesc;
-		rtq2208_ldo_match[(*ldo_idx)++].desc = desc;
+		desc->active_discharge_reg = RTQ2208_REG_LDO_DVS_CTRL;
+		desc->active_discharge_on = curr_info->dis_on;
+		desc->active_discharge_mask = curr_info->dis_mask;
+		desc->vsel_reg = RTQ2208_REG_LDO_DVS_CTRL;
+		desc->vsel_mask = curr_info->vsel_mask;
 
 		rdesc->suspend_config_reg = curr_info->base;
 		rdesc->suspend_enable_mask = RTQ2208_LDO_EN_STR_MASK;
@@ -444,8 +491,7 @@ static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, in
 static int rtq2208_parse_regulator_dt_data(int n_regulator, const unsigned int *regulator_idx_table,
 		struct rtq2208_regulator_desc *rdesc[RTQ2208_LDO_MAX], struct device *dev)
 {
-	struct of_regulator_match rtq2208_ldo_match[2];
-	int mtp_sel, ret, i, idx, ldo_idx = 0;
+	int mtp_sel, i, idx, ret;
 
 	/* get mtp_sel0 or mtp_sel1 */
 	mtp_sel = device_property_read_bool(dev, "richtek,mtp-sel-high");
@@ -457,11 +503,14 @@ static int rtq2208_parse_regulator_dt_data(int n_regulator, const unsigned int *
 		if (!rdesc[i])
 			return -ENOMEM;
 
-		rtq2208_init_regulator_desc(rdesc[i], mtp_sel, idx, rtq2208_ldo_match, &ldo_idx);
+		rtq2208_init_regulator_desc(rdesc[i], mtp_sel, idx);
+
+		/* init ldo dvs ability */
+		if (idx >= RTQ2208_LDO2)
+			rtq2208_ldo_match[idx - RTQ2208_LDO2].desc = &rdesc[i]->desc;
 	}
 
-	/* init ldo fixed_uV */
-	ret = rtq2208_of_get_fixed_voltage(dev, rtq2208_ldo_match, ldo_idx);
+	ret = rtq2208_of_get_ldo_dvs_ability(dev);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to get ldo fixed_uV\n");
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not
  2024-04-29 10:16 ` [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not Alina Yu
@ 2024-04-29 10:37   ` Krzysztof Kozlowski
  2024-04-29 16:09   ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2024-04-29 10:37 UTC (permalink / raw
  To: Alina Yu, lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt,
	conor+dt
  Cc: linux-kernel, devicetree, johnny_lai, cy_huang

On 29/04/2024 12:16, Alina Yu wrote:
> Since there is no way to check is ldo is adjustable or not.
> 'richtek,use-fix-dvs' is added for that. user is supposed to know whether vout of ldo is adjustable.

1. Please wrap commit message according to Linux coding style /
submission process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

2. Start sentences with capital letters. LDO is acronym.

3. Constraints already tell you that, don't they? Explain why they are
not enough and you need new property.

> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> ---
>  .../devicetree/bindings/regulator/richtek,rtq2208.yaml         | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> index 609c066..3951679 100644
> --- a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> @@ -75,6 +75,14 @@ properties:
>          description:
>            regulator description for ldo[1-2].
>  
> +        properties:
> +          richtek,use-fix-dvs:
> +            type: boolean
> +            description: |
> +              ldo vout ability is determined by this setting. If it's set, the voltage is unadjustable.
> +              There is no risk-free method for software to determine whether the ldo vout is fixed or not.
> +              Therefore, it can only be done in this way.

Wrap according to Linux style (as expressed in Linux coding style document).


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not
  2024-04-29 10:16 ` [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not Alina Yu
  2024-04-29 10:37   ` Krzysztof Kozlowski
@ 2024-04-29 16:09   ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2024-04-29 16:09 UTC (permalink / raw
  To: Alina Yu
  Cc: lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-kernel, devicetree, johnny_lai, cy_huang

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

On Mon, Apr 29, 2024 at 06:16:46PM +0800, Alina Yu wrote:
> Since there is no way to check is ldo is adjustable or not.
> 'richtek,use-fix-dvs' is added for that. user is supposed to know whether vout of ldo is adjustable.

As Krzysztof said we already know if the voltage can change since in
order for Linux to change the voltage there must be a voltage range
specified (and see comment on patch 2).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put_matches is called
  2024-04-29 10:16 ` [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put_matches is called Alina Yu
@ 2024-04-29 16:13   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2024-04-29 16:13 UTC (permalink / raw
  To: Alina Yu
  Cc: lgirdwood, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-kernel, devicetree, johnny_lai, cy_huang

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

On Mon, Apr 29, 2024 at 06:16:47PM +0800, Alina Yu wrote:

> ramp_delay range of bucks is changed.
> The maximum ramp up and down range is shorten from 64mVstep/us tor 16mVstep/us/.
> The LDO's Vout is adjustable if the haardware setting allow it, and it can be set either 1800mv or 3300mv.
> Additionally, the discharge register is chaned to other position.
> The discharge register has been moved to another position.
> In this version, a software bug has been fixed. rtq2208_ldo_match is no longer a local variable,
> which prevents invalid memory access when devm_of_regulator_put_matches is called.

As covered in submitting-patches.rst since this is a bunch of different
changes that don't really overlap it should be split into multiple
patches, for example the change of the ramp time for the DCDCs should be
separate to all the LDO stuff.

> -		if (init_data->constraints.min_uV == init_data->constraints.max_uV)
> -			rdesc->desc.fixed_uV = init_data->constraints.min_uV;
> +		fixed_uV = of_property_read_bool(match->of_node, "richtek,use-fix-dvs");
> +
> +		if (fixed_uV) {
> +			desc->n_voltages = 1;
> +			desc->fixed_uV = init_data->constraints.min_uV;
> +			desc->ops = &rtq2208_regulator_ldo_fix_ops;
> +		} else {
> +			desc->n_voltages = ARRAY_SIZE(rtq2208_ldo_volt_table);
> +			desc->volt_table = rtq2208_ldo_volt_table;
> +			desc->ops = &rtq2208_regulator_ldo_adj_ops;
> +		}

The driver shouldn't need to look at the constraints to set up the
operations for the hardware, if the regulator can be configured for
multiple voltages just register it that way and let the core figure out
if we ever actually want to change the voltage.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-04-29 16:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-29 10:16 [PATCH 0/2] Fix rtq2208 buck ramp_delay and ldo discharge and dvs setting Alina Yu
2024-04-29 10:16 ` [PATCH 1/2] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not Alina Yu
2024-04-29 10:37   ` Krzysztof Kozlowski
2024-04-29 16:09   ` Mark Brown
2024-04-29 10:16 ` [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put_matches is called Alina Yu
2024-04-29 16:13   ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).