[PATCH v3 0/2] dc395x: fix compiler warnings and improve formatting of the macros

[PATCH v3 0/2] dc395x: fix compiler warnings and improve formatting of the macros

[Xinhui Yang]

This series of patches clears the compiler warnings for the dc395x
driver. 

The first patch introduces a new macro that casts the value returned by
a read operation to void, since some values returned by some specific
read operations (which just simply clears the FIFO buffer or resets the
interrupt status) can be ignored. Creating a new macro that casts the
return value to void to fix the warning.

During the fix, checkpatch.pl complained about missing whitespace
between macro arguments and missing parentheses around complex
expressions. To align with the changes in the first patch, the
formatting of macros above and below the introduced macro are also
fixed.

Since in Patch v2 [1] Bart pointed out that such change can't be made
to the stable tree, the patch is splitted to two parts.

---
Changes since v2 [1]:
- Split the patch into two parts, the first one fixes the warning, and
  the second one improves the formatting of the surrounding macros.
- Make the description of the formatting changes more clear.

Changes since v1 [2]:
- Add Cc: tag to include this patch to the stable tree.
- Add additional description about the formatting changes.

[1]: https://lore.kernel.org/linux-scsi/20250922152609.827311-1-cyan@cyano.uk/
[2]: https://lore.kernel.org/linux-scsi/20250922143619.824129-1-cyan@cyano.uk/

---
Xinhui Yang (2):
  scsi: dc395x: correctly discard the return value in certain reads
  scsi: dc395x: improve code formatting for the macros

 drivers/scsi/dc395x.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

-- 
2.51.0

[PATCH v3 1/2] scsi: dc395x: correctly discard the return value in certain reads

[Xinhui Yang]

There are certain read operations performed in this code which doesn't
really don't need its return value. Those read operations either clears
the FIFO buffer, or clears the interruption status. However, unused read
triggers compiler warnings. With CONFIG_WERROR on, these warnings get
converted into errors:

drivers/scsi/dc395x.c: In function ‘__dc395x_eh_bus_reset’:
drivers/scsi/dc395x.c:97:49: error: value computed is not used [-Werror=unused-value]
   97 | #define DC395x_read8(acb,address)               (u8)(inb(acb->io_port_base + (address)))
      |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/dc395x.c:1003:9: note: in expansion of macro ‘DC395x_read8’
 1003 |         DC395x_read8(acb, TRM_S1040_SCSI_INTSTATUS);
      |         ^~~~~~~~~~~~
drivers/scsi/dc395x.c: In function ‘data_io_transfer’:
drivers/scsi/dc395x.c:97:49: error: value computed is not used [-Werror=unused-value]
   97 | #define DC395x_read8(acb,address)               (u8)(inb(acb->io_port_base + (address)))
      |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/dc395x.c:2032:33: note: in expansion of macro ‘DC395x_read8’
 2032 |                                 DC395x_read8(acb, TRM_S1040_SCSI_FIFO);

Create a new macro DC395x_peek8() to deliberately cast the return value
to void, which tells the compiler we really don't need the return value
of such read operations.

Cc: stable@vger.kernel.org
Signed-off-by: Xinhui Yang <cyan@cyano.uk>
---
 drivers/scsi/dc395x.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 386c8359e1cc..aed4f21e8143 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -94,6 +94,12 @@
 #define DC395x_LOCK_IO(dev,flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock, flags)
 #define DC395x_UNLOCK_IO(dev,flags)		spin_unlock_irqrestore(((struct Scsi_Host *)dev)->host_lock, flags)
 
+/*
+ * read operations that may trigger side effects in the hardware,
+ * but the value can or should be discarded.
+ */
+#define DC395x_peek8(acb, address)		((void)(inb(acb->io_port_base + (address))))
+/* normal read write operations goes here. */
 #define DC395x_read8(acb,address)		(u8)(inb(acb->io_port_base + (address)))
 #define DC395x_read16(acb,address)		(u16)(inw(acb->io_port_base + (address)))
 #define DC395x_read32(acb,address)		(u32)(inl(acb->io_port_base + (address)))
@@ -1000,7 +1006,7 @@ static int __dc395x_eh_bus_reset(struct scsi_cmnd *cmd)
 	DC395x_write8(acb, TRM_S1040_DMA_CONTROL, CLRXFIFO);
 	clear_fifo(acb, "eh_bus_reset");
 	/* Delete pending IRQ */
