2023-10-12 23:29:36

by Soumya Negi

[permalink] [raw]
Subject: [PATCH v2 0/2] staging: rts5208: Fix checkpatch macro warnings

In the driver staging/rts5208, checkpatch.pl warns of possible precedence
issues in the header rtsx.h because macro arguments are not parenthesized.
This patch set handles the respective macros differently(as needed) in
each patch to fix the warnings. Patches can be applied in any order.

Soumya Negi (2):
staging: rts5208: Refactor macros to static inline functions
staging: rts5208: Remove macros scsi_lock(), scsi_unlock()
---
v2: No change since v1. Resending patch set because the patches were not
delivered correctly on first attempt.

drivers/staging/rts5208/rtsx.c | 24 ++++++-------
drivers/staging/rts5208/rtsx.h | 66 ++++++++++++++++++++--------------
2 files changed, 52 insertions(+), 38 deletions(-)

--
2.42.0


2023-10-12 23:29:51

by Soumya Negi

[permalink] [raw]
Subject: [PATCH v2 1/2] staging: rts5208: Refactor macros to static inline functions

Driver rts5208 uses macros to read/write data & to perform generic PCI
functions. Rewrite these macros as static inline functions in the header
file.

Suggested-by: Dan Carpenter <[email protected]>
Suggested-by: Julia Lawall <[email protected]>
Signed-off-by: Soumya Negi <[email protected]>
---
Note: The macros wait_timeout_x() & wait_timeout() have not been redfined as
functions because they pass defined hex constants as arguments(hence can't be
type-checked).

drivers/staging/rts5208/rtsx.h | 59 +++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx.h b/drivers/staging/rts5208/rtsx.h
index 2e101da83220..ea29646b8c38 100644
--- a/drivers/staging/rts5208/rtsx.h
+++ b/drivers/staging/rts5208/rtsx.h
@@ -39,25 +39,6 @@
/*
* macros for easy use
*/
-#define rtsx_writel(chip, reg, value) \
- iowrite32(value, (chip)->rtsx->remap_addr + reg)
-#define rtsx_readl(chip, reg) \
- ioread32((chip)->rtsx->remap_addr + reg)
-#define rtsx_writew(chip, reg, value) \
- iowrite16(value, (chip)->rtsx->remap_addr + reg)
-#define rtsx_readw(chip, reg) \
- ioread16((chip)->rtsx->remap_addr + reg)
-#define rtsx_writeb(chip, reg, value) \
- iowrite8(value, (chip)->rtsx->remap_addr + reg)
-#define rtsx_readb(chip, reg) \
- ioread8((chip)->rtsx->remap_addr + reg)
-
-#define rtsx_read_config_byte(chip, where, val) \
- pci_read_config_byte((chip)->rtsx->pci, where, val)
-
-#define rtsx_write_config_byte(chip, where, val) \
- pci_write_config_byte((chip)->rtsx->pci, where, val)
-
#define wait_timeout_x(task_state, msecs) \
do { \
set_current_state((task_state)); \
@@ -147,4 +128,44 @@ enum xfer_buf_dir {TO_XFER_BUF, FROM_XFER_BUF};
#include "rtsx_sys.h"
#include "general.h"

+static inline void rtsx_writel(struct rtsx_chip *chip, u32 reg, u32 value)
+{
+ iowrite32(value, chip->rtsx->remap_addr + reg);
+}
+
+static inline u32 rtsx_readl(struct rtsx_chip *chip, u32 reg)
+{
+ return ioread32(chip->rtsx->remap_addr + reg);
+}
+
+static inline void rtsx_writew(struct rtsx_chip *chip, u32 reg, u16 value)
+{
+ iowrite16(value, chip->rtsx->remap_addr + reg);
+}
+
+static inline u16 rtsx_readw(struct rtsx_chip *chip, u32 reg)
+{
+ return ioread16(chip->rtsx->remap_addr + reg);
+}
+
+static inline void rtsx_writeb(struct rtsx_chip *chip, u32 reg, u8 value)
+{
+ iowrite8(value, chip->rtsx->remap_addr + reg);
+}
+
+static inline u8 rtsx_readb(struct rtsx_chip *chip, u32 reg)
+{
+ return ioread8((chip)->rtsx->remap_addr + reg);
+}
+
+static inline int rtsx_read_config_byte(struct rtsx_chip *chip, int where, u8 *val)
+{
+ return pci_read_config_byte(chip->rtsx->pci, where, val);
+}
+
+static inline int rtsx_write_config_byte(struct rtsx_chip *chip, int where, u8 val)
+{
+ return pci_write_config_byte(chip->rtsx->pci, where, val);
+}
+
#endif /* __REALTEK_RTSX_H */
--
2.42.0

2023-10-12 23:29:57

by Soumya Negi

[permalink] [raw]
Subject: [PATCH v2 2/2] staging: rts5208: Remove macros scsi_lock(), scsi_unlock()

The scsi_lock() and scsi_unlock() macros protect the sm_state and the
single queue element srb for write access in the driver. As suggested,
these names are very generic. Remove the macros from header file and call
spin_lock_irq() & spin_unlock_irq() directly instead.

Suggested-by: Dan Carpenter <[email protected]>
Suggested-by: Julia Lawall <[email protected]>
Signed-off-by: Soumya Negi <[email protected]>
---
drivers/staging/rts5208/rtsx.c | 24 ++++++++++++------------
drivers/staging/rts5208/rtsx.h | 7 -------
2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
index 08543a3936da..86d32e3b3282 100644
--- a/drivers/staging/rts5208/rtsx.c
+++ b/drivers/staging/rts5208/rtsx.c
@@ -117,7 +117,7 @@ static int slave_configure(struct scsi_device *sdev)
} while (0)

