2022-07-28 05:09:01

by Daniil Lunev

[permalink] [raw]
Subject: [PATCH] ufs: core: print capabilities in controller's sysfs node

Allows userspace to check supported by the controller/device
functionality, e.g. write booster.

Signed-off-by: Daniil Lunev <[email protected]>

---

drivers/ufs/core/ufs-sysfs.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index 0a088b47d5570..b0c294c367519 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -254,6 +254,14 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr,
return res < 0 ? res : count;
}

+static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct ufs_hba *hba = dev_get_drvdata(dev);
+
+ return sysfs_emit(buf, "0x%08llx\n", hba->caps);
+}
+
static DEVICE_ATTR_RW(rpm_lvl);
static DEVICE_ATTR_RO(rpm_target_dev_state);
static DEVICE_ATTR_RO(rpm_target_link_state);
@@ -262,6 +270,7 @@ static DEVICE_ATTR_RO(spm_target_dev_state);
static DEVICE_ATTR_RO(spm_target_link_state);
static DEVICE_ATTR_RW(auto_hibern8);
static DEVICE_ATTR_RW(wb_on);
+static DEVICE_ATTR_RO(caps);

static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
&dev_attr_rpm_lvl.attr,
@@ -272,6 +281,7 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
&dev_attr_spm_target_link_state.attr,
&dev_attr_auto_hibern8.attr,
&dev_attr_wb_on.attr,
+ &dev_attr_caps.attr,
NULL
};

--
2.31.0


2022-07-28 06:58:04

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] ufs: core: print capabilities in controller's sysfs node

On Thu, Jul 28, 2022 at 02:49:25PM +1000, Daniil Lunev wrote:
> Allows userspace to check supported by the controller/device
> functionality, e.g. write booster.
>
> Signed-off-by: Daniil Lunev <[email protected]>

You have to also document this in Documentation/ABI/ when you add a new
sysfs file. Please do so here.

thanks,

greg k-h

2022-07-28 13:53:28

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] ufs: core: print capabilities in controller's sysfs node

On 7/27/22 21:49, Daniil Lunev wrote:
> +static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "0x%08llx\n", hba->caps);
> +}

This code change includes all of the UFSHCD_CAP_* constants in the
kernel ABI. Is that really what we want? I'm wondering whether it
perhaps would be better only to export those capabilities to user space
that user space needs to know about.

Thanks,

Bart.

2022-07-28 23:12:54

by Daniil Lunev

[permalink] [raw]
Subject: Re: [PATCH] ufs: core: print capabilities in controller's sysfs node

On Thu, Jul 28, 2022 at 11:37 PM Bart Van Assche <[email protected]> wrote:
> This code change includes all of the UFSHCD_CAP_* constants in the
> kernel ABI. Is that really what we want? I'm wondering whether it
> perhaps would be better only to export those capabilities to user space
> that user space needs to know about.
Adding the filtering would introduce an extra maintenance burden and
will likely go out
of sync. I don't see harm in exposing all capabilities, both intrinsic
to the controller and
negotiated with the device. Do you see any scenario where that would be harmful?
--Daniil

2022-07-28 23:34:42

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] ufs: core: print capabilities in controller's sysfs node

On 7/28/22 16:08, Daniil Lunev wrote:
> On Thu, Jul 28, 2022 at 11:37 PM Bart Van Assche <[email protected]> wrote:
>> This code change includes all of the UFSHCD_CAP_* constants in the
>> kernel ABI. Is that really what we want? I'm wondering whether it
>> perhaps would be better only to export those capabilities to user space
>> that user space needs to know about.
> Adding the filtering would introduce an extra maintenance burden and
> will likely go out
> of sync. I don't see harm in exposing all capabilities, both intrinsic
> to the controller and
> negotiated with the device. Do you see any scenario where that would be harmful?

Please only export what you need instead of exporting all capabilities.

Thanks,

Bart.