linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] libata cleanups and improvements
@ 2021-08-02  9:02 Damien Le Moal
  2021-08-02  9:02 ` [PATCH 1/7] libata: cleanup device sleep capability detection Damien Le Moal
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

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(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/7] libata: cleanup device sleep capability detection
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02  9:02 ` [PATCH 2/7] libata: cleanup ata_dev_configure() Damien Le Moal
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

Move the code to retrieve the device sleep capability and timings out of
ata_dev_configure() into the helper function ata_dev_config_devslp().
While at it, mark the device as supporting the device sleep capability
only if the sata settings page was retrieved successfully to ensure that
the timing information is correctly initialized.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 55 +++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 61c762961ca8..9b39a4e2e567 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2363,6 +2363,37 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 		dev->flags |= ATA_DFLAG_TRUSTED;
 }
 
+static void ata_dev_config_devslp(struct ata_device *dev)
+{
+	u8 *sata_setting = dev->link->ap->sector_buf;
+	unsigned int err_mask;
+	int i, j;
+
+	/*
+	 * Check device sleep capability. Get DevSlp timing variables
+	 * from SATA Settings page of Identify Device Data Log.
+	 */
+	if (!ata_id_has_devslp(dev->id))
+		return;
+
+	err_mask = ata_read_log_page(dev,
+				     ATA_LOG_IDENTIFY_DEVICE,
+				     ATA_LOG_SATA_SETTINGS,
+				     sata_setting, 1);
+	if (err_mask) {
+		ata_dev_dbg(dev,
+			    "failed to get SATA Settings Log, Emask 0x%x\n",
+			    err_mask);
+		return;
+	}
+
+	dev->flags |= ATA_DFLAG_DEVSLP;
+	for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
+		j = ATA_LOG_DEVSLP_OFFSET + i;
+		dev->devslp_timing[i] = sata_setting[j];
+	}
+}
+
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -2565,29 +2596,7 @@ int ata_dev_configure(struct ata_device *dev)
 			}
 		}
 
-		/* Check and mark DevSlp capability. Get DevSlp timing variables
-		 * from SATA Settings page of Identify Device Data Log.
-		 */
-		if (ata_id_has_devslp(dev->id)) {
-			u8 *sata_setting = ap->sector_buf;
-			int i, j;
-
-			dev->flags |= ATA_DFLAG_DEVSLP;
-			err_mask = ata_read_log_page(dev,
-						     ATA_LOG_IDENTIFY_DEVICE,
-						     ATA_LOG_SATA_SETTINGS,
-						     sata_setting,
-						     1);
-			if (err_mask)
-				ata_dev_dbg(dev,
-					    "failed to get Identify Device Data, Emask 0x%x\n",
-					    err_mask);
-			else
-				for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
-					j = ATA_LOG_DEVSLP_OFFSET + i;
-					dev->devslp_timing[i] = sata_setting[j];
-				}
-		}
+		ata_dev_config_devslp(dev);
 		ata_dev_config_sense_reporting(dev);
 		ata_dev_config_zac(dev);
 		ata_dev_config_trusted(dev);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/7] libata: cleanup ata_dev_configure()
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
  2021-08-02  9:02 ` [PATCH 1/7] libata: cleanup device sleep capability detection Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02  9:02 ` [PATCH 3/7] libata: cleanup NCQ priority handling Damien Le Moal
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

Introduce the helper functions ata_dev_config_lba() and
ata_dev_config_chs() to configure the addressing capabilities of a
device. Each helper takes a string as argument for the addressing
information printed after these helpers execution completes.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 110 ++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 9b39a4e2e567..1849f858761b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2363,6 +2363,52 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 		dev->flags |= ATA_DFLAG_TRUSTED;
 }
 
