All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Enable UFS provisioning via Linux
@ 2018-06-15 21:00 Evan Green
  2018-06-15 21:00 ` [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs Evan Green
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Evan Green @ 2018-06-15 21:00 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	Stanislav Nijnikov, Greg Kroah-Hartman, Bart Van Assche,
	Adrian Hunter, linux-kernel, linux-scsi
  Cc: Evan Green

This series enables provisioning UFS devices using the existing sysfs
interface. This functionality is primarily useful along the assembly
line, but might also be useful for end users that receive devices that
aren't locked down.

Changes since v1:
	- Reworked the interface to show each unit of the config
descriptor as a separate directory, rather than the previous method I
had of a file for selecting the unit, and then a common set of files
that interacted with whichever unit was selected. I did some kobject
magic to accomplish this. I noticed from Greg KH's reply to Sayali's
patches [1] that configfs might be the preferred method. Let me know
if I should abandon this series in favor of Sayali's, with the
possible exception of "Make sysfs attributes writable".
	- Squashed documentation changes into their respective code
changes.
	- I decided to keep the config descriptor attributes as their
own files, rather than hiding writes behind device descriptor and unit
descriptor, as I think that's more future proof and true to the UFS spec.

[1] https://lkml.org/lkml/2018/6/8/210

Evan Green (4):
  scsi: ufs: Add Configuration Descriptor to sysfs
  scsi: ufs: Make sysfs attributes writable
  scsi: ufs: Refactor descriptor read for write
  scsi: ufs: Enable writing config descriptor

 Documentation/ABI/testing/sysfs-driver-ufs | 156 ++++++++++--
 drivers/scsi/ufs/ufs-sysfs.c               | 367 +++++++++++++++++++++++++++--
 drivers/scsi/ufs/ufs-sysfs.h               |   1 +
 drivers/scsi/ufs/ufs.h                     |  29 +++
 drivers/scsi/ufs/ufshcd.c                  | 110 ++++++---
 drivers/scsi/ufs/ufshcd.h                  |  30 ++-
 6 files changed, 620 insertions(+), 73 deletions(-)

-- 
2.13.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs
  2018-06-15 21:00 [PATCH v2 0/4] Enable UFS provisioning via Linux Evan Green
@ 2018-06-15 21:00 ` Evan Green
  2018-06-16  7:16   ` Greg Kroah-Hartman
  2018-06-15 21:00 ` [PATCH v2 2/4] scsi: ufs: Make sysfs attributes writable Evan Green
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Evan Green @ 2018-06-15 21:00 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	Stanislav Nijnikov, Greg Kroah-Hartman, Bart Van Assche,
	Adrian Hunter, linux-kernel, linux-scsi
  Cc: Evan Green

This change adds the configuration descriptor to the UFS
sysfs interface. This is done in preparation for making the
interface writable, which will enable provisioning UFS devices
via Linux.

The configuration descriptor is laid out as a header, then a set of
(usually 8) copies of the same descriptor for each unit.

Signed-off-by: Evan Green <evgreen@chromium.org>
---
Changes since v1:
	- Squashed documentation changes into this change.
	- Reworked sysfs layout so that instead of a sysfs file for a
