On Sat, Jun 13, 2015 at 11:49:16PM -0600, Sagar Dharia wrote: > +static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b) > +{ > + if (a->manf_id == b->manf_id && a->prod_code == b->prod_code && > + a->dev_index == b->dev_index && > + a->instance == b->instance) > + return 0; > + return -EIO; > +} -EIO? That seems a bit random. > +static int slim_pm_resume(struct device *dev) > +{ > + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; > + > + if (pm) > + return pm_generic_resume(dev); > + else > + return 0; > +} I'm really surprised this needs to be open coded, none of this references anything Slimbus specific. Why not fix the need for open coding? There's also a bunch of other ops like poweroff and freeze/thaw that you're not doing anything with. > +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl) > +{ Why are these operations split? > + * @reset_device: This callback is called after framer is booted. > + * Driver should do the needful to reset the device, > + * so that device acquires sync and be operational. boot_device or init_device perhaps? Reset sounds like it takes a running device and resets it.