On Mon, 2019-07-15 at 14:21 -0700, john.hubb...@gmail.com wrote:
> From: John Hubbard <jhubb...@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.sick...@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...


_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to