On Wed, Oct 26, 2022 at 03:39:12PM +0300, Jarkko Nikula wrote: > Align all defines to the same column. ... > +#define DW_IC_SDA_HOLD_MIN_VERS 0x3131312A > +#define DW_IC_COMP_TYPE_VALUE 0x44570140 While at it, I would add comments that show ASCII values of these definitions. ... > +#define DW_IC_INTR_DEFAULT_MASK (DW_IC_INTR_RX_FULL | \ > + DW_IC_INTR_TX_ABRT | \ > + DW_IC_INTR_STOP_DET) > +#define DW_IC_INTR_MASTER_MASK (DW_IC_INTR_DEFAULT_MASK | \ > + DW_IC_INTR_TX_EMPTY) > +#define DW_IC_INTR_SLAVE_MASK (DW_IC_INTR_DEFAULT_MASK | \ > + DW_IC_INTR_RX_UNDER | \ > + DW_IC_INTR_RD_REQ) While at it, I would reformat these (and similar below) as #define DW_IC_INTR_DEFAULT_MASK \ (DW_IC_INTR_RX_FULL | DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET) ... > int (*init)(struct dw_i2c_dev *dev); > int (*set_sda_hold_time)(struct dw_i2c_dev *dev); These... > int mode; > - struct i2c_bus_recovery_info rinfo; > + struct i2c_bus_recovery_info rinfo; ...and now this are consistent, but these all are inconsistent with, e.g. int mode; where we have $type $name; -- With Best Regards, Andy Shevchenko