On Fri, Mar 13, 2015 at 09:50:14AM +0000, Srinivas Kandagatla wrote: A couple of *very* minor points below, otherwise this looks OK to me. > +struct eeprom_device *eeprom_register(struct eeprom_config *config) > +{ > + struct eeprom_device *eeprom; > + int rval; > + > + if (!config->regmap || !config->size) { > + dev_err(config->dev, "Regmap not found\n"); > + return ERR_PTR(-EINVAL); > + } You have a struct device in the config and the regmap API has dev_get_regmap() which for most devices that don't have multiple regmaps will give the right regmap. It would be nice to support this as a convenience for users. > + eeprom = kzalloc(sizeof(*eeprom), GFP_KERNEL); > + if (!eeprom) > + return ERR_PTR(-ENOMEM); ... > + rval = device_add(&eeprom->dev); > + if (rval) > + return ERR_PTR(rval); Don't you need a kfree() if device_add() fails?