+static int ata_dev_config_lba(struct ata_device *dev,
+			      char *info, size_t infosz)
+{
+	const u16 *id = dev->id;
+	int info_ofst;
+
+	dev->flags |= ATA_DFLAG_LBA;
+
+	if (ata_id_has_lba48(id)) {
+		dev->flags |= ATA_DFLAG_LBA48;
+		strcpy(info, "LBA48 ");
+
+		if (dev->n_sectors >= (1UL << 28) &&
+		    ata_id_has_flush_ext(id))
+			dev->flags |= ATA_DFLAG_FLUSH_EXT;
+	} else {
+		strcpy(info, "LBA ");
+	}
+	info_ofst = strlen(info);
+
+	/* config NCQ */
+	return ata_dev_config_ncq(dev, info + info_ofst,
+				  infosz - info_ofst);
+}
+
+static void ata_dev_config_chs(struct ata_device *dev,
+			       char *info, size_t infosz)
+{
+	const u16 *id = dev->id;
+
+	/* Default translation */
+	dev->cylinders	= id[1];
+	dev->heads	= id[3];
+	dev->sectors	= id[6];
+
+	if (ata_id_current_chs_valid(id)) {
+		/* Current CHS translation is valid. */
+		dev->cylinders = id[54];
+		dev->heads     = id[55];
+		dev->sectors   = id[56];
+	}
+
+	snprintf(info, infosz, "CHS %u/%u/%u",
+		 dev->cylinders, dev->heads, dev->sectors);
+}
+
 static void ata_dev_config_devslp(struct ata_device *dev)
 {
 	u8 *sata_setting = dev->link->ap->sector_buf;
@@ -2418,6 +2464,7 @@ int ata_dev_configure(struct ata_device *dev)
 	char revbuf[7];		/* XYZ-99\0 */
 	char fwrevbuf[ATA_ID_FW_REV_LEN+1];
 	char modelbuf[ATA_ID_PROD_LEN+1];
+	char lba_info[40];
 	int rc;
 
 	if (!ata_dev_enabled(dev) && ata_msg_info(ap)) {
@@ -2539,61 +2586,22 @@ int ata_dev_configure(struct ata_device *dev)
 		}
 
 		if (ata_id_has_lba(id)) {
-			const char *lba_desc;
-			char ncq_desc[24];
-
-			lba_desc = "LBA";
-			dev->flags |= ATA_DFLAG_LBA;
-			if (ata_id_has_lba48(id)) {
-				dev->flags |= ATA_DFLAG_LBA48;
-				lba_desc = "LBA48";
-
-				if (dev->n_sectors >= (1UL << 28) &&
-				    ata_id_has_flush_ext(id))
-					dev->flags |= ATA_DFLAG_FLUSH_EXT;
-			}
-
-			/* config NCQ */
-			rc = ata_dev_config_ncq(dev, ncq_desc, sizeof(ncq_desc));
+			rc = ata_dev_config_lba(dev, lba_info, sizeof(lba_info));
 			if (rc)
 				return rc;
-
-			/* print device info to dmesg */
-			if (ata_msg_drv(ap) && print_info) {
-				ata_dev_info(dev, "%s: %s, %s, max %s\n",
-					     revbuf, modelbuf, fwrevbuf,
-					     ata_mode_string(xfer_mask));
-				ata_dev_info(dev,
-					     "%llu sectors, multi %u: %s %s\n",
-					(unsigned long long)dev->n_sectors,
-					dev->multi_count, lba_desc, ncq_desc);
-			}
 		} else {
-			/* CHS */
-
-			/* Default translation */
-			dev->cylinders	= id[1];
-			dev->heads	= id[3];
-			dev->sectors	= id[6];
-
-			if (ata_id_current_chs_valid(id)) {
-				/* Current CHS translation is valid. */
-				dev->cylinders = id[54];
-				dev->heads     = id[55];
-				dev->sectors   = id[56];
-			}
+			ata_dev_config_chs(dev, lba_info, sizeof(lba_info));
+		}
 
-			/* print device info to dmesg */
-			if (ata_msg_drv(ap) && print_info) {
-				ata_dev_info(dev, "%s: %s, %s, max %s\n",
-					     revbuf,	modelbuf, fwrevbuf,
-					     ata_mode_string(xfer_mask));
-				ata_dev_info(dev,
-					     "%llu sectors, multi %u, CHS %u/%u/%u\n",
-					     (unsigned long long)dev->n_sectors,
-					     dev->multi_count, dev->cylinders,
-					     dev->heads, dev->sectors);
-			}
+		/* print device info to dmesg */
+		if (ata_msg_drv(ap) && print_info) {
+			ata_dev_info(dev, "%s: %s, %s, max %s\n",
+				     revbuf, modelbuf, fwrevbuf,
+				     ata_mode_string(xfer_mask));
+			ata_dev_info(dev,
+				     "%llu sectors, multi %u, %s\n",
+				     (unsigned long long)dev->n_sectors,
+				     dev->multi_count, lba_info);
 		}
 
 		ata_dev_config_devslp(dev);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/7] libata: cleanup NCQ priority handling
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
  2021-08-02  9:02 ` [PATCH 1/7] libata: cleanup device sleep capability detection Damien Le Moal
  2021-08-02  9:02 ` [PATCH 2/7] libata: cleanup ata_dev_configure() Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02  9:02 ` [PATCH 4/7] libata: fix ata_read_log_page() warning Damien Le Moal
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

The ata device flag ATA_DFLAG_NCQ_PRIO indicates if a device supports
the NCQ Priority feature while the ATA_DFLAG_NCQ_PRIO_ENABLE device
flag indicates if the feature is enabled. Enabling NCQ priority use is
controlled by the user through the device sysfs attribute
ncq_prio_enable. As a result, the ATA_DFLAG_NCQ_PRIO flag should not be
cleared when ATA_DFLAG_NCQ_PRIO_ENABLE is not set as the device still
supports the feature even after the user disables it. This leads to the
following cleanups:
- In ata_build_rw_tf(), set a command high priority bit based on the
  ATA_DFLAG_NCQ_PRIO_ENABLE flag, not on the ATA_DFLAG_NCQ flag. That
  is, set a command high priority only if the user enabled NCQ priority
  use.
- In ata_dev_config_ncq_prio(), ATA_DFLAG_NCQ_PRIO should not be cleared
  if ATA_DFLAG_NCQ_PRIO_ENABLE is not set. If the device does not
  support NCQ priority, both ATA_DFLAG_NCQ_PRIO and
  ATA_DFLAG_NCQ_PRIO_ENABLE must be cleared.

With the above ata_dev_config_ncq_prio() change, ATA_DFLAG_NCQ_PRIO flag
is set on device scan and revalidation. There is no need to trigger a
device revalidation in ata_ncq_prio_enable_store() when the user enables
the use of NCQ priority. Remove the revalidation code from that funciton
to simplify it. Also change the return value from -EIO to -EINVAL when a
user tries to enable NCQ priority for a device that does not support
this feature.  While at it, also simplify ata_ncq_prio_enable_show().

Overall, there is no functional change introduced by this patch.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 32 ++++++++++++++------------------
 drivers/ata/libata-sata.c | 37 ++++++++++++-------------------------
 2 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 1849f858761b..21afb59f359f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -706,11 +706,9 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 		if (tf->flags & ATA_TFLAG_FUA)
 			tf->device |= 1 << 7;
 
-		if (dev->flags & ATA_DFLAG_NCQ_PRIO) {
-			if (class == IOPRIO_CLASS_RT)
-				tf->hob_nsect |= ATA_PRIO_HIGH <<
-						 ATA_SHIFT_PRIO;
-		}
+		if (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE &&
+		    class == IOPRIO_CLASS_RT)
+			tf->hob_nsect |= ATA_PRIO_HIGH << ATA_SHIFT_PRIO;
 	} else if (dev->flags & ATA_DFLAG_LBA) {
 		tf->flags |= ATA_TFLAG_LBA;
 
@@ -2173,11 +2171,6 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
 
-	if (!(dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE)) {
-		dev->flags &= ~ATA_DFLAG_NCQ_PRIO;
-		return;
-	}
-
 	err_mask = ata_read_log_page(dev,
 				     ATA_LOG_IDENTIFY_DEVICE,
 				     ATA_LOG_SATA_SETTINGS,
@@ -2185,18 +2178,21 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 				     1);
 	if (err_mask) {
 		ata_dev_dbg(dev,
-			    "failed to get Identify Device data, Emask 0x%x\n",
+			    "failed to get SATA settings log, Emask 0x%x\n",
 			    err_mask);
-		return;
+		goto not_supported;
 	}
 
-	if (ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3)) {
-		dev->flags |= ATA_DFLAG_NCQ_PRIO;
-	} else {
-		dev->flags &= ~ATA_DFLAG_NCQ_PRIO;
-		ata_dev_dbg(dev, "SATA page does not support priority\n");
-	}
+	if (!(ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3)))
+		goto not_supported;
+
+	dev->flags |= ATA_DFLAG_NCQ_PRIO;
+
+	return;
 
+not_supported:
+	dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
+	dev->flags &= ~ATA_DFLAG_NCQ_PRIO;
 }
 
 static int ata_dev_config_ncq(struct ata_device *dev,
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 8adeab76dd38..dc397ebda089 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -839,23 +839,17 @@ static ssize_t ata_ncq_prio_enable_show(struct device *device,
 					char *buf)
 {
 	struct scsi_device *sdev = to_scsi_device(device);
-	struct ata_port *ap;
+	struct ata_port *ap = ata_shost_to_port(sdev->host);
 	struct ata_device *dev;
 	bool ncq_prio_enable;
 	int rc = 0;
 
-	ap = ata_shost_to_port(sdev->host);
-
 	spin_lock_irq(ap->lock);
 	dev = ata_scsi_find_dev(ap, sdev);
-	if (!dev) {
+	if (!dev)
 		rc = -ENODEV;
-		goto unlock;
-	}
-
-	ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE;
-
-unlock:
+	else
+		ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE;
 	spin_unlock_irq(ap->lock);
 
 	return rc ? rc : snprintf(buf, 20, "%u\n", ncq_prio_enable);
@@ -869,7 +863,7 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device,
 	struct ata_port *ap;
 	struct ata_device *dev;
 	long int input;
-	int rc;
+	int rc = 0;
 
 	rc = kstrtol(buf, 10, &input);
 	if (rc)
@@ -883,27 +877,20 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device,
 		return  -ENODEV;
 
 	spin_lock_irq(ap->lock);
+
+	if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
+		rc = -EINVAL;
+		goto unlock;
+	}
+
 	if (input)
 		dev->flags |= ATA_DFLAG_NCQ_PRIO_ENABLE;
 	else
 		dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
 
-	dev->link->eh_info.action |= ATA_EH_REVALIDATE;
-	dev->link->eh_info.flags |= ATA_EHI_QUIET;
-	ata_port_schedule_eh(ap);
+unlock:
 	spin_unlock_irq(ap->lock);
 
-	ata_port_wait_eh(ap);
-
-	if (input) {
-		spin_lock_irq(ap->lock);
-		if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
-			dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
-			rc = -EIO;
-		}
-		spin_unlock_irq(ap->lock);
-	}
-
 	return rc ? rc : len;
 }
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/7] libata: fix ata_read_log_page() warning
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
                   ` (2 preceding siblings ...)
  2021-08-02  9:02 ` [PATCH 3/7] libata: cleanup NCQ priority handling Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02  9:02 ` [PATCH 5/7] libata: print feature list on device scan Damien Le Moal
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

