2024-05-10 12:14:09

by Alina Yu

[permalink] [raw]
Subject: [PATCH v3 0/6] Fix rtq2208 BUCK ramp_delay and LDO dvs setting

Hi,

This series patches is for hardware modification of RTQ2208.
ramp_delay range of BUCK is changed.
The maximum ramp up and down range of BUCK are shorten
from 64mVstep/us to 16mVstep/us.
The LDO's Vout is adjustable if the hardware setting allow it,
and it can be set either 1800mv or 3300mv.
Additionally, 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.

Thanks,
Alina yu
---
Change in v3:
- In Patch 1/2:
- As discussing in v2 series,
add 'richtek,fixed-microvolt' property to specify the fixed voltage
- In Patch 2/2:
- Move Fixes to the start of the series
- Seperate LDO vsel and discharge change to seperate patches
- Add "richtek,fixed-microvolt" to specify LDO fixed voltage
- Check specified desc->fixed_uV matches with constraints->min_uV and constraints->max_uV
---
Alina Yu (6):
regulator: rtq2208: Fix invalid memory access when
devm_of_regulator_put_matches is called
regulator: rtq2208: Fix LDO vsel setting
regulator: rtq2208: Fix LDO discharge register
regulator: rtq2208: Fix the BUCK ramp_delay range to maximum of
16mVstep/us
regulator: rtq2208: Fix LDO to be compatible with both fixed and
adjustable vout
regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is
adjustable or not

.../bindings/regulator/richtek,rtq2208.yaml | 8 ++
drivers/regulator/rtq2208-regulator.c | 151 ++++++++++++++-------
2 files changed, 110 insertions(+), 49 deletions(-)

--
2.7.4



2024-05-10 12:14:30

by Alina Yu

[permalink] [raw]
Subject: [PATCH v3 1/6] regulator: rtq2208: Fix invalid memory access when devm_of_regulator_put_matches is called

In this patch, a software bug has been fixed.
rtq2208_ldo_match is no longer a local variable.
It prevents invalid memory access when devm_of_regulator_put_matches
is called.

Signed-off-by: Alina Yu <[email protected]>
---
v3
- Move Fixes to the start of the series
---
drivers/regulator/rtq2208-regulator.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
index 2d54844..dfa293a 100644
--- a/drivers/regulator/rtq2208-regulator.c
+++ b/drivers/regulator/rtq2208-regulator.c
@@ -224,6 +224,11 @@ static const struct regulator_ops rtq2208_regulator_ldo_ops = {
.set_suspend_disable = rtq2208_set_suspend_disable,
};

