Skip to contentSkip to conversation

thunderbolt: rename shadowed variables bit to interrupt_bit and auto_clear_bit

PATCH Applied Tom Rix 3 participants 5 messages Mar 15, 2023 – Mar 20, 2023

Subsystems (2)
lore
Share & tools
lore

Tom Rix

author
#0 lore raw

PATCH 1/1 thunderbolt: rename shadowed variables bit to interrupt_bit and auto_clear_bit

cppcheck reports
drivers/thunderbolt/nhi.c:74:7: style: Local variable 'bit' shadows outer variable [shadowVariable]

  int bit;
      ^
drivers/thunderbolt/nhi.c:66:6: note: Shadowed declaration
 int bit = ring_interrupt_index(ring) & 31;
     ^
drivers/thunderbolt/nhi.c:74:7: note: Shadow variable
  int bit;
      ^

For readablity rename the outer to interrupt_bit and the innner to auto_clear_bit.

---
 drivers/thunderbolt/nhi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/thunderbolt/nhi.c +8 -8
index 318d20bd5b69..d0d26d288be8 100644
@@ -63,15 +63,15 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 {
 	int reg = REG_RING_INTERRUPT_BASE +
 		  ring_interrupt_index(ring) / 32 * 4;
-	int bit = ring_interrupt_index(ring) & 31;
-	int mask = 1 << bit;
+	int interrupt_bit = ring_interrupt_index(ring) & 31;
+	int mask = 1 << interrupt_bit;
 	u32 old, new;
 
 	if (ring->irq > 0) {
 		u32 step, shift, ivr, misc;
 		void __iomem *ivr_base;
+		int auto_clear_bit;
 		int index;
-		int bit;
 
 		if (ring->is_tx)
 			index = ring->hop;
@@ -91,11 +91,11 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 		 */
 		misc = ioread32(ring->nhi->iobase + REG_DMA_MISC);
 		if (ring->nhi->quirks & QUIRK_AUTO_CLEAR_INT)
-			bit = REG_DMA_MISC_INT_AUTO_CLEAR;
+			auto_clear_bit = REG_DMA_MISC_INT_AUTO_CLEAR;
 		else
-			bit = REG_DMA_MISC_DISABLE_AUTO_CLEAR;
-		if (!(misc & bit))
-			iowrite32(misc | bit, ring->nhi->iobase + REG_DMA_MISC);
+			auto_clear_bit = REG_DMA_MISC_DISABLE_AUTO_CLEAR;
+		if (!(misc & auto_clear_bit))
+			iowrite32(misc | auto_clear_bit, ring->nhi->iobase + REG_DMA_MISC);
 
 		ivr_base = ring->nhi->iobase + REG_INT_VEC_ALLOC_BASE;
 		step = index / REG_INT_VEC_ALLOC_REGS * REG_INT_VEC_ALLOC_BITS;
@@ -115,7 +115,7 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 
 	dev_dbg(&ring->nhi->pdev->dev,
 		"%s interrupt at register %#x bit %d (%#x -> %#x)\n",
-		active ? "enabling" : "disabling", reg, bit, old, new);
+		active ? "enabling" : "disabling", reg, interrupt_bit, old, new);
 
 	if (new == old)
 		dev_WARN(&ring->nhi->pdev->dev,
Signature · 1 line
2.27.0
#1 lore raw

+Cc Mario

Quoted 10 lines · Tom Rix

cppcheck reports
drivers/thunderbolt/nhi.c:74:7: style: Local variable 'bit' shadows outer variable [shadowVariable]

  int bit;
      ^
drivers/thunderbolt/nhi.c:66:6: note: Shadowed declaration
 int bit = ring_interrupt_index(ring) & 31;
     ^
drivers/thunderbolt/nhi.c:74:7: note: Shadow variable
  int bit;
      ^
Quoted 2 lines

For readablity rename the outer to interrupt_bit and the innner to auto_clear_bit.

Thanks for the patch! Yeah, this did not show up in any of the kbuild tests perhaps they are missing cppcheck :(

I'm thinking that I'll just move the two commits from "fixes" to "next" and add this one on top (and drop the stable tags) as the code that we should be sending to stable should not need additional fixes IMHO. I know Mario is on vacation so probably cannot answer here so let's deal with this when he is back.

Quoted 55 lines
---
 drivers/thunderbolt/nhi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/thunderbolt/nhi.c +8 -8
index 318d20bd5b69..d0d26d288be8 100644
@@ -63,15 +63,15 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 {
 	int reg = REG_RING_INTERRUPT_BASE +
 		  ring_interrupt_index(ring) / 32 * 4;
-	int bit = ring_interrupt_index(ring) & 31;
-	int mask = 1 << bit;
+	int interrupt_bit = ring_interrupt_index(ring) & 31;
+	int mask = 1 << interrupt_bit;
 	u32 old, new;
 
 	if (ring->irq > 0) {
 		u32 step, shift, ivr, misc;
 		void __iomem *ivr_base;
+		int auto_clear_bit;
 		int index;
-		int bit;
 
 		if (ring->is_tx)
 			index = ring->hop;
@@ -91,11 +91,11 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 		 */
 		misc = ioread32(ring->nhi->iobase + REG_DMA_MISC);
 		if (ring->nhi->quirks & QUIRK_AUTO_CLEAR_INT)
-			bit = REG_DMA_MISC_INT_AUTO_CLEAR;
+			auto_clear_bit = REG_DMA_MISC_INT_AUTO_CLEAR;
 		else
-			bit = REG_DMA_MISC_DISABLE_AUTO_CLEAR;
-		if (!(misc & bit))
-			iowrite32(misc | bit, ring->nhi->iobase + REG_DMA_MISC);
+			auto_clear_bit = REG_DMA_MISC_DISABLE_AUTO_CLEAR;
+		if (!(misc & auto_clear_bit))
+			iowrite32(misc | auto_clear_bit, ring->nhi->iobase + REG_DMA_MISC);
 
 		ivr_base = ring->nhi->iobase + REG_INT_VEC_ALLOC_BASE;
 		step = index / REG_INT_VEC_ALLOC_REGS * REG_INT_VEC_ALLOC_BITS;
@@ -115,7 +115,7 @@ static void ring_interrupt_active(struct tb_ring *ring, bool active)
 
 	dev_dbg(&ring->nhi->pdev->dev,
 		"%s interrupt at register %#x bit %d (%#x -> %#x)\n",
-		active ? "enabling" : "disabling", reg, bit, old, new);
+		active ? "enabling" : "disabling", reg, interrupt_bit, old, new);
 
 	if (new == old)
 		dev_WARN(&ring->nhi->pdev->dev,
Signature · 1 line
2.27.0
#2 lore raw
Quoted 16 lines · Mika Westerberg

+Cc Mario

Quoted 10 lines · Tom Rix

cppcheck reports
drivers/thunderbolt/nhi.c:74:7: style: Local variable 'bit' shadows outer variable [shadowVariable]

  int bit;
      ^
drivers/thunderbolt/nhi.c:66:6: note: Shadowed declaration
 int bit = ring_interrupt_index(ring) & 31;
     ^
drivers/thunderbolt/nhi.c:74:7: note: Shadow variable
  int bit;
      ^
Quoted 2 lines

For readablity rename the outer to interrupt_bit and the innner to auto_clear_bit.

Quoted 8 lines

Thanks for the patch! Yeah, this did not show up in any of the kbuild tests perhaps they are missing cppcheck :(

I'm thinking that I'll just move the two commits from "fixes" to "next" and add this one on top (and drop the stable tags) as the code that we should be sending to stable should not need additional fixes IMHO. I know Mario is on vacation so probably cannot answer here so let's deal with this when he is back.

Applied to thunderbolt.git/next (along with the two commits from Mario).

#3 lore raw
Quoted 19 lines · Mika Westerberg
Quoted 18 lines · Mika Westerberg

+Cc Mario

Quoted 10 lines · Tom Rix

cppcheck reports
drivers/thunderbolt/nhi.c:74:7: style: Local variable 'bit' shadows outer variable [shadowVariable]

   int bit;
       ^
drivers/thunderbolt/nhi.c:66:6: note: Shadowed declaration
  int bit = ring_interrupt_index(ring) & 31;
      ^
drivers/thunderbolt/nhi.c:74:7: note: Shadow variable
   int bit;
       ^
Quoted 2 lines

For readablity rename the outer to interrupt_bit and the innner to auto_clear_bit.

Thanks for the patch! Yeah, this did not show up in any of the kbuild tests perhaps they are missing cppcheck :(

Quoted 6 lines
Quoted 5 lines

I'm thinking that I'll just move the two commits from "fixes" to "next" and add this one on top (and drop the stable tags) as the code that we should be sending to stable should not need additional fixes IMHO. I know Mario is on vacation so probably cannot answer here so let's deal with this when he is back.

Applied to thunderbolt.git/next (along with the two commits from Mario).

Thanks for the fix Tom!

Mika - It's unfortunate that a fixup was needed but I'd still like if we can get these 3 commits into 6.3-rc and also to stable.

#4 lore raw
Quoted 26 lines · Mario Limonciello
Quoted 24 lines · Mika Westerberg
Quoted 23 lines · Mika Westerberg

+Cc Mario

Quoted 10 lines · Tom Rix

cppcheck reports
drivers/thunderbolt/nhi.c:74:7: style: Local variable 'bit' shadows outer variable [shadowVariable]

   int bit;
       ^
drivers/thunderbolt/nhi.c:66:6: note: Shadowed declaration
  int bit = ring_interrupt_index(ring) & 31;
      ^
drivers/thunderbolt/nhi.c:74:7: note: Shadow variable
   int bit;
       ^
Quoted 2 lines

For readablity rename the outer to interrupt_bit and the innner to auto_clear_bit.

Thanks for the patch! Yeah, this did not show up in any of the kbuild tests perhaps they are missing cppcheck :(

I'm thinking that I'll just move the two commits from "fixes" to "next" and add this one on top (and drop the stable tags) as the code that we should be sending to stable should not need additional fixes IMHO. I know Mario is on vacation so probably cannot answer here so let's deal with this when he is back.

Quoted 6 lines
Quoted 1 line

Applied to thunderbolt.git/next (along with the two commits from Mario).

Thanks for the fix Tom!

Mika - It's unfortunate that a fixup was needed but I'd still like if we can get these 3 commits into 6.3-rc and also to stable.

Fair enough. I moved them into fixes now.