Skip to contentSkip to conversation

scsi: ufs-qcom: remove redundant condition check

PATCH In review Subhash Jadavani 2 participants 3 messages Feb 17, 2017 – Feb 22, 2017 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 scsi: ufs-qcom: remove redundant condition check

Dan Carpenter <[email protected]> reported this:
---
The patch 9c46b8676271: "scsi: ufs-qcom: dump additional testbus registers" from Feb 3, 2017, leads to the following static checker warning:

    drivers/scsi/ufs/ufs-qcom.c:1531 ufs_qcom_testbus_cfg_is_ok()
    warn: impossible condition
		'(host->testbus.select_minor > 255) => (0-255 > 255)'

drivers/scsi/ufs/ufs-qcom.c
1517 static bool ufs_qcom_testbus_cfg_is_ok(struct ufs_qcom_host *host) 1518 {
1519 if (host->testbus.select_major >= TSTBUS_MAX) {
1520 dev_err(host->hba->dev,
1521 "%s: UFS_CFG1[TEST_BUS_SEL} may not equal 0x%05X\n",
1522 __func__, host->testbus.select_major);
1523 return false;
1524 }
1525
1526 /*

  1527           * Not performing check for each individual select_major
  1528           * mappings of select_minor, since there is no harm in
  1529           * configuring a non-existent select_minor
  1530           */
  1531          if (host->testbus.select_minor > 0xFF) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It might make sense to keep this check. I don't know. But it's confusing that 0xFF is a magic number. Better to make it a define.

1532 dev_err(host->hba->dev,
1533 "%s: 0x%05X is not a legal testbus option\n",
1534 __func__, host->testbus.select_minor);
1535 return false;
1536 }
1537
1538 return true;
1539 }
---

As data type of "select_minor" is u8, above check is redundant. This change removes it.

---
 drivers/scsi/ufs/ufs-qcom.c | 12 ------------
 1 file changed, 12 deletions(-)
drivers/scsi/ufs/ufs-qcom.c +0 -12
index ce5d023..c87d770 100644
@@ -1523,18 +1523,6 @@ static bool ufs_qcom_testbus_cfg_is_ok(struct ufs_qcom_host *host)
 		return false;
 	}
 
-	/*
-	 * Not performing check for each individual select_major
-	 * mappings of select_minor, since there is no harm in
-	 * configuring a non-existent select_minor
-	 */
-	if (host->testbus.select_minor > 0xFF) {
-		dev_err(host->hba->dev,
-			"%s: 0x%05X is not a legal testbus option\n",
-			__func__, host->testbus.select_minor);
-		return false;
-	}
-
 	return true;
 }
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Subhash" == Subhash Jadavani <[email protected]> writes:

Subhash,

Subhash> Dan Carpenter <[email protected]> reported this: --- The Subhash> patch 9c46b8676271: "scsi: ufs-qcom: dump additional testbus Subhash> registers" from Feb 3, 2017, leads to the following static Subhash> checker warning:

You had used "---" to enclose Dan's email. But that's also the "ignore everything beyond this point" delimiter so I ended up with an empty commit message.

I fixed it up. Applied to 4.11/scsi-queue.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
#2 lore raw
Quoted 9 lines · Martin K. Petersen
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Subhash" == Subhash Jadavani <[email protected]> writes:

Subhash,

Subhash> Dan Carpenter <[email protected]> reported this: --- The Subhash> patch 9c46b8676271: "scsi: ufs-qcom: dump additional testbus Subhash> registers" from Feb 3, 2017, leads to the following static Subhash> checker warning:

Quoted 5 lines

You had used "---" to enclose Dan's email. But that's also the "ignore everything beyond this point" delimiter so I ended up with an empty commit message.

I fixed it up. Applied to 4.11/scsi-queue.

Thanks and sorry about that, was skeptical about it.

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project