Support for the READ LOG PAGE DMA EXT command is indicated by words 119
and 120 of a device identify data. This is tested in
ata_read_log_page() with ata_id_has_read_log_dma_ext() and the
READ LOG PAGE DMA command used if the device reports supports for it.

However, some devices lie about this support and using the DMA version
of the command fails, generating the warning message "READ LOG DMA EXT
failed, trying PIO". Since READ LOG PAGE DMA EXT is an optional command,
this warning is not at all important but may be scary for the user.
Change ata_read_log_page() to suppres this warning and to print an
error message if both DMA and PIO attempts failed.

With this change, there is no need to print again an error message when
ata_read_log_page() returns an error. So simplify the users of this
function.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 47 +++++++++++----------------------------
 1 file changed, 13 insertions(+), 34 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 21afb59f359f..68ef43de0ed2 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2021,13 +2021,15 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
 	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
 				     buf, sectors * ATA_SECT_SIZE, 0);
 
-	if (err_mask && dma) {
-		dev->horkage |= ATA_HORKAGE_NO_DMA_LOG;
-		ata_dev_warn(dev, "READ LOG DMA EXT failed, trying PIO\n");
-		goto retry;
+	if (err_mask) {
+		if (dma) {
+			dev->horkage |= ATA_HORKAGE_NO_DMA_LOG;
+			goto retry;
+		}
+		ata_dev_err(dev, "Read log page 0x%02x failed, Emask 0x%x\n",
+			    (unsigned int)page, err_mask);
 	}
 
-	DPRINTK("EXIT, err_mask=%x\n", err_mask);
 	return err_mask;
 }
 