unit selector and then a common set of unit attributes, each unit in
the config descriptor has its own directory. This required a little
bit of kobject magic. Alternatively I could use standard device
attributes and simply allocate N*M of them from a template. I have
that coded up, and can go with that if preferred, but I thought
this was a little nicer since it wasted less memory.

 Documentation/ABI/testing/sysfs-driver-ufs | 156 ++++++++++++++++++++
 drivers/scsi/ufs/ufs-sysfs.c               | 226 +++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-sysfs.h               |   1 +
 drivers/scsi/ufs/ufs.h                     |  29 ++++
 drivers/scsi/ufs/ufshcd.c                  |   1 +
 drivers/scsi/ufs/ufshcd.h                  |  15 ++
 6 files changed, 428 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 016724ec26d5..eebd95fb1db5 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -237,6 +237,162 @@ Description:	This file shows the command maximum timeout for a change
 		The file is read only.
 
 
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/boot_enable
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows whether or not the UFS boot feature is enabled.
+		This is one of the UFS configuration descriptor parameters.
+		More information about the descriptor can be found in the UFS
+		2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/descriptor_access_enable
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows whether or not access will be permitted to the
+		Device Descriptor after the partial initialization phase of the
+		boot sequence. This is one of the UFS configuration descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/high_priority_lun
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the identifier of the high priority logical
+		unit. This is one of the UFS configuration descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/init_active_icc_level
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the ICC level in active mode after device
+		initialization or hardware reset. This is one of the UFS
+		configuration descriptor parameters. More information about the
+		descriptor can be found in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/initial_power_mode
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the power mode after device initialization or
+		hardware reset. This is one of the UFS configuration descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/number_of_luns
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the number of logical units that the device will
+		support. This is one of the UFS configuration descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/periodic_rtc_update
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the frequency and method of real time clock
+		updates. This is one of the UFS configuration descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/secure_removal_type
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the secure removal type of the UFS device. This
+		is one of the UFS configuration descriptor parameters. More
+		information about the descriptor can be found in the UFS 2.1
+		specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/allocation_units
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the number of allocation units assigned to the
+		particular logical unit. This is one of the UFS configuration
+		unit descriptor parameters. More information about the
+		descriptor can be found in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/boot_lun_id
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the boot LUN ID for this particular logical
+		unit, indicating whether it is Boot A, Boot B, or not special.
+		This is one of the UFS configuration unit descriptor parameters.
+		More information about the descriptor can be found in the UFS
+		2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/context_capabilities
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the context capabilities for the particular
+		logical unit. This is one of the UFS configuration unit
+		descriptor parameters. More information about the descriptor
+		can be found in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/data_reliability
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the data reliability for the particular logical
+		unit. This is one of the UFS configuration unit descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/logical_block_size
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the logical block size for the particular
+		logical unit as a power of two. This is one of the UFS
+		configuration unit descriptor parameters. More information
+		about the descriptor can be found in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/lu_enable
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows whether or not the particular logical unit is
+		enabled. This is one of the UFS configuration unit descriptor
+		parameters. More information about the descriptor can be found
+		in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/lu_write_protect
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the write protect status for the particular
+		logical unit. This is one of the UFS configuration unit
+		descriptor parameters. More information about the descriptor
+		can be found in the UFS 2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/memory_type
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the memory type for the particular logical unit.
+		This is one of the UFS configuration unit descriptor parameters.
+		More information about the descriptor can be found in the UFS
+		2.1 specification.
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/provisioning_type
+Date:		May 2018
+Contact:	Evan Green <evgreen@chromium.org>
+Description:	This file shows the provisioning type information for the
+		particular logical unit. This is one of the UFS configuration
+		uint descriptor parameters. More information about the
+		descriptor can be found in the UFS 2.1 specification.
+		The file is read only.
+
+
 What:		/sys/bus/platform/drivers/ufshcd/*/interconnect_descriptor/unipro_version
 Date:		February 2018
 Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 8d9332bb7d0c..7ac1440c82eb 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -327,6 +327,132 @@ static const struct attribute_group ufs_sysfs_device_descriptor_group = {
 	.attrs = ufs_sysfs_device_descriptor,
 };
 
+/*
+ * Define a unique attribute structure for configuration descriptors, since
+ * the sysfs attributes look at both the description and the parent kobject
+ * to find the index.
+ */
+
+struct ufs_config_desc_attr {
+	struct attribute attr;
+	u8 offset;
+	u8 size;
+};
+
+#define to_ufs_cfg_obj(_kobj) container_of(_kobj, struct ufs_cfg_object, kobj)
+#define to_ufs_cfg_desc_attr(_attr) \
+	container_of(_attr, struct ufs_config_desc_attr, attr)
+
+#define UFS_CONFIG_DESC_PARAM(_name, _uname, _size)			\
+static struct ufs_config_desc_attr ufs_cfg_attr_##_name = {		\
+	.attr = {.name = __stringify(_name),				\
+		 .mode = VERIFY_OCTAL_PERMISSIONS(0444) }, \
+	.offset = CONFIGURATION_DESC_PARAM##_uname,			\
+	.size = _size							\
+}
+
+UFS_CONFIG_DESC_PARAM(number_of_luns, _NUM_LU, 1);
+UFS_CONFIG_DESC_PARAM(boot_enable, _BOOT_ENBL, 1);
+UFS_CONFIG_DESC_PARAM(descriptor_access_enable, _DESC_ACCSS_ENBL, 1);
+UFS_CONFIG_DESC_PARAM(initial_power_mode, _INIT_PWR_MODE, 1);
+UFS_CONFIG_DESC_PARAM(high_priority_lun, _HIGH_PR_LUN, 1);
+UFS_CONFIG_DESC_PARAM(secure_removal_type, _SEC_RMV_TYPE, 1);
+UFS_CONFIG_DESC_PARAM(init_active_icc_level, _ACTVE_ICC_LVL, 1);
+UFS_CONFIG_DESC_PARAM(periodic_rtc_update, _FRQ_RTC, 2);
+
+static struct attribute *ufs_sysfs_config_descriptor[] = {
+	&ufs_cfg_attr_number_of_luns.attr,
+	&ufs_cfg_attr_boot_enable.attr,
+	&ufs_cfg_attr_descriptor_access_enable.attr,
+	&ufs_cfg_attr_initial_power_mode.attr,
+	&ufs_cfg_attr_high_priority_lun.attr,
+	&ufs_cfg_attr_secure_removal_type.attr,
+	&ufs_cfg_attr_init_active_icc_level.attr,
+	&ufs_cfg_attr_periodic_rtc_update.attr,
+	NULL,
+};
+
+#define UFS_CONFIG_UNIT_DESC_PARAM(_name, _uname, _size)		\
+static struct ufs_config_desc_attr ufs_cfg_unit_attr_##_name = {	\
+	.attr = {.name = __stringify(_name),				\
+		 .mode = VERIFY_OCTAL_PERMISSIONS(0444) },		\
+	.offset = CONFIGURATION_UNIT_DESC_PARAM##_uname,		\
+	.size = _size							\
+}
+
+UFS_CONFIG_UNIT_DESC_PARAM(lu_enable, _LU_ENABLE, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(boot_lun_id, _BOOT_LUN_ID, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(lu_write_protect, _LU_WR_PROTECT, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(memory_type, _MEM_TYPE, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(allocation_units, _NUM_ALLOC_UNITS, 4);
+UFS_CONFIG_UNIT_DESC_PARAM(data_reliability, _DATA_RELIABILITY, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(logical_block_size, _LOGICAL_BLK_SIZE, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(provisioning_type, _PROVISIONING_TYPE, 1);
+UFS_CONFIG_UNIT_DESC_PARAM(context_capabilities, _CTX_CAPABILITIES, 2);
+
+static struct attribute *ufs_sysfs_config_unit_descriptor[] = {
+	&ufs_cfg_unit_attr_lu_enable.attr,
+	&ufs_cfg_unit_attr_boot_lun_id.attr,
+	&ufs_cfg_unit_attr_lu_write_protect.attr,
+	&ufs_cfg_unit_attr_memory_type.attr,
+	&ufs_cfg_unit_attr_allocation_units.attr,
+	&ufs_cfg_unit_attr_data_reliability.attr,
+	&ufs_cfg_unit_attr_logical_block_size.attr,
+	&ufs_cfg_unit_attr_provisioning_type.attr,
+	&ufs_cfg_unit_attr_context_capabilities.attr,
+	NULL
+};
+
+static ssize_t ufs_cfg_attr_show(struct kobject *kobj, struct attribute *attr,
+			     char *buf)
+{
+	struct ufs_config_desc_attr *cfg_attr = to_ufs_cfg_desc_attr(attr);
+	struct ufs_cfg_object *cfg_obj = to_ufs_cfg_obj(kobj);
+	u8 offset = cfg_attr->offset;
+	struct device *dev;
+	struct ufs_hba *hba;
+
+	/* For unit config descriptors, add the unit's offset and get the
+	 * device parent two up.
+	 */
+	if (cfg_obj->index >= 0) {
+		offset += CONFIGURATION_DESC_PARAM_UNIT0 +
+			(CONFIGURATION_UNIT_DESC_SIZE * cfg_obj->index);
+
+		dev = kobj_to_dev(cfg_obj->kobj.parent->parent);
+
+	} else {
+		dev = kobj_to_dev(cfg_obj->kobj.parent);
+	}
+
+	hba = dev_get_drvdata(dev);
+	return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_CONFIGURATION, 0,
+					 offset, buf, cfg_attr->size);
+}
+
+static const struct sysfs_ops ufs_sysfs_config_descriptor_ops = {
+	.show	= ufs_cfg_attr_show,
+};
+
+static void ufs_cfg_kobject_release(struct kobject *kobj)
+{
+	struct ufs_cfg_object *cfg = to_ufs_cfg_obj(kobj);
+
+	kfree(cfg);
+}
+
+static struct kobj_type ufs_cfg_kobject_type = {
+	.release = ufs_cfg_kobject_release,
+	.sysfs_ops = &ufs_sysfs_config_descriptor_ops,
+	.default_attrs = ufs_sysfs_config_descriptor,
+};
+
+static struct kobj_type ufs_cfg_unit_kobject_type = {
+	.release = ufs_cfg_kobject_release,
+	.sysfs_ops = &ufs_sysfs_config_descriptor_ops,
+	.default_attrs = ufs_sysfs_config_unit_descriptor,
+};
+
 #define UFS_INTERCONNECT_DESC_PARAM(_name, _uname, _size)		\
 	UFS_DESC_PARAM(_name, _uname, INTERCONNECT, _size)
 
@@ -800,6 +926,21 @@ const struct attribute_group ufs_sysfs_lun_attributes_group = {
 	.attrs = ufs_sysfs_lun_attributes,
 };
 
+static void ufs_sysfs_destroy_unit_groups(struct device *dev)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < hba->cfg_object_count; i++) {
+		kobject_uevent(&hba->cfg_objects[i]->kobj, KOBJ_REMOVE);
+		kobject_put(&hba->cfg_objects[i]->kobj);
+	}
+
+	kfree(hba->cfg_objects);
+	hba->cfg_objects = NULL;
+	hba->cfg_object_count = 0;
+}
+
 void ufs_sysfs_add_nodes(struct device *dev)
 {
 	int ret;
@@ -811,7 +952,92 @@ void ufs_sysfs_add_nodes(struct device *dev)
 			__func__, ret);
 }
 
+void ufs_sysfs_add_units(struct device *dev)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int conf_len, i, inited = 0, unit_count, ret;
+
+	/* Determine the number of units from the descriptor length. */
+	ret = ufshcd_map_desc_id_to_length(hba,
+				QUERY_DESC_IDN_CONFIGURATION, &conf_len);
+
+	if (ret || conf_len < CONFIGURATION_DESC_PARAM_UNIT0)
+		return;
+
+	unit_count = (conf_len - CONFIGURATION_DESC_PARAM_UNIT0) /
+			CONFIGURATION_UNIT_DESC_SIZE;
+
+	/* Skip if the right number of units are already set up. */
+	if (hba->cfg_object_count == unit_count + 1)
+		return;
+
+	/* Clean up any old unit descriptor groups. */
+	if (hba->cfg_object_count)
+		ufs_sysfs_destroy_unit_groups(dev);
+
+	/*
+	 * Create kobjects for the config descriptor and each unit descriptor.
+	 * The first element is the config descriptor itself.
+	 */
+	hba->cfg_objects = kzalloc(
+		(unit_count + 1) * sizeof(void *), GFP_ATOMIC);
+
+	if (!hba->cfg_objects)
+		return;
+
+	hba->cfg_objects[0] = kzalloc(sizeof(struct ufs_cfg_object),
+				      GFP_ATOMIC);
+	if (!hba->cfg_objects[0])
+		goto err;
+
+	hba->cfg_objects[0]->index = -1;
+	ret = kobject_init_and_add(&hba->cfg_objects[0]->kobj,
+				   &ufs_cfg_kobject_type, &dev->kobj,
+				   "%s", "config_descriptor");
+
+	if (ret) {
+		kfree(hba->cfg_objects[0]);
+		goto err;
+	}
+
+	inited++;
+	for (i = 0; i < unit_count; i++, inited++) {
+		hba->cfg_objects[i + 1] = kzalloc(sizeof(struct ufs_cfg_object),
+						  GFP_ATOMIC);
+
+		if (!hba->cfg_objects[i + 1])
+			goto err;
+
+		hba->cfg_objects[i + 1]->index = i;
+		ret = kobject_init_and_add(&hba->cfg_objects[i + 1]->kobj,
+					   &ufs_cfg_unit_kobject_type,
+					   &hba->cfg_objects[0]->kobj,
+					   "unit%d", i);
+
+		if (ret) {
+			kfree(hba->cfg_objects[i + 1]);
+			goto err;
+		}
+	}
+
+	hba->cfg_object_count = unit_count + 1;
+	for (i = 0; i < inited; i++)
+		kobject_uevent(&hba->cfg_objects[i]->kobj, KOBJ_ADD);
+
+	return;
+
+err:
+	dev_err(dev, "error %d creating unit sysfs groups\n", ret);
+	for (i = inited - 1; i >= 0; i--)
+		kobject_put(&hba->cfg_objects[i]->kobj);
+
+	kfree(hba->cfg_objects);
+	hba->cfg_objects = NULL;
+	hba->cfg_object_count = 0;
+}
+
 void ufs_sysfs_remove_nodes(struct device *dev)
 {
 	sysfs_remove_groups(&dev->kobj, ufs_sysfs_groups);
+	ufs_sysfs_destroy_unit_groups(dev);
 }
diff --git a/drivers/scsi/ufs/ufs-sysfs.h b/drivers/scsi/ufs/ufs-sysfs.h
index e5621e59a432..a1002f339b1a 100644
--- a/drivers/scsi/ufs/ufs-sysfs.h
+++ b/drivers/scsi/ufs/ufs-sysfs.h
@@ -10,6 +10,7 @@
 #include "ufshcd.h"
 
 void ufs_sysfs_add_nodes(struct device *dev);
+void ufs_sysfs_add_units(struct device *dev);
 void ufs_sysfs_remove_nodes(struct device *dev);
 
 extern const struct attribute_group ufs_sysfs_unit_descriptor_group;
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 14e5bf7af0bb..a5712b1a07ad 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -260,6 +260,35 @@ enum device_desc_param {
 	DEVICE_DESC_PARAM_PRDCT_REV		= 0x2A,
 };
 
+/* Configuration descriptor parameter offsets in bytes */
+enum configuration_desc_param {
+	CONFIGURATION_DESC_PARAM_LEN			= 0x0,
+	CONFIGURATION_DESC_PARAM_TYPE			= 0x1,
+	CONFIGURATION_DESC_PARAM_NUM_LU			= 0x2,
+	CONFIGURATION_DESC_PARAM_BOOT_ENBL		= 0x3,
+	CONFIGURATION_DESC_PARAM_DESC_ACCSS_ENBL	= 0x4,
+	CONFIGURATION_DESC_PARAM_INIT_PWR_MODE		= 0x5,
+	CONFIGURATION_DESC_PARAM_HIGH_PR_LUN		= 0x6,
+	CONFIGURATION_DESC_PARAM_SEC_RMV_TYPE		= 0x7,
+	CONFIGURATION_DESC_PARAM_ACTVE_ICC_LVL		= 0x8,
+	CONFIGURATION_DESC_PARAM_FRQ_RTC		= 0x9,
+	CONFIGURATION_DESC_PARAM_UNIT0			= 0x10,
+};
+
+/* Configuration unit descriptor parameter offsets in bytes */
+enum configuration_unit_desc_param {
+	CONFIGURATION_UNIT_DESC_PARAM_LU_ENABLE		= 0x0,
+	CONFIGURATION_UNIT_DESC_PARAM_BOOT_LUN_ID	= 0x1,
+	CONFIGURATION_UNIT_DESC_PARAM_LU_WR_PROTECT	= 0x2,
+	CONFIGURATION_UNIT_DESC_PARAM_MEM_TYPE		= 0x3,
+	CONFIGURATION_UNIT_DESC_PARAM_NUM_ALLOC_UNITS	= 0x4,
+	CONFIGURATION_UNIT_DESC_PARAM_DATA_RELIABILITY	= 0x8,
+	CONFIGURATION_UNIT_DESC_PARAM_LOGICAL_BLK_SIZE	= 0x9,
+	CONFIGURATION_UNIT_DESC_PARAM_PROVISIONING_TYPE	= 0xA,
+	CONFIGURATION_UNIT_DESC_PARAM_CTX_CAPABILITIES	= 0xB,
+	CONFIGURATION_UNIT_DESC_SIZE			= 0x10,
+};
+
 /* Interconnect descriptor parameters offsets in bytes*/
 enum interconnect_desc_param {
 	INTERCONNECT_DESC_PARAM_LEN		= 0x0,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 397081d320b1..0baba3fdb112 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6603,6 +6603,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
 
 	/* set the state as operational after switching to desired gear */
 	hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
+	ufs_sysfs_add_units(hba->dev);
 
 	/*
 	 * If we are in error handling context or in power management callbacks
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f51758f1e5cc..bb540a3fd8de 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -448,6 +448,16 @@ struct ufs_stats {
 };
 
 /**
+ * struct ufs_cfg_object - Configuration descriptor unit object.
+ * @kobj: Kernel object
+ * @index: Stores the index of the unit being described.
+ */
+struct ufs_cfg_object {
+	struct kobject kobj;
+	int index;
+};
+
+/**
  * struct ufs_hba - per adapter private structure
  * @mmio_base: UFSHCI base register address
  * @ucdl_base_addr: UFS Command Descriptor base address
@@ -501,6 +511,8 @@ struct ufs_stats {
  * @is_urgent_bkops_lvl_checked: keeps track if the urgent bkops level for
  *  device is known or not.
  * @scsi_block_reqs_cnt: reference counting for scsi block requests
+ * @cfg_objects: Stores the array of kobjects created for the config descriptor.
+ * @cfg_object_count: Stores the number of elements in cfg_objects.
  */
 struct ufs_hba {
 	void __iomem *mmio_base;
@@ -702,6 +714,9 @@ struct ufs_hba {
 	struct rw_semaphore clk_scaling_lock;
 	struct ufs_desc_size desc_size;
 	atomic_t scsi_block_reqs_cnt;
+
+	struct ufs_cfg_object **cfg_objects;
+	size_t cfg_object_count;
 };
 
 /* Returns true if clocks can be gated. Otherwise false */
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] scsi: ufs: Make sysfs attributes writable
  2018-06-15 21:00 [PATCH v2 0/4] Enable UFS provisioning via Linux Evan Green
  2018-06-15 21:00 ` [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs Evan Green
@ 2018-06-15 21:00 ` Evan Green
  2018-06-15 21:00 ` [PATCH v2 3/4] scsi: ufs: Refactor descriptor read for write Evan Green
  2018-06-15 21:00 ` [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor Evan Green
  3 siblings, 0 replies; 9+ messages in thread
From: Evan Green @ 2018-06-15 21:00 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	Stanislav Nijnikov, Greg Kroah-Hartman, Bart Van Assche,
	Adrian Hunter, linux-kernel, linux-scsi
  Cc: Evan Green

This change makes the UFS controller's sysfs attributes writable, which
will enable users to provision unprovisioned UFS devices, or
re-provision unlocked UFS devices.

Signed-off-by: Evan Green <evgreen@chromium.org>
---
Changes since v1:
	- Fixed u32 cast in _store function as suggested by Bart.
	- Squashed documentation change into this change.

 Documentation/ABI/testing/sysfs-driver-ufs | 17 +--------
 drivers/scsi/ufs/ufs-sysfs.c               | 58 ++++++++++++++++++++----------
 2 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index eebd95fb1db5..d7477fffe0d1 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -841,7 +841,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the boot lun enabled UFS device attribute.
 		The full information about the attribute could be found at
 		UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/current_power_mode
 Date:		February 2018
@@ -849,7 +848,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the current power mode UFS device attribute.
 		The full information about the attribute could be found at
 		UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/active_icc_level
 Date:		February 2018
@@ -857,7 +855,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the active icc level UFS device attribute.
 		The full information about the attribute could be found at
 		UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/ooo_data_enabled
 Date:		February 2018
@@ -865,7 +862,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the out of order data transfer enabled UFS
 		device attribute. The full information about the attribute
 		could be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/bkops_status
 Date:		February 2018
@@ -873,7 +869,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the background operations status UFS device
 		attribute. The full information about the attribute could
 		be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/purge_status
 Date:		February 2018
@@ -881,7 +876,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the purge operation status UFS device
 		attribute. The full information about the attribute could
 		be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/max_data_in_size
 Date:		February 2018
@@ -889,7 +883,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file shows the maximum data size in a DATA IN
 		UPIU. The full information about the attribute could
 		be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/max_data_out_size
 Date:		February 2018
@@ -897,7 +890,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file shows the maximum number of bytes that can be
 		requested with a READY TO TRANSFER UPIU. The full information
 		about the attribute could be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/reference_clock_frequency
 Date:		February 2018
@@ -905,14 +897,13 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the reference clock frequency UFS device
 		attribute. The full information about the attribute could
 		be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/configuration_descriptor_lock
 Date:		February 2018
 Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file shows whether the configuration descriptor is locked.
 		The full information about the attribute could be found at
-		UFS specifications 2.1. The file is read only.
+		UFS specifications 2.1.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/max_number_of_rtt
 Date:		February 2018
@@ -921,7 +912,6 @@ Description:	This file provides the maximum current number of
 		outstanding RTTs in device that is allowed. The full
 		information about the attribute could be found at
 		UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/exception_event_control
 Date:		February 2018
@@ -929,7 +919,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the exception event control UFS device
 		attribute. The full information about the attribute could
 		be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/exception_event_status
 Date:		February 2018
@@ -937,7 +926,6 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the exception event status UFS device
 		attribute. The full information about the attribute could
 		be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/ffu_status
 Date:		February 2018
@@ -945,14 +933,12 @@ Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file provides the ffu status UFS device attribute.
 		The full information about the attribute could be found at
 		UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/psa_state
 Date:		February 2018
 Contact:	Stanislav Nijnikov <stanislav.nijnikov@wdc.com>
 Description:	This file show the PSA feature status. The full information
 		about the attribute could be found at UFS specifications 2.1.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/attributes/psa_data_size
 Date:		February 2018
@@ -961,7 +947,6 @@ Description:	This file shows the amount of data that the host plans to
 		load to all logical units in pre-soldering state.
 		The full information about the attribute could be found at
 		UFS specifications 2.1.
-		The file is read only.
 
 
 What:		/sys/class/scsi_device/*/device/dyn_cap_needed
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 7ac1440c82eb..4726c05bb085 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -781,7 +781,7 @@ static const struct attribute_group ufs_sysfs_flags_group = {
 	.attrs = ufs_sysfs_device_flags,
 };
 
-#define UFS_ATTRIBUTE(_name, _uname)					\
+#define UFS_ATTRIBUTE_SHOW(_name, _uname)				\
 static ssize_t _name##_show(struct device *dev,				\
 	struct device_attribute *attr, char *buf)			\
 {									\
@@ -791,25 +791,45 @@ static ssize_t _name##_show(struct device *dev,				\
 		QUERY_ATTR_IDN##_uname, 0, 0, &value))			\
 		return -EINVAL;						\
 	return sprintf(buf, "0x%08X\n", value);				\
-}									\
-static DEVICE_ATTR_RO(_name)
+}
 
-UFS_ATTRIBUTE(boot_lun_enabled, _BOOT_LU_EN);
-UFS_ATTRIBUTE(current_power_mode, _POWER_MODE);
-UFS_ATTRIBUTE(active_icc_level, _ACTIVE_ICC_LVL);
-UFS_ATTRIBUTE(ooo_data_enabled, _OOO_DATA_EN);
-UFS_ATTRIBUTE(bkops_status, _BKOPS_STATUS);
-UFS_ATTRIBUTE(purge_status, _PURGE_STATUS);
-UFS_ATTRIBUTE(max_data_in_size, _MAX_DATA_IN);
-UFS_ATTRIBUTE(max_data_out_size, _MAX_DATA_OUT);
-UFS_ATTRIBUTE(reference_clock_frequency, _REF_CLK_FREQ);
-UFS_ATTRIBUTE(configuration_descriptor_lock, _CONF_DESC_LOCK);
-UFS_ATTRIBUTE(max_number_of_rtt, _MAX_NUM_OF_RTT);
-UFS_ATTRIBUTE(exception_event_control, _EE_CONTROL);
-UFS_ATTRIBUTE(exception_event_status, _EE_STATUS);
-UFS_ATTRIBUTE(ffu_status, _FFU_STATUS);
-UFS_ATTRIBUTE(psa_state, _PSA_STATE);
-UFS_ATTRIBUTE(psa_data_size, _PSA_DATA_SIZE);
+#define UFS_ATTRIBUTE_RO(_name, _uname)					\
+UFS_ATTRIBUTE_SHOW(_name, _uname)					\
+DEVICE_ATTR_RO(_name)
+
+#define UFS_ATTRIBUTE_RW(_name, _uname)					\
+UFS_ATTRIBUTE_SHOW(_name, _uname)					\
+static ssize_t _name##_store(struct device *dev,			\
+		struct device_attribute *attr, const char *buf,		\
+		size_t count)						\
+{									\
+	struct ufs_hba *hba = dev_get_drvdata(dev);			\
+	u32 value;							\
+	if (kstrtou32(buf, 0, &value))					\
+		return -EINVAL;						\
+	if (ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,	\
+		QUERY_ATTR_IDN##_uname, 0, 0, &value))			\
+		return -EINVAL;						\
+	return count;							\
+}									\
+static DEVICE_ATTR_RW(_name)
+
+UFS_ATTRIBUTE_RW(boot_lun_enabled, _BOOT_LU_EN);
+UFS_ATTRIBUTE_RO(current_power_mode, _POWER_MODE);
+UFS_ATTRIBUTE_RW(active_icc_level, _ACTIVE_ICC_LVL);
+UFS_ATTRIBUTE_RW(ooo_data_enabled, _OOO_DATA_EN);
+UFS_ATTRIBUTE_RO(bkops_status, _BKOPS_STATUS);
+UFS_ATTRIBUTE_RO(purge_status, _PURGE_STATUS);
+UFS_ATTRIBUTE_RW(max_data_in_size, _MAX_DATA_IN);
+UFS_ATTRIBUTE_RW(max_data_out_size, _MAX_DATA_OUT);
+UFS_ATTRIBUTE_RW(reference_clock_frequency, _REF_CLK_FREQ);
+UFS_ATTRIBUTE_RW(configuration_descriptor_lock, _CONF_DESC_LOCK);
+UFS_ATTRIBUTE_RW(max_number_of_rtt, _MAX_NUM_OF_RTT);
+UFS_ATTRIBUTE_RW(exception_event_control, _EE_CONTROL);
+UFS_ATTRIBUTE_RW(exception_event_status, _EE_STATUS);
+UFS_ATTRIBUTE_RO(ffu_status, _FFU_STATUS);
+UFS_ATTRIBUTE_RO(psa_state, _PSA_STATE);
+UFS_ATTRIBUTE_RO(psa_data_size, _PSA_DATA_SIZE);
 
 static struct attribute *ufs_sysfs_attributes[] = {
 	&dev_attr_boot_lun_enabled.attr,
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] scsi: ufs: Refactor descriptor read for write
  2018-06-15 21:00 [PATCH v2 0/4] Enable UFS provisioning via Linux Evan Green
  2018-06-15 21:00 ` [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs Evan Green
  2018-06-15 21:00 ` [PATCH v2 2/4] scsi: ufs: Make sysfs attributes writable Evan Green
@ 2018-06-15 21:00 ` Evan Green
  2018-06-15 21:00 ` [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor Evan Green
  3 siblings, 0 replies; 9+ messages in thread
From: Evan Green @ 2018-06-15 21:00 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	Stanislav Nijnikov, Greg Kroah-Hartman, Bart Van Assche,
	Adrian Hunter, linux-kernel, linux-scsi
  Cc: Evan Green

This change refactors the ufshcd_read_desc_param function into
one that can both read and write descriptors. Most of the low-level
plumbing for writing to descriptors was already there, this change
simply enables that code path, and manages the incoming data buffer
appropriately.

Signed-off-by: Evan Green <evgreen@chromium.org>
---
Changes since v1:
	- Fail if requesting a region fully outside the descriptor.
	- Clear the remaining buffer if requesting a region partially
outside the descriptor.
	- Add a mutex around the read-modify-write operation as
suggested by Bart.
	- Fixed comment style in this function as suggested by Bart.
	- Refactored error prints to the end of the function, which I
think addresses the appearance that the function is nested too deeply
(as it only actually has 3 levels of nesting).

 drivers/scsi/ufs/ufs-sysfs.c |   4 +-
 drivers/scsi/ufs/ufshcd.c    | 109 +++++++++++++++++++++++++++++++------------
 drivers/scsi/ufs/ufshcd.h    |  15 +++---
 3 files changed, 90 insertions(+), 38 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 4726c05bb085..d0365e8bf839 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -227,8 +227,8 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba,
 	if (param_size > 8)
 		return -EINVAL;
 
-	ret = ufshcd_read_desc_param(hba, desc_id, desc_index,
-				param_offset, desc_buf, param_size);
+	ret = ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_READ_DESC, desc_id,
+				desc_index, param_offset, desc_buf, param_size);
 	if (ret)
 		return -EINVAL;
 	switch (param_size) {
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0baba3fdb112..110157877823 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3092,22 +3092,24 @@ int ufshcd_map_desc_id_to_length(struct ufs_hba *hba,
 EXPORT_SYMBOL(ufshcd_map_desc_id_to_length);
 
 /**
- * ufshcd_read_desc_param - read the specified descriptor parameter
+ * ufshcd_rw_desc_param - read or write the specified descriptor parameter
  * @hba: Pointer to adapter instance
+ * @opcode: indicates whether to read or write
  * @desc_id: descriptor idn value
  * @desc_index: descriptor index
- * @param_offset: offset of the parameter to read
- * @param_read_buf: pointer to buffer where parameter would be read
- * @param_size: sizeof(param_read_buf)
+ * @param_offset: offset of the parameter to read or write
+ * @param_buf: pointer to buffer to be read or written
+ * @param_size: sizeof(param_buf)
  *
  * Return 0 in case of success, non-zero otherwise
  */
-int ufshcd_read_desc_param(struct ufs_hba *hba,
-			   enum desc_idn desc_id,
-			   int desc_index,
-			   u8 param_offset,
-			   u8 *param_read_buf,
-			   u8 param_size)
+int ufshcd_rw_desc_param(struct ufs_hba *hba,
+			 enum query_opcode opcode,
+			 enum desc_idn desc_id,
+			 int desc_index,
+			 u8 param_offset,
+			 u8 *param_buf,
+			 u8 param_size)
 {
 	int ret;
 	u8 *desc_buf;
@@ -3118,7 +3120,8 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 	if (desc_id >= QUERY_DESC_IDN_MAX || !param_size)
 		return -EINVAL;
 
-	/* Get the max length of descriptor from structure filled up at probe
+	/*
+	 * Get the max length of descriptor from structure filled up at probe
 	 * time.
 	 */
 	ret = ufshcd_map_desc_id_to_length(hba, desc_id, &buff_len);
@@ -3130,26 +3133,53 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 		return ret;
 	}
 
+	if (param_offset > buff_len)
+		return -EINVAL;
+
+	/* Lock the descriptor mutex to prevent simultaneous changes. */
+	mutex_lock(&hba->desc_mutex);
+
 	/* Check whether we need temp memory */
 	if (param_offset != 0 || param_size < buff_len) {
-		desc_buf = kmalloc(buff_len, GFP_KERNEL);
-		if (!desc_buf)
-			return -ENOMEM;
+		desc_buf = kzalloc(buff_len, GFP_KERNEL);
+		if (!desc_buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		/*
+		 * If it's a write, first read the complete descriptor, then
+		 * copy in the parts being changed.
+		 */
+		if (opcode == UPIU_QUERY_OPCODE_WRITE_DESC) {
+			if (param_offset + param_size > buff_len) {
+				ret = -EINVAL;
+				goto out;
+			}
+
+			ret = ufshcd_query_descriptor_retry(hba,
+						UPIU_QUERY_OPCODE_READ_DESC,
+						desc_id, desc_index, 0,
+						desc_buf, &buff_len);
+
+			if (ret)
+				goto out;
+
+			memcpy(desc_buf + param_offset, param_buf, param_size);
+		}
+
 	} else {
-		desc_buf = param_read_buf;
+		desc_buf = param_buf;
 		is_kmalloc = false;
 	}
 
-	/* Request for full descriptor */
-	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
+	/* Read or write the entire descriptor. */
+	ret = ufshcd_query_descriptor_retry(hba, opcode,
 					desc_id, desc_index, 0,
 					desc_buf, &buff_len);
 
-	if (ret) {
-		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d",
-			__func__, desc_id, desc_index, param_offset, ret);
+	if (ret)
 		goto out;
-	}
 
 	/* Sanity check */
 	if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id) {
@@ -3159,13 +3189,29 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
 		goto out;
 	}
 
-	/* Check wherher we will not copy more data, than available */
-	if (is_kmalloc && param_size > buff_len)
-		param_size = buff_len;
+	/*
+	 * Copy data to the output. The offset is already validated to be
+	 * within the buffer.
+	 */
+	if (is_kmalloc && (opcode == UPIU_QUERY_OPCODE_READ_DESC)) {
+		if (param_offset + param_size > buff_len) {
+			memset(param_buf + buff_len - param_offset, 0,
+			       param_offset + param_size - buff_len);
+
+			param_size = buff_len - param_offset;
+		}
+
+		memcpy(param_buf, &desc_buf[param_offset], param_size);
+	}
 
-	if (is_kmalloc)
-		memcpy(param_read_buf, &desc_buf[param_offset], param_size);
 out:
+	mutex_unlock(&hba->desc_mutex);
+	if (ret)
+		dev_err(hba->dev, "Failed %s descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d",
+			opcode == UPIU_QUERY_OPCODE_READ_DESC ?
+			"reading" : "writing",
+			desc_id, desc_index, param_offset, ret);
+
 	if (is_kmalloc)
 		kfree(desc_buf);
 	return ret;
@@ -3177,7 +3223,8 @@ static inline int ufshcd_read_desc(struct ufs_hba *hba,
 				   u8 *buf,
 				   u32 size)
 {
-	return ufshcd_read_desc_param(hba, desc_id, desc_index, 0, buf, size);
+	return ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_READ_DESC, desc_id,
+					desc_index, 0, buf, size);
 }
 
 static inline int ufshcd_read_power_desc(struct ufs_hba *hba,
@@ -3283,8 +3330,9 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
 	if (!ufs_is_valid_unit_desc_lun(lun))
 		return -EOPNOTSUPP;
 
-	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
-				      param_offset, param_read_buf, param_size);
+	return ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_READ_DESC,
+				    QUERY_DESC_IDN_UNIT, lun, param_offset,
+				    param_read_buf, param_size);
 }
 
 /**
@@ -8019,8 +8067,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	INIT_WORK(&hba->eh_work, ufshcd_err_handler);
 	INIT_WORK(&hba->eeh_work, ufshcd_exception_event_handler);
 
-	/* Initialize UIC command mutex */
+	/* Initialize UIC command and descriptor mutexes */
 	mutex_init(&hba->uic_cmd_mutex);
