mbox series

[0/7] libata cleanups and improvements

Message ID 20210802090232.1166195-1-damien.lemoal@wdc.com
Headers show
Series libata cleanups and improvements | expand

Message

Damien Le Moal Aug. 2, 2021, 9:02 a.m. UTC 
The first three patches of this series cleanup libata-core code in the
area of device configuration (ata_dev_configure() function).
Patch 4 improves ata_read_log_page() handling to avoid unnecessary
warning messages and patch 5 adds an informational message on device
scan to advertize the features supported by a device.

Path 6 adds the new sysfs ahci device attribute ncq_prio_supported to
indicate that a disk supports NCQ priority. Patch 7 does the same for
the mpt3sas driver, adding the sas_ncq_prio_supported device attribute.

Damien Le Moal (7):
  libata: cleanup device sleep capability detection
  libata: cleanup ata_dev_configure()
  libata: cleanup NCQ priority handling
  libata: fix ata_read_log_page() warning
  libata: print feature list on device scan
  libahci: Introduce ncq_prio_supported sysfs sttribute
  scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute

 drivers/ata/libahci.c              |   1 +
 drivers/ata/libata-core.c          | 249 +++++++++++++++--------------
 drivers/ata/libata-sata.c          |  61 ++++---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c |  20 +++
 include/linux/libata.h             |   5 +
 5 files changed, 191 insertions(+), 145 deletions(-)

Comments

Damien Le Moal Aug. 2, 2021, 10:52 p.m. UTC | #1 
On 2021/08/03 1:00, Bart Van Assche wrote:
> On 8/2/21 2:02 AM, Damien Le Moal wrote:
>> +/**
>> + * sas_ncq_prio_supported_show - Indicate if device supports NCQ priority
>> + * @dev: pointer to embedded device
>> + * @attr: sas_ncq_prio_supported attribute desciptor
>> + * @buf: the buffer returned
>> + *
>> + * A sysfs 'read/write' sdev attribute, only works with SATA
>> + */
>> +static ssize_t
>> +sas_ncq_prio_supported_show(struct device *dev,
>> +			    struct device_attribute *attr, char *buf)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +
>> +	return snprintf(buf, PAGE_SIZE, "%d\n",
>> +			scsih_ncq_prio_supp(sdev));
>> +}
>> +static DEVICE_ATTR_RO(sas_ncq_prio_supported);
> 
> Since this is new code, how about using sysfs_emit() instead of snprintf()?

OK. Will do.

> 
> Thanks,
> 
> Bart.
>
Johannes Thumshirn Aug. 3, 2021, 7:55 a.m. UTC | #2 
On 02/08/2021 11:03, Damien Le Moal wrote:
> +/**
> + * sas_ncq_prio_supported_show - Indicate if device supports NCQ priority
> + * @dev: pointer to embedded device
> + * @attr: sas_ncq_prio_supported attribute desciptor
> + * @buf: the buffer returned
> + *
> + * A sysfs 'read/write' sdev attribute, only works with SATA
> + */

[...]

> +static DEVICE_ATTR_RO(sas_ncq_prio_supported);
> +

Shouldn't that comment read: 
"A sysfs 'read only' sdev attribute, only works with SATA"
Damien Le Moal Aug. 3, 2021, 8:03 a.m. UTC | #3 
On 2021/08/03 16:55, Johannes Thumshirn wrote:
> On 02/08/2021 11:03, Damien Le Moal wrote:
>> +/**
>> + * sas_ncq_prio_supported_show - Indicate if device supports NCQ priority
>> + * @dev: pointer to embedded device
>> + * @attr: sas_ncq_prio_supported attribute desciptor
>> + * @buf: the buffer returned
>> + *
>> + * A sysfs 'read/write' sdev attribute, only works with SATA
>> + */
> 
> [...]
> 
>> +static DEVICE_ATTR_RO(sas_ncq_prio_supported);
>> +
> 
> Shouldn't that comment read: 
> "A sysfs 'read only' sdev attribute, only works with SATA"

Oops. Indeed it should :)