@@ -2056,12 +2058,8 @@ static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
 	 */
 	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, 0, ap->sector_buf,
 				1);
-	if (err) {
-		ata_dev_info(dev,
-			     "failed to get Device Identify Log Emask 0x%x\n",
-			     err);
+	if (err)
 		return false;
-	}
 
 	for (i = 0; i < ap->sector_buf[8]; i++) {
 		if (ap->sector_buf[9 + i] == page)
@@ -2125,11 +2123,7 @@ static void ata_dev_config_ncq_send_recv(struct ata_device *dev)
 	}
 	err_mask = ata_read_log_page(dev, ATA_LOG_NCQ_SEND_RECV,
 				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get NCQ Send/Recv Log Emask 0x%x\n",
-			    err_mask);
-	} else {
+	if (!err_mask) {
 		u8 *cmds = dev->ncq_send_recv_cmds;
 
 		dev->flags |= ATA_DFLAG_NCQ_SEND_RECV;
@@ -2155,11 +2149,7 @@ static void ata_dev_config_ncq_non_data(struct ata_device *dev)
 	}
 	err_mask = ata_read_log_page(dev, ATA_LOG_NCQ_NON_DATA,
 				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get NCQ Non-Data Log Emask 0x%x\n",
-			    err_mask);
-	} else {
+	if (!err_mask) {
 		u8 *cmds = dev->ncq_non_data_cmds;
 
 		memcpy(cmds, ap->sector_buf, ATA_LOG_NCQ_NON_DATA_SIZE);
@@ -2176,12 +2166,8 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 				     ATA_LOG_SATA_SETTINGS,
 				     ap->sector_buf,
 				     1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get SATA settings log, Emask 0x%x\n",
-			    err_mask);
+	if (err_mask)
 		goto not_supported;
-	}
 
 	if (!(ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3)))
 		goto not_supported;
@@ -2342,11 +2328,8 @@ static void ata_dev_config_trusted(struct ata_device *dev)
 
 	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, ATA_LOG_SECURITY,
 			ap->sector_buf, 1);
-	if (err) {
-		ata_dev_dbg(dev,
-			    "failed to read Security Log, Emask 0x%x\n", err);
+	if (err)
 		return;
-	}
 
 	trusted_cap = get_unaligned_le64(&ap->sector_buf[40]);
 	if (!(trusted_cap & (1ULL << 63))) {
@@ -2422,12 +2405,8 @@ static void ata_dev_config_devslp(struct ata_device *dev)
 				     ATA_LOG_IDENTIFY_DEVICE,
 				     ATA_LOG_SATA_SETTINGS,
 				     sata_setting, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get SATA Settings Log, Emask 0x%x\n",
-			    err_mask);
+	if (err_mask)
 		return;
-	}
 
 	dev->flags |= ATA_DFLAG_DEVSLP;
 	for (i = 0; i < ATA_LOG_DEVSLP_SIZE; i++) {
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/7] libata: print feature list on device scan
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
                   ` (3 preceding siblings ...)
  2021-08-02  9:02 ` [PATCH 4/7] libata: fix ata_read_log_page() warning Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02  9:02 ` [PATCH 6/7] libahci: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
  2021-08-02  9:02 ` [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported " Damien Le Moal
  6 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

Print a list of features supported by a drive when it is configured in
ata_dev_configure() using the new function ata_dev_print_features().
The features printed are not already advertized and are: trusted
send-recev support, device attention support, device sleep support,
NCQ send-recv support and NCQ priority support.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 17 +++++++++++++++++
 include/linux/libata.h    |  4 ++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 68ef43de0ed2..77118719e494 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2415,6 +2415,20 @@ static void ata_dev_config_devslp(struct ata_device *dev)
 	}
 }
 
+static void ata_dev_print_features(struct ata_device *dev)
+{
+	if (!(dev->flags & ATA_DFLAG_FEATURES_MASK))
+		return;
+
+	ata_dev_info(dev,
+		     "Features:%s%s%s%s%s\n",
+		     dev->flags & ATA_DFLAG_TRUSTED ? " Trust" : "",
+		     dev->flags & ATA_DFLAG_DA ? " Dev-Attention" : "",
+		     dev->flags & ATA_DFLAG_DEVSLP ? " Dev-Sleep" : "",
+		     dev->flags & ATA_DFLAG_NCQ_SEND_RECV ? " NCQ-sndrcv" : "",
+		     dev->flags & ATA_DFLAG_NCQ_PRIO ? " NCQ-prio" : "");
+}
+
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -2584,6 +2598,9 @@ int ata_dev_configure(struct ata_device *dev)
 		ata_dev_config_zac(dev);
 		ata_dev_config_trusted(dev);
 		dev->cdb_len = 32;
