diff mbox series

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

Message ID 20220803074955.v6.1.Ibf9efc9be50783eeee55befa2270b7d38552354c@changeid (mailing list archive)
State Superseded
Headers show
Series [v6] ufs: core: print UFSHCD capabilities in controller's sysfs node | expand

Commit Message

Daniil Lunev Aug. 2, 2022, 9:50 p.m. UTC
Allows userspace to check if Clock Scaling and Write Booster are
supported.

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

---

Changes in v6:
* Add comment to clarify meaning of the "capbilities" sysfs group.

Changes in v5:
* Correct wording for clock scaling.
* Correct wording for the commit message.

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               | 37 ++++++++++++++++++++++
 2 files changed, 63 insertions(+)

Comments

Greg Kroah-Hartman Aug. 3, 2022, 6:12 a.m. UTC | #1
On Wed, Aug 03, 2022 at 07:50:07AM +1000, Daniil Lunev wrote:
> Allows userspace to check if Clock Scaling and Write Booster are
> supported.
> 
> Signed-off-by: Daniil Lunev <dlunev@chromium.org>
> 
> ---
> 
> Changes in v6:
> * Add comment to clarify meaning of the "capbilities" sysfs group.
> 
> Changes in v5:
> * Correct wording for clock scaling.
> * Correct wording for the commit message.
> 
> 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               | 37 ++++++++++++++++++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
> index 6b248abb1bd71..1750a9b84ce0f 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 supported.
> +		1  Clock scaling is supported.
> +		== ============================
> +
> +		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..4149cdf19de92 100644
> --- a/drivers/ufs/core/ufs-sysfs.c
> +++ b/drivers/ufs/core/ufs-sysfs.c
> @@ -279,6 +279,42 @@ 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);
> +
> +/*
> + * The "capabilities" sysfs group represents the effective capabilities of the
> + * host-device pair, i.e. the capabilities which are enabled in the driver for
> + * the specific host controller, supported by the host controller and are
> + * supported and/or have compatible configuration on the device side.

Why is this information not also in the Documentation/ABI/ entries as
well?

thanks,

greg k-h
Daniil Lunev Aug. 3, 2022, 6:34 a.m. UTC | #2
> Why is this information not also in the Documentation/ABI/ entries as
> well?
How would you want me to word it in there? For each entry? There
will be more in the future, would we need to just copy it as a
boilerplate? Bart suggested it be added in the code, do you see
a good way to mention the same in the doc?
Thanks,
--Daniil
Daniil Lunev Aug. 9, 2022, 9:16 p.m. UTC | #3
Hello,

Are there any more amendments you would want me to make or is it good to go?

Thanks,
Daniil


On Wed, Aug 3, 2022 at 4:34 PM Daniil Lunev <dlunev@chromium.org> wrote:
>
> > Why is this information not also in the Documentation/ABI/ entries as
> > well?
> How would you want me to word it in there? For each entry? There
> will be more in the future, would we need to just copy it as a
> boilerplate? Bart suggested it be added in the code, do you see
> a good way to mention the same in the doc?
> Thanks,
> --Daniil
Greg Kroah-Hartman Aug. 10, 2022, 5:03 a.m. UTC | #4
A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?


http://daringfireball.net/2007/07/on_top

On Wed, Aug 10, 2022 at 07:16:34AM +1000, Daniil Lunev wrote:
> Hello,
> 
> Are there any more amendments you would want me to make or is it good to go?

I have no context here at all, sorry, nor do I have any idea what you
are expecting from us :(

Also, it's the middle of the merge window, we can't do anything right
now.  Resend your patch series after -rc1 is out and we can review it
then.

thanks,

greg k-h
Eric Biggers Aug. 11, 2022, 6:19 a.m. UTC | #5
On Wed, Aug 03, 2022 at 04:34:55PM +1000, Daniil Lunev wrote:
> > > +
> > > +/*
> > > + * The "capabilities" sysfs group represents the effective capabilities of the
> > > + * host-device pair, i.e. the capabilities which are enabled in the driver for
> > > + * the specific host controller, supported by the host controller and are
> > > + * supported and/or have compatible configuration on the device side.
> >
> > Why is this information not also in the Documentation/ABI/ entries as
> > well?
> >
> How would you want me to word it in there? For each entry? There
> will be more in the future, would we need to just copy it as a
> boilerplate? Bart suggested it be added in the code, do you see
> a good way to mention the same in the doc?
> Thanks,
> --Daniil

This is describing the directory, so please include it in a documentation entry
for the directory itself (/sys/bus/platform/drivers/ufshcd/*/capabilities/) just
above the documentation entries for the files in the directory.

- Eric
Daniil Lunev Aug. 11, 2022, 7:32 p.m. UTC | #6
Hi Eric,
> This is describing the directory, so please include it in a documentation entry
> for the directory itself (/sys/bus/platform/drivers/ufshcd/*/capabilities/) just
> above the documentation entries for the files in the directory.

Should I keep the comment in this file as well, or remove it here when
adding to the documentation?

--Daniil
Eric Biggers Aug. 11, 2022, 7:36 p.m. UTC | #7
On Fri, Aug 12, 2022 at 05:32:34AM +1000, Daniil Lunev wrote:
> Hi Eric,
> > This is describing the directory, so please include it in a documentation entry
> > for the directory itself (/sys/bus/platform/drivers/ufshcd/*/capabilities/) just
> > above the documentation entries for the files in the directory.
> 
> Should I keep the comment in this file as well, or remove it here when
> adding to the documentation?
> 
> --Daniil

Avoiding redundancy is nice.  Maybe make the code just refer to the
documentation?

- Eric
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 6b248abb1bd71..1750a9b84ce0f 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 supported.
+		1  Clock scaling is supported.
+		== ============================
+
+		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..4149cdf19de92 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -279,6 +279,42 @@  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);
+
+/*
+ * The "capabilities" sysfs group represents the effective capabilities of the
+ * host-device pair, i.e. the capabilities which are enabled in the driver for
+ * the specific host controller, supported by the host controller and are
+ * supported and/or have compatible configuration on the device side.
+ */
+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 +1170,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,