+	mutex_init(&hba->desc_mutex);
 
 	/* Initialize mutex for device management commands */
 	mutex_init(&hba->dev_cmd.lock);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index bb540a3fd8de..c6305a458100 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -513,6 +513,7 @@ struct ufs_cfg_object {
  * @scsi_block_reqs_cnt: reference counting for scsi block requests
  * @cfg_objects: Stores the array of kobjects created for the config descriptor.
  * @cfg_object_count: Stores the number of elements in cfg_objects.
+ * @desc_mutex: Mutex to serialize modifications to UFS descriptors.
  */
 struct ufs_hba {
 	void __iomem *mmio_base;
@@ -717,6 +718,7 @@ struct ufs_hba {
 
 	struct ufs_cfg_object **cfg_objects;
 	size_t cfg_object_count;
+	struct mutex desc_mutex;
 };
 
 /* Returns true if clocks can be gated. Otherwise false */
@@ -886,12 +888,13 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
 				  enum desc_idn idn, u8 index,
 				  u8 selector,
 				  u8 *desc_buf, int *buf_len);
-int ufshcd_read_desc_param(struct ufs_hba *hba,
-			   enum desc_idn desc_id,
-			   int desc_index,
-			   u8 param_offset,
-			   u8 *param_read_buf,
-			   u8 param_size);
+int ufshcd_rw_desc_param(struct ufs_hba *hba,
+			 enum query_opcode opcode,
+			 enum desc_idn desc_id,
+			 int desc_index,
+			 u8 param_offset,
+			 u8 *param_buf,
+			 u8 param_size);
 int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 		      enum attr_idn idn, u8 index, u8 selector, u32 *attr_val);
 int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor
  2018-06-15 21:00 [PATCH v2 0/4] Enable UFS provisioning via Linux Evan Green
                   ` (2 preceding siblings ...)
  2018-06-15 21:00 ` [PATCH v2 3/4] scsi: ufs: Refactor descriptor read for write Evan Green
