2023-05-28 00:02:51

by Randy Dunlap

[permalink] [raw]
Subject: scsi/stex time_after() build warnings

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.
--
~Randy
---
drivers/scsi/stex.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)

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;



2023-05-28 20:53:55

by Bart Van Assche

[permalink] [raw]
Subject: Re: scsi/stex time_after() build warnings

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.

2023-05-28 23:08:10

by Randy Dunlap

[permalink] [raw]
Subject: Re: scsi/stex time_after() build warnings

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 <[email protected]>
Tested-by: Randy Dunlap <[email protected]> # build-tested

--
~Randy