Re: [PATCH v5 3/6] block: introduce device_add_of_disk()

[Christoph Hellwig]

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.