+
+		if (ata_msg_drv(ap) && print_info)
+			ata_dev_print_features(dev);
 	}
 
 	/* ATAPI-specific feature tests */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3fcd24236793..b23f28cfc8e0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -161,6 +161,10 @@ enum {
 	ATA_DFLAG_D_SENSE	= (1 << 29), /* Descriptor sense requested */
 	ATA_DFLAG_ZAC		= (1 << 30), /* ZAC device */
 
+	ATA_DFLAG_FEATURES_MASK	= ATA_DFLAG_TRUSTED | ATA_DFLAG_DA | \
+				  ATA_DFLAG_DEVSLP | ATA_DFLAG_NCQ_SEND_RECV | \
+				  ATA_DFLAG_NCQ_PRIO,
+
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
 	ATA_DEV_ATA_UNSUP	= 2,	/* ATA device (unsupported) */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 6/7] libahci: Introduce ncq_prio_supported sysfs sttribute
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
                   ` (4 preceding siblings ...)
  2021-08-02  9:02 ` [PATCH 5/7] libata: print feature list on device scan Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02  9:02 ` [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported " Damien Le Moal
  6 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

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..4a067384086b 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 : snprintf(buf, 20, "%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;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute
  2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
                   ` (5 preceding siblings ...)
  2021-08-02  9:02 ` [PATCH 6/7] libahci: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
@ 2021-08-02  9:02 ` Damien Le Moal
  2021-08-02 16:00   ` Bart Van Assche
  2021-08-03  7:55   ` Johannes Thumshirn
  6 siblings, 2 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02  9:02 UTC (permalink / raw)
  To: Jens Axboe, linux-block, linux-ide, Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

Similarly to AHCI, introduce the device sysfs attribute
sas_ncq_prio_supported to advertize if a SATA device supports the NCQ
priority feature. Without this new attribute, the user can only
discover if a SATA device supports NCQ priority by trying to enable
the feature use with the sas_ncq_prio_enable sysfs device attribute,
which fails when the device does not support high priroity commands.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
index b66140e4c370..7bce8288fb4f 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c
@@ -3918,6 +3918,25 @@ sas_device_handle_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(sas_device_handle);
 
