Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 24 Aug 2020 12:00:45 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
Cc:     Lee Jones <lee.jones@linaro.org>, Rob Herring <robh+dt@kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Sebastian Reichel <sre@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>,
        linux-actions@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        linux-pm@vger.kernel.org
Subject: Re: [PATCH v2 3/6] regulator: Add regulator driver for ATC260x PMICs
Message-ID: <20200824110045.GA4676@sirena.org.uk>
References: <cover.1598043782.git.cristian.ciocaltea@gmail.com>
 <8da70f0b19de17fb8edead7ff06461ae2451b0e9.1598043782.git.cristian.ciocaltea@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="2fHTh5uZTiUOsy+g"
Content-Disposition: inline
In-Reply-To: <8da70f0b19de17fb8edead7ff06461ae2451b0e9.1598043782.git.cristian.ciocaltea@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Aug 22, 2020 at 01:19:49AM +0300, Cristian Ciocaltea wrote:

> +static int atc260x_set_voltage_time_sel(struct regulator_dev *rdev,
> +					unsigned int old_selector,
> +					unsigned int new_selector)
> +{
> +	struct atc260x_regulator_data *data = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +
> +	if (new_selector > old_selector)
> +		return id > data->last_dcdc_reg_id ? data->voltage_time_ldo
> +						   : data->voltage_time_dcdc;

Please write normal conditional statements to make things easier to
read.  It also looks like this would be more robustly written by just
having separate ops for DCDCs and LDOs, this could easily break if
another device is supported in the driver.

> +static const struct of_device_id atc260x_regulator_of_match[] = {
> +	{ .compatible = "actions,atc2603c-regulator" },
> +	{ .compatible = "actions,atc2609a-regulator" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, atc260x_regulator_of_match);

We don't need compatibles here, this is just reflecting the current
Linux device model into the OS neutral DT bindings.  Another OS may
choose to split regulators up differently.  We should just instantiate
the regulator device from the MFD based on identifying the chip overall.

--2fHTh5uZTiUOsy+g
Content-Type: application/pgp-signature; name="signature.asc"

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl9DndwACgkQJNaLcl1U
h9CXoAf6AzUFwxgd0S5ClrLwTuv/z8XNl93mNAnQI+Lgte0VWytWqrO+qCtOPcAq
sQgGC4jUOMfZR2vcx+UGMuCCoYjDO75ApLLyN4Z79wUeNApY/VVVoSpMCy1/yvtf
wneUL9d6TNL0S/Po6/RBm0kII2vbhPcRXdJxGo1JAefjao0cc6OC7ep6OtxsnQF9
sr9aCXfvloI95sl6bc/QVaWqd5/dP7WuPINxzY6Xd20TJ4bL9gVmwv87uBdZXLZZ
8jjoAg+/7ma13fxdt7CuaGJLuXZNW+WusMKy/Bx5CqFwqpDkEPuRpA5zXXPI8/5c
axFmXEOlJ7MAB8WZg4fNbeBHCsbekg==
=H2HJ
-----END PGP SIGNATURE-----

--2fHTh5uZTiUOsy+g--