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 <[email protected]>
---
Changes since v3:
- Added static to UFS_ATTRIBUTE_RO [Stanislav]
- Fixed read-only exception_event_status [Stanislav]
- Added warnings in documentation for write-once
attributes [Stanislav]
Configfs was determined to be the preferred mechanism for writing the
config descriptor, but attributes also need to be written during setup,
and are already present in sysfs. Making these attributes writable is
also helpful for debugging and experimentation.
Changes since v2:
- Removed the configuration descriptor changes from the series,
since configfs was the preferred way to write to that, leaving only
this change.
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
Documentation/ABI/testing/sysfs-driver-ufs | 29 +++++++---------
drivers/scsi/ufs/ufs-sysfs.c | 56 ++++++++++++++++++++----------
2 files changed, 51 insertions(+), 34 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 016724ec26d5..3ed8aaac2faf 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -685,7 +685,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -693,7 +692,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -701,7 +699,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -709,7 +706,9 @@ Contact: Stanislav Nijnikov <[email protected]>
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.
+ Warning: This attribute can only be written one time
+ within the lifetime of the device. Once written, it cannot be
+ changed.
What: /sys/bus/platform/drivers/ufshcd/*/attributes/bkops_status
Date: February 2018
@@ -717,7 +716,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -725,7 +723,9 @@ Contact: Stanislav Nijnikov <[email protected]>
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.
+ Warning: This attribute can only be written one time
+ within the lifetime of the device. Once written, it cannot be
+ changed.
What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_in_size
Date: February 2018
@@ -733,7 +733,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -741,7 +740,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -749,14 +747,19 @@ Contact: Stanislav Nijnikov <[email protected]>
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.
+ Warning: This attribute can only be written one time
+ within the lifetime of the device. Once written, it cannot be
+ changed.
What: /sys/bus/platform/drivers/ufshcd/*/attributes/configuration_descriptor_lock
Date: February 2018
Contact: Stanislav Nijnikov <[email protected]>
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.
+ Warning: This attribute can only be written one time
+ within the lifetime of the device. Once written, it cannot be
+ changed.
What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_number_of_rtt
Date: February 2018
@@ -765,7 +768,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
@@ -773,7 +775,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -781,7 +782,6 @@ Contact: Stanislav Nijnikov <[email protected]>
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
@@ -789,14 +789,12 @@ Contact: Stanislav Nijnikov <[email protected]>
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 <[email protected]>
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
@@ -805,7 +803,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 8d9332bb7d0c..344f5025bc36 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -655,7 +655,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) \
{ \
@@ -665,25 +665,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); \
-} \
+}
+
+#define UFS_ATTRIBUTE_RO(_name, _uname) \
+UFS_ATTRIBUTE_SHOW(_name, _uname) \
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_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_RO(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.16.4
> -----Original Message-----
> From: Evan Green <[email protected]>
> Sent: Thursday, August 9, 2018 1:45 AM
> To: Vinayak Holikatti <[email protected]>; James E.J. Bottomley <[email protected]>; Martin K. Petersen
> <[email protected]>; Stanislav Nijnikov <[email protected]>; Adrian Hunter <[email protected]>; linux-
> [email protected]; [email protected]; Bart Van Assche <[email protected]>
> Cc: Evan Green <[email protected]>
> Subject: [PATCH v4] scsi: ufs: Make sysfs attributes writable
>
> 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 <[email protected]>
Reviewed-by: Stanislav Nijnikov <[email protected]>
On 09/08/18 01:44, Evan Green wrote:
> 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 <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> Changes since v3:
> - Added static to UFS_ATTRIBUTE_RO [Stanislav]
> - Fixed read-only exception_event_status [Stanislav]
> - Added warnings in documentation for write-once
> attributes [Stanislav]
>
> Configfs was determined to be the preferred mechanism for writing the
> config descriptor, but attributes also need to be written during setup,
> and are already present in sysfs. Making these attributes writable is
> also helpful for debugging and experimentation.
>
> Changes since v2:
> - Removed the configuration descriptor changes from the series,
> since configfs was the preferred way to write to that, leaving only
> this change.
>
> 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
>
> Documentation/ABI/testing/sysfs-driver-ufs | 29 +++++++---------
> drivers/scsi/ufs/ufs-sysfs.c | 56 ++++++++++++++++++++----------
> 2 files changed, 51 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
> index 016724ec26d5..3ed8aaac2faf 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -685,7 +685,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -693,7 +692,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -701,7 +699,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -709,7 +706,9 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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.
> + Warning: This attribute can only be written one time
> + within the lifetime of the device. Once written, it cannot be
> + changed.
>
> What: /sys/bus/platform/drivers/ufshcd/*/attributes/bkops_status
> Date: February 2018
> @@ -717,7 +716,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -725,7 +723,9 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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.
> + Warning: This attribute can only be written one time
> + within the lifetime of the device. Once written, it cannot be
> + changed.
>
> What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_data_in_size
> Date: February 2018
> @@ -733,7 +733,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -741,7 +740,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -749,14 +747,19 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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.
> + Warning: This attribute can only be written one time
> + within the lifetime of the device. Once written, it cannot be
> + changed.
>
> What: /sys/bus/platform/drivers/ufshcd/*/attributes/configuration_descriptor_lock
> Date: February 2018
> Contact: Stanislav Nijnikov <[email protected]>
> 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.
> + Warning: This attribute can only be written one time
> + within the lifetime of the device. Once written, it cannot be
> + changed.
>
> What: /sys/bus/platform/drivers/ufshcd/*/attributes/max_number_of_rtt
> Date: February 2018
> @@ -765,7 +768,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
> @@ -773,7 +775,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -781,7 +782,6 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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
> @@ -789,14 +789,12 @@ Contact: Stanislav Nijnikov <[email protected]>
> 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 <[email protected]>
> 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
> @@ -805,7 +803,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 8d9332bb7d0c..344f5025bc36 100644
> --- a/drivers/scsi/ufs/ufs-sysfs.c
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -655,7 +655,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) \
> { \
> @@ -665,25 +665,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); \
> -} \
> +}
> +
> +#define UFS_ATTRIBUTE_RO(_name, _uname) \
> +UFS_ATTRIBUTE_SHOW(_name, _uname) \
> 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_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_RO(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,
>
On 22/08/18 14:00, Adrian Hunter wrote:
> On 09/08/18 01:44, Evan Green wrote:
>> 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 <[email protected]>
>
> Acked-by: Adrian Hunter <[email protected]>
>
This patch has my Ack and Reviewed-by: Stanislav Nijnikov, so can it be
queued for v4.20? There is also Evan's "[PATCH] scsi: ufs: Make sysfs flags
writable"
Hi,
On Tue, Sep 4, 2018 at 3:29 AM Adrian Hunter <[email protected]> wrote:
>
> On 22/08/18 14:00, Adrian Hunter wrote:
> > On 09/08/18 01:44, Evan Green wrote:
> >> 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 <[email protected]>
> >
> > Acked-by: Adrian Hunter <[email protected]>
> >
>
> This patch has my Ack and Reviewed-by: Stanislav Nijnikov, so can it be
> queued for v4.20? There is also Evan's "[PATCH] scsi: ufs: Make sysfs flags
> writable"
Ping?
I came across this patch and Evan's other one and noticed that they
haven't been applied though a batch of other SCSI patches for 4.20
were applied about a week ago. Martin: is there something about these
patches that needs to change before they can land?
We're about to land the two patches into the Chrome OS kernel tree
which will remove any built-in reminders we'll have to keep track of
upstream progress, so this is probably the final ping from this end
until the next major Chrome OS kernel rebase. ;-)
-Doug
Doug,
> I came across this patch and Evan's other one and noticed that they
> haven't been applied though a batch of other SCSI patches for 4.20
> were applied about a week ago. Martin: is there something about these
> patches that needs to change before they can land?
I have simply been awaiting some sort of consensus on the various
competing approaches. Lots of patches posted with tiny incremental fixes
but very little discussion about the merits of one over the other.
--
Martin K. Petersen Oracle Linux Engineering
Martin,
On Tue, Sep 25, 2018 at 6:08 PM Martin K. Petersen
<[email protected]> wrote:
>
> Doug,
>
> > I came across this patch and Evan's other one and noticed that they
> > haven't been applied though a batch of other SCSI patches for 4.20
> > were applied about a week ago. Martin: is there something about these
> > patches that needs to change before they can land?
>
> I have simply been awaiting some sort of consensus on the various
> competing approaches. Lots of patches posted with tiny incremental fixes
> but very little discussion about the merits of one over the other.
Ah, perfect information! Thank you! I was just confused because I
didn't understand all the status and it just looked like silence here.
Maybe someone on this thread can start a discussion with all the
stakeholders (people who have been involved in competing patches or
other tiny bits and pieces) and summarize their view of the current
status? Maybe that would help get the ball rolling again?
Thanks again! :)
-Doug
On Tue, Sep 25, 2018 at 6:46 PM Doug Anderson <[email protected]> wrote:
>
> Martin,
>
> On Tue, Sep 25, 2018 at 6:08 PM Martin K. Petersen
> <[email protected]> wrote:
> >
> > Doug,
> >
> > > I came across this patch and Evan's other one and noticed that they
> > > haven't been applied though a batch of other SCSI patches for 4.20
> > > were applied about a week ago. Martin: is there something about these
> > > patches that needs to change before they can land?
> >
> > I have simply been awaiting some sort of consensus on the various
> > competing approaches. Lots of patches posted with tiny incremental fixes
> > but very little discussion about the merits of one over the other.
>
> Ah, perfect information! Thank you! I was just confused because I
> didn't understand all the status and it just looked like silence here.
>
> Maybe someone on this thread can start a discussion with all the
> stakeholders (people who have been involved in competing patches or
> other tiny bits and pieces) and summarize their view of the current
> status? Maybe that would help get the ball rolling again?
>
Ah, I did not realize that's what was being gated on.
These patches complement, rather than compete with, the other patches
out there. There are two components to completely provisioning a UFS
device: writing the configuration descriptors, and setting
attributes/flags. My original series [1] did contain support for the
provisioning portion, but I opted to leave that to Sayali's patch [2]
that uses configfs, rather than duplicate effort. Sayali's other patch
[3] does handle setting the reference clock frequency, which has some
overlap with this patch in that both set bRefClkFreq. But this patch
and the flag patch [4] are still needed for provisioning activity like
locking the descriptors down once they're set up, and enable other
device experimentation. In other words, they're independent.
There was also another independent fix [5] for devices that start in
sleep mode, which Linux currently can't handle. That patch got no
reviews, which is a shame, and I should probably resend as multiple
patches or at least with some additional information.
-Evan
[1] https://lkml.org/lkml/2018/5/29/969
[2] https://lkml.org/lkml/2018/9/14/293
[3] https://lkml.org/lkml/2018/9/14/292
[4] https://patchwork.kernel.org/patch/10570811/
[5] https://lkml.org/lkml/2018/8/10/669
> > >
> > > I have simply been awaiting some sort of consensus on the various
> > > competing approaches. Lots of patches posted with tiny incremental fixes
> > > but very little discussion about the merits of one over the other.
> >
> > Ah, perfect information! Thank you! I was just confused because I
> > didn't understand all the status and it just looked like silence here.
> >
> > Maybe someone on this thread can start a discussion with all the
> > stakeholders (people who have been involved in competing patches or
> > other tiny bits and pieces) and summarize their view of the current
> > status? Maybe that would help get the ball rolling again?
> >
>
> Ah, I did not realize that's what was being gated on.
>
> These patches complement, rather than compete with, the other patches
> out there. There are two components to completely provisioning a UFS
> device: writing the configuration descriptors, and setting
> attributes/flags. My original series [1] did contain support for the
> provisioning portion, but I opted to leave that to Sayali's patch [2]
> that uses configfs, rather than duplicate effort. Sayali's other patch
> [3] does handle setting the reference clock frequency, which has some
> overlap with this patch in that both set bRefClkFreq. But this patch
> and the flag patch [4] are still needed for provisioning activity like
> locking the descriptors down once they're set up, and enable other
> device experimentation. In other words, they're independent.
>
Also, in this context there is the series in
https://www.spinics.net/lists/linux-scsi/msg123479.html
which allows to send UPIUs via a bsg device.
It's not a provisioning series per-se like Evan's and Sayali's.
It covers the provisioning functionality,
But also allow to send task management UPIU, and UIC commands,
Which can be used for testing and validation.
Thanks,
Avri
> There was also another independent fix [5] for devices that start in
> sleep mode, which Linux currently can't handle. That patch got no
> reviews, which is a shame, and I should probably resend as multiple
> patches or at least with some additional information.
>
> -Evan
>
> [1] https://lkml.org/lkml/2018/5/29/969
> [2] https://lkml.org/lkml/2018/9/14/293
> [3] https://lkml.org/lkml/2018/9/14/292
> [4] https://patchwork.kernel.org/patch/10570811/
> [5] https://lkml.org/lkml/2018/8/10/669
On Thu, Sep 27, 2018 at 06:32:47AM +0000, Avri Altman wrote:
> Also, in this context there is the series in
> https://www.spinics.net/lists/linux-scsi/msg123479.html
> which allows to send UPIUs via a bsg device.
>
> It's not a provisioning series per-se like Evan's and Sayali's.
> It covers the provisioning functionality,
> But also allow to send task management UPIU, and UIC commands,
> Which can be used for testing and validation.
And as someone having been involved with review of a few different
UFS provisioning bits this is what I think we should be merging.
Instead of being in a rat race of adding ever new sysfs or configfs
attributes for things that don't matter to normal driver operation
I'd rather have a relatively clean pass through interface and move
policy to userspace. Especially given that there are plenty of
vendor specific commands at these levels as well.
On Thu, Sep 27, 2018 at 7:01 AM Christoph Hellwig <[email protected]> wrote:
>
> On Thu, Sep 27, 2018 at 06:32:47AM +0000, Avri Altman wrote:
> > Also, in this context there is the series in
> > https://www.spinics.net/lists/linux-scsi/msg123479.html
> > which allows to send UPIUs via a bsg device.
> >
> > It's not a provisioning series per-se like Evan's and Sayali's.
> > It covers the provisioning functionality,
> > But also allow to send task management UPIU, and UIC commands,
> > Which can be used for testing and validation.
>
> And as someone having been involved with review of a few different
> UFS provisioning bits this is what I think we should be merging.
>
> Instead of being in a rat race of adding ever new sysfs or configfs
> attributes for things that don't matter to normal driver operation
> I'd rather have a relatively clean pass through interface and move
> policy to userspace. Especially given that there are plenty of
> vendor specific commands at these levels as well.
There's no policy in my patches (nor Sayali's), nor are there any
vendor-specific commands here. The sysfs interface has exposed knobs
defined by the UFS specification, which to me seems like the kernel
providing a sane abstraction of device functionality. I don't see
there being a rat race, as these attributes and the config descriptor
are all that's needed to provision a device, and any reasonable future
versions of the UFS spec would likely be backwards compatible with
respect to attributes and flags.
The patches Avri linked to seem fine as well, but I don't see why
there's not room for both the "roll your own driver completely in user
mode" approach, and the "kernel provides a reasonable abstraction of
device functionality" approach to co-exist. We do the same sort of
thing for simple buses like I2C for example, where you can both write
a kernel driver or do bus transactions directly from user mode.
-Evan