-	DC395x_read8(acb, TRM_S1040_SCSI_INTSTATUS);
+	DC395x_peek8(acb, TRM_S1040_SCSI_INTSTATUS);
 	set_basic_config(acb);
 
 	reset_dev_param(acb);
@@ -2029,8 +2035,8 @@ static void data_io_transfer(struct AdapterCtlBlk *acb,
 			DC395x_write8(acb, TRM_S1040_SCSI_CONFIG2,
 				      CFG2_WIDEFIFO);
 			if (io_dir & DMACMD_DIR) {
-				DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
-				DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
+				DC395x_peek8(acb, TRM_S1040_SCSI_FIFO);
+				DC395x_peek8(acb, TRM_S1040_SCSI_FIFO);
 			} else {
 				/* Danger, Robinson: If you find KGs
 				 * scattered over the wide disk, the driver
@@ -2044,7 +2050,7 @@ static void data_io_transfer(struct AdapterCtlBlk *acb,
 			/* Danger, Robinson: If you find a collection of Ks on your disk
 			 * something broke :-( */
 			if (io_dir & DMACMD_DIR)
-				DC395x_read8(acb, TRM_S1040_SCSI_FIFO);
+				DC395x_peek8(acb, TRM_S1040_SCSI_FIFO);
 			else
 				DC395x_write8(acb, TRM_S1040_SCSI_FIFO, 'K');
 		}
@@ -2892,7 +2898,7 @@ static void set_basic_config(struct AdapterCtlBlk *acb)
 	    DMA_FIFO_HALF_HALF | DMA_ENHANCE /*| DMA_MEM_MULTI_READ */ ;
 	DC395x_write16(acb, TRM_S1040_DMA_CONFIG, wval);
 	/* Clear pending interrupt status */
-	DC395x_read8(acb, TRM_S1040_SCSI_INTSTATUS);
+	DC395x_peek8(acb, TRM_S1040_SCSI_INTSTATUS);
 	/* Enable SCSI interrupt    */
 	DC395x_write8(acb, TRM_S1040_SCSI_INTEN, 0x7F);
 	DC395x_write8(acb, TRM_S1040_DMA_INTEN, EN_SCSIINTR | EN_DMAXFERERROR
@@ -3799,7 +3805,7 @@ static void adapter_uninit_chip(struct AdapterCtlBlk *acb)
 		reset_scsi_bus(acb);
 
 	/* clear any pending interrupt state */
-	DC395x_read8(acb, TRM_S1040_SCSI_INTSTATUS);
+	DC395x_peek8(acb, TRM_S1040_SCSI_INTSTATUS);
 }
 
 
-- 
2.51.0

[PATCH v3 2/2] scsi: dc395x: improve code formatting for the macros

[Xinhui Yang]

These DC395x_* macros does not have white spaces around their arguments,
thus checkpatch.pl throws an error for each change in the macros.

Also, there are no surrounding parentheses in the expressions for the
read and write macros, which checkpatch.pl also complained about.

This patch does only formatting improvements to make the macro
definitions align with the previous patch.

Signed-off-by: Xinhui Yang <cyan@cyano.uk>
---
 drivers/scsi/dc395x.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index aed4f21e8143..cff6fa20e53c 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -91,8 +91,8 @@
 #endif
 
 
-#define DC395x_LOCK_IO(dev,flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock, flags)
-#define DC395x_UNLOCK_IO(dev,flags)		spin_unlock_irqrestore(((struct Scsi_Host *)dev)->host_lock, flags)
+#define DC395x_LOCK_IO(dev, flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock, flags)
+#define DC395x_UNLOCK_IO(dev, flags)		spin_unlock_irqrestore(((struct Scsi_Host *)dev)->host_lock, flags)
 
 /*
  * read operations that may trigger side effects in the hardware,
@@ -100,12 +100,12 @@
  */
 #define DC395x_peek8(acb, address)		((void)(inb(acb->io_port_base + (address))))
 /* normal read write operations goes here. */
-#define DC395x_read8(acb,address)		(u8)(inb(acb->io_port_base + (address)))
-#define DC395x_read16(acb,address)		(u16)(inw(acb->io_port_base + (address)))
-#define DC395x_read32(acb,address)		(u32)(inl(acb->io_port_base + (address)))
-#define DC395x_write8(acb,address,value)	outb((value), acb->io_port_base + (address))
-#define DC395x_write16(acb,address,value)	outw((value), acb->io_port_base + (address))
-#define DC395x_write32(acb,address,value)	outl((value), acb->io_port_base + (address))
+#define DC395x_read8(acb, address)		((u8)    (inb(acb->io_port_base + (address))))
+#define DC395x_read16(acb, address)		((u16)   (inw(acb->io_port_base + (address))))
+#define DC395x_read32(acb, address)		((u32)   (inl(acb->io_port_base + (address))))
+#define DC395x_write8(acb, address, value)	(outb((value), acb->io_port_base + (address)))
+#define DC395x_write16(acb, address, value)	(outw((value), acb->io_port_base + (address)))
+#define DC395x_write32(acb, address, value)	(outl((value), acb->io_port_base + (address)))
 
 #define TAG_NONE 255
 
-- 
2.51.0

Re: [PATCH v3 2/2] scsi: dc395x: improve code formatting for the macros

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree.
Subject: [PATCH v3 2/2] scsi: dc395x: improve code formatting for the macros
Link: https://lore.kernel.org/stable/20250923125226.1883391-3-cyan%40cyano.uk

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 2/2] scsi: dc395x: improve code formatting for the macros

