scsi/stex time_after() build warnings

Message ID 89df78d0-fb52-9045-2074-1d6f0afae4d9@infradead.org
State New
Headers
Series scsi/stex time_after() build warnings |

Commit Message

Randy Dunlap May 27, 2023, 11:08 p.m. UTC
  Hi,

When I build stex.o for i386, I see these build warnings:

In file included from ../include/linux/bitops.h:7,
                 from ../include/linux/kernel.h:22,
                 from ../drivers/scsi/stex.c:13:
../drivers/scsi/stex.c: In function ‘stex_common_handshake’:
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1035:29: note: in expansion of macro ‘time_after’
 1035 |                         if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
      |                             ^~~~~~~~~~
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1085:21: note: in expansion of macro ‘time_after’
 1085 |                 if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
      |                     ^~~~~~~~~~
../drivers/scsi/stex.c: In function ‘stex_ss_handshake’:
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1121:29: note: in expansion of macro ‘time_after’
 1121 |                         if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
      |                             ^~~~~~~~~~
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1133:29: note: in expansion of macro ‘time_after’
 1133 |                         if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
      |                             ^~~~~~~~~~
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1186:29: note: in expansion of macro ‘time_after’
 1186 |                         if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
      |                             ^~~~~~~~~~
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1199:29: note: in expansion of macro ‘time_after’
 1199 |                         if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
      |                             ^~~~~~~~~~
