ACK: [SRU][Trusty][PATCH] NVMe: Export NVMe attributes to sysfs group

[Colin Ian King]

On 14/12/16 18:10, Dan Streetman wrote:
> From: Keith Busch <keith.busch at intel.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1649635
> 
> Adds all controller information to attribute list exposed to sysfs, and
> appends the reset_controller attribute to it. The nvme device is created
> with this attribute list, so driver no long manages its attributes.
> 
> Backport note:
> This backport required changes to how the attributes are exported, due
> to the large number of changes between this kernel and the upstream
> kernel where this functionality is added.  Specifically, in the upstream
> commit, the driver already has been changed over to use device_create()
> to manage the device's attributes, which included creating a new device
> class for nvme devices; the upstream commit also had already added a sysfs
> interface to reset the controller.  Those changes are too large to
> backport just to provide three string attributes via sysfs, so instead
> this directly uses device_create_file() for the drive's attribute strings,
> and does not export the interface to reset the controller.  That allows
> this patch to stay as simple as possible.
> 
> Reported-by: Sujith Pandel <sujithpshankar at gmail.com>
> Cc: Sujith Pandel <sujithpshankar@ gmail.com>
> Cc: David Milburn <dmilburn at redhat.com>
> Signed-off-by: Keith Busch <keith.busch at intel.com>
> Signed-off-by: Jens Axboe <axboe at fb.com>
> (backported from upstream commit 779ff75617099f4defe14e20443b95019a4c5ae8)
> Signed-off-by: Dan Streetman <dan.streetman at canonical.com>
> ---
>  drivers/block/nvme-core.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> index ce8087e..9abcabc 100644
> --- a/drivers/block/nvme-core.c
> +++ b/drivers/block/nvme-core.c
> @@ -143,6 +143,19 @@ static unsigned nvme_queue_extra(int depth)
>  	return DIV_ROUND_UP(depth, 8) + (depth * sizeof(struct nvme_cmd_info));
>  }
>  
> +#define nvme_show_function(field)						\
> +static ssize_t  field##_show(struct device *dev,				\
> +			     struct device_attribute *attr, char *buf)		\
> +{										\
> +	struct nvme_dev *ndev = pci_get_drvdata(to_pci_dev(dev));		\
> +	return sprintf(buf, "%.*s\n", (int)sizeof(ndev->field),	ndev->field);	\
> +}										\
> +static DEVICE_ATTR(field, S_IRUGO, field##_show, NULL);
> +
> +nvme_show_function(model);
> +nvme_show_function(serial);
> +nvme_show_function(firmware_rev);
> +
>  /**
>   * alloc_cmdid() - Allocate a Command ID
>   * @nvmeq: The queue that will be used for this command
> @@ -2573,9 +2586,22 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (result)
>  		goto remove;
>  
> +	if (device_create_file(&pdev->dev, &dev_attr_model))
> +		goto deregister;
> +	if (device_create_file(&pdev->dev, &dev_attr_serial))
> +		goto remove_model;
> +	if (device_create_file(&pdev->dev, &dev_attr_firmware_rev))
> +		goto remove_serial;
> +
>  	dev->initialized = 1;
>  	return 0;
>  
> + remove_serial:
> +	device_remove_file(&pdev->dev, &dev_attr_serial);
> + remove_model:
> +	device_remove_file(&pdev->dev, &dev_attr_model);
> + deregister:
> +	misc_deregister(&dev->miscdev);
>   remove:
>  	nvme_dev_remove(dev);
>  	nvme_free_namespaces(dev);
> @@ -2607,6 +2633,9 @@ static void nvme_remove(struct pci_dev *pdev)
>  	list_del_init(&dev->node);
>  	spin_unlock(&dev_list_lock);
>  
> +	device_remove_file(&pdev->dev, &dev_attr_model);
> +	device_remove_file(&pdev->dev, &dev_attr_serial);
> +	device_remove_file(&pdev->dev, &dev_attr_firmware_rev);
>  	pci_set_drvdata(pdev, NULL);
>  	flush_work(&dev->reset_work);
>  	misc_deregister(&dev->miscdev);
> 
Looks fine to me.

Acked-by: Colin Ian King <colin.king at canonical.com>