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:16:40 +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 29/34] i2c: tegra: Improve formatting of variables
Message-ID: <20200917121640.GH3515672@ulmo>
References: <20200908224006.25636-1-digetx@gmail.com>
 <20200908224006.25636-30-digetx@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="+E165xXSMiO4rFaE"
Content-Disposition: inline
In-Reply-To: <20200908224006.25636-30-digetx@gmail.com>
User-Agent: Mutt/1.14.6 (2020-07-11)
Precedence: bulk


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

On Wed, Sep 09, 2020 at 01:40:01AM +0300, Dmitry Osipenko wrote:
> Reorder definition of variables in the code to have them sorted by length
> and grouped logically, also replace "unsigned long" with "u32". Do this in
> order to make code easier to read.
>=20
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 97 ++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 52 deletions(-)
>=20
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegr=
a.c
> index ac40c87f1c21..2376f502d299 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -259,42 +259,48 @@ struct tegra_i2c_hw_feature {
>   */
>  struct tegra_i2c_dev {
>  	struct device *dev;
> -	const struct tegra_i2c_hw_feature *hw;
>  	struct i2c_adapter adapter;
> -	struct clk *div_clk;
> -	struct clk_bulk_data *clocks;
> -	unsigned int nclocks;
> +
> +	const struct tegra_i2c_hw_feature *hw;
>  	struct reset_control *rst;
> -	void __iomem *base;
> -	phys_addr_t base_phys;
>  	unsigned int cont_id;
>  	unsigned int irq;
> -	bool is_dvc;
> -	bool is_vi;
> +
> +	phys_addr_t base_phys;
> +	void __iomem *base;
> +
> +	struct clk_bulk_data *clocks;
> +	unsigned int nclocks;
> +
> +	struct clk *div_clk;
> +	u32 bus_clk_rate;
> +
>  	struct completion msg_complete;
> +	size_t msg_buf_remaining;
>  	int msg_err;
>  	u8 *msg_buf;
> -	size_t msg_buf_remaining;
> -	bool msg_read;
> -	u32 bus_clk_rate;
> -	bool is_multimaster_mode;
> +
> +	struct completion dma_complete;
>  	struct dma_chan *tx_dma_chan;
>  	struct dma_chan *rx_dma_chan;
> +	unsigned int dma_buf_size;
>  	dma_addr_t dma_phys;
>  	u32 *dma_buf;
> -	unsigned int dma_buf_size;
> -	bool is_curr_dma_xfer;
> -	struct completion dma_complete;
> +
> +	bool is_multimaster_mode;
>  	bool is_curr_atomic_xfer;
> +	bool is_curr_dma_xfer;
> +	bool msg_read;
> +	bool is_dvc;
> +	bool is_vi;
>  };
> =20
> -static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val,
> -		       unsigned long reg)
> +static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, u32 reg)

I actually prefer unsigned long/int over u32 for offsets because it
makes it clearer that this is not in fact a 32-bit value that we're
writing into a register. This is especially true for these register
accessors where the "offset" is called "reg" and may be easily
mistaken for a register value.

Thierry

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

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

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl9jU6gACgkQ3SOs138+
s6Gt4w//eQTYuWVJf9AGLpSfI2NEwi8LwUZVYc3MP1NkbsWWs5+feDeRyWJ52ucy
Cf/2aoF8erQDZflPM/JXe3hdPEw5xkIVwg6KrLiYqZyU2W2exwuamxmuwIvo9P+G
7QHpfWvxMvMgADqNvVrHrTXFbxP1jTXSTLccmKa+8dYyw1rRNPaI3pF/6/rwEMbm
T6JI/Y/e3L21y9O4L8UuWfFcZk50sXYdcKh2ya2cdjcsI0UW45YiN6VdKclUvTcM
jvI0in5lgTPhHO555UQ+1rvkp/+lmu8+sTmkdnuyXBqIT44SYp7kYWdb5ZXbgklz
SxUxtFrlAJjlfWJunxbG7Lm48H/09NqVg2mwphCg7ixUL/DAm1qszmCj5DWOR2XU
HjDd2ENyXlgefuXLB5InIAVGTCdGhOjo+1IiYPFgE4mUjxaT0901797Au5+OMBg3
0i6I8p17gUkLeDm1fHusAf53KN/vcWHrWRyklv5Ki8S57c5Xjbu4eYD1iBy8qcGn
Iqp/BMjwgx8m3xR2OJSBZYCWi/GAN5kFb5qJVNHkkSUUSr2ThWVphw64Jqm+ERSB
J/EyBtrMd40oyepjfV52FCqY3sa4gxS9L6TtI8y35+D4YhIfkxGvuhGixoLrYlZV
IfIzQbPwvZ2E6+Yqq3o7fSQMlJt9VSNA1rwq1KpblT3D85jf6h8=
=2HC5
-----END PGP SIGNATURE-----

--+E165xXSMiO4rFaE--