+/**
+ * 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);
+
 /**
  * sas_ncq_prio_enable_show - send prioritized io commands to device
  * @dev: pointer to embedded device
@@ -3960,6 +3979,7 @@ static DEVICE_ATTR_RW(sas_ncq_prio_enable);
 struct device_attribute *mpt3sas_dev_attrs[] = {
 	&dev_attr_sas_address,
 	&dev_attr_sas_device_handle,
+	&dev_attr_sas_ncq_prio_supported,
 	&dev_attr_sas_ncq_prio_enable,
 	NULL,
 };
-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute
  2021-08-02  9:02 ` [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported " Damien Le Moal
@ 2021-08-02 16:00   ` Bart Van Assche
  2021-08-02 22:52     ` Damien Le Moal
  2021-08-03  7:55   ` Johannes Thumshirn
  1 sibling, 1 reply; 13+ messages in thread
From: Bart Van Assche @ 2021-08-02 16:00 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-block, linux-ide,
	Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

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()?

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute
  2021-08-02 16:00   ` Bart Van Assche
@ 2021-08-02 22:52     ` Damien Le Moal
  0 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-02 22:52 UTC (permalink / raw)
  To: Bart Van Assche, Jens Axboe, linux-block, linux-ide,
	Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

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.
> 


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute
  2021-08-02  9:02 ` [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported " Damien Le Moal
  2021-08-02 16:00   ` Bart Van Assche
@ 2021-08-03  7:55   ` Johannes Thumshirn
  2021-08-03  8:03     ` Damien Le Moal
  1 sibling, 1 reply; 13+ messages in thread
From: Johannes Thumshirn @ 2021-08-03  7:55 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-block, linux-ide,
	Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

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"

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported sysfs sttribute
  2021-08-03  7:55   ` Johannes Thumshirn
@ 2021-08-03  8:03     ` Damien Le Moal
  0 siblings, 0 replies; 13+ messages in thread
From: Damien Le Moal @ 2021-08-03  8:03 UTC (permalink / raw)
  To: Johannes Thumshirn, Jens Axboe, linux-block, linux-ide,
	Martin K . Petersen, linux-scsi
  Cc: Sathya Prakash, Sreekanth Reddy, Suganath Prabu Subramani

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 :)



-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/7] libata: cleanup ata_dev_configure()
       [not found] <202108030231.7Ta42o7Q-lkp@intel.com>
@ 2021-08-03  9:08 ` kernel test robot
  0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2021-08-03  9:08 UTC (permalink / raw)
  To: Damien Le Moal, Jens Axboe, linux-block, linux-ide,
	Martin K . Petersen, linux-scsi
  Cc: clang-built-linux, kbuild-all, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani

[-- Attachment #1: Type: text/plain, Size: 16863 bytes --]


Hi Damien,

I love your patch! Perhaps something to improve:

[auto build test WARNING on block/for-next]
[also build test WARNING on mkp-scsi/for-next scsi/for-next v5.14-rc3 
next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: 
https://github.com/0day-ci/linux/commits/Damien-Le-Moal/libata-cleanups-and-improvements/20210802-170443
base: 
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git 
for-next
:::::: branch date: 9 hours ago
:::::: commit date: 9 hours ago
config: x86_64-randconfig-c001-20210802 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 
4f71f59bf3d9914188a11d0c41bedbb339d36ff5)
reproduce (this is a W=1 build):
         wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
-O ~/bin/make.cross
         chmod +x ~/bin/make.cross
         # install x86_64 cross compiling tool for clang build
         # apt-get install binutils-x86-64-linux-gnu
         # 
https://github.com/0day-ci/linux/commit/5622af0f73c9776b5bdf272dee77d57d62ea03fa
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review 
Damien-Le-Moal/libata-cleanups-and-improvements/20210802-170443
         git checkout 5622af0f73c9776b5bdf272dee77d57d62ea03fa
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross 
ARCH=x86_64 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
    note: (skipping 3 expansions in backtrace; use 
-fmacro-backtrace-limit=0 to see all)
    include/linux/minmax.h:104:48: note: expanded from macro 'min_t'
    #define min_t(type, x, y)       __careful_cmp((type)(x), (type)(y), <)
                                    ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
    include/linux/minmax.h:38:14: note: expanded from macro '__careful_cmp'
                    __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), 
op))
                    ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    include/linux/minmax.h:31:25: note: expanded from macro '__cmp_once'
                    typeof(x) unique_x = (x);               \
                                          ^
    drivers/hwmon/max6621.c:321:10: note: '?' condition is false
                            val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
                                  ^
    include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
    #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                   ^
    include/linux/minmax.h:124:48: note: expanded from macro 'clamp_t'
    #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                                   ^
    include/linux/minmax.h:112:27: note: expanded from macro 'max_t'
    #define max_t(type, x, y)       __careful_cmp((type)(x), (type)(y), >)
                                    ^
    include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                    __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), 
op))
                    ^
    include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                    __cmp(unique_x, unique_y, op); })
                    ^
    include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                             ^
    drivers/hwmon/max6621.c:321:10: note: '__UNIQUE_ID___x523' is < 
'__UNIQUE_ID___y524'
                            val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
                                  ^
    include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
    #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    include/linux/minmax.h:124:36: note: expanded from macro 'clamp_t'
    #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    include/linux/minmax.h:104:27: note: expanded from macro 'min_t'
    #define min_t(type, x, y)       __careful_cmp((type)(x), (type)(y), <)
                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                    __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), 
op))
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                    __cmp(unique_x, unique_y, op); })
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                             ^~~
    drivers/hwmon/max6621.c:321:10: note: '?' condition is true
                            val = clamp_val(val, MAX6621_TEMP_INPUT_MIN,
                                  ^
    include/linux/minmax.h:137:32: note: expanded from macro 'clamp_val'
    #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
                                   ^
    include/linux/minmax.h:124:36: note: expanded from macro 'clamp_t'
    #define clamp_t(type, val, lo, hi) min_t(type, max_t(type, val, lo), hi)
                                       ^
    include/linux/minmax.h:104:27: note: expanded from macro 'min_t'
    #define min_t(type, x, y)       __careful_cmp((type)(x), (type)(y), <)
                                    ^
    include/linux/minmax.h:38:3: note: expanded from macro '__careful_cmp'
                    __cmp_once(x, y, __UNIQUE_ID(__x), __UNIQUE_ID(__y), 
op))
                    ^
    include/linux/minmax.h:33:3: note: expanded from macro '__cmp_once'
                    __cmp(unique_x, unique_y, op); })
                    ^
    include/linux/minmax.h:28:26: note: expanded from macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                             ^
    drivers/hwmon/max6621.c:323:10: note: Calling 'max6621_temp_mc2reg'
                            val = max6621_temp_mc2reg(val);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~
    drivers/hwmon/max6621.c:135:23: note: The result of the left shift 
is undefined because the left operand is negative
            return (val / 1000L) << MAX6621_REG_TEMP_SHIFT;
                   ~~~~~~~~~~~~~ ^
    Suppressed 8 warnings (8 in non-user code).
    Use -header-filter=.* to display errors from all non-system headers. 
Use -system-headers to display errors from system headers as well.
    9 warnings generated.
    Suppressed 9 warnings (8 in non-user code, 1 with check filters).
    Use -header-filter=.* to display errors from all non-system headers. 
Use -system-headers to display errors from system headers as well.
    9 warnings generated.
    Suppressed 9 warnings (8 in non-user code, 1 with check filters).
    Use -header-filter=.* to display errors from all non-system headers. 
Use -system-headers to display errors from system headers as well.
    9 warnings generated.
    Suppressed 9 warnings (8 in non-user code, 1 with check filters).
    Use -header-filter=.* to display errors from all non-system headers. 
Use -system-headers to display errors from system headers as well.
    12 warnings generated.
    drivers/nvme/target/fcloop.c:1226:3: warning: Value stored to 'ret' 
is never read [clang-analyzer-deadcode.DeadStores]
                    ret = -EINVAL;
                    ^     ~~~~~~~
    drivers/nvme/target/fcloop.c:1226:3: note: Value stored to 'ret' is 
never read
                    ret = -EINVAL;
                    ^     ~~~~~~~
    Suppressed 11 warnings (11 in non-user code).
    Use -header-filter=.* to display errors from all non-system headers. 
Use -system-headers to display errors from system headers as well.
    15 warnings generated.
>> drivers/ata/libata-core.c:2376:3: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                    strcpy(info, "LBA48 ");
                    ^~~~~~
    drivers/ata/libata-core.c:2376:3: note: Call to function 'strcpy' is 
insecure as it does not provide bounding of the memory buffer. Replace 
unbounded copy functions with analogous functions that support length 
arguments such as 'strlcpy'. CWE-119
                    strcpy(info, "LBA48 ");
                    ^~~~~~
    drivers/ata/libata-core.c:2382:3: warning: Call to function 'strcpy' 
is insecure as it does not provide bounding of the memory buffer. 
Replace unbounded copy functions with analogous functions that support 
length arguments such as 'strlcpy'. CWE-119 
[clang-analyzer-security.insecureAPI.strcpy]
                    strcpy(info, "LBA ");
                    ^~~~~~
    drivers/ata/libata-core.c:2382:3: note: Call to function 'strcpy' is 
insecure as it does not provide bounding of the memory buffer. Replace 
unbounded copy functions with analogous functions that support length 
arguments such as 'strlcpy'. CWE-119
                    strcpy(info, "LBA ");
                    ^~~~~~
    drivers/ata/libata-core.c:5471:18: warning: Access to field 
'pio_mask' results in a dereference of a null pointer (loaded from 
variable 'pi') [clang-analyzer-core.NullDereference]
                    ap->pio_mask = pi->pio_mask;
                                   ^~
    drivers/ata/libata-core.c:5462:6: note: Assuming 'host' is non-null
            if (!host)
                ^~~~~
    drivers/ata/libata-core.c:5462:2: note: Taking false branch
            if (!host)
            ^
    drivers/ata/libata-core.c:5465:21: note: Null pointer value stored 
to 'pi'
            for (i = 0, j = 0, pi = NULL; i < host->n_ports; i++) {
                               ^~~~~~~~~
    drivers/ata/libata-core.c:5465:32: note: Assuming 'i' is < field 
'n_ports'
            for (i = 0, j = 0, pi = NULL; i < host->n_ports; i++) {
                                          ^~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5465:2: note: Loop condition is true. 
Entering loop body
            for (i = 0, j = 0, pi = NULL; i < host->n_ports; i++) {
            ^
    drivers/ata/libata-core.c:5468:7: note: Assuming the condition is false
                    if (ppi[j])
                        ^~~~~~
    drivers/ata/libata-core.c:5468:3: note: Taking false branch
                    if (ppi[j])
                    ^
    drivers/ata/libata-core.c:5471:18: note: Access to field 'pio_mask' 
results in a dereference of a null pointer (loaded from variable 'pi')
                    ap->pio_mask = pi->pio_mask;
                                   ^~
    drivers/ata/libata-core.c:5593:6: warning: Access to field 
'host_stop' results in a dereference of a null pointer (loaded from 
field 'ops') [clang-analyzer-core.NullDereference]
            if (host->ops->host_stop)
                ^
    drivers/ata/libata-core.c:5840:7: note: Calling 'ata_host_start'
            rc = ata_host_start(host);
                 ^~~~~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5576:6: note: Assuming the condition is false
            if (host->flags & ATA_HOST_STARTED)
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5576:2: note: Taking false branch
            if (host->flags & ATA_HOST_STARTED)
            ^
    drivers/ata/libata-core.c:5581:14: note: Assuming 'i' is < field 
'n_ports'
            for (i = 0; i < host->n_ports; i++) {
                        ^~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5581:2: note: Loop condition is true. 
Entering loop body
            for (i = 0; i < host->n_ports; i++) {
            ^
    drivers/ata/libata-core.c:5586:7: note: Assuming field 'ops' is null
                    if (!host->ops && !ata_port_is_dummy(ap))
                        ^~~~~~~~~~
    drivers/ata/libata-core.c:5586:7: note: Assuming pointer value is null
                    if (!host->ops && !ata_port_is_dummy(ap))
                        ^~~~~~~~~~
    drivers/ata/libata-core.c:5586:7: note: Left side of '&&' is true
    drivers/ata/libata-core.c:5586:21: note: Assuming the condition is false
                    if (!host->ops && !ata_port_is_dummy(ap))
                                      ^~~~~~~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5586:3: note: Taking false branch
                    if (!host->ops && !ata_port_is_dummy(ap))
                    ^
    drivers/ata/libata-core.c:5589:7: note: Assuming field 'port_stop' 
is null
                    if (ap->ops->port_stop)
                        ^~~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5589:3: note: Taking false branch
                    if (ap->ops->port_stop)
                    ^
    drivers/ata/libata-core.c:5581:14: note: Assuming 'i' is >= field 
'n_ports'
            for (i = 0; i < host->n_ports; i++) {
                        ^~~~~~~~~~~~~~~~~
    drivers/ata/libata-core.c:5581:2: note: Loop condition is false. 
Execution continues on line 5593
            for (i = 0; i < host->n_ports; i++) {
            ^
    drivers/ata/libata-core.c:5593:6: note: Access to field 'host_stop' 
results in a dereference of a null pointer (loaded from field 'ops')
            if (host->ops->host_stop)
                ^     ~~~
    Suppressed 11 warnings (11 in non-user code).
    Use -header-filter=.* to display errors from all non-system headers. 
Use -system-headers to display errors from system headers as well.
    8 warnings generated.
    include/linux/hid.h:1007:9: warning: Access to field 'name' results 
in a dereference of a null pointer (loaded from variable 'input') 
[clang-analyzer-core.NullDereference]
                                        input->name, c, type);
                                        ^
    drivers/hid/hid-lenovo.c:321:2: note: Control jumps to 'case 12544:' 
  at line 329
            switch (hdev->product) {
            ^
    drivers/hid/hid-lenovo.c:335:10: note: Calling 
'lenovo_input_mapping_scrollpoint'
                    return lenovo_input_mapping_scrollpoint(hdev, hi, field,
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    drivers/hid/hid-lenovo.c:224:6: note: Assuming field 'hid' is equal 
to HID_GD_Z
            if (usage->hid == HID_GD_Z) {
                ^~~~~~~~~~~~~~~~~~~~~~
    drivers/hid/hid-lenovo.c:224:2: note: Taking true branch

vim +2376 drivers/ata/libata-core.c

818831c8b22f75 Christoph Hellwig 2017-06-04  2365  5622af0f73c977 Damien 
Le Moal    2021-08-02  2366  static int ata_dev_config_lba(struct 
ata_device *dev,
5622af0f73c977 Damien Le Moal    2021-08-02  2367  			      char *info, 
size_t infosz)
5622af0f73c977 Damien Le Moal    2021-08-02  2368  {
5622af0f73c977 Damien Le Moal    2021-08-02  2369  	const u16 *id = dev->id;
5622af0f73c977 Damien Le Moal    2021-08-02  2370  	int info_ofst;
5622af0f73c977 Damien Le Moal    2021-08-02  2371  5622af0f73c977 Damien 
Le Moal    2021-08-02  2372  	dev->flags |= ATA_DFLAG_LBA;
5622af0f73c977 Damien Le Moal    2021-08-02  2373  5622af0f73c977 Damien 
Le Moal    2021-08-02  2374  	if (ata_id_has_lba48(id)) {
5622af0f73c977 Damien Le Moal    2021-08-02  2375  		dev->flags |= 
ATA_DFLAG_LBA48;
5622af0f73c977 Damien Le Moal    2021-08-02 @2376  		strcpy(info, "LBA48 ");
5622af0f73c977 Damien Le Moal    2021-08-02  2377  5622af0f73c977 Damien 
Le Moal    2021-08-02  2378  		if (dev->n_sectors >= (1UL << 28) &&
5622af0f73c977 Damien Le Moal    2021-08-02  2379  		 
ata_id_has_flush_ext(id))
5622af0f73c977 Damien Le Moal    2021-08-02  2380  			dev->flags |= 
ATA_DFLAG_FLUSH_EXT;
5622af0f73c977 Damien Le Moal    2021-08-02  2381  	} else {
5622af0f73c977 Damien Le Moal    2021-08-02  2382  		strcpy(info, "LBA ");
5622af0f73c977 Damien Le Moal    2021-08-02  2383  	}
5622af0f73c977 Damien Le Moal    2021-08-02  2384  	info_ofst = 
strlen(info);
5622af0f73c977 Damien Le Moal    2021-08-02  2385  5622af0f73c977 Damien 
Le Moal    2021-08-02  2386  	/* config NCQ */
5622af0f73c977 Damien Le Moal    2021-08-02  2387  	return 
ata_dev_config_ncq(dev, info + info_ofst,
5622af0f73c977 Damien Le Moal    2021-08-02  2388  				  infosz - 
info_ofst);
5622af0f73c977 Damien Le Moal    2021-08-02  2389  }
5622af0f73c977 Damien Le Moal    2021-08-02  2390
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34103 bytes --]