@ 2018-06-15 21:00 ` Evan Green
  2018-06-15 21:33     ` kbuild test robot
  3 siblings, 1 reply; 9+ messages in thread
From: Evan Green @ 2018-06-15 21:00 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	Stanislav Nijnikov, Greg Kroah-Hartman, Bart Van Assche,
	Adrian Hunter, linux-kernel, linux-scsi
  Cc: Evan Green

This change enables writing to fields of the config descriptor
via sysfs. It can be used to provision an unprovisioned UFS
device, or reprovision an unlocked device.

Signed-off-by: Evan Green <evgreen@chromium.org>
---
Changes since v2:
        - New common _store function, since I'm using kobj_type now.
        - Tried to make the endianness more explicit as suggested by Bart.
I opted not to go as far as creating rw_desc_param{16,32} and friends,
as those would simply turn around and call the generic function, which
didn't seem all that useful.

 Documentation/ABI/testing/sysfs-driver-ufs | 17 ------
 drivers/scsi/ufs/ufs-sysfs.c               | 83 +++++++++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 19 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index d7477fffe0d1..fef594d912de 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -244,7 +244,6 @@ Description:	This file shows whether or not the UFS boot feature is enabled.
 		This is one of the UFS configuration descriptor parameters.
 		More information about the descriptor can be found in the UFS
 		2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/descriptor_access_enable
 Date:		May 2018
