[PATCH v3 0/2] Expose UFSHCD capabilities in sysfs.

[PATCH v3 0/2] Expose UFSHCD capabilities in sysfs.

[Daniil Lunev]

The patchset introduces new sysfs nodes, which userspace can check to
determine support for certain capabilities. Specifically the patchset
exposes Clock Scaling, Write Booster, and Inline Crypto Engine
capabilities.

Changes in v3:
* Expose each capability as an individual node
* Add preleminary CL to align checking for capabilities
* Modify documentation to represent new scheme

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

Daniil Lunev (2):
  ufs: add function to check CRYPTO capability
  ufs: core: print UFSHCD capabilities in controller's sysfs node

 Documentation/ABI/testing/sysfs-driver-ufs | 39 ++++++++++++++++++++
 drivers/ufs/core/ufs-sysfs.c               | 41 ++++++++++++++++++++++
 drivers/ufs/core/ufshcd-crypto.c           |  8 ++---
 drivers/ufs/host/ufs-mediatek.c            |  2 +-
 drivers/ufs/host/ufs-qcom-ice.c            |  4 +--
 drivers/ufs/host/ufshcd-pci.c              |  2 +-
 include/ufs/ufshcd.h                       |  5 +++
 7 files changed, 93 insertions(+), 8 deletions(-)

-- 
2.31.0

[PATCH v3 1/2] ufs: add function to check CRYPTO capability

[Daniil Lunev]

To align with other capability check functions.

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

 drivers/ufs/core/ufshcd-crypto.c | 8 ++++----
 drivers/ufs/host/ufs-mediatek.c  | 2 +-
 drivers/ufs/host/ufs-qcom-ice.c  | 4 ++--
 drivers/ufs/host/ufshcd-pci.c    | 2 +-
 include/ufs/ufshcd.h             | 5 +++++
 5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/core/ufshcd-crypto.c b/drivers/ufs/core/ufshcd-crypto.c
