Skip to contentSkip to conversation

phy: brcm-sata: fix a timeout test in init

PATCH In review Dan Carpenter 2 participants 4 messages Jun 19, 2017 – Jun 20, 2017

Subsystems (2)
Kernel janitors Generic PHY framework
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 phy: brcm-sata: fix a timeout test in init

We want to timeout with try set to zero so this should be a pre-op instead of post-op.

Signed-off-by Dan Carpenter <[email protected]>
drivers/phy/broadcom/phy-brcm-sata.c +1 -1
index ccbc3d994998..48fb016ce689 100644
@@ -329,7 +329,7 @@ static int brcm_nsp_sata_init(struct brcm_sata_port *port)
 
 	/* Wait for pll_seq_done bit */
 	try = 50;
-	while (try--) {
+	while (--try) {
 		val = brcm_sata_phy_rd(base, BLOCK0_REG_BANK,
 					BLOCK0_XGXSSTATUS);
 		if (val & BLOCK0_XGXSSTATUS_PLL_LOCK)
#1 lore raw
Quoted 10 lines · Dan Carpenter

We want to timeout with try set to zero so this should be a pre-op instead of post-op.

Signed-off-by Dan Carpenter <[email protected]>
drivers/phy/broadcom/phy-brcm-sata.c +0 -0
index ccbc3d994998..48fb016ce689 100644

drivers/phy/broadcom/phy-brcm-sata.c static int brcm_nsp_sata_init(struct brcm_sata_port *port) · line 332

Quoted 5 lines
drivers/phy/broadcom/phy-brcm-sata.c +1 -1
@@ -329,7 +329,7 @@ static int brcm_nsp_sata_init(struct brcm_sata_port *port)
 
  	/* Wait for pll_seq_done bit */
  	try = 50;
-	while (try--) {
+	while (--try) {

Do we want to try reading the status 50 times? If yes, won't your change break that? It will rather run the loop 49 times.

Thanks
Vivek

Quoted 3 lines
drivers/phy/broadcom/phy-brcm-sata.c +0 -0
  		val = brcm_sata_phy_rd(base, BLOCK0_REG_BANK,
  					BLOCK0_XGXSSTATUS);
  		if (val & BLOCK0_XGXSSTATUS_PLL_LOCK)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#2 lore raw
Quoted 8 lines · Vivek Gautam
Quoted 5 lines · Dan Carpenter

We want to timeout with try set to zero so this should be a pre-op instead of post-op.

Signed-off-by Dan Carpenter <[email protected]>
Quoted 13 lines

drivers/phy/broadcom/phy-brcm-sata.c static int brcm_nsp_sata_init(struct brcm_sata_port *port) · line 331

Quoted 9 lines
drivers/phy/broadcom/phy-brcm-sata.c +1 -1
index ccbc3d994998..48fb016ce689 100644
@@ -329,7 +329,7 @@ static int brcm_nsp_sata_init(struct brcm_sata_port *port)
  	/* Wait for pll_seq_done bit */
  	try = 50;
-	while (try--) {
+	while (--try) {

Do we want to try reading the status 50 times? If yes, won't your change break that? It will rather run the loop 49 times.

Yeah. I know. I'm pretty sure that 50 is a rough number, and not an exact thing.

regards,
dan carpenter

#3 lore raw
Quoted 20 lines · Dan Carpenter
Quoted 7 lines · Vivek Gautam
Quoted 5 lines · Dan Carpenter

We want to timeout with try set to zero so this should be a pre-op instead of post-op.

Signed-off-by Dan Carpenter <[email protected]>
Quoted 12 lines

drivers/phy/broadcom/phy-brcm-sata.c static int brcm_nsp_sata_init(struct brcm_sata_port *port) · line 331

Quoted 9 lines
drivers/phy/broadcom/phy-brcm-sata.c +1 -1
index ccbc3d994998..48fb016ce689 100644
@@ -329,7 +329,7 @@ static int brcm_nsp_sata_init(struct brcm_sata_port *port)
   	/* Wait for pll_seq_done bit */
   	try = 50;
-	while (try--) {
+	while (--try) {

Do we want to try reading the status 50 times? If yes, won't your change break that? It will rather run the loop 49 times.

Quoted 2 lines

Yeah. I know. I'm pretty sure that 50 is a rough number, and not an exact thing.

Right, I agree that this must be rough count.
nice catch btw.

Best regards
Vivek

Quoted 4 lines

regards,
dan carpenter

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project