@@ -254,7 +253,6 @@ Description:	This file shows whether or not access will be permitted to the
 		boot sequence. This is one of the UFS configuration descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/high_priority_lun
 Date:		May 2018
@@ -263,7 +261,6 @@ Description:	This file shows the identifier of the high priority logical
 		unit. This is one of the UFS configuration descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/init_active_icc_level
 Date:		May 2018
@@ -272,7 +269,6 @@ Description:	This file shows the ICC level in active mode after device
 		initialization or hardware reset. This is one of the UFS
 		configuration descriptor parameters. More information about the
 		descriptor can be found in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/initial_power_mode
 Date:		May 2018
@@ -281,7 +277,6 @@ Description:	This file shows the power mode after device initialization or
 		hardware reset. This is one of the UFS configuration descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/number_of_luns
 Date:		May 2018
@@ -290,7 +285,6 @@ Description:	This file shows the number of logical units that the device will
 		support. This is one of the UFS configuration descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/periodic_rtc_update
 Date:		May 2018
@@ -299,7 +293,6 @@ Description:	This file shows the frequency and method of real time clock
 		updates. This is one of the UFS configuration descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/secure_removal_type
 Date:		May 2018
@@ -308,7 +301,6 @@ Description:	This file shows the secure removal type of the UFS device. This
 		is one of the UFS configuration descriptor parameters. More
 		information about the descriptor can be found in the UFS 2.1
 		specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/allocation_units
 Date:		May 2018
