On Tue, Feb 28, 2017 at 4:37 AM, Eva Rachel Retuya <eraretuya@xxxxxxxxx> wrote: > Move I2C-specific code into its own file and rely on regmap to access > registers. The core code provides access to x, y, z and scale readings. Portion of minor comments. > +config ADXL345_I2C > + tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver" > depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m) I'm wondering if it works in a form of depends on INPUT_ADXL34X=n > +int adxl345_common_probe(struct device *dev, struct regmap *regmap, > + const char *name); > +int adxl345_common_remove(struct device *dev); I think a "common" word is redundant. > - * IIO driver for ADXL345 > - * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or > - * 0x53 (ALT ADDRESS pin grounded) > + * IIO core driver for ADXL345 Should not it be at the beginning of header comment? > +static int adxl345_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct regmap *regmap; > + const char *name = NULL; Reverse tree order, please. > + > + regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&client->dev, "Error initializing i2c regmap: %d\n", > + (int)PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + if (id) > + name = id->name; > + > + return adxl345_common_probe(&client->dev, regmap, name); Do you need temporary variable? return adxl345_probe(&client->dev, regmap, id ? id->name : NULL); -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html