linux-kernel - Re: [PATCH 2/2] regulator: rtq2208: Fix RTQ2208 buck ramp delay and ldo vout setting and segmentaion fault when devm_of_regulator_put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Zi_HRJ6xJ8cEhKAy@finisterre.sirena.org.uk>
Date: Tue, 30 Apr 2024 01:13:56 +0900
From: Mark Brown <broonie@...nel.org>
To: Alina Yu <alina_yu@...htek.com>
Cc: lgirdwood@...il.com, robh+dt@...nel.org,
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	johnny_lai@...htek.com, cy_huang@...htek.com
Subject: 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

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)