Messages in this thread Patch in this message | | | Subject | Re: [PATCH] staging: kpc2000: whitespace and line length cleanup | From | Joe Perches <> | Date | Mon, 15 Jul 2019 15:21:18 -0700 |
| |
On Mon, 2019-07-15 at 14:21 -0700, john.hubbard@gmail.com wrote: > From: John Hubbard <jhubbard@nvidia.com> > > This commit was created by running indent(1): > `indent -linux` > > ...and then applying some manual corrections and > cleanup afterward, to keep it sane. No functional changes > were made.
I don't find many of these whitespace changes "better".
Sometimes, it's just fine to have > 80 char lines.
Alignment formatting was OK before this and now has many odd uses that make reading for a human harder rather than simpler or easier.
> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c [] > @@ -33,9 +33,9 @@ MODULE_LICENSE("GPL"); > MODULE_AUTHOR("Matt.Sickler@Daktronics.com"); > > struct i2c_device { > - unsigned long smba; > - struct i2c_adapter adapter; > - unsigned int features; > + unsigned long smba; > + struct i2c_adapter adapter; > + unsigned int features;
Here the spaces before the identifier are converted to tab aligned
> }; > > /***************************** > @@ -52,9 +52,9 @@ struct i2c_device { > #define SMBHSTDAT0(p) ((5 * REG_SIZE) + (p)->smba) > #define SMBHSTDAT1(p) ((6 * REG_SIZE) + (p)->smba) > #define SMBBLKDAT(p) ((7 * REG_SIZE) + (p)->smba) > -#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and later */ > -#define SMBAUXSTS(p) ((12 * REG_SIZE) + (p)->smba) /* ICH4 and later */ > -#define SMBAUXCTL(p) ((13 * REG_SIZE) + (p)->smba) /* ICH4 and later */ > +#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and later */ > +#define SMBAUXSTS(p) ((12 * REG_SIZE) + (p)->smba) /* ICH4 and later */ > +#define SMBAUXCTL(p) ((13 * REG_SIZE) + (p)->smba) /* ICH4 and later */
But here the #define value still has spaces but the comment uses tabs. Why tab align the comments but not the #define value?
> @@ -136,17 +138,21 @@ static int i801_check_pre(struct i2c_device *priv) [] > status &= STATUS_FLAGS; > if (status) { > - //dev_dbg(&priv->adapter.dev, "Clearing status flags (%02x)\n", status); > + //dev_dbg(&priv->adapter.dev, > + //"Clearing status flags (%02x)\n", status);
This was better before.
An improvement might be to add more macros like:
#define i2c_err(priv, fmt, ...) dev_err(&(priv)->adapter.dev, fmt, ##__VA_ARGS__) #define i2c_dbg(priv, fmt, ...) dev_dbg(&(priv)->adapter.dev, fmt, ##__VA_ARGS__)
So all these uses of dev_<level>(&priv->adapter.dev, ...) become much shorter visually and the line wrapping becomes rather better.
> outb_p(status, SMBHSTSTS(priv)); > status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS; > if (status) { > - dev_err(&priv->adapter.dev, "Failed clearing status flags (%02x)\n", status); > + dev_err(&priv->adapter.dev, > + "Failed clearing status flags (%02x)\n", > + status);
e.g.: i2c_err(priv, "Failed clearing status flags (%02x)\n", status);
etc...
[]
> @@ -301,7 +322,8 @@ static int i801_block_transaction_byte_by_byte(struct i2c_device *priv, union i2 > else > smbcmd = I801_BLOCK_LAST; > } else { > - if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_READ) > + if (command == I2C_SMBUS_I2C_BLOCK_DATA > + && read_write == I2C_SMBUS_READ)
logic continuations should be at EOL.
if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write == I2C_SMBUS_READ)
[] > @@ -558,13 +614,14 @@ static u32 i801_func(struct i2c_adapter *adapter) > I2C_FUNC_SMBUS_WORD_DATA | /* _READ_WORD_DATA _WRITE_WORD_DATA */ > I2C_FUNC_SMBUS_BLOCK_DATA | /* _READ_BLOCK_DATA _WRITE_BLOCK_DATA */ > !I2C_FUNC_SMBUS_I2C_BLOCK | /* _READ_I2C_BLOCK _WRITE_I2C_BLOCK */ > - !I2C_FUNC_SMBUS_EMUL; /* _QUICK _BYTE _BYTE_DATA _WORD_DATA _PROC_CALL _WRITE_BLOCK_DATA _I2C_BLOCK _PEC */ > + /* _QUICK _BYTE _BYTE_DATA _WORD_DATA _PROC_CALL _WRITE_BLOCK_DATA _I2C_BLOCK _PEC : */ > + !I2C_FUNC_SMBUS_EMUL; > return f; > } > > static const struct i2c_algorithm smbus_algorithm = { > - .smbus_xfer = i801_access, > - .functionality = i801_func, > + .smbus_xfer = i801_access, > + .functionality = i801_func,
Many people prefer the aligned function names.
etc...
| |