@@ -317,7 +309,6 @@ Description:	This file shows the number of allocation units assigned to the
 		particular logical unit. This is one of the UFS configuration
 		unit descriptor parameters. More information about the
 		descriptor can be found in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/boot_lun_id
 Date:		May 2018
@@ -327,7 +318,6 @@ Description:	This file shows the boot LUN ID for this particular logical
 		This is one of the UFS configuration unit descriptor parameters.
 		More information about the descriptor can be found in the UFS
 		2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/context_capabilities
 Date:		May 2018
@@ -336,7 +326,6 @@ Description:	This file shows the context capabilities for the particular
 		logical unit. This is one of the UFS configuration unit
 		descriptor parameters. More information about the descriptor
 		can be found in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/data_reliability
 Date:		May 2018
@@ -345,7 +334,6 @@ Description:	This file shows the data reliability for the particular logical
 		unit. This is one of the UFS configuration unit descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/logical_block_size
 Date:		May 2018
@@ -354,7 +342,6 @@ Description:	This file shows the logical block size for the particular
 		logical unit as a power of two. This is one of the UFS
 		configuration unit descriptor parameters. More information
 		about the descriptor can be found in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/lu_enable
 Date:		May 2018
@@ -363,7 +350,6 @@ Description:	This file shows whether or not the particular logical unit is
 		enabled. This is one of the UFS configuration unit descriptor
 		parameters. More information about the descriptor can be found
 		in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/lu_write_protect
 Date:		May 2018
@@ -372,7 +358,6 @@ Description:	This file shows the write protect status for the particular
 		logical unit. This is one of the UFS configuration unit
 		descriptor parameters. More information about the descriptor
 		can be found in the UFS 2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/memory_type
 Date:		May 2018
@@ -381,7 +366,6 @@ Description:	This file shows the memory type for the particular logical unit.
 		This is one of the UFS configuration unit descriptor parameters.
 		More information about the descriptor can be found in the UFS
 		2.1 specification.
-		The file is read only.
 
 What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/unit*/provisioning_type
 Date:		May 2018
@@ -390,7 +374,6 @@ Description:	This file shows the provisioning type information for the
 		particular logical unit. This is one of the UFS configuration
 		uint descriptor parameters. More information about the
 		descriptor can be found in the UFS 2.1 specification.