../drivers/scsi/stex.c: In function ‘stex_yos_reset’:
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1355:21: note: in expansion of macro ‘time_after’
 1355 |                 if (time_after(jiffies, before + ST_INTERNAL_TIMEOUT * HZ)) {
      |                     ^~~~~~~~~~
../drivers/scsi/stex.c: In function ‘stex_hba_stop’:
../include/linux/typecheck.h:12:25: warning: comparison of distinct pointer types lacks a cast
   12 |         (void)(&__dummy == &__dummy2); \
      |                         ^~
../include/linux/jiffies.h:106:10: note: in expansion of macro ‘typecheck’
  106 |          typecheck(unsigned long, b) && \
      |          ^~~~~~~~~
../drivers/scsi/stex.c:1902:21: note: in expansion of macro ‘time_after’
 1902 |                 if (time_after(jiffies, before + ST_INTERNAL_TIMEOUT * HZ)) {
      |                     ^~~~~~~~~~



but I don't see this type of warning for any other SCSI drivers
and I don't see this problem for stex.o on x86_64.

Does anyone have a clue about why this happens?


If I modify each timer_after() check to use a calculated unsigned long
variable, the warnings do not happen (as in the patch below).

Thanks.
  

Comments

Bart Van Assche May 28, 2023, 8:30 p.m. UTC | #1
On 5/27/23 16:08, Randy Dunlap wrote:
> When I build stex.o for i386, I see these build warnings:

Are you perhaps using gcc 13?

Has the following alternative patch been considered? I think this patch 
is lower risk than the patch in the original email:

diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index 5b230e149c3d..8ffb75be99bc 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -109,7 +109,9 @@ enum {
  	TASK_ATTRIBUTE_HEADOFQUEUE		= 0x1,
  	TASK_ATTRIBUTE_ORDERED			= 0x2,
  	TASK_ATTRIBUTE_ACA			= 0x4,
+};

+enum {
  	SS_STS_NORMAL				= 0x80000000,
  	SS_STS_DONE				= 0x40000000,
  	SS_STS_HANDSHAKE			= 0x20000000,
@@ -121,7 +123,9 @@ enum {
  	SS_I2H_REQUEST_RESET			= 0x2000,

  	SS_MU_OPERATIONAL			= 0x80000000,
+};

+enum {
  	STEX_CDB_LENGTH				= 16,
  	STATUS_VAR_LEN				= 128,


Thanks,

Bart.
  
Randy Dunlap May 28, 2023, 10:24 p.m. UTC | #2
Hi Bart,

On 5/28/23 13:30, Bart Van Assche wrote:
> On 5/27/23 16:08, Randy Dunlap wrote:
>> When I build stex.o for i386, I see these build warnings:
> 
> Are you perhaps using gcc 13?

Yes.

> Has the following alternative patch been considered? I think this patch is lower risk than the patch in the original email:
> 
> diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
> index 5b230e149c3d..8ffb75be99bc 100644
> --- a/drivers/scsi/stex.c
> +++ b/drivers/scsi/stex.c
> @@ -109,7 +109,9 @@ enum {
>      TASK_ATTRIBUTE_HEADOFQUEUE        = 0x1,
>      TASK_ATTRIBUTE_ORDERED            = 0x2,
>      TASK_ATTRIBUTE_ACA            = 0x4,
> +};
> 
> +enum {
>      SS_STS_NORMAL                = 0x80000000,
>      SS_STS_DONE                = 0x40000000,
>      SS_STS_HANDSHAKE            = 0x20000000,
> @@ -121,7 +123,9 @@ enum {
>      SS_I2H_REQUEST_RESET            = 0x2000,
> 
>      SS_MU_OPERATIONAL            = 0x80000000,
> +};
> 
> +enum {
>      STEX_CDB_LENGTH                = 16,
>      STATUS_VAR_LEN                = 128,
> 
> 

Yes, that also works. Thanks.

Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org> # build-tested
  

Patch

diff -- a/drivers/scsi/stex.c b/drivers/scsi/stex.c
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -1025,14 +1025,15 @@  static int stex_common_handshake(struct
 	struct handshake_frame *h;
 	dma_addr_t status_phys;
 	u32 data;
-	unsigned long before;
+	unsigned long before, timeout;
 
 	if (readl(base + OMR0) != MU_HANDSHAKE_SIGNATURE) {
 		writel(MU_INBOUND_DOORBELL_HANDSHAKE, base + IDBL);
 		readl(base + IDBL);
 		before = jiffies;
 		while (readl(base + OMR0) != MU_HANDSHAKE_SIGNATURE) {
-			if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
+			timeout = before + MU_MAX_DELAY * HZ;
+			if (time_after(jiffies, timeout)) {
 				printk(KERN_ERR DRV_NAME
 					"(%s): no handshake signature\n",
 					pci_name(hba->pdev));
@@ -1082,7 +1083,8 @@  static int stex_common_handshake(struct
 	udelay(10);
 	before = jiffies;
 	while (readl(base + OMR0) != MU_HANDSHAKE_SIGNATURE) {
-		if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
+		timeout = before + MU_MAX_DELAY * HZ;
+		if (time_after(jiffies, timeout)) {
 			printk(KERN_ERR DRV_NAME
 				"(%s): no signature after handshake frame\n",
 				pci_name(hba->pdev));
@@ -1110,7 +1112,7 @@  static int stex_ss_handshake(struct st_h
 	struct handshake_frame *h;
 	__le32 *scratch;
 	u32 data, scratch_size, mailboxdata, operationaldata;
-	unsigned long before;
+	unsigned long before, timeout;
 	int ret = 0;
 
 	before = jiffies;
@@ -1118,7 +1120,8 @@  static int stex_ss_handshake(struct st_h
 	if (hba->cardtype == st_yel) {
 		operationaldata = readl(base + YIOA_STATUS);
 		while (operationaldata != SS_MU_OPERATIONAL) {
-			if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
+			timeout = before + MU_MAX_DELAY * HZ;
+			if (time_after(jiffies, timeout)) {
 				printk(KERN_ERR DRV_NAME
 					"(%s): firmware not operational\n",
 					pci_name(hba->pdev));
@@ -1130,7 +1133,8 @@  static int stex_ss_handshake(struct st_h
 	} else {
 		operationaldata = readl(base + PSCRATCH3);
 		while (operationaldata != SS_MU_OPERATIONAL) {
-			if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
+			timeout = before + MU_MAX_DELAY * HZ;
+			if (time_after(jiffies, timeout)) {
 				printk(KERN_ERR DRV_NAME
 					"(%s): firmware not operational\n",
 					pci_name(hba->pdev));
@@ -1183,7 +1187,8 @@  static int stex_ss_handshake(struct st_h
 	scratch = hba->scratch;
 	if (hba->cardtype == st_yel) {
 		while (!(le32_to_cpu(*scratch) & SS_STS_HANDSHAKE)) {
-			if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
+			timeout = before + MU_MAX_DELAY * HZ;
+			if (time_after(jiffies, timeout)) {
 				printk(KERN_ERR DRV_NAME
 					"(%s): no signature after handshake frame\n",
 					pci_name(hba->pdev));
@@ -1196,7 +1201,8 @@  static int stex_ss_handshake(struct st_h
 	} else {
 		mailboxdata = readl(base + MAILBOX_BASE + MAILBOX_HNDSHK_STS);
 		while (mailboxdata != SS_STS_HANDSHAKE) {
-			if (time_after(jiffies, before + MU_MAX_DELAY * HZ)) {
+			timeout = before + MU_MAX_DELAY * HZ;
+			if (time_after(jiffies, timeout)) {
 				printk(KERN_ERR DRV_NAME
 					"(%s): no signature after handshake frame\n",
 					pci_name(hba->pdev));
@@ -1344,7 +1350,7 @@  static void stex_hard_reset(struct st_hb
 static int stex_yos_reset(struct st_hba *hba)
 {
 	void __iomem *base;
-	unsigned long flags, before;
+	unsigned long flags, before, timeout;
 	int ret = 0;
 
 	base = hba->mmio_base;
@@ -1352,7 +1358,8 @@  static int stex_yos_reset(struct st_hba
 	readl(base + IDBL); /* flush */
 	before = jiffies;
 	while (hba->out_req_cnt > 0) {
-		if (time_after(jiffies, before + ST_INTERNAL_TIMEOUT * HZ)) {
+		timeout = before + ST_INTERNAL_TIMEOUT * HZ;
+		if (time_after(jiffies, timeout)) {
 			printk(KERN_WARNING DRV_NAME
 				"(%s): reset timeout\n", pci_name(hba->pdev));
 			ret = -1;
@@ -1852,7 +1859,7 @@  static void stex_hba_stop(struct st_hba
 	struct req_msg *req;
 	struct st_msg_header *msg_h;
 	unsigned long flags;
-	unsigned long before;
+	unsigned long before, timeout;
 	u16 tag = 0;
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
@@ -1899,7 +1906,8 @@  static void stex_hba_stop(struct st_hba
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	before = jiffies;
 	while (hba->ccb[tag].req_type & PASSTHRU_REQ_TYPE) {
-		if (time_after(jiffies, before + ST_INTERNAL_TIMEOUT * HZ)) {
+		timeout = before + ST_INTERNAL_TIMEOUT * HZ;
+		if (time_after(jiffies, timeout)) {
 			hba->ccb[tag].req_type = 0;
 			hba->mu_status = MU_STATE_STOP;
 			return;