index 198360fe5e8e1..f819488bbde14 100644
--- a/drivers/ufs/core/ufshcd-crypto.c
+++ b/drivers/ufs/core/ufshcd-crypto.c
@@ -118,7 +118,7 @@ static int ufshcd_crypto_keyslot_evict(struct blk_crypto_profile *profile,
 
 bool ufshcd_crypto_enable(struct ufs_hba *hba)
 {
-	if (!(hba->caps & UFSHCD_CAP_CRYPTO))
+	if (!ufshcd_is_crypto_supported(hba))
 		return false;
 
 	/* Reset might clear all keys, so reprogram all the keys. */
@@ -165,7 +165,7 @@ int ufshcd_hba_init_crypto_capabilities(struct ufs_hba *hba)
 	 * hasn't advertised that crypto is supported.
 	 */
 	if (!(hba->capabilities & MASK_CRYPTO_SUPPORT) ||
-	    !(hba->caps & UFSHCD_CAP_CRYPTO))
+	    !ufshcd_is_crypto_supported(hba))
 		goto out;
 
 	hba->crypto_capabilities.reg_val =
@@ -225,7 +225,7 @@ void ufshcd_init_crypto(struct ufs_hba *hba)
 {
 	int slot;
 
-	if (!(hba->caps & UFSHCD_CAP_CRYPTO))
+	if (!ufshcd_is_crypto_supported(hba))
 		return;
 
 	/* Clear all keyslots - the number of keyslots is (CFGC + 1) */
@@ -235,6 +235,6 @@ void ufshcd_init_crypto(struct ufs_hba *hba)
 
 void ufshcd_crypto_register(struct ufs_hba *hba, struct request_queue *q)
 {
-	if (hba->caps & UFSHCD_CAP_CRYPTO)
+	if (ufshcd_is_crypto_supported(hba))
 		blk_crypto_register(&hba->crypto_profile, q);
 }
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index beabc3ccd30b3..4bdf6a709126d 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -182,7 +182,7 @@ static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba,
 			ufs_mtk_host_reset(hba);
 		}
 
-		if (hba->caps & UFSHCD_CAP_CRYPTO)
+		if (ufshcd_is_crypto_supported(hba))
 			ufs_mtk_crypto_enable(hba);
 
 		if (host->caps & UFS_MTK_CAP_DISABLE_AH8) {
diff --git a/drivers/ufs/host/ufs-qcom-ice.c b/drivers/ufs/host/ufs-qcom-ice.c
index 745e48ec598f8..180a015b6973d 100644
--- a/drivers/ufs/host/ufs-qcom-ice.c
+++ b/drivers/ufs/host/ufs-qcom-ice.c
@@ -161,7 +161,7 @@ static void qcom_ice_optimization_enable(struct ufs_qcom_host *host)
 
 int ufs_qcom_ice_enable(struct ufs_qcom_host *host)
 {
-	if (!(host->hba->caps & UFSHCD_CAP_CRYPTO))
+	if (!ufshcd_is_crypto_supported(host->hba))
 		return 0;
 	qcom_ice_low_power_mode_enable(host);
 	qcom_ice_optimization_enable(host);
@@ -189,7 +189,7 @@ int ufs_qcom_ice_resume(struct ufs_qcom_host *host)
 {
 	int err;
 
-	if (!(host->hba->caps & UFSHCD_CAP_CRYPTO))
+	if (!ufshcd_is_crypto_supported(host->hba))
 		return 0;
 
 	err = qcom_ice_wait_bist_status(host);
diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c
index 04166bda41daa..c06ccef348065 100644
--- a/drivers/ufs/host/ufshcd-pci.c
+++ b/drivers/ufs/host/ufshcd-pci.c
@@ -89,7 +89,7 @@ static int ufs_intel_hce_enable_notify(struct ufs_hba *hba,
 				       enum ufs_notify_change_status status)
 {
 	/* Cannot enable ICE until after HC enable */
-	if (status == POST_CHANGE && hba->caps & UFSHCD_CAP_CRYPTO) {
+	if (status == POST_CHANGE && ufshcd_is_crypto_supported(hba)) {
 		u32 hce = ufshcd_readl(hba, REG_CONTROLLER_ENABLE);
 
 		hce |= CRYPTO_GENERAL_ENABLE;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a92271421718e..ddbf470f8f455 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1005,6 +1005,11 @@ static inline bool ufshcd_is_wb_allowed(struct ufs_hba *hba)
 	return hba->caps & UFSHCD_CAP_WB_EN;
 }
 
+static inline bool ufshcd_is_crypto_supported(struct ufs_hba *hba)
+{
+	return hba->caps & UFSHCD_CAP_CRYPTO;
+}
+
 #define ufshcd_writel(hba, val, reg)	\
 	writel((val), (hba)->mmio_base + (reg))
 #define ufshcd_readl(hba, reg)	\
-- 
2.31.0

Re: [PATCH v3 1/2] ufs: add function to check CRYPTO capability

[Greg Kroah-Hartman]

On Fri, Jul 29, 2022 at 12:05:07PM +1000, Daniil Lunev wrote:
> To align with other capability check functions.

This is not a valid changelog text, sorry.  Please read the
documentation for how to write a valid one.

> Signed-off-by: Daniil Lunev <dlunev@chromium.org>
> 
>  drivers/ufs/core/ufshcd-crypto.c | 8 ++++----
>  drivers/ufs/host/ufs-mediatek.c  | 2 +-
>  drivers/ufs/host/ufs-qcom-ice.c  | 4 ++--
>  drivers/ufs/host/ufshcd-pci.c    | 2 +-
>  include/ufs/ufshcd.h             | 5 +++++
>  5 files changed, 13 insertions(+), 8 deletions(-)

Something went wrong with your patch, there is no --- line, so git will
apply it with the diffstat, right?

Did you hand-edit this?

thanks,

greg k-h

Re: [PATCH v3 1/2] ufs: add function to check CRYPTO capability

[Daniil Lunev]

> This is not a valid changelog text, sorry.  Please read the
> documentation for how to write a valid one.
Sorry for that. Though the latest patchset is v4 and that one
doesn't contain this patch since Eric Biggers pointed out the
crypto information can be obtained from device's queue sysfs
node. But I will try to be better further on.

> Something went wrong with your patch, there is no --- line, so git will
> apply it with the diffstat, right?
>
> Did you hand-edit this?

Hm, I don't recall hand editing it, but the possibility of pilot error is
always there. I am still getting used to the upstream tooling and
processes. Sorry for the inconvenience, but as I mentioned, this
patch is dropped in v4 version (and sorry for high traffic of changes
on this patchset)

Thanks,
Daniil

[PATCH v3 2/2] ufs: core: print UFSHCD capabilities in controller's sysfs node

[Daniil Lunev]

Allows userspace to check if Clock Scaling, Write Booster and Inline
Crypto Engine functionality can be enabled.

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

---

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 | 39 ++++++++++++++++++++
 drivers/ufs/core/ufs-sysfs.c               | 41 ++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 6b248abb1bd71..805d76f7d7aef 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -1591,6 +1591,45 @@ 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/bus/platform/drivers/ufshcd/*/capabilities/crypto
+What:		/sys/bus/platform/devices/*.ufs/capabilities/crypto
+Date:		July 2022
+Contact:	Daniil Lunev <dlunev@chromium.org>
+Description:	Indicates status of Inline Crypto Engine support.
+
+		== ============================
+		0  Inline Crypto Engine can not be used.
+		1  Inline Crypto Engine can be used.
+		== ============================
+
+		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..4a3e58d545fe5 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -279,6 +279,46 @@ 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 ssize_t crypto_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_crypto_supported(hba));
+}
+
+static DEVICE_ATTR_RO(clock_scaling);
+static DEVICE_ATTR_RO(write_booster);
+static DEVICE_ATTR_RO(crypto);
+
+static struct attribute *ufs_sysfs_capabilities_attrs[] = {
+	&dev_attr_clock_scaling.attr,
+	&dev_attr_write_booster.attr,
+	&dev_attr_crypto.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 +1174,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 v3 2/2] ufs: core: print UFSHCD capabilities in controller's sysfs node

[Eric Biggers]

On Fri, Jul 29, 2022 at 12:05:08PM +1000, Daniil Lunev wrote:
> +What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/crypto
> +What:		/sys/bus/platform/devices/*.ufs/capabilities/crypto
> +Date:		July 2022
> +Contact:	Daniil Lunev <dlunev@chromium.org>
> +Description:	Indicates status of Inline Crypto Engine support.
> +
> +		== ============================
> +		0  Inline Crypto Engine can not be used.
> +		1  Inline Crypto Engine can be used.
> +		== ============================
> +
> +		The file is read only.

Why is this needed when /sys/block/$disk/queue/crypto/ already exists and
provides more information?

- Eric

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

[Greg Kroah-Hartman]

On Fri, Jul 29, 2022 at 12:05:08PM +1000, Daniil Lunev wrote:
> Allows userspace to check if Clock Scaling, Write Booster and Inline
> Crypto Engine functionality can be enabled.
> 
> Signed-off-by: Daniil Lunev <dlunev@chromium.org>
> 
> ---
> 
> 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 | 39 ++++++++++++++++++++
>  drivers/ufs/core/ufs-sysfs.c               | 41 ++++++++++++++++++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
> index 6b248abb1bd71..805d76f7d7aef 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -1591,6 +1591,45 @@ Description:	This entry shows the status of HPB.
>  
>  		The file is read only.
>  
> +What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/clock_scaling

This shouldn't be linked to as a driver file, it's a device file.  So no
need for this line.

> +What:		/sys/bus/platform/devices/*.ufs/capabilities/clock_scaling

Since when are all ufs devices platform devices?  Do we not have UFS
controllers on other types of busses?

thanks,

greg k-h

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

[Daniil Lunev]

> >
> > +What:                /sys/bus/platform/drivers/ufshcd/*/capabilities/clock_scaling
>
> This shouldn't be linked to as a driver file, it's a device file.  So no
> need for this line.
>
> > +What:                /sys/bus/platform/devices/*.ufs/capabilities/clock_scaling
>
> Since when are all ufs devices platform devices?  Do we not have UFS
> controllers on other types of busses?

I have pretty much copped the structure of the entries across this file. Nearly
all of the entries link both device and driver paths and nearly all of
the entries
mention the platform-based path (which you correctly mentioned is not
factually correct, since we do have controllers on the pci bus). Please advise
if it is ok to keep it like this for consistency or what would be the
appropriate
way to adjust the documentation?
--Daniil

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

[Greg Kroah-Hartman]

On Fri, Jul 29, 2022 at 06:29:45PM +1000, Daniil Lunev wrote:
> > >
> > > +What:                /sys/bus/platform/drivers/ufshcd/*/capabilities/clock_scaling
> >
> > This shouldn't be linked to as a driver file, it's a device file.  So no
> > need for this line.
> >
> > > +What:                /sys/bus/platform/devices/*.ufs/capabilities/clock_scaling
> >
> > Since when are all ufs devices platform devices?  Do we not have UFS
> > controllers on other types of busses?
> 
> I have pretty much copped the structure of the entries across this file. Nearly
> all of the entries link both device and driver paths and nearly all of
> the entries
> mention the platform-based path (which you correctly mentioned is not
> factually correct, since we do have controllers on the pci bus). Please advise
> if it is ok to keep it like this for consistency or what would be the
> appropriate
> way to adjust the documentation?

Ah, ok, that's odd.  Let's just leave this as-is for now, hopefully
someone else cleans this up later.

thanks,

greg k-h