[PATCH v4] ufs: core: print UFSHCD capabilities in controller's sysfs node

[PATCH v4] ufs: core: print UFSHCD capabilities in controller's sysfs node

[Daniil Lunev]

Allows userspace to check if Clock Scaling, Write Booster functionality
status.

Signed-off-by: Daniil Lunev <dlunev@chromium.org>

---

Changes in v4:
* Dropped crypto node per Eric Biggers mentioning it can be queried from
  disk's queue node

Changes in v3:
* Expose each capability individually.
* Update documentation to represent new scheme.

Changes in v2:
* Add documentation entry for the new sysfs node.

 Documentation/ABI/testing/sysfs-driver-ufs | 26 ++++++++++++++++++
 drivers/ufs/core/ufs-sysfs.c               | 31 ++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 6b248abb1bd71..ddc405f87786f 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -1591,6 +1591,32 @@ Description:	This entry shows the status of HPB.
 
 		The file is read only.
 
+What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/clock_scaling
+What:		/sys/bus/platform/devices/*.ufs/capabilities/clock_scaling
+Date:		July 2022
+Contact:	Daniil Lunev <dlunev@chromium.org>
+Description:	Indicates status of clock scaling.
+
+		== ============================
+		0  Clock scaling is not enabled.
+		1  Clock scaling is enabled.
+		== ============================
+
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/write_booster
+What:		/sys/bus/platform/devices/*.ufs/capabilities/write_booster
+Date:		July 2022
+Contact:	Daniil Lunev <dlunev@chromium.org>
+Description:	Indicates status of Write Booster.
+
+		== ============================
+		0  Write Booster can not be enabled.
+		1  Write Booster can be enabled.
+		== ============================
+
+		The file is read only.
+
 What:		/sys/class/scsi_device/*/device/hpb_param_sysfs/activation_thld
 Date:		February 2021
 Contact:	Avri Altman <avri.altman@wdc.com>
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 0a088b47d5570..5c53349337dd8 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -279,6 +279,36 @@ static const struct attribute_group ufs_sysfs_default_group = {
 	.attrs = ufs_sysfs_ufshcd_attrs,
 };
 
+static ssize_t clock_scaling_show(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", ufshcd_is_clkscaling_supported(hba));
+}
+
+static ssize_t write_booster_show(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", ufshcd_is_wb_allowed(hba));
+}
+
+static DEVICE_ATTR_RO(clock_scaling);
+static DEVICE_ATTR_RO(write_booster);
+
+static struct attribute *ufs_sysfs_capabilities_attrs[] = {
+	&dev_attr_clock_scaling.attr,
+	&dev_attr_write_booster.attr,
+	NULL
+};
+
+static const struct attribute_group ufs_sysfs_capabilities_group = {
+	.name = "capabilities",
+	.attrs = ufs_sysfs_capabilities_attrs,
+};
+
 static ssize_t monitor_enable_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
@@ -1134,6 +1164,7 @@ static const struct attribute_group ufs_sysfs_attributes_group = {
 
 static const struct attribute_group *ufs_sysfs_groups[] = {
 	&ufs_sysfs_default_group,
+	&ufs_sysfs_capabilities_group,
 	&ufs_sysfs_monitor_group,
 	&ufs_sysfs_device_descriptor_group,
 	&ufs_sysfs_interconnect_descriptor_group,
-- 
2.31.0

Re: [PATCH v4] ufs: core: print UFSHCD capabilities in controller's sysfs node

[Bart Van Assche]

On 7/28/22 22:13, Daniil Lunev wrote:
> Allows userspace to check if Clock Scaling, Write Booster functionality
> status.

The above sentence is not complete. Did you perhaps want to write "are 
supported by the host controller" instead of "status"?

> +What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/clock_scaling
> +What:		/sys/bus/platform/devices/*.ufs/capabilities/clock_scaling
> +Date:		July 2022
> +Contact:	Daniil Lunev <dlunev@chromium.org>
> +Description:	Indicates status of clock scaling.
> +
> +		== ============================
> +		0  Clock scaling is not enabled.
> +		1  Clock scaling is enabled.
> +		== ============================
> +
> +		The file is read only.

I don't think the above documentation is correct. My understanding is 
that the UFSHCD_CAP_CLK_SCALING flag indicates whether or not the host 
controller supports clock scaling. It does not indicate whether or not 
clock scaling is enabled.

> +What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/write_booster
> +What:		/sys/bus/platform/devices/*.ufs/capabilities/write_booster
> +Date:		July 2022
> +Contact:	Daniil Lunev <dlunev@chromium.org>
> +Description:	Indicates status of Write Booster.
> +
> +		== ============================
> +		0  Write Booster can not be enabled.
> +		1  Write Booster can be enabled.
> +		== ============================
> +
> +		The file is read only.

Please change "can not / can be enabled" into "is not supported by the 
host controller / is supported by the host controller".

Thanks,

Bart.

Re: [PATCH v4] ufs: core: print UFSHCD capabilities in controller's sysfs node

[Daniil Lunev]

HI Bart
> The above sentence is not complete. Did you perhaps want to write "are
> supported by the host controller" instead of "status"?
Will fix it in the next version.

> I don't think the above documentation is correct. My understanding is
> that the UFSHCD_CAP_CLK_SCALING flag indicates whether or not the host
> controller supports clock scaling. It does not indicate whether or not
> clock scaling is enabled.
Ah, right, there is a control for it in sysfs, it is just hidden in
ufshcd.c I was under
a wrong impression that it is like the writebooster capability that
has quite a bit of
conditions for staying present. WIll fix it in the next version.

> Please change "can not / can be enabled" into "is not supported by the
> host controller / is supported by the host controller".
That would be incorrect. The "caps" variable semantics is a bit weird
in the sense
that it is used at times to convey "active" capabilities, not just
supported one. For
example, for the writebooster capability to be present in caps, first
controller driver
should indicate it is ready to support it, then the part that is
attached to the host
controller has to indicate support in the device descriptor, then WB has to be
configured and its lifetime should not be exhausted. If any of those parameters
are not satisfied, the capability will be removed from the set despite generally
being supported. I am not sure how to properly word it, but just
saying "controller
supports it" would becounter-factual (especially since the controller
doesn't really
knows anything about writebooster per-ce, it is part's functionality).
What would
be suggested wording in that case?

--Daniil

Re: [PATCH v4] ufs: core: print UFSHCD capabilities in controller's sysfs node

[Bart Van Assche]

On 8/1/22 14:29, Daniil Lunev wrote:
>> Please change "can not / can be enabled" into "is not supported by the
>> host controller / is supported by the host controller".
>
> That would be incorrect. The "caps" variable semantics is a bit weird
> in the sense that it is used at times to convey "active" 
> capabilities, not just supported one. For example, for the 
> writebooster capability to be present in caps, first controller 
> driver should indicate it is ready to support it, then the part that 
> is attached to the host controller has to indicate support in the 
> device descriptor, then WB has to be configured and its lifetime
> should not be exhausted. If any of those parameters are not
> satisfied, the capability will be removed from the set despite 
> generally being supported. I am not sure how to properly word it, but
> just saying "controller supports it" would be counter-factual
> (especially since the controller doesn't really knows anything about
> writebooster per-se, it is part's functionality). What would be
> suggested wording in that case?
Given the above I think we can keep the current wording. This also makes 
me wonder why the UFSHCD_CAP_WB_EN flag occurs in the hba->caps member 
variable. That member variable is used to track controller capabilities. 
My understanding is that the WriteBooster functionality is a UFS device 
feature and also that no host controller support is required to control 
the WriteBooster feature.

Thanks,

Bart.