| Message ID | 20210806111145.445697-9-damien.lemoal@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libata cleanups and improvements | expand |
On 8/6/21 1:11 PM, Damien Le Moal wrote: > Currently, the only way a user can determine if a SATA device supports > NCQ priority is to try to enable the use of this feature using the > ncq_prio_enable sysfs device attribute. If enabling the feature fails, > it is because the device does not support NCQ priority. Otherwise, the > feature is enabled and indicates that the device supports NCQ priority. > > Improve this odd interface by introducing the read-only > ncq_prio_supported sysfs device attribute to indicate if a SATA device > supports NCQ priority. The value of this attribute reflects if the > device flag ATA_DFLAG_NCQ_PRIO is set or cleared. > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > drivers/ata/libahci.c | 1 + > drivers/ata/libata-sata.c | 24 ++++++++++++++++++++++++ > include/linux/libata.h | 1 + > 3 files changed, 26 insertions(+) > > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c > index fec2e9754aed..5b3fa2cbe722 100644 > --- a/drivers/ata/libahci.c > +++ b/drivers/ata/libahci.c > @@ -125,6 +125,7 @@ EXPORT_SYMBOL_GPL(ahci_shost_attrs); > struct device_attribute *ahci_sdev_attrs[] = { > &dev_attr_sw_activity, > &dev_attr_unload_heads, > + &dev_attr_ncq_prio_supported, > &dev_attr_ncq_prio_enable, > NULL > }; > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c > index dc397ebda089..5566fd4bb38f 100644 > --- a/drivers/ata/libata-sata.c > +++ b/drivers/ata/libata-sata.c > @@ -834,6 +834,30 @@ 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) > +{ > + 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; > + int rc = 0; > + > + spin_lock_irq(ap->lock); > + 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); > +} > + > +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) > diff --git a/include/linux/libata.h b/include/linux/libata.h > index b23f28cfc8e0..a2d1bae7900b 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -539,6 +539,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes) > extern struct device_attribute dev_attr_unload_heads; > #ifdef CONFIG_SATA_HOST > extern struct device_attribute dev_attr_link_power_management_policy; > +extern struct device_attribute dev_attr_ncq_prio_supported; > extern struct device_attribute dev_attr_ncq_prio_enable; > extern struct device_attribute dev_attr_em_message_type; > extern struct device_attribute dev_attr_em_message; > Alternative would have been to make the 'ncq_prio_enable' attribute visible only when NCQ prio is supported, but not sure if that works out. So: Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On 2021/08/06 20:37, Hannes Reinecke wrote: > On 8/6/21 1:11 PM, Damien Le Moal wrote: >> Currently, the only way a user can determine if a SATA device supports >> NCQ priority is to try to enable the use of this feature using the >> ncq_prio_enable sysfs device attribute. If enabling the feature fails, >> it is because the device does not support NCQ priority. Otherwise, the >> feature is enabled and indicates that the device supports NCQ priority. >> >> Improve this odd interface by introducing the read-only >> ncq_prio_supported sysfs device attribute to indicate if a SATA device >> supports NCQ priority. The value of this attribute reflects if the >> device flag ATA_DFLAG_NCQ_PRIO is set or cleared. >> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> >> --- >> drivers/ata/libahci.c | 1 + >> drivers/ata/libata-sata.c | 24 ++++++++++++++++++++++++ >> include/linux/libata.h | 1 + >> 3 files changed, 26 insertions(+) >> >> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c >> index fec2e9754aed..5b3fa2cbe722 100644 >> --- a/drivers/ata/libahci.c >> +++ b/drivers/ata/libahci.c >> @@ -125,6 +125,7 @@ EXPORT_SYMBOL_GPL(ahci_shost_attrs); >> struct device_attribute *ahci_sdev_attrs[] = { >> &dev_attr_sw_activity, >> &dev_attr_unload_heads, >> + &dev_attr_ncq_prio_supported, >> &dev_attr_ncq_prio_enable, >> NULL >> }; >> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c >> index dc397ebda089..5566fd4bb38f 100644 >> --- a/drivers/ata/libata-sata.c >> +++ b/drivers/ata/libata-sata.c >> @@ -834,6 +834,30 @@ 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) >> +{ >> + 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; >> + int rc = 0; >> + >> + spin_lock_irq(ap->lock); >> + 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); >> +} >> + >> +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) >> diff --git a/include/linux/libata.h b/include/linux/libata.h >> index b23f28cfc8e0..a2d1bae7900b 100644 >> --- a/include/linux/libata.h >> +++ b/include/linux/libata.h >> @@ -539,6 +539,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes) >> extern struct device_attribute dev_attr_unload_heads; >> #ifdef CONFIG_SATA_HOST >> extern struct device_attribute dev_attr_link_power_management_policy; >> +extern struct device_attribute dev_attr_ncq_prio_supported; >> extern struct device_attribute dev_attr_ncq_prio_enable; >> extern struct device_attribute dev_attr_em_message_type; >> extern struct device_attribute dev_attr_em_message; >> > Alternative would have been to make the 'ncq_prio_enable' attribute > visible only when NCQ prio is supported, but not sure if that works out. Indeed. But from the start (4.10) ncq_prio_enable is always shown, even for devices that do not support NCQ prio. I am not sure if changing that is acceptable as this is a user IF. > > So: > > Reviewed-by: Hannes Reinecke <hare@suse.de> > > Cheers, > > Hannes >
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index fec2e9754aed..5b3fa2cbe722 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -125,6 +125,7 @@ EXPORT_SYMBOL_GPL(ahci_shost_attrs); struct device_attribute *ahci_sdev_attrs[] = { &dev_attr_sw_activity, &dev_attr_unload_heads, + &dev_attr_ncq_prio_supported, &dev_attr_ncq_prio_enable, NULL }; diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c index dc397ebda089..5566fd4bb38f 100644 --- a/drivers/ata/libata-sata.c +++ b/drivers/ata/libata-sata.c @@ -834,6 +834,30 @@ 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) +{ + 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; + int rc = 0; + + spin_lock_irq(ap->lock); + 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); +} + +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) diff --git a/include/linux/libata.h b/include/linux/libata.h index b23f28cfc8e0..a2d1bae7900b 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -539,6 +539,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes) extern struct device_attribute dev_attr_unload_heads; #ifdef CONFIG_SATA_HOST extern struct device_attribute dev_attr_link_power_management_policy; +extern struct device_attribute dev_attr_ncq_prio_supported; extern struct device_attribute dev_attr_ncq_prio_enable; extern struct device_attribute dev_attr_em_message_type; extern struct device_attribute dev_attr_em_message;
Currently, the only way a user can determine if a SATA device supports NCQ priority is to try to enable the use of this feature using the ncq_prio_enable sysfs device attribute. If enabling the feature fails, it is because the device does not support NCQ priority. Otherwise, the feature is enabled and indicates that the device supports NCQ priority. Improve this odd interface by introducing the read-only ncq_prio_supported sysfs device attribute to indicate if a SATA device supports NCQ priority. The value of this attribute reflects if the device flag ATA_DFLAG_NCQ_PRIO is set or cleared. Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- drivers/ata/libahci.c | 1 + drivers/ata/libata-sata.c | 24 ++++++++++++++++++++++++ include/linux/libata.h | 1 + 3 files changed, 26 insertions(+)