Thanks, this looks much better. A few minor nitpicks, though: > -int __must_check device_add_disk(struct device *parent, struct gendisk *disk, > - const struct attribute_group **groups) > +static int __device_add_disk(struct device *parent, struct gendisk *disk, > + const struct attribute_group **groups, > + struct fwnode_handle *fwnode) I don't think we need a separate helper if device_add_disk simply wraps the OF version by passing a NULL fwnode. > +int __must_check device_add_of_disk(struct device *parent, struct gendisk *disk, > + const struct attribute_group **groups, > + struct fwnode_handle *fwnode) > +{ > + return __device_add_disk(parent, disk, groups, fwnode); > +} I'd name this as add_disk_fwnode as the of in device_add_of_disk reads as in add the device of the disk, and the fwnode is what gets passed. The device_ is a bit redundant and just there for historic reasons as the original add_disk predates the device model. Can you also add a kerneldoc comment for the new helper? > +EXPORT_SYMBOL(device_add_of_disk); EXPORT_SYMBO_GPL, please.