diff mbox

[v5,08/11] scsi: host template attribute groups

Message ID 1517933183-30892-9-git-send-email-stanislav.nijnikov@wdc.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Stanislav Nijnikov Feb. 6, 2018, 4:06 p.m. UTC 
The patch introduces an additional field in the scsi_host_template
structure - struct attribute_group **sdev_group.
This field allows to define groups of attributes. It will provide an
ability to use binary attributes as well as device attributes and
to group them under subfolders if necessary.

Signed-off-by: Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/scsi/scsi_sysfs.c | 12 ++++++++++++
 include/scsi/scsi_host.h  |  6 ++++++
 2 files changed, 18 insertions(+)

Comments

Bart Van Assche Feb. 6, 2018, 7:06 p.m. UTC | #1 
On Tue, 2018-02-06 at 18:06 +0200, Stanislav Nijnikov wrote:
> +	if (sdev->host->hostt->sdev_groups) {

> +		error = sysfs_create_groups(&sdev->sdev_gendev.kobj,

> +			sdev->host->hostt->sdev_groups);

> +			if (error)

> +				return error;

> +	}


Please follow the indentation style that is used elsewhere in the kernel and
move the sdev->host->hostt->sdev_groups argument to the right such that it
starts one position to the right of the sysfs_create_groups() opening
parenthesis. Please also indent the if (error) statement correctly.

> @@ -1326,6 +1333,11 @@ void __scsi_remove_device(struct scsi_device *sdev)

>  	if (sdev->sdev_state == SDEV_DEL)

>  		return;

>  

> +	if (sdev->host->hostt->sdev_groups) {

> +		sysfs_remove_groups(&sdev->sdev_gendev.kobj,

> +			sdev->host->hostt->sdev_groups);

> +	}


If sdev->is_visible is false then the device_add() call for sdev->sdev_gendev
may have failed so calling sysfs_remove_groups() is not necessary. Please move
this call down to just before the device_del(dev) since that call also removes
sysfs attributes. Additionally, I think that checkpatch will complain about the
above code and will report that braces are not necessary for single-line
statements?

Thanks,

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index cbc0fe2..fe5d1cb 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1309,6 +1309,13 @@  int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 		}
 	}
 
+	if (sdev->host->hostt->sdev_groups) {
+		error = sysfs_create_groups(&sdev->sdev_gendev.kobj,
+			sdev->host->hostt->sdev_groups);
+			if (error)
+				return error;
+	}
+
 	scsi_autopm_put_device(sdev);
 	return error;
 }
@@ -1326,6 +1333,11 @@  void __scsi_remove_device(struct scsi_device *sdev)
 	if (sdev->sdev_state == SDEV_DEL)
 		return;
 
+	if (sdev->host->hostt->sdev_groups) {
+		sysfs_remove_groups(&sdev->sdev_gendev.kobj,
+			sdev->host->hostt->sdev_groups);
+	}
+
 	if (sdev->is_visible) {
 		/*
 		 * If scsi_internal_target_block() is running concurrently,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 1a1df0d..1931758 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -477,6 +477,12 @@  struct scsi_host_template {
 	struct device_attribute **sdev_attrs;
 
 	/*
+	 * Pointer to the SCSI device attribute groups for this host,
+	 * NULL terminated.
+	 */
+	const struct attribute_group **sdev_groups;
+
+	/*
 	 * List of hosts per template.
 	 *
 	 * This is only for use by scsi_module.c for legacy templates.