Re: [PATCH v7 29/34] i2c: tegra: Improve formatting of variables

Thierry Reding
2020-09-17T12:18:46Z

[-- Attachment #1: Type: text/plain, Size: 2490 bytes --]

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.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> drivers/i2c/busses/i2c-tegra.c | 97 ++++++++++++++++------------------
> 1 file changed, 45 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.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;
> };
>
> -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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]