/* queue a command */
-/* This is always called with scsi_lock(host) held */
+/* This is always called with spin_lock_irq(host->host_lock) held */
static int queuecommand_lck(struct scsi_cmnd *srb)
{
void (*done)(struct scsi_cmnd *) = scsi_done;
@@ -159,18 +159,18 @@ static int command_abort(struct scsi_cmnd *srb)
struct rtsx_dev *dev = host_to_rtsx(host);
struct rtsx_chip *chip = dev->chip;

- scsi_lock(host);
+ spin_lock_irq(host->host_lock);

/* Is this command still active? */
if (chip->srb != srb) {
- scsi_unlock(host);
+ spin_unlock_irq(host->host_lock);
dev_info(&dev->pci->dev, "-- nothing to abort\n");
return FAILED;
}

rtsx_set_stat(chip, RTSX_STAT_ABORT);

- scsi_unlock(host);
+ spin_unlock_irq(host->host_lock);

/* Wait for the aborted command to finish */
wait_for_completion(&dev->notify);
@@ -366,7 +366,7 @@ static int rtsx_control_thread(void *__dev)
}

/* lock access to the state */
- scsi_lock(host);
+ spin_lock_irq(host->host_lock);

/* has the command aborted ? */
if (rtsx_chk_stat(chip, RTSX_STAT_ABORT)) {
@@ -374,7 +374,7 @@ static int rtsx_control_thread(void *__dev)
goto skip_for_abort;
}

- scsi_unlock(host);
+ spin_unlock_irq(host->host_lock);

/* reject the command if the direction indicator
* is UNKNOWN
@@ -402,7 +402,7 @@ static int rtsx_control_thread(void *__dev)
}

/* lock access to the state */
- scsi_lock(host);
+ spin_lock_irq(host->host_lock);

/* did the command already complete because of a disconnect? */
if (!chip->srb)
@@ -424,7 +424,7 @@ static int rtsx_control_thread(void *__dev)

/* finished working on this command */
chip->srb = NULL;
- scsi_unlock(host);
+ spin_unlock_irq(host->host_lock);

/* unlock the device pointers */
mutex_unlock(&dev->dev_mutex);
@@ -603,9 +603,9 @@ static void quiesce_and_remove_host(struct rtsx_dev *dev)
* interrupt a SCSI-scan or device-reset delay
*/
mutex_lock(&dev->dev_mutex);
- scsi_lock(host);
+ spin_lock_irq(host->host_lock);
rtsx_set_stat(chip, RTSX_STAT_DISCONNECT);
- scsi_unlock(host);
+ spin_unlock_irq(host->host_lock);
mutex_unlock(&dev->dev_mutex);
wake_up(&dev->delay_wait);
wait_for_completion(&dev->scanning_done);
@@ -621,10 +621,10 @@ static void quiesce_and_remove_host(struct rtsx_dev *dev)
mutex_lock(&dev->dev_mutex);
if (chip->srb) {
chip->srb->result = DID_NO_CONNECT << 16;
- scsi_lock(host);
+ spin_lock_irq(host->host_lock);
scsi_done(dev->chip->srb);
chip->srb = NULL;
- scsi_unlock(host);
+ spin_unlock_irq(host->host_lock);
}
mutex_unlock(&dev->dev_mutex);

diff --git a/drivers/staging/rts5208/rtsx.h b/drivers/staging/rts5208/rtsx.h
index ea29646b8c38..ec6f5b07390b 100644
--- a/drivers/staging/rts5208/rtsx.h
+++ b/drivers/staging/rts5208/rtsx.h
@@ -108,13 +108,6 @@ static inline struct rtsx_dev *host_to_rtsx(struct Scsi_Host *host)
return (struct rtsx_dev *)host->hostdata;
}

-/*
- * The scsi_lock() and scsi_unlock() macros protect the sm_state and the
- * single queue element srb for write access
- */
-#define scsi_unlock(host) spin_unlock_irq(host->host_lock)
-#define scsi_lock(host) spin_lock_irq(host->host_lock)
-
#define lock_state(chip) spin_lock_irq(&((chip)->rtsx->reg_lock))
#define unlock_state(chip) spin_unlock_irq(&((chip)->rtsx->reg_lock))

--
2.42.0