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

[Xinhui Yang <cyan@cyano.uk>]

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