-		The file is read only.
 
 
 What:		/sys/bus/platform/drivers/ufshcd/*/interconnect_descriptor/unipro_version
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index d0365e8bf839..9e6e592280bb 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -252,6 +252,56 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba,
 	return ret;
 }
 
+static ssize_t ufs_sysfs_write_desc_param(struct ufs_hba *hba,
+				  enum desc_idn desc_id,
+				  u8 desc_index,
+				  u8 param_offset,
+				  const char *buf,
+				  ssize_t buf_size,
+				  u8 width)
+{
+	int ret;
+	unsigned long long value;
+	u8 value8;
+	__be16 value16;
+	__be32 value32;
+	__be64 value64;
+	u8 *valueptr;
+
+	if (kstrtoull(buf, 0, &value))
+		return -EINVAL;
+
+	switch (width) {
+	case 1:
+		value8 = (u8)value;
+		valueptr = &value8;
+		break;
+
+	case 2:
+		value16 = cpu_to_be16(value);
+		valueptr = (u8 *)&value16;
+		break;
+
+	case 4:
+		value32 = cpu_to_be32(value);
+		valueptr = (u8 *)&value32;
+		break;
+
+	case 8:
+		value64 = cpu_to_be64(value);
+		valueptr = (u8 *)&value64;
+		break;
+	}
+
+	ret = ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_WRITE_DESC, desc_id,
+				desc_index, param_offset,
+				valueptr, width);
+	if (ret)
+		return -EINVAL;
+
+	return buf_size;
+}
+
 #define UFS_DESC_PARAM(_name, _puname, _duname, _size)			\
 static ssize_t _name##_show(struct device *dev,				\
 	struct device_attribute *attr, char *buf)			\
@@ -346,7 +396,7 @@ struct ufs_config_desc_attr {
 #define UFS_CONFIG_DESC_PARAM(_name, _uname, _size)			\
 static struct ufs_config_desc_attr ufs_cfg_attr_##_name = {		\
 	.attr = {.name = __stringify(_name),				\
-		 .mode = VERIFY_OCTAL_PERMISSIONS(0444) }, \
+		 .mode = VERIFY_OCTAL_PERMISSIONS(0644) }, 		\
 	.offset = CONFIGURATION_DESC_PARAM##_uname,			\
 	.size = _size							\
 }
@@ -375,7 +425,7 @@ static struct attribute *ufs_sysfs_config_descriptor[] = {
 #define UFS_CONFIG_UNIT_DESC_PARAM(_name, _uname, _size)		\
 static struct ufs_config_desc_attr ufs_cfg_unit_attr_##_name = {	\
 	.attr = {.name = __stringify(_name),				\
-		 .mode = VERIFY_OCTAL_PERMISSIONS(0444) },		\
+		 .mode = VERIFY_OCTAL_PERMISSIONS(0644) },		\
 	.offset = CONFIGURATION_UNIT_DESC_PARAM##_uname,		\
 	.size = _size							\
 }
@@ -430,8 +480,37 @@ static ssize_t ufs_cfg_attr_show(struct kobject *kobj, struct attribute *attr,
 					 offset, buf, cfg_attr->size);
 }
 
+static ssize_t ufs_cfg_attr_store(struct kobject *kobj, struct attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct ufs_config_desc_attr *cfg_attr = to_ufs_cfg_desc_attr(attr);
+	struct ufs_cfg_object *cfg_obj = to_ufs_cfg_obj(kobj);
+	u8 offset = cfg_attr->offset;
+	struct device *dev;
+	struct ufs_hba *hba;
+
+	/*
+	 * For unit config descriptors, add the unit's offset and get the
+	 * device parent two up.
+	 */
+	if (cfg_obj->index >= 0) {
+		offset += CONFIGURATION_DESC_PARAM_UNIT0 +
+			(CONFIGURATION_UNIT_DESC_SIZE * cfg_obj->index);
+
+		dev = kobj_to_dev(cfg_obj->kobj.parent->parent);
+
+	} else {
+		dev = kobj_to_dev(cfg_obj->kobj.parent);
+	}
+
+	hba = dev_get_drvdata(dev);
+	return ufs_sysfs_write_desc_param(hba, QUERY_DESC_IDN_CONFIGURATION, 0,
+					 offset, buf, count, cfg_attr->size);
+}
+
 static const struct sysfs_ops ufs_sysfs_config_descriptor_ops = {
 	.show	= ufs_cfg_attr_show,
+	.store	= ufs_cfg_attr_store,
 };
 
 static void ufs_cfg_kobject_release(struct kobject *kobj)
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor
  2018-06-15 21:00 ` [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor Evan Green
@ 2018-06-15 21:33     ` kbuild test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-06-15 21:33 UTC (permalink / raw)
  To: Evan Green
  Cc: kbuild-all, Vinayak Holikatti, James E.J. Bottomley,
	Martin K. Petersen, Stanislav Nijnikov, Greg Kroah-Hartman,
	Bart Van Assche, Adrian Hunter, linux-kernel, linux-scsi,
	Evan Green

[-- Attachment #1: Type: text/plain, Size: 5378 bytes --]

Hi Evan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20180615]
[cannot apply to scsi/for-next v4.17]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Evan-Green/Enable-UFS-provisioning-via-Linux/20180616-050548
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-x016-201823 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   include/linux/huge_mm.h:230:34: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:533:24: sparse: constant 0xffffc90000000000 is so big it is unsigned long
   include/linux/mm.h:533:48: sparse: constant 0xffffc90000000000 is so big it is unsigned long
   include/linux/mm.h:624:29: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:1098:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:1796:27: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:1888:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:151:25: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:236:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:387:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:387:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/dma-mapping.h:235:35: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/dma-mapping.h:238:33: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/highmem.h:51:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1721:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1721:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1723:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1723:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   drivers/scsi/ufs/ufs-sysfs.c:897:1: sparse: symbol 'dev_attr_current_power_mode' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:900:1: sparse: symbol 'dev_attr_bkops_status' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:901:1: sparse: symbol 'dev_attr_purge_status' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:909:1: sparse: symbol 'dev_attr_ffu_status' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:910:1: sparse: symbol 'dev_attr_psa_state' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:911:1: sparse: symbol 'dev_attr_psa_data_size' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c: In function 'ufs_cfg_attr_store':
>> drivers/scsi/ufs/ufs-sysfs.c:296:6: warning: 'valueptr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     ret = ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_WRITE_DESC, desc_id,
     ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        desc_index, param_offset,
        ~~~~~~~~~~~~~~~~~~~~~~~~~
        valueptr, width);
        ~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufs-sysfs.c:269:6: note: 'valueptr' was declared here
     u8 *valueptr;
         ^~~~~~~~

vim +/valueptr +296 drivers/scsi/ufs/ufs-sysfs.c

   254	
   255	static ssize_t ufs_sysfs_write_desc_param(struct ufs_hba *hba,
   256					  enum desc_idn desc_id,
   257					  u8 desc_index,
   258					  u8 param_offset,
   259					  const char *buf,
   260					  ssize_t buf_size,
   261					  u8 width)
   262	{
   263		int ret;
   264		unsigned long long value;
   265		u8 value8;
   266		__be16 value16;
   267		__be32 value32;
   268		__be64 value64;
   269		u8 *valueptr;
   270	
   271		if (kstrtoull(buf, 0, &value))
   272			return -EINVAL;
   273	
   274		switch (width) {
   275		case 1:
   276			value8 = (u8)value;
   277			valueptr = &value8;
   278			break;
   279	
   280		case 2:
   281			value16 = cpu_to_be16(value);
   282			valueptr = (u8 *)&value16;
   283			break;
   284	
   285		case 4:
   286			value32 = cpu_to_be32(value);
   287			valueptr = (u8 *)&value32;
   288			break;
   289	
   290		case 8:
   291			value64 = cpu_to_be64(value);
   292			valueptr = (u8 *)&value64;
   293			break;
   294		}
   295	
 > 296		ret = ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_WRITE_DESC, desc_id,
   297					desc_index, param_offset,
   298					valueptr, width);
   299		if (ret)
   300			return -EINVAL;
   301	
   302		return buf_size;
   303	}
   304	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28553 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor
