* [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas
@ 2024-03-02 20:16 Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers Igor Pylypiv
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
This patch series adds sas_ncq_prio_supported and sas_ncq_prio_enable
sysfs sttributes for libsas managed SATA devices. Existing libata sysfs
attributes cannot be used directly because the ata_port location is
different for libsas.
Changes since v2:
- Added libsas SATA sysfs attributes to aic94xx and isci.
Changes since v1:
- Dropped the "sas_" prefix to align sysfs sttributes naming with AHCI.
- Dropped ternary operators to make the code more readable.
- Corrected the formatting %u -> %d in sysfs_emit().
- Changed kstrtol() to kstrtobool() in [ata|sas]_ncq_prio_enable_store().
- Changed comments to use the "/* */" style instead of "//".
- Added libsas SATA sysfs attributes to mvsas and hisi_sas.
- Dropped the 'Reviewed-by' tags because they were not sent in-reply
to the patch emails.
Igor Pylypiv (7):
ata: libata-sata: Factor out NCQ Priority configuration helpers
scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices
scsi: pm80xx: Add libsas SATA sysfs attributes group
scsi: mvsas: Add libsas SATA sysfs attributes group
scsi: hisi_sas: Add libsas SATA sysfs attributes group
scsi: aic94xx: Add libsas SATA sysfs attributes group
scsi: isci: Add libsas SATA sysfs attributes group
drivers/ata/libata-sata.c | 139 ++++++++++++++++++-------
drivers/scsi/aic94xx/aic94xx_init.c | 8 ++
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 6 ++
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 6 ++
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 6 ++
drivers/scsi/isci/init.c | 6 ++
drivers/scsi/libsas/sas_ata.c | 92 ++++++++++++++++
drivers/scsi/mvsas/mv_init.c | 7 ++
drivers/scsi/pm8001/pm8001_ctl.c | 5 +
drivers/scsi/pm8001/pm8001_init.c | 1 +
drivers/scsi/pm8001/pm8001_sas.h | 1 +
include/linux/libata.h | 4 +
include/scsi/sas_ata.h | 6 ++
13 files changed, 247 insertions(+), 40 deletions(-)
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:34 ` John Garry
2024-03-02 20:16 ` [PATCH v3 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices Igor Pylypiv
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
Export libata NCQ Priority configuration helpers to be reused
for libsas managed SATA devices.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/ata/libata-sata.c | 139 +++++++++++++++++++++++++++-----------
include/linux/libata.h | 4 ++
2 files changed, 103 insertions(+), 40 deletions(-)
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 0fb1934875f2..a8d5e36d5211 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -848,29 +848,73 @@ DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
ata_scsi_lpm_show, ata_scsi_lpm_store);
EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
+/**
+ * ata_ncq_prio_supported - Check if device supports NCQ Priority
+ * @ap: ATA port of the target device
+ * @sdev: SCSI device
+ *
+ * Helper to check if device supports NCQ Priority feature,
+ * usable with both libsas and libata.
+ */
+int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev)
+{
+ struct ata_device *dev;
+ unsigned long flags;
+ int rc;
+
+ spin_lock_irqsave(ap->lock, flags);
+ dev = ata_scsi_find_dev(ap, sdev);
+ if (!dev)
+ rc = -ENODEV;
+ else
+ rc = !!(dev->flags & ATA_DFLAG_NCQ_PRIO);
+ spin_unlock_irqrestore(ap->lock, flags);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(ata_ncq_prio_supported);
+
static ssize_t ata_ncq_prio_supported_show(struct device *device,
struct device_attribute *attr,
char *buf)
{
struct scsi_device *sdev = to_scsi_device(device);
struct ata_port *ap = ata_shost_to_port(sdev->host);
+ int rc;
+
+ rc = ata_ncq_prio_supported(ap, sdev);
+ if (rc < 0)
+ return rc;
+
+ return sysfs_emit(buf, "%d\n", rc);
+}
+
+DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
+EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
+
+/**
+ * ata_ncq_prio_enabled - Check if NCQ Priority is enabled
+ * @ap: ATA port of the target device
+ * @sdev: SCSI device
+ *
+ * Helper to check if NCQ Priority feature is enabled,
+ * usable with both libsas and libata.
+ */
+int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev)
+{
struct ata_device *dev;
- bool ncq_prio_supported;
- int rc = 0;
+ unsigned long flags;
+ int rc;
- spin_lock_irq(ap->lock);
+ spin_lock_irqsave(ap->lock, flags);
dev = ata_scsi_find_dev(ap, sdev);
if (!dev)
rc = -ENODEV;
else
- ncq_prio_supported = dev->flags & ATA_DFLAG_NCQ_PRIO;
- spin_unlock_irq(ap->lock);
-
- return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_supported);
+ rc = !!(dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED);
+ spin_unlock_irqrestore(ap->lock, flags);
+ return rc;
}
-
-DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
-EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
+EXPORT_SYMBOL_GPL(ata_ncq_prio_enabled);
static ssize_t ata_ncq_prio_enable_show(struct device *device,
struct device_attribute *attr,
@@ -878,50 +922,45 @@ static ssize_t ata_ncq_prio_enable_show(struct device *device,
{
struct scsi_device *sdev = to_scsi_device(device);
struct ata_port *ap = ata_shost_to_port(sdev->host);
- struct ata_device *dev;
- bool ncq_prio_enable;
- int rc = 0;
+ int rc;
- spin_lock_irq(ap->lock);
- dev = ata_scsi_find_dev(ap, sdev);
- if (!dev)
- rc = -ENODEV;
- else
- ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED;
- spin_unlock_irq(ap->lock);
+ rc = ata_ncq_prio_enabled(ap, sdev);
+ if (rc < 0)
+ return rc;
- return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_enable);
+ return sysfs_emit(buf, "%d\n", rc);
}
-static ssize_t ata_ncq_prio_enable_store(struct device *device,
- struct device_attribute *attr,
- const char *buf, size_t len)
+/**
+ * ata_ncq_prio_enable - Enable/disable NCQ Priority
+ * @ap: ATA port of the target device
+ * @sdev: SCSI device
+ * @enable: true - enable NCQ Priority, false - disable NCQ Priority
+ *
+ * Helper to enable/disable NCQ Priority feature, usable with both
+ * libsas and libata.
+ */
+int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
+ bool enable)
{
- struct scsi_device *sdev = to_scsi_device(device);
- struct ata_port *ap;
struct ata_device *dev;
- long int input;
+ unsigned long flags;
int rc = 0;
- rc = kstrtol(buf, 10, &input);
- if (rc)
- return rc;
- if ((input < 0) || (input > 1))
- return -EINVAL;
+ spin_lock_irqsave(ap->lock, flags);
- ap = ata_shost_to_port(sdev->host);
dev = ata_scsi_find_dev(ap, sdev);
- if (unlikely(!dev))
- return -ENODEV;
-
- spin_lock_irq(ap->lock);
+ if (unlikely(!dev)) {
+ rc = -ENODEV;
+ goto unlock;
+ }
if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
rc = -EINVAL;
goto unlock;
}
- if (input) {
+ if (enable) {
if (dev->flags & ATA_DFLAG_CDL_ENABLED) {
ata_dev_err(dev,
"CDL must be disabled to enable NCQ priority\n");
@@ -934,9 +973,29 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device,
}
unlock:
- spin_unlock_irq(ap->lock);
+ spin_unlock_irqrestore(ap->lock, flags);
+ return rc;
+}
+EXPORT_SYMBOL_GPL(ata_ncq_prio_enable);
+
+static ssize_t ata_ncq_prio_enable_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct scsi_device *sdev = to_scsi_device(device);
+ struct ata_port *ap = ata_shost_to_port(sdev->host);
+ bool enable;
+ int rc;
+
+ rc = kstrtobool(buf, &enable);
+ if (rc)
+ return rc;
+
+ rc = ata_ncq_prio_enable(ap, sdev, enable);
+ if (rc)
+ return rc;
- return rc ? rc : len;
+ return len;
}
DEVICE_ATTR(ncq_prio_enable, S_IRUGO | S_IWUSR,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 26d68115afb8..f3ff2bf3ec6b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1157,6 +1157,10 @@ extern int ata_scsi_change_queue_depth(struct scsi_device *sdev,
int queue_depth);
extern int ata_change_queue_depth(struct ata_port *ap, struct scsi_device *sdev,
int queue_depth);
+extern int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev);
+extern int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev);
+extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
+ bool enable);
extern struct ata_device *ata_dev_pair(struct ata_device *adev);
extern int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:22 ` John Garry
2024-03-02 20:16 ` [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group Igor Pylypiv
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
Libata sysfs attributes cannot be used for libsas managed SATA devices
because the ata_port location is different for libsas.
Defined sysfs attributes (visible for SATA devices only):
- /sys/block/sda/device/ncq_prio_enable
- /sys/block/sda/device/ncq_prio_supported
The newly defined attributes will pass the correct ata_port to libata
helper functions.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/scsi/libsas/sas_ata.c | 92 +++++++++++++++++++++++++++++++++++
include/scsi/sas_ata.h | 6 +++
2 files changed, 98 insertions(+)
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 12e2653846e3..e4d07134a0e4 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -964,3 +964,95 @@ int sas_execute_ata_cmd(struct domain_device *device, u8 *fis, int force_phy_id)
force_phy_id, &tmf_task);
}
EXPORT_SYMBOL_GPL(sas_execute_ata_cmd);
+
+static ssize_t sas_ncq_prio_supported_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct scsi_device *sdev = to_scsi_device(device);
+ struct domain_device *ddev = sdev_to_domain_dev(sdev);
+ int rc;
+
+ /* This attribute shall be visible for SATA devices only */
+ if (WARN_ON(!dev_is_sata(ddev)))
+ return -EINVAL;
+
+ rc = ata_ncq_prio_supported(ddev->sata_dev.ap, sdev);
+ if (rc < 0)
+ return rc;
+
+ return sysfs_emit(buf, "%d\n", rc);
+}
+
+DEVICE_ATTR(ncq_prio_supported, S_IRUGO, sas_ncq_prio_supported_show, NULL);
+
+static ssize_t sas_ncq_prio_enable_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct scsi_device *sdev = to_scsi_device(device);
+ struct domain_device *ddev = sdev_to_domain_dev(sdev);
+ int rc;
+
+ /* This attribute shall be visible for SATA devices only */
+ if (WARN_ON(!dev_is_sata(ddev)))
+ return -EINVAL;
+
+ rc = ata_ncq_prio_enabled(ddev->sata_dev.ap, sdev);
+ if (rc < 0)
+ return rc;
+
+ return sysfs_emit(buf, "%d\n", rc);
+}
+
+static ssize_t sas_ncq_prio_enable_store(struct device *device,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct scsi_device *sdev = to_scsi_device(device);
+ struct domain_device *ddev = sdev_to_domain_dev(sdev);
+ bool enable;
+ int rc;
+
+ /* This attribute shall be visible for SATA devices only */
+ if (WARN_ON(!dev_is_sata(ddev)))
+ return -EINVAL;
+
+ rc = kstrtobool(buf, &enable);
+ if (rc)
+ return rc;
+
+ rc = ata_ncq_prio_enable(ddev->sata_dev.ap, sdev, enable);
+ if (rc)
+ return rc;
+
+ return len;
+}
+
+DEVICE_ATTR(ncq_prio_enable, S_IRUGO | S_IWUSR,
+ sas_ncq_prio_enable_show, sas_ncq_prio_enable_store);
+
+static struct attribute *sas_ata_sdev_attrs[] = {
+ &dev_attr_ncq_prio_supported.attr,
+ &dev_attr_ncq_prio_enable.attr,
+ NULL
+};
+
+static umode_t sas_ata_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int i)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct domain_device *ddev = sdev_to_domain_dev(sdev);
+
+ if (!dev_is_sata(ddev))
+ return 0;
+
+ return attr->mode;
+}
+
+const struct attribute_group sas_ata_sdev_attr_group = {
+ .attrs = sas_ata_sdev_attrs,
+ .is_visible = sas_ata_attr_is_visible,
+};
+EXPORT_SYMBOL_GPL(sas_ata_sdev_attr_group);
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 2f8c719840a6..cded782fdf33 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -39,6 +39,8 @@ int smp_ata_check_ready_type(struct ata_link *link);
int sas_discover_sata(struct domain_device *dev);
int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *phy,
struct domain_device *child, int phy_id);
+
+extern const struct attribute_group sas_ata_sdev_attr_group;
#else
static inline void sas_ata_disabled_notice(void)
@@ -123,6 +125,10 @@ static inline int sas_ata_add_dev(struct domain_device *parent, struct ex_phy *p
sas_ata_disabled_notice();
return -ENODEV;
}
+
+static const struct attribute_group sas_ata_sdev_attr_group = {
+ .attrs = NULL,
+};
#endif
#endif /* _SAS_ATA_H_ */
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:16 ` John Garry
2024-03-04 8:43 ` Jinpu Wang
2024-03-02 20:16 ` [PATCH v3 4/7] scsi: mvsas: " Igor Pylypiv
` (3 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
The added sysfs attributes group enables the configuration of NCQ Priority
feature for HBAs that rely on libsas to manage SATA devices.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/scsi/pm8001/pm8001_ctl.c | 5 +++++
drivers/scsi/pm8001/pm8001_init.c | 1 +
drivers/scsi/pm8001/pm8001_sas.h | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index 5c26a13ffbd2..9ffe1a868d0f 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -1039,3 +1039,8 @@ const struct attribute_group *pm8001_host_groups[] = {
&pm8001_host_attr_group,
NULL
};
+
+const struct attribute_group *pm8001_sdev_groups[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index ed6b7d954dda..e6b1108f6117 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -134,6 +134,7 @@ static const struct scsi_host_template pm8001_sht = {
.compat_ioctl = sas_ioctl,
#endif
.shost_groups = pm8001_host_groups,
+ .sdev_groups = pm8001_sdev_groups,
.track_queue_depth = 1,
.cmd_per_lun = 32,
.map_queues = pm8001_map_queues,
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 3ccb7371902f..ced6721380a8 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -717,6 +717,7 @@ int pm80xx_fatal_errors(struct pm8001_hba_info *pm8001_ha);
void pm8001_free_dev(struct pm8001_device *pm8001_dev);
/* ctl shared API */
extern const struct attribute_group *pm8001_host_groups[];
+extern const struct attribute_group *pm8001_sdev_groups[];
#define PM8001_INVALID_TAG ((u32)-1)
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 4/7] scsi: mvsas: Add libsas SATA sysfs attributes group
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
` (2 preceding siblings ...)
2024-03-02 20:16 ` [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:16 ` John Garry
2024-03-02 20:16 ` [PATCH v3 5/7] scsi: hisi_sas: " Igor Pylypiv
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
The added sysfs attributes group enables the configuration of NCQ Priority
feature for HBAs that rely on libsas to manage SATA devices.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/scsi/mvsas/mv_init.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 43ebb331e216..f1090bb5f2c9 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -26,6 +26,7 @@ static const struct mvs_chip_info mvs_chips[] = {
};
static const struct attribute_group *mvst_host_groups[];
+static const struct attribute_group *mvst_sdev_groups[];
#define SOC_SAS_NUM 2
@@ -53,6 +54,7 @@ static const struct scsi_host_template mvs_sht = {
.compat_ioctl = sas_ioctl,
#endif
.shost_groups = mvst_host_groups,
+ .sdev_groups = mvst_sdev_groups,
.track_queue_depth = 1,
};
@@ -779,6 +781,11 @@ static struct attribute *mvst_host_attrs[] = {
ATTRIBUTE_GROUPS(mvst_host);
+static const struct attribute_group *mvst_sdev_groups[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
+
module_init(mvs_init);
module_exit(mvs_exit);
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 5/7] scsi: hisi_sas: Add libsas SATA sysfs attributes group
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
` (3 preceding siblings ...)
2024-03-02 20:16 ` [PATCH v3 4/7] scsi: mvsas: " Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:18 ` John Garry
2024-03-02 20:16 ` [PATCH v3 6/7] scsi: aic94xx: " Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 7/7] scsi: isci: " Igor Pylypiv
6 siblings, 1 reply; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
The added sysfs attributes group enables the configuration of NCQ Priority
feature for HBAs that rely on libsas to manage SATA devices.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 6 ++++++
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 6 ++++++
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 6 ++++++
3 files changed, 18 insertions(+)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3c555579f9a1..97864b56a71f 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1734,6 +1734,11 @@ static struct attribute *host_v1_hw_attrs[] = {
ATTRIBUTE_GROUPS(host_v1_hw);
+static const struct attribute_group *sdev_groups_v1_hw[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
+
static const struct scsi_host_template sht_v1_hw = {
.name = DRV_NAME,
.proc_name = DRV_NAME,
@@ -1758,6 +1763,7 @@ static const struct scsi_host_template sht_v1_hw = {
.compat_ioctl = sas_ioctl,
#endif
.shost_groups = host_v1_hw_groups,
+ .sdev_groups = sdev_groups_v1_hw,
.host_reset = hisi_sas_host_reset,
};
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 73b378837da7..b5d379ebe05d 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -3544,6 +3544,11 @@ static struct attribute *host_v2_hw_attrs[] = {
ATTRIBUTE_GROUPS(host_v2_hw);
+static const struct attribute_group *sdev_groups_v2_hw[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
+
static void map_queues_v2_hw(struct Scsi_Host *shost)
{
struct hisi_hba *hisi_hba = shost_priv(shost);
@@ -3585,6 +3590,7 @@ static const struct scsi_host_template sht_v2_hw = {
.compat_ioctl = sas_ioctl,
#endif
.shost_groups = host_v2_hw_groups,
+ .sdev_groups = sdev_groups_v2_hw,
.host_reset = hisi_sas_host_reset,
.map_queues = map_queues_v2_hw,
.host_tagset = 1,
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index b56fbc61a15a..9b69ea16a1e6 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2929,6 +2929,11 @@ static struct attribute *host_v3_hw_attrs[] = {
ATTRIBUTE_GROUPS(host_v3_hw);
+static const struct attribute_group *sdev_groups_v3_hw[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
+
#define HISI_SAS_DEBUGFS_REG(x) {#x, x}
struct hisi_sas_debugfs_reg_lu {
@@ -3340,6 +3345,7 @@ static const struct scsi_host_template sht_v3_hw = {
.compat_ioctl = sas_ioctl,
#endif
.shost_groups = host_v3_hw_groups,
+ .sdev_groups = sdev_groups_v3_hw,
.tag_alloc_policy = BLK_TAG_ALLOC_RR,
.host_reset = hisi_sas_host_reset,
.host_tagset = 1,
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 6/7] scsi: aic94xx: Add libsas SATA sysfs attributes group
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
` (4 preceding siblings ...)
2024-03-02 20:16 ` [PATCH v3 5/7] scsi: hisi_sas: " Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:18 ` John Garry
2024-03-02 20:16 ` [PATCH v3 7/7] scsi: isci: " Igor Pylypiv
6 siblings, 1 reply; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
The added sysfs attributes group enables the configuration of NCQ Priority
feature for HBAs that rely on libsas to manage SATA devices.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/scsi/aic94xx/aic94xx_init.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 8a3340d8d7ad..ccccd0eb6275 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -14,6 +14,7 @@
#include <linux/firmware.h>
#include <linux/slab.h>
+#include <scsi/sas_ata.h>
#include <scsi/scsi_host.h>
#include "aic94xx.h"
@@ -34,6 +35,7 @@ MODULE_PARM_DESC(use_msi, "\n"
static struct scsi_transport_template *aic94xx_transport_template;
static int asd_scan_finished(struct Scsi_Host *, unsigned long);
static void asd_scan_start(struct Scsi_Host *);
+static const struct attribute_group *asd_sdev_groups[];
static const struct scsi_host_template aic94xx_sht = {
.module = THIS_MODULE,
@@ -60,6 +62,7 @@ static const struct scsi_host_template aic94xx_sht = {
.compat_ioctl = sas_ioctl,
#endif
.track_queue_depth = 1,
+ .sdev_groups = asd_sdev_groups,
};
static int asd_map_memio(struct asd_ha_struct *asd_ha)
@@ -951,6 +954,11 @@ static void asd_remove_driver_attrs(struct device_driver *driver)
driver_remove_file(driver, &driver_attr_version);
}
+static const struct attribute_group *asd_sdev_groups[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
+
static struct sas_domain_function_template aic94xx_transport_functions = {
.lldd_dev_found = asd_dev_found,
.lldd_dev_gone = asd_dev_gone,
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v3 7/7] scsi: isci: Add libsas SATA sysfs attributes group
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
` (5 preceding siblings ...)
2024-03-02 20:16 ` [PATCH v3 6/7] scsi: aic94xx: " Igor Pylypiv
@ 2024-03-02 20:16 ` Igor Pylypiv
2024-03-04 8:18 ` John Garry
6 siblings, 1 reply; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-02 20:16 UTC (permalink / raw
To: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel, Igor Pylypiv
The added sysfs attributes group enables the configuration of NCQ Priority
feature for HBAs that rely on libsas to manage SATA devices.
Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
drivers/scsi/isci/init.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 6277162a028b..8658dcd61b87 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -149,6 +149,11 @@ static struct attribute *isci_host_attrs[] = {
ATTRIBUTE_GROUPS(isci_host);
+static const struct attribute_group *isci_sdev_groups[] = {
+ &sas_ata_sdev_attr_group,
+ NULL
+};
+
static const struct scsi_host_template isci_sht = {
.module = THIS_MODULE,
@@ -176,6 +181,7 @@ static const struct scsi_host_template isci_sht = {
.compat_ioctl = sas_ioctl,
#endif
.shost_groups = isci_host_groups,
+ .sdev_groups = isci_sdev_groups,
.track_queue_depth = 1,
};
--
2.44.0.278.ge034bb2e1d-goog
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group
2024-03-02 20:16 ` [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group Igor Pylypiv
@ 2024-03-04 8:16 ` John Garry
2024-03-04 8:43 ` Jinpu Wang
1 sibling, 0 replies; 17+ messages in thread
From: John Garry @ 2024-03-04 8:16 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> The added sysfs attributes group enables the configuration of NCQ Priority
> feature for HBAs that rely on libsas to manage SATA devices.
>
> Signed-off-by: Igor Pylypiv<ipylypiv@google.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 4/7] scsi: mvsas: Add libsas SATA sysfs attributes group
2024-03-02 20:16 ` [PATCH v3 4/7] scsi: mvsas: " Igor Pylypiv
@ 2024-03-04 8:16 ` John Garry
0 siblings, 0 replies; 17+ messages in thread
From: John Garry @ 2024-03-04 8:16 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> The added sysfs attributes group enables the configuration of NCQ Priority
> feature for HBAs that rely on libsas to manage SATA devices.
>
> Signed-off-by: Igor Pylypiv<ipylypiv@google.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/7] scsi: hisi_sas: Add libsas SATA sysfs attributes group
2024-03-02 20:16 ` [PATCH v3 5/7] scsi: hisi_sas: " Igor Pylypiv
@ 2024-03-04 8:18 ` John Garry
0 siblings, 0 replies; 17+ messages in thread
From: John Garry @ 2024-03-04 8:18 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> The added sysfs attributes group enables the configuration of NCQ Priority
> feature for HBAs that rely on libsas to manage SATA devices.
>
> Signed-off-by: Igor Pylypiv<ipylypiv@google.com>
Apart from comment on v1 HW:
Reviewed-by: John Garry <john.g.garry@oracle.com>
> ---
> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 6 ++++++
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 6 ++++++
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 6 ++++++
> 3 files changed, 18 insertions(+)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> index 3c555579f9a1..97864b56a71f 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> @@ -1734,6 +1734,11 @@ static struct attribute *host_v1_hw_attrs[] = {
>
> ATTRIBUTE_GROUPS(host_v1_hw);
>
> +static const struct attribute_group *sdev_groups_v1_hw[] = {
> + &sas_ata_sdev_attr_group,
IIRC, this driver never supported SATA
> + NULL
> +};
> +
> static const struct scsi_host_template sht_v1_hw = {
> .name = DRV_NAME,
> .proc_name = DRV_NAME,
> @@ -1758,6 +1763,7 @@ static const struct scsi_host_template sht_v1_hw = {
> .compat_ioctl = sas_ioctl,
> #endif
> .shost_groups = host_v1_hw_groups,
> + .sdev_groups = sdev_groups_v1_hw,
> .host_reset = hisi_sas_host_reset,
> };
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 6/7] scsi: aic94xx: Add libsas SATA sysfs attributes group
2024-03-02 20:16 ` [PATCH v3 6/7] scsi: aic94xx: " Igor Pylypiv
@ 2024-03-04 8:18 ` John Garry
0 siblings, 0 replies; 17+ messages in thread
From: John Garry @ 2024-03-04 8:18 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> The added sysfs attributes group enables the configuration of NCQ Priority
> feature for HBAs that rely on libsas to manage SATA devices.
>
> Signed-off-by: Igor Pylypiv<ipylypiv@google.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 7/7] scsi: isci: Add libsas SATA sysfs attributes group
2024-03-02 20:16 ` [PATCH v3 7/7] scsi: isci: " Igor Pylypiv
@ 2024-03-04 8:18 ` John Garry
0 siblings, 0 replies; 17+ messages in thread
From: John Garry @ 2024-03-04 8:18 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> The added sysfs attributes group enables the configuration of NCQ Priority
> feature for HBAs that rely on libsas to manage SATA devices.
>
> Signed-off-by: Igor Pylypiv<ipylypiv@google.com>
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices
2024-03-02 20:16 ` [PATCH v3 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices Igor Pylypiv
@ 2024-03-04 8:22 ` John Garry
0 siblings, 0 replies; 17+ messages in thread
From: John Garry @ 2024-03-04 8:22 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> Libata sysfs attributes cannot be used for libsas managed SATA devices
> because the ata_port location is different for libsas.
>
> Defined sysfs attributes (visible for SATA devices only):
> - /sys/block/sda/device/ncq_prio_enable
> - /sys/block/sda/device/ncq_prio_supported
>
> The newly defined attributes will pass the correct ata_port to libata
> helper functions.
>
> Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
This looks ok:
Reviewed-by: John Garry <john.g.garry@oracle.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers
2024-03-02 20:16 ` [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers Igor Pylypiv
@ 2024-03-04 8:34 ` John Garry
2024-03-04 22:15 ` Igor Pylypiv
0 siblings, 1 reply; 17+ messages in thread
From: John Garry @ 2024-03-04 8:34 UTC (permalink / raw
To: Igor Pylypiv, Damien Le Moal, Niklas Cassel, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche
Cc: TJ Adams, linux-ide, linux-scsi, linux-kernel
On 02/03/2024 20:16, Igor Pylypiv wrote:
> Export libata NCQ Priority configuration helpers to be reused
> for libsas managed SATA devices.
>
> Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
This looks ok. Some small code comments below, though.
> ---
> drivers/ata/libata-sata.c | 139 +++++++++++++++++++++++++++-----------
> include/linux/libata.h | 4 ++
> 2 files changed, 103 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index 0fb1934875f2..a8d5e36d5211 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -848,29 +848,73 @@ DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
> ata_scsi_lpm_show, ata_scsi_lpm_store);
> EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
>
> +/**
> + * ata_ncq_prio_supported - Check if device supports NCQ Priority
> + * @ap: ATA port of the target device
> + * @sdev: SCSI device
> + *
> + * Helper to check if device supports NCQ Priority feature,
> + * usable with both libsas and libata.
> + */
> +int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev)
> +{
> + struct ata_device *dev;
> + unsigned long flags;
> + int rc;
> +
> + spin_lock_irqsave(ap->lock, flags);
> + dev = ata_scsi_find_dev(ap, sdev);
> + if (!dev)
> + rc = -ENODEV;
> + else
> + rc = !!(dev->flags & ATA_DFLAG_NCQ_PRIO);
> + spin_unlock_irqrestore(ap->lock, flags);
> + return rc;
> +}
I'm not the biggest fan of functions which effectively return both a
boolean and an error code.
I like this patten more:
int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device
*sdev, bool *supported)
{
...
if (rc)
return rc;
*supported = true/false.
return 0;
}
No big deal, though.
> +EXPORT_SYMBOL_GPL(ata_ncq_prio_supported);
> +
> static ssize_t ata_ncq_prio_supported_show(struct device *device,
> struct device_attribute *attr,
> char *buf)
> {
> struct scsi_device *sdev = to_scsi_device(device);
> struct ata_port *ap = ata_shost_to_port(sdev->host);
> + int rc;
> +
> + rc = ata_ncq_prio_supported(ap, sdev);
> + if (rc < 0)
> + return rc;
> +
> + return sysfs_emit(buf, "%d\n", rc);
> +}
> +
> +DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
> +EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
> +
> +/**
> + * ata_ncq_prio_enabled - Check if NCQ Priority is enabled
> + * @ap: ATA port of the target device
> + * @sdev: SCSI device
> + *
> + * Helper to check if NCQ Priority feature is enabled,
> + * usable with both libsas and libata.
It's usable by anything which uses libata, really. For the moment that
is libsas and libata.
> + */
> +int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev)
> +{
> struct ata_device *dev;
> - bool ncq_prio_supported;
> - int rc = 0;
> + unsigned long flags;
> + int rc;
>
> - spin_lock_irq(ap->lock);
> + spin_lock_irqsave(ap->lock, flags);
> dev = ata_scsi_find_dev(ap, sdev);
> if (!dev)
> rc = -ENODEV;
> else
> - ncq_prio_supported = dev->flags & ATA_DFLAG_NCQ_PRIO;
> - spin_unlock_irq(ap->lock);
> -
> - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_supported);
> + rc = !!(dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED);
> + spin_unlock_irqrestore(ap->lock, flags);
> + return rc;
> }
> -
> -DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
> -EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
> +EXPORT_SYMBOL_GPL(ata_ncq_prio_enabled);
>
> static ssize_t ata_ncq_prio_enable_show(struct device *device,
> struct device_attribute *attr,
> @@ -878,50 +922,45 @@ static ssize_t ata_ncq_prio_enable_show(struct device *device,
> {
> struct scsi_device *sdev = to_scsi_device(device);
> struct ata_port *ap = ata_shost_to_port(sdev->host);
> - struct ata_device *dev;
> - bool ncq_prio_enable;
> - int rc = 0;
> + int rc;
>
> - spin_lock_irq(ap->lock);
> - dev = ata_scsi_find_dev(ap, sdev);
> - if (!dev)
> - rc = -ENODEV;
> - else
> - ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED;
> - spin_unlock_irq(ap->lock);
> + rc = ata_ncq_prio_enabled(ap, sdev);
> + if (rc < 0)
> + return rc;
>
> - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_enable);
> + return sysfs_emit(buf, "%d\n", rc);
> }
>
> -static ssize_t ata_ncq_prio_enable_store(struct device *device,
> - struct device_attribute *attr,
> - const char *buf, size_t len)
> +/**
> + * ata_ncq_prio_enable - Enable/disable NCQ Priority
> + * @ap: ATA port of the target device
> + * @sdev: SCSI device
> + * @enable: true - enable NCQ Priority, false - disable NCQ Priority
> + *
> + * Helper to enable/disable NCQ Priority feature, usable with both
> + * libsas and libata.
> + */
> +int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
> + bool enable)
> {
> - struct scsi_device *sdev = to_scsi_device(device);
> - struct ata_port *ap;
> struct ata_device *dev;
> - long int input;
> + unsigned long flags;
> int rc = 0;
>
> - rc = kstrtol(buf, 10, &input);
> - if (rc)
> - return rc;
> - if ((input < 0) || (input > 1))
> - return -EINVAL;
> + spin_lock_irqsave(ap->lock, flags);
>
> - ap = ata_shost_to_port(sdev->host);
> dev = ata_scsi_find_dev(ap, sdev);
> - if (unlikely(!dev))
> - return -ENODEV;
> -
> - spin_lock_irq(ap->lock);
> + if (unlikely(!dev)) {
no need for unlikely() - this is not fathpath
> + rc = -ENODEV;
> + goto unlock;
> + }
>
> if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
> rc = -EINVAL;
> goto unlock;
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group
2024-03-02 20:16 ` [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group Igor Pylypiv
2024-03-04 8:16 ` John Garry
@ 2024-03-04 8:43 ` Jinpu Wang
1 sibling, 0 replies; 17+ messages in thread
From: Jinpu Wang @ 2024-03-04 8:43 UTC (permalink / raw
To: Igor Pylypiv
Cc: Damien Le Moal, Niklas Cassel, John Garry, Jason Yan,
James E.J. Bottomley, Martin K. Petersen, Jack Wang,
Hannes Reinecke, Xiang Chen, Artur Paszkiewicz, Bart Van Assche,
TJ Adams, linux-ide, linux-scsi, linux-kernel
On Sat, Mar 2, 2024 at 9:16 PM Igor Pylypiv <ipylypiv@google.com> wrote:
>
> The added sysfs attributes group enables the configuration of NCQ Priority
> feature for HBAs that rely on libsas to manage SATA devices.
>
> Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/scsi/pm8001/pm8001_ctl.c | 5 +++++
> drivers/scsi/pm8001/pm8001_init.c | 1 +
> drivers/scsi/pm8001/pm8001_sas.h | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
> index 5c26a13ffbd2..9ffe1a868d0f 100644
> --- a/drivers/scsi/pm8001/pm8001_ctl.c
> +++ b/drivers/scsi/pm8001/pm8001_ctl.c
> @@ -1039,3 +1039,8 @@ const struct attribute_group *pm8001_host_groups[] = {
> &pm8001_host_attr_group,
> NULL
> };
> +
> +const struct attribute_group *pm8001_sdev_groups[] = {
> + &sas_ata_sdev_attr_group,
> + NULL
> +};
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index ed6b7d954dda..e6b1108f6117 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -134,6 +134,7 @@ static const struct scsi_host_template pm8001_sht = {
> .compat_ioctl = sas_ioctl,
> #endif
> .shost_groups = pm8001_host_groups,
> + .sdev_groups = pm8001_sdev_groups,
> .track_queue_depth = 1,
> .cmd_per_lun = 32,
> .map_queues = pm8001_map_queues,
> diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
> index 3ccb7371902f..ced6721380a8 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.h
> +++ b/drivers/scsi/pm8001/pm8001_sas.h
> @@ -717,6 +717,7 @@ int pm80xx_fatal_errors(struct pm8001_hba_info *pm8001_ha);
> void pm8001_free_dev(struct pm8001_device *pm8001_dev);
> /* ctl shared API */
> extern const struct attribute_group *pm8001_host_groups[];
> +extern const struct attribute_group *pm8001_sdev_groups[];
>
> #define PM8001_INVALID_TAG ((u32)-1)
>
> --
> 2.44.0.278.ge034bb2e1d-goog
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers
2024-03-04 8:34 ` John Garry
@ 2024-03-04 22:15 ` Igor Pylypiv
0 siblings, 0 replies; 17+ messages in thread
From: Igor Pylypiv @ 2024-03-04 22:15 UTC (permalink / raw
To: John Garry
Cc: Damien Le Moal, Niklas Cassel, Jason Yan, James E.J. Bottomley,
Martin K. Petersen, Jack Wang, Hannes Reinecke, Xiang Chen,
Artur Paszkiewicz, Bart Van Assche, TJ Adams, linux-ide,
linux-scsi, linux-kernel
On Mon, Mar 04, 2024 at 08:34:52AM +0000, John Garry wrote:
> On 02/03/2024 20:16, Igor Pylypiv wrote:
> > Export libata NCQ Priority configuration helpers to be reused
> > for libsas managed SATA devices.
> >
> > Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
>
> This looks ok. Some small code comments below, though.
>
> > ---
> > drivers/ata/libata-sata.c | 139 +++++++++++++++++++++++++++-----------
> > include/linux/libata.h | 4 ++
> > 2 files changed, 103 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> > index 0fb1934875f2..a8d5e36d5211 100644
> > --- a/drivers/ata/libata-sata.c
> > +++ b/drivers/ata/libata-sata.c
> > @@ -848,29 +848,73 @@ DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
> > ata_scsi_lpm_show, ata_scsi_lpm_store);
> > EXPORT_SYMBOL_GPL(dev_attr_link_power_management_policy);
> > +/**
> > + * ata_ncq_prio_supported - Check if device supports NCQ Priority
> > + * @ap: ATA port of the target device
> > + * @sdev: SCSI device
> > + *
> > + * Helper to check if device supports NCQ Priority feature,
> > + * usable with both libsas and libata.
> > + */
> > +int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev)
> > +{
> > + struct ata_device *dev;
> > + unsigned long flags;
> > + int rc;
> > +
> > + spin_lock_irqsave(ap->lock, flags);
> > + dev = ata_scsi_find_dev(ap, sdev);
> > + if (!dev)
> > + rc = -ENODEV;
> > + else
> > + rc = !!(dev->flags & ATA_DFLAG_NCQ_PRIO);
> > + spin_unlock_irqrestore(ap->lock, flags);
> > + return rc;
> > +}
>
> I'm not the biggest fan of functions which effectively return both a boolean
> and an error code.
>
> I like this patten more:
>
> int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev,
> bool *supported)
> {
> ...
> if (rc)
> return rc;
> *supported = true/false.
> return 0;
> }
>
> No big deal, though.
Thanks for the review, John. Separating out an error code and a boolean
makes the code a bit more readable. Sent out v4 with the changes.
>
> > +EXPORT_SYMBOL_GPL(ata_ncq_prio_supported);
> > +
> > static ssize_t ata_ncq_prio_supported_show(struct device *device,
> > struct device_attribute *attr,
> > char *buf)
> > {
> > struct scsi_device *sdev = to_scsi_device(device);
> > struct ata_port *ap = ata_shost_to_port(sdev->host);
> > + int rc;
> > +
> > + rc = ata_ncq_prio_supported(ap, sdev);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return sysfs_emit(buf, "%d\n", rc);
> > +}
> > +
> > +DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
> > +EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
> > +
> > +/**
> > + * ata_ncq_prio_enabled - Check if NCQ Priority is enabled
> > + * @ap: ATA port of the target device
> > + * @sdev: SCSI device
> > + *
> > + * Helper to check if NCQ Priority feature is enabled,
> > + * usable with both libsas and libata.
>
> It's usable by anything which uses libata, really. For the moment that is
> libsas and libata.
>
> > + */
> > +int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev)
> > +{
> > struct ata_device *dev;
> > - bool ncq_prio_supported;
> > - int rc = 0;
> > + unsigned long flags;
> > + int rc;
> > - spin_lock_irq(ap->lock);
> > + spin_lock_irqsave(ap->lock, flags);
> > dev = ata_scsi_find_dev(ap, sdev);
> > if (!dev)
> > rc = -ENODEV;
> > else
> > - ncq_prio_supported = dev->flags & ATA_DFLAG_NCQ_PRIO;
> > - spin_unlock_irq(ap->lock);
> > -
> > - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_supported);
> > + rc = !!(dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED);
> > + spin_unlock_irqrestore(ap->lock, flags);
> > + return rc;
> > }
> > -
> > -DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL);
> > -EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported);
> > +EXPORT_SYMBOL_GPL(ata_ncq_prio_enabled);
> > static ssize_t ata_ncq_prio_enable_show(struct device *device,
> > struct device_attribute *attr,
> > @@ -878,50 +922,45 @@ static ssize_t ata_ncq_prio_enable_show(struct device *device,
> > {
> > struct scsi_device *sdev = to_scsi_device(device);
> > struct ata_port *ap = ata_shost_to_port(sdev->host);
> > - struct ata_device *dev;
> > - bool ncq_prio_enable;
> > - int rc = 0;
> > + int rc;
> > - spin_lock_irq(ap->lock);
> > - dev = ata_scsi_find_dev(ap, sdev);
> > - if (!dev)
> > - rc = -ENODEV;
> > - else
> > - ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED;
> > - spin_unlock_irq(ap->lock);
> > + rc = ata_ncq_prio_enabled(ap, sdev);
> > + if (rc < 0)
> > + return rc;
> > - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_enable);
> > + return sysfs_emit(buf, "%d\n", rc);
> > }
> > -static ssize_t ata_ncq_prio_enable_store(struct device *device,
> > - struct device_attribute *attr,
> > - const char *buf, size_t len)
> > +/**
> > + * ata_ncq_prio_enable - Enable/disable NCQ Priority
> > + * @ap: ATA port of the target device
> > + * @sdev: SCSI device
> > + * @enable: true - enable NCQ Priority, false - disable NCQ Priority
> > + *
> > + * Helper to enable/disable NCQ Priority feature, usable with both
> > + * libsas and libata.
> > + */
> > +int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
> > + bool enable)
> > {
> > - struct scsi_device *sdev = to_scsi_device(device);
> > - struct ata_port *ap;
> > struct ata_device *dev;
> > - long int input;
> > + unsigned long flags;
> > int rc = 0;
> > - rc = kstrtol(buf, 10, &input);
> > - if (rc)
> > - return rc;
> > - if ((input < 0) || (input > 1))
> > - return -EINVAL;
> > + spin_lock_irqsave(ap->lock, flags);
> > - ap = ata_shost_to_port(sdev->host);
> > dev = ata_scsi_find_dev(ap, sdev);
> > - if (unlikely(!dev))
> > - return -ENODEV;
> > -
> > - spin_lock_irq(ap->lock);
> > + if (unlikely(!dev)) {
>
> no need for unlikely() - this is not fathpath
>
> > + rc = -ENODEV;
> > + goto unlock;
> > + }
> > if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
> > rc = -EINVAL;
> > goto unlock;
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-03-04 22:15 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-02 20:16 [PATCH v3 0/7] NCQ Priority sysfs sttributes for libsas Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 1/7] ata: libata-sata: Factor out NCQ Priority configuration helpers Igor Pylypiv
2024-03-04 8:34 ` John Garry
2024-03-04 22:15 ` Igor Pylypiv
2024-03-02 20:16 ` [PATCH v3 2/7] scsi: libsas: Define NCQ Priority sysfs attributes for SATA devices Igor Pylypiv
2024-03-04 8:22 ` John Garry
2024-03-02 20:16 ` [PATCH v3 3/7] scsi: pm80xx: Add libsas SATA sysfs attributes group Igor Pylypiv
2024-03-04 8:16 ` John Garry
2024-03-04 8:43 ` Jinpu Wang
2024-03-02 20:16 ` [PATCH v3 4/7] scsi: mvsas: " Igor Pylypiv
2024-03-04 8:16 ` John Garry
2024-03-02 20:16 ` [PATCH v3 5/7] scsi: hisi_sas: " Igor Pylypiv
2024-03-04 8:18 ` John Garry
2024-03-02 20:16 ` [PATCH v3 6/7] scsi: aic94xx: " Igor Pylypiv
2024-03-04 8:18 ` John Garry
2024-03-02 20:16 ` [PATCH v3 7/7] scsi: isci: " Igor Pylypiv
2024-03-04 8:18 ` John Garry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).