[-- Attachment #3: Attached Message Part --]
[-- Type: text/plain, Size: 150 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-08-03  9:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02  9:02 [PATCH 0/7] libata cleanups and improvements Damien Le Moal
2021-08-02  9:02 ` [PATCH 1/7] libata: cleanup device sleep capability detection Damien Le Moal
2021-08-02  9:02 ` [PATCH 2/7] libata: cleanup ata_dev_configure() Damien Le Moal
2021-08-02  9:02 ` [PATCH 3/7] libata: cleanup NCQ priority handling Damien Le Moal
2021-08-02  9:02 ` [PATCH 4/7] libata: fix ata_read_log_page() warning Damien Le Moal
2021-08-02  9:02 ` [PATCH 5/7] libata: print feature list on device scan Damien Le Moal
2021-08-02  9:02 ` [PATCH 6/7] libahci: Introduce ncq_prio_supported sysfs sttribute Damien Le Moal
2021-08-02  9:02 ` [PATCH 7/7] scsi: mpt3sas: Introduce sas_ncq_prio_supported " Damien Le Moal
2021-08-02 16:00   ` Bart Van Assche
2021-08-02 22:52     ` Damien Le Moal
2021-08-03  7:55   ` Johannes Thumshirn
2021-08-03  8:03     ` Damien Le Moal
     [not found] <202108030231.7Ta42o7Q-lkp@intel.com>
2021-08-03  9:08 ` [PATCH 2/7] libata: cleanup ata_dev_configure() kernel test robot

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 NNTP newsgroup(s).