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:   Thu, 17 Sep 2020 14:21:05 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Dmitry Osipenko <digetx@gmail.com>
Cc:     Jonathan Hunter <jonathanh@nvidia.com>,
        Laxman Dewangan <ldewangan@nvidia.com>,
        Wolfram Sang <wsa@the-dreams.de>,
        =?utf-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= <mirq-linux@rere.qmqm.pl>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        linux-i2c@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 30/34] i2c: tegra: Clean up variable names
Message-ID: <20200917122105.GI3515672@ulmo>
References: <20200908224006.25636-1-digetx@gmail.com>
 <20200908224006.25636-31-digetx@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="XLWMkxR+mZNQ4WTO"
Content-Disposition: inline
In-Reply-To: <20200908224006.25636-31-digetx@gmail.com>
User-Agent: Mutt/1.14.6 (2020-07-11)
Precedence: bulk


--XLWMkxR+mZNQ4WTO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Sep 09, 2020 at 01:40:02AM +0300, Dmitry Osipenko wrote:
> Rename "ret" variables to "err" in order to make code a bit more
> expressive, emphasizing that the returned value is an error code.
> Same vice versa, where appropriate.
>=20
> Rename variable "reg" to "val" in order to better reflect the actual
> usage of the variable in the code and to make naming consistent with
> the rest of the code.
>=20
> Use briefer names for a few members of the tegra_i2c_dev structure in
> order to improve readability of the code.
>=20
> All dev/&pdev->dev are replaced with i2c_dev->dev in order to have uniform
> code style across the driver.
>=20
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 173 ++++++++++++++++-----------------
>  1 file changed, 86 insertions(+), 87 deletions(-)

That's indeed a nice improvement. One thing did spring out at me,
though.

> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegr=
a.c
[...]
> @@ -1831,20 +1830,20 @@ static int __maybe_unused tegra_i2c_runtime_suspe=
nd(struct device *dev)
> =20
>  	clk_bulk_disable(i2c_dev->nclocks, i2c_dev->clocks);
> =20
> -	return pinctrl_pm_select_idle_state(i2c_dev->dev);
> +	return pinctrl_pm_select_idle_state(dev);
>  }
> =20
>  static int __maybe_unused tegra_i2c_suspend(struct device *dev)
>  {
>  	struct tegra_i2c_dev *i2c_dev =3D dev_get_drvdata(dev);
> -	int err =3D 0;
> +	int ret =3D 0;
> =20
>  	i2c_mark_adapter_suspended(&i2c_dev->adapter);
> =20
>  	if (!pm_runtime_status_suspended(dev))
> -		err =3D tegra_i2c_runtime_suspend(dev);
> +		ret =3D tegra_i2c_runtime_suspend(dev);
> =20
> -	return err;
> +	return ret;
>  }

Isn't this exactly the opposite of what the commit message says (and the
rest of the patch does)?

Thierry

--XLWMkxR+mZNQ4WTO
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl9jVLEACgkQ3SOs138+
s6GC6w/+IyW7glSDNb080etzU2+oeqx/jVPyinPTxjE6/IcKuvVd5LWJNFLOaJ4W
FeumA7waD71+2k+On9pJOyGUJNzIBJZkxE/ZkeWEE2ZnVMT4/OMEYce25dY7pYO7
ZS28oqIIJxkM1B9m8uxaMEVaaoOGNtowVK/gQSvfdfm4tk5P11RaKb0D3HjXa6cL
2oITcSdciVhHVTP+qeSLYNIZ3mNinplUn4hmIgkhr9d+SyLRf33riqFOGerKsCEW
u96fPqydLev4TQWyMs0TuMAzgC8Xer40i/Cn5OS6WLj2x3/WBISWL4WpMolykf4r
iMhmTStys/yN4EVJD6qUJ0XGqTtlTNAvPPSon2hUOIbjKzhi6Zxt6VleYLbPhOFE
Cs2r8ouMApBBvU4TBgG7+H29SChKHGHOtpjb9CmETdu/DHwVvUz3ShHAR8ae3RDm
YKj1NlFqAMgshb7GtFa1p0lVXOhxNNZKYx2dbX7bCeM6m4N4OW0S0Wo8beODDPZF
Wfzrx6P9+1xMiZjw+gNZoa4TgHVNWePUgfNN1jBO1BXgi52ciIvMErble5RuRClG
5nQ2M1glVLMlC5O/LakR2Tb9usscH2d9vu2kSLWst+u7IYXY5ympuAFvXrHCc608
bSX8SZnvB4SIA99lozNyzRhnXl5l5/1ytKEoi9Qd/2Qua4urTUQ=
=dMIY
-----END PGP SIGNATURE-----

--XLWMkxR+mZNQ4WTO--