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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6957eafb-8be2-3323-9c55-bb6eee6ab6bf@gmail.com>
Date:   Thu, 17 Sep 2020 18:13:36 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Wolfram Sang <wsa@...-dreams.de>,
        Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        linux-i2c@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 29/34] i2c: tegra: Improve formatting of variables

17.09.2020 15:16, Thierry Reding пишет:
> 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@...il.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.

That is a bit questionable, at least it definitely won't save me from a
mistake :)