Received-SPF: pass (google.com: domain of linux-kernel+bounces-162661-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 30 Apr 2024 01:13:56 +0900
From: Mark Brown <broonie@kernel.org>
To: Alina Yu <alina_yu@richtek.com>
Cc: lgirdwood@gmail.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	johnny_lai@richtek.com, cy_huang@richtek.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
Message-ID: <Zi_HRJ6xJ8cEhKAy@finisterre.sirena.org.uk>
References: <1714385807-22393-1-git-send-email-alina_yu@richtek.com>
 <1714385807-22393-3-git-send-email-alina_yu@richtek.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="CMFEKwFL0XCdxy2X"
Content-Disposition: inline
In-Reply-To: <1714385807-22393-3-git-send-email-alina_yu@richtek.com>


--CMFEKwFL0XCdxy2X
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--CMFEKwFL0XCdxy2X
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmYvx0MACgkQJNaLcl1U
h9A9ygf/b3X8dbsjTS9LgrAuYbruOGAJnJaScqP+1gX0zNEp0EFUHrtJPJ4T4mZU
QqwtkJWQgMcw/9pt62Av3woKE2P684R4nExzH0rUOxdchYNHQN71NB8BingBqX+a
NZp5iqeorVGrRBdSjbDmI6w0UYS0cRbGhxn6gZ4HCtkC+9THf0N6YFdrA6RuhwRx
BUWG6LZbDxdPD/B9ePz5R8OIy7W3u/sGf5iy5KIc/KoLTh+EGnHrM6Ldr2s+ZoEn
fv5pyBI0ZgzsvKxhtPhEkxvZTJMIA30cpYm3sxzoTj9Pjz9lJPs/iddRu04N+RMU
81nq1XlHyqNRI7o8eE5HFb4cQP4jeg==
=BqI3
-----END PGP SIGNATURE-----

--CMFEKwFL0XCdxy2X--