+static struct of_regulator_match rtq2208_ldo_match[] = {
+ {.name = "ldo2", },
+ {.name = "ldo1", },
+};
+
static unsigned int rtq2208_of_map_mode(unsigned int mode)
{
switch (mode) {
@@ -335,8 +340,7 @@ static const struct linear_range rtq2208_vout_range[] = {
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;
@@ -351,14 +355,14 @@ 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;
@@ -373,8 +377,7 @@ static int rtq2208_of_get_fixed_voltage(struct device *dev,
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)
+static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, int mtp_sel, int idx)
{
struct regulator_desc *desc;
static const struct {
@@ -432,10 +435,6 @@ static void rtq2208_init_regulator_desc(struct rtq2208_regulator_desc *rdesc, in
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;
-
rdesc->suspend_config_reg = curr_info->base;
rdesc->suspend_enable_mask = RTQ2208_LDO_EN_STR_MASK;
}
@@ -444,8 +443,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 +455,15 @@ 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


2024-05-10 12:15:16

by Alina Yu

[permalink] [raw]
Subject: [PATCH v3 5/6] regulator: rtq2208: Fix LDO to be compatible with both fixed and adjustable vout

In this patch, LDO's adjustable and fixed Vout settings are compatible.
The LDO Vout ability depends on "richtek,fixed-microvolt".
If adjustable, the Vout can be set to either 1800mV or 3300mV.

Signed-off-by: Alina Yu <[email protected]>
---
v3
- Add "richtek,fixed-microvolt" to specify LDO fixed voltage
- Check specified desc->fixed_uV matches with constraints->min_uV and constraints->max_uV
---
drivers/regulator/rtq2208-regulator.c | 43 ++++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
index 2e9387f..c2c1689 100644
--- a/drivers/regulator/rtq2208-regulator.c
+++ b/drivers/regulator/rtq2208-regulator.c
@@ -219,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,
@@ -228,6 +228,23 @@ 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", },
@@ -358,8 +375,9 @@ 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;
+ u32 fixed_uV;
int ret, i;

if (!dev->of_node)
@@ -379,13 +397,26 @@ static int rtq2208_of_get_ldo_dvs_ability(struct device *dev)
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;
+ /* specify working fixed voltage if the propery exists */
+ ret = of_property_read_u32(match->of_node, "richtek,fixed-microvolt", &fixed_uV);
+
+ if (!ret) {
+ if (fixed_uV != init_data->constraints.min_uV ||
+ fixed_uV != init_data->constraints.max_uV)
+ return -EINVAL;
+ desc->n_voltages = 1;
+ desc->fixed_uV = fixed_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;
--
2.7.4


2024-05-15 15:47:52

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] regulator: rtq2208: Fix invalid memory access when devm_of_regulator_put_matches is called

On Fri, May 10, 2024 at 08:06:20PM +0800, Alina Yu wrote:
> In this patch, a software bug has been fixed.
> rtq2208_ldo_match is no longer a local variable.
> It prevents invalid memory access when devm_of_regulator_put_matches
> is called.

This doesn't apply against current code, please check and resend (on
Linus' tree rather than mine at this point given the merge window).


Attachments:
(No filename) (388.00 B)
signature.asc (499.00 B)
Download all attachments

2024-05-27 09:32:16

by Alina Yu

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] regulator: rtq2208: Fix invalid memory access when devm_of_regulator_put_matches is called

On Wed, May 15, 2024 at 04:47:37PM +0100, Mark Brown wrote:
> On Fri, May 10, 2024 at 08:06:20PM +0800, Alina Yu wrote:
> > In this patch, a software bug has been fixed.
> > rtq2208_ldo_match is no longer a local variable.
> > It prevents invalid memory access when devm_of_regulator_put_matches
> > is called.
>
> This doesn't apply against current code, please check and resend (on
> Linus' tree rather than mine at this point given the merge window).

Hi,
Mark

I apologize for the interruption.

I've resubmitted the new series for review at the following link:
https://lore.kernel.org/all/7c28d2e61d2fc13066ba4814d1ecfab8f344aaad.1715846612.git.alina_yu@richtek.com/

This series is based on the previous submission found at:
https://lore.kernel.org/all/5d56b79c94de63fc86b5a70b7e374da4240fee8b.1714467553.git.alina_yu@richtek.com/

I would greatly appreciate it if you could let me know if this will be merged in the future branch,
or if there are any mistakes that I need to address.


Thanks,
Alina


2024-05-29 11:22:37

by Mark Brown

[permalink] [raw]
Subject: Re: (subset) [PATCH v3 0/6] Fix rtq2208 BUCK ramp_delay and LDO dvs setting

On Fri, 10 May 2024 20:06:19 +0800, Alina Yu wrote:
> This series patches is for hardware modification of RTQ2208.
> ramp_delay range of BUCK is changed.
> The maximum ramp up and down range of BUCK are shorten
> from 64mVstep/us to 16mVstep/us.
> The LDO's Vout is adjustable if the hardware setting allow it,
> and it can be set either 1800mv or 3300mv.
> Additionally, 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.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/6] regulator: rtq2208: Fix invalid memory access when devm_of_regulator_put_matches is called
commit: 72b6a2d6506843375c7b91197f49ef38ca0c6d0f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark