Received-SPF: pass (google.com: domain of linux-kernel+bounces-30166-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 18 Jan 2024 13:49:55 +0000
From: Mark Brown <broonie@kernel.org>
To: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jean Delvare <jdelvare@suse.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Jonathan Corbet <corbet@lwn.net>,
	Liam Girdwood <lgirdwood@gmail.com>, Rob Herring <robh@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hwmon@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH v5 5/5] hwmon: Add support for Amphenol ChipCap 2
Message-ID: <226d3abd-e372-4c66-b2b0-cc86e6a4bb27@sirena.org.uk>
References: <20240115-topic-chipcap2-v5-0-0cc7a15aeece@gmail.com>
 <20240115-topic-chipcap2-v5-5-0cc7a15aeece@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="QrXWaaDFLUIiRC/N"
Content-Disposition: inline
In-Reply-To: <20240115-topic-chipcap2-v5-5-0cc7a15aeece@gmail.com>


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

On Mon, Jan 15, 2024 at 09:02:25PM +0100, Javier Carrasco wrote:

> +static int cc2_enable(struct cc2_data *data)
> +{
> +	int ret;
> +
> +	if (regulator_is_enabled(data->regulator))
> +		return 0;

This is generally a sign that the regulator API usage is not good, the
driver should not rely on references to the regulator held by anything
else since whatever else is holding the regulator on could turn it off
at any time.  If the driver did the enable itself then it should know
that it did so and not need to query.

> +	ret = regulator_enable(data->regulator);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * TODO: the startup-delay-us property of the regulator might be
> +	 * added to the delay (if provided).
> +	 * Currently there is no interface to read its value apart from
> +	 * a direct access to regulator->rdev->constraints->enable_time,
> +	 * which is discouraged like any direct access to the regulator_dev
> +	 * structure. This would be relevant in cases where the startup delay
> +	 * is in the range of milliseconds.
> +	 */
> +	usleep_range(CC2_STARTUP_TIME_US, CC2_STARTUP_TIME_US + 125);

Note that the regulator startup delay is the time taken for the
regulator to power up so if the device needs additional delay then that
will always need to be in addition to whatever the regulator is doing.

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

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmWpLIIACgkQJNaLcl1U
h9B65wf/Wg0QW0LIWGgdWQ7IuOfqDh3LYsyHxFindkzlWUik31DSPhRG1Y8mGpCn
HttmSyq3JRctGJdKnMpoiFfK6WpQk8SWvqN6c8S1sXUAHzJ+kUuiIrBgg7Rttd2a
aYyK2M6NxTC3VBx3BGgcPOGsgdIxF5jBG9UBE2//kBpqSICHtc1D4P7XfTGsf4nX
2XLXqW7HqqIQFS6w9pi785wY6D0JDPUR8guOOUV+RJM2dWHxO54vx1dmINy9il+0
1z098JjEQIMfqs1T0t9tH06wbkNGQCfAi/AnKGSEYwkxcneiU1eo65GwsMV3H163
MnMMsNykw5LhV9yVpqLOYKPn/yF6lw==
=N/9a
-----END PGP SIGNATURE-----

--QrXWaaDFLUIiRC/N--