[James Bottomley]

On Tue, 2025-09-23 at 20:52 +0800, Xinhui Yang wrote:
> These DC395x_* macros does not have white spaces around their
> arguments,
> thus checkpatch.pl throws an error for each change in the macros.
> 
> Also, there are no surrounding parentheses in the expressions for the
> read and write macros, which checkpatch.pl also complained about.
> 
> This patch does only formatting improvements to make the macro
> definitions align with the previous patch.
> 
> Signed-off-by: Xinhui Yang <cyan@cyano.uk>
> ---
>  drivers/scsi/dc395x.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
> index aed4f21e8143..cff6fa20e53c 100644
> --- a/drivers/scsi/dc395x.c
> +++ b/drivers/scsi/dc395x.c
> @@ -91,8 +91,8 @@
>  #endif
>  
>  
> -#define
> DC395x_LOCK_IO(dev,flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock,flags)
> -#define
> DC395x_UNLOCK_IO(dev,flags)		spin_unlock_irqrestore(((struct Scsi_Host*)dev)->host_lock,flags)
> +#define DC395x_LOCK_IO(dev,
> flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock, flags)
> +#define DC395x_UNLOCK_IO(dev,
> flags)		spin_unlock_irqrestore(((struct Scsi_Host *)dev)->host_lock, flags)
>  
>  /*
>   * read operations that may trigger side effects in the hardware,
> @@ -100,12 +100,12 @@
>   */
>  #define DC395x_peek8(acb, address)		((void)(inb(acb-
> >io_port_base + (address))))
>  /* normal read write operations goes here. */
> -#define DC395x_read8(acb,address)		(u8)(inb(acb-
> >io_port_base + (address)))
> -#define DC395x_read16(acb,address)		(u16)(inw(acb-
> >io_port_base + (address)))
> -#define DC395x_read32(acb,address)		(u32)(inl(acb-
> >io_port_base + (address)))
> -#define DC395x_write8(acb,address,value)	outb((value), acb-
> >io_port_base + (address))
> -#define DC395x_write16(acb,address,value)	outw((value), acb-
> >io_port_base + (address))
> -#define DC395x_write32(acb,address,value)	outl((value), acb-
> >io_port_base + (address))
> +#define DC395x_read8(acb, address)		((u8)    (inb(acb-
> >io_port_base + (address))))
> +#define DC395x_read16(acb, address)		((u16)   (inw(acb-
> >io_port_base + (address))))
> +#define DC395x_read32(acb, address)		((u32)   (inl(acb-
> >io_port_base + (address))))

This doesn't look right.  The problem checkpatch is complaining about
is surely that the cast makes it a compound statement.  However, since
inb inw and inl all return the types they're being cast to the correct
solution is surely to remove the cast making these single statements
that don't need parentheses.