@ 2018-06-15 21:33     ` kbuild test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-06-15 21:33 UTC (permalink / raw)
  Cc: kbuild-all, Vinayak Holikatti, James E.J. Bottomley,
	Martin K. Petersen, Stanislav Nijnikov, Greg Kroah-Hartman,
	Bart Van Assche, Adrian Hunter, linux-kernel, linux-scsi,
	Evan Green

[-- Attachment #1: Type: text/plain, Size: 5378 bytes --]

Hi Evan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20180615]
[cannot apply to scsi/for-next v4.17]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Evan-Green/Enable-UFS-provisioning-via-Linux/20180616-050548
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-x016-201823 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   include/linux/huge_mm.h:230:34: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:533:24: sparse: constant 0xffffc90000000000 is so big it is unsigned long
   include/linux/mm.h:533:48: sparse: constant 0xffffc90000000000 is so big it is unsigned long
   include/linux/mm.h:624:29: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:1098:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:1796:27: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/mm.h:1888:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:151:25: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:236:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:387:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/scatterlist.h:387:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/dma-mapping.h:235:35: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/dma-mapping.h:238:33: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/highmem.h:51:16: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1721:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1721:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1723:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   include/linux/blkdev.h:1723:14: sparse: constant 0xffffea0000000000 is so big it is unsigned long
   drivers/scsi/ufs/ufs-sysfs.c:897:1: sparse: symbol 'dev_attr_current_power_mode' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:900:1: sparse: symbol 'dev_attr_bkops_status' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:901:1: sparse: symbol 'dev_attr_purge_status' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:909:1: sparse: symbol 'dev_attr_ffu_status' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:910:1: sparse: symbol 'dev_attr_psa_state' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c:911:1: sparse: symbol 'dev_attr_psa_data_size' was not declared. Should it be static?
   drivers/scsi/ufs/ufs-sysfs.c: In function 'ufs_cfg_attr_store':
>> drivers/scsi/ufs/ufs-sysfs.c:296:6: warning: 'valueptr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     ret = ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_WRITE_DESC, desc_id,
     ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        desc_index, param_offset,
        ~~~~~~~~~~~~~~~~~~~~~~~~~
        valueptr, width);
        ~~~~~~~~~~~~~~~~
   drivers/scsi/ufs/ufs-sysfs.c:269:6: note: 'valueptr' was declared here
     u8 *valueptr;
         ^~~~~~~~

vim +/valueptr +296 drivers/scsi/ufs/ufs-sysfs.c

   254	
   255	static ssize_t ufs_sysfs_write_desc_param(struct ufs_hba *hba,
   256					  enum desc_idn desc_id,
   257					  u8 desc_index,
   258					  u8 param_offset,
   259					  const char *buf,
   260					  ssize_t buf_size,
   261					  u8 width)
   262	{
   263		int ret;
   264		unsigned long long value;
   265		u8 value8;
   266		__be16 value16;
   267		__be32 value32;
   268		__be64 value64;
   269		u8 *valueptr;
   270	
   271		if (kstrtoull(buf, 0, &value))
   272			return -EINVAL;
   273	
   274		switch (width) {
   275		case 1:
   276			value8 = (u8)value;
   277			valueptr = &value8;
   278			break;
   279	
   280		case 2:
   281			value16 = cpu_to_be16(value);
   282			valueptr = (u8 *)&value16;
   283			break;
   284	
   285		case 4:
   286			value32 = cpu_to_be32(value);
   287			valueptr = (u8 *)&value32;
   288			break;
   289	
   290		case 8:
   291			value64 = cpu_to_be64(value);
   292			valueptr = (u8 *)&value64;
   293			break;
   294		}
   295	
 > 296		ret = ufshcd_rw_desc_param(hba, UPIU_QUERY_OPCODE_WRITE_DESC, desc_id,
   297					desc_index, param_offset,
   298					valueptr, width);
   299		if (ret)
   300			return -EINVAL;
   301	
   302		return buf_size;
   303	}
   304	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28553 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs
  2018-06-15 21:00 ` [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs Evan Green
@ 2018-06-16  7:16   ` Greg Kroah-Hartman
  2018-06-18 16:54     ` Evan Green
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2018-06-16  7:16 UTC (permalink / raw)
  To: Evan Green
  Cc: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	Stanislav Nijnikov, Bart Van Assche, Adrian Hunter, linux-kernel,
	linux-scsi

On Fri, Jun 15, 2018 at 02:00:46PM -0700, Evan Green wrote:
> This change adds the configuration descriptor to the UFS
> sysfs interface. This is done in preparation for making the
> interface writable, which will enable provisioning UFS devices
> via Linux.
> 
> The configuration descriptor is laid out as a header, then a set of
> (usually 8) copies of the same descriptor for each unit.
> 
> Signed-off-by: Evan Green <evgreen@chromium.org>
> ---
> Changes since v1:
> 	- Squashed documentation changes into this change.
> 	- Reworked sysfs layout so that instead of a sysfs file for a
> unit selector and then a common set of unit attributes, each unit in
> the config descriptor has its own directory. This required a little
> bit of kobject magic. Alternatively I could use standard device
> attributes and simply allocate N*M of them from a template. I have
> that coded up, and can go with that if preferred, but I thought
> this was a little nicer since it wasted less memory.

Ick, don't use "raw" kobjects please, as userspace will not see them
correctly in the libraries that track devices and attributes, like
libudev.

And what is wrong with using configfs?  I thought that was the better
way to go for something like this.  You are configuring the device,
which is exactly what configfs was created for, to keep people from
having to do this type of mess in sysfs.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs
  2018-06-16  7:16   ` Greg Kroah-Hartman
@ 2018-06-18 16:54     ` Evan Green
  0 siblings, 0 replies; 9+ messages in thread
From: Evan Green @ 2018-06-18 16:54 UTC (permalink / raw)
  To: gregkh
  Cc: Vinayak Holikatti, jejb, martin.petersen, stanislav.nijnikov,
	Bart.VanAssche, adrian.hunter, linux-kernel, linux-scsi

On Sat, Jun 16, 2018 at 12:16 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jun 15, 2018 at 02:00:46PM -0700, Evan Green wrote:
> > This change adds the configuration descriptor to the UFS
> > sysfs interface. This is done in preparation for making the
> > interface writable, which will enable provisioning UFS devices
> > via Linux.
> >
> > The configuration descriptor is laid out as a header, then a set of
> > (usually 8) copies of the same descriptor for each unit.
> >
> > Signed-off-by: Evan Green <evgreen@chromium.org>
> > ---
> > Changes since v1:
> >       - Squashed documentation changes into this change.
> >       - Reworked sysfs layout so that instead of a sysfs file for a
> > unit selector and then a common set of unit attributes, each unit in
> > the config descriptor has its own directory. This required a little
> > bit of kobject magic. Alternatively I could use standard device
> > attributes and simply allocate N*M of them from a template. I have
> > that coded up, and can go with that if preferred, but I thought
> > this was a little nicer since it wasted less memory.
>
> Ick, don't use "raw" kobjects please, as userspace will not see them
> correctly in the libraries that track devices and attributes, like
> libudev.
>
> And what is wrong with using configfs?  I thought that was the better
> way to go for something like this.  You are configuring the device,
> which is exactly what configfs was created for, to keep people from
> having to do this type of mess in sysfs.
>
> thanks,
>
> greg k-h

Ok. Sayali's got patches for doing this via configfs, so let's go with his.
-Evan

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-06-18 16:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-15 21:00 [PATCH v2 0/4] Enable UFS provisioning via Linux Evan Green
2018-06-15 21:00 ` [PATCH v2 1/4] scsi: ufs: Add Configuration Descriptor to sysfs Evan Green
2018-06-16  7:16   ` Greg Kroah-Hartman
2018-06-18 16:54     ` Evan Green
2018-06-15 21:00 ` [PATCH v2 2/4] scsi: ufs: Make sysfs attributes writable Evan Green
2018-06-15 21:00 ` [PATCH v2 3/4] scsi: ufs: Refactor descriptor read for write Evan Green
2018-06-15 21:00 ` [PATCH v2 4/4] scsi: ufs: Enable writing config descriptor Evan Green
2018-06-15 21:33   ` kbuild test robot
2018-06-15 21:33     ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.