| Message ID | 20240306012226.3398927-2-ipylypiv@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | NCQ Priority sysfs sttributes for libsas | expand |
Hello Igor, On Tue, Mar 05, 2024 at 05:22:20PM -0800, Igor Pylypiv wrote: > Export libata NCQ Priority configuration helpers to be reused > for libsas managed SATA devices. > > Acked-by: Damien Le Moal <dlemoal@kernel.org> > Reviewed-by: Jason Yan <yanaijie@huawei.com> > Signed-off-by: Igor Pylypiv <ipylypiv@google.com> > --- > drivers/ata/libata-sata.c | 140 +++++++++++++++++++++++++++----------- > include/linux/libata.h | 6 ++ > 2 files changed, 107 insertions(+), 39 deletions(-) > > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c > index 0fb1934875f2..f00dd02dc6f8 100644 > --- a/drivers/ata/libata-sata.c > +++ b/drivers/ata/libata-sata.c > @@ -848,80 +848,122 @@ 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); > > -static ssize_t ata_ncq_prio_supported_show(struct device *device, > - struct device_attribute *attr, > - char *buf) > +/** > + * ata_ncq_prio_supported - Check if device supports NCQ Priority > + * @ap: ATA port of the target device > + * @sdev: SCSI device > + * @supported: Address of a boolean to store the result > + * > + * Helper to check if device supports NCQ Priority feature. This kdoc is missing a "Return:", see: https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html (Same comment for other functions.) > + */ > +int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev, > + bool *supported) > { > - 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_supported; > + unsigned long flags; > int rc = 0; > > - spin_lock_irq(ap->lock); > + spin_lock_irqsave(ap->lock, flags); You should mention why you are changing this from a spin_lock_irq() to a spin_lock_irqsave() in the commit message. > 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); > + *supported = dev->flags & ATA_DFLAG_NCQ_PRIO; > + spin_unlock_irqrestore(ap->lock, flags); > + return rc; You removed the blank link between spin_unlock and return, please keep this empty line. (Same comment for other functions.) > +} > +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); > + bool supported; > + int rc; > > - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_supported); > + rc = ata_ncq_prio_supported(ap, sdev, &supported); > + if (rc) > + return rc; > + > + return sysfs_emit(buf, "%d\n", supported); > } > > DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL); > EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported); > > -static ssize_t ata_ncq_prio_enable_show(struct device *device, > - struct device_attribute *attr, > - char *buf) > +/** > + * ata_ncq_prio_enabled - Check if NCQ Priority is enabled > + * @ap: ATA port of the target device > + * @sdev: SCSI device > + * @enabled: Address of a boolean to store the result > + * > + * Helper to check if NCQ Priority feature is enabled. > + */ > +int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev, > + bool *enabled) > { > - 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; > + unsigned long flags; > int rc = 0; > > - 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_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED; > - spin_unlock_irq(ap->lock); > - > - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_enable); > + *enabled = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED; > + spin_unlock_irqrestore(ap->lock, flags); > + return rc; > } > +EXPORT_SYMBOL_GPL(ata_ncq_prio_enabled); > > -static ssize_t ata_ncq_prio_enable_store(struct device *device, > - struct device_attribute *attr, > - const char *buf, size_t len) > +static ssize_t ata_ncq_prio_enable_show(struct device *device, > + struct device_attribute *attr, > + char *buf) > { > struct scsi_device *sdev = to_scsi_device(device); > - struct ata_port *ap; > - struct ata_device *dev; > - long int input; > - int rc = 0; > + struct ata_port *ap = ata_shost_to_port(sdev->host); > + bool enabled; > + int rc; > > - rc = kstrtol(buf, 10, &input); > + rc = ata_ncq_prio_enabled(ap, sdev, &enabled); > if (rc) > return rc; > - if ((input < 0) || (input > 1)) > - return -EINVAL; > > - ap = ata_shost_to_port(sdev->host); > - dev = ata_scsi_find_dev(ap, sdev); > - if (unlikely(!dev)) > - return -ENODEV; > + return sysfs_emit(buf, "%d\n", enabled); > +} > + > +/** > + * 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. > + */ > +int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev, > + bool enable) > +{ > + struct ata_device *dev; > + unsigned long flags; > + int rc = 0; > + > + spin_lock_irqsave(ap->lock, flags); > > - spin_lock_irq(ap->lock); > + dev = ata_scsi_find_dev(ap, sdev); > + if (!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 +976,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..6dd9a4f9ca7c 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -1157,6 +1157,12 @@ 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, > + bool *supported); > +extern int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev, > + bool *enabled); > +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 >
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index 0fb1934875f2..f00dd02dc6f8 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -848,80 +848,122 @@ 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); -static ssize_t ata_ncq_prio_supported_show(struct device *device, - struct device_attribute *attr, - char *buf) +/** + * ata_ncq_prio_supported - Check if device supports NCQ Priority + * @ap: ATA port of the target device + * @sdev: SCSI device + * @supported: Address of a boolean to store the result + * + * Helper to check if device supports NCQ Priority feature. + */ +int ata_ncq_prio_supported(struct ata_port *ap, struct scsi_device *sdev, + bool *supported) { - 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_supported; + unsigned long flags; int rc = 0; - 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); + *supported = 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); + bool supported; + int rc; - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_supported); + rc = ata_ncq_prio_supported(ap, sdev, &supported); + if (rc) + return rc; + + return sysfs_emit(buf, "%d\n", supported); } DEVICE_ATTR(ncq_prio_supported, S_IRUGO, ata_ncq_prio_supported_show, NULL); EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_supported); -static ssize_t ata_ncq_prio_enable_show(struct device *device, - struct device_attribute *attr, - char *buf) +/** + * ata_ncq_prio_enabled - Check if NCQ Priority is enabled + * @ap: ATA port of the target device + * @sdev: SCSI device + * @enabled: Address of a boolean to store the result + * + * Helper to check if NCQ Priority feature is enabled. + */ +int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev, + bool *enabled) { - 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; + unsigned long flags; int rc = 0; - 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_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED; - spin_unlock_irq(ap->lock); - - return rc ? rc : sysfs_emit(buf, "%u\n", ncq_prio_enable); + *enabled = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLED; + spin_unlock_irqrestore(ap->lock, flags); + return rc; } +EXPORT_SYMBOL_GPL(ata_ncq_prio_enabled); -static ssize_t ata_ncq_prio_enable_store(struct device *device, - struct device_attribute *attr, - const char *buf, size_t len) +static ssize_t ata_ncq_prio_enable_show(struct device *device, + struct device_attribute *attr, + char *buf) { struct scsi_device *sdev = to_scsi_device(device); - struct ata_port *ap; - struct ata_device *dev; - long int input; - int rc = 0; + struct ata_port *ap = ata_shost_to_port(sdev->host); + bool enabled; + int rc; - rc = kstrtol(buf, 10, &input); + rc = ata_ncq_prio_enabled(ap, sdev, &enabled); if (rc) return rc; - if ((input < 0) || (input > 1)) - return -EINVAL; - ap = ata_shost_to_port(sdev->host); - dev = ata_scsi_find_dev(ap, sdev); - if (unlikely(!dev)) - return -ENODEV; + return sysfs_emit(buf, "%d\n", enabled); +} + +/** + * 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. + */ +int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev, + bool enable) +{ + struct ata_device *dev; + unsigned long flags; + int rc = 0; + + spin_lock_irqsave(ap->lock, flags); - spin_lock_irq(ap->lock); + dev = ata_scsi_find_dev(ap, sdev); + if (!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 +976,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..6dd9a4f9ca7c 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1157,6 +1157,12 @@ 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, + bool *supported); +extern int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev, + bool *enabled); +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);