> +#define DC395x_write8(acb, address, value)	(outb((value), acb-
> >io_port_base + (address)))
> +#define DC395x_write16(acb, address, value)	(outw((value), acb-
> >io_port_base + (address)))
> +#define DC395x_write32(acb, address, value)	(outl((value), acb-
> >io_port_base + (address)))

And these are single statements which shouldn't need parentheses.  Are
you sure checkpatch is complaining about this, because if it is then
checkpatch needs fixing.

Regards,

James

Re: [PATCH v3 2/2] scsi: dc395x: improve code formatting for the macros

[Xinhui Yang]

Hi James,

在 2025/9/23 21:09, James Bottomley 写道:
> On Tue, 2025-09-23 at 20:52 +0800, Xinhui Yang wrote:
>> These DC395x_* macros does not have white spaces around their
>> arguments,
>> thus checkpatch.pl throws an error for each change in the macros.
>>
>> Also, there are no surrounding parentheses in the expressions for the
>> read and write macros, which checkpatch.pl also complained about.
>>
>> This patch does only formatting improvements to make the macro
>> definitions align with the previous patch.
>>
>> Signed-off-by: Xinhui Yang <cyan@cyano.uk>
>> ---
>>  drivers/scsi/dc395x.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
>> index aed4f21e8143..cff6fa20e53c 100644
>> --- a/drivers/scsi/dc395x.c
>> +++ b/drivers/scsi/dc395x.c
>> @@ -91,8 +91,8 @@
>>  #endif
>>  
>>  
>> -#define
>> DC395x_LOCK_IO(dev,flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock,flags)
>> -#define
>> DC395x_UNLOCK_IO(dev,flags)		spin_unlock_irqrestore(((struct Scsi_Host*)dev)->host_lock,flags)
>> +#define DC395x_LOCK_IO(dev,
>> flags)		spin_lock_irqsave(((struct Scsi_Host *)dev)->host_lock, flags)
>> +#define DC395x_UNLOCK_IO(dev,
>> flags)		spin_unlock_irqrestore(((struct Scsi_Host *)dev)->host_lock, flags)
>>  
>>  /*
>>   * read operations that may trigger side effects in the hardware,
>> @@ -100,12 +100,12 @@
>>   */
>>  #define DC395x_peek8(acb, address)		((void)(inb(acb-
>>> io_port_base + (address))))
>>  /* normal read write operations goes here. */
>> -#define DC395x_read8(acb,address)		(u8)(inb(acb-
>>> io_port_base + (address)))
>> -#define DC395x_read16(acb,address)		(u16)(inw(acb-
>>> io_port_base + (address)))
>> -#define DC395x_read32(acb,address)		(u32)(inl(acb-
>>> io_port_base + (address)))
>> -#define DC395x_write8(acb,address,value)	outb((value), acb-
>>> io_port_base + (address))
>> -#define DC395x_write16(acb,address,value)	outw((value), acb-
>>> io_port_base + (address))
>> -#define DC395x_write32(acb,address,value)	outl((value), acb-
>>> io_port_base + (address))
>> +#define DC395x_read8(acb, address)		((u8)    (inb(acb-
>>> io_port_base + (address))))
>> +#define DC395x_read16(acb, address)		((u16)   (inw(acb-
>>> io_port_base + (address))))
>> +#define DC395x_read32(acb, address)		((u32)   (inl(acb-
>>> io_port_base + (address))))
> 
> This doesn't look right.  The problem checkpatch is complaining about
> is surely that the cast makes it a compound statement.  However, since
> inb inw and inl all return the types they're being cast to the correct
> solution is surely to remove the cast making these single statements
> that don't need parentheses.

Thanks, I checked the definitions and you are right - these read macros
should have their casts removed, as they now return u8, u16 and u32
respectively.

>> +#define DC395x_write8(acb, address, value)	(outb((value), acb-
>>> io_port_base + (address)))
>> +#define DC395x_write16(acb, address, value)	(outw((value), acb-
>>> io_port_base + (address)))
>> +#define DC395x_write32(acb, address, value)	(outl((value), acb-
>>> io_port_base + (address)))
> 
> And these are single statements which shouldn't need parentheses.  Are
> you sure checkpatch is complaining about this, because if it is then
> checkpatch needs fixing.

Thanks for pointing this out, they don't need additional parentheses.

> Regards,
> 
> James
> 

Thanks,
Xinhui