stmmac: indent an if statement

PATCH Applied Dan Carpenter 4 participants 12 messages Jan 12, 2017 – Jan 17, 2017

Subsystems (5)

Networking Network drivers Kernel janitors Arm STM32 +1 more subsystems

lore

Share & tools

lore

Dan Carpenter

author

2017-01-12 18:46 UTC #0 lore raw

PATCH 1/1 stmmac: indent an if statement

The break statement should be indented one more tab.

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

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +1 -1

index ac32f9e..4daa8a3 100644
@@ -191,7 +191,7 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
 		for_each_child_of_node(np, plat->mdio_node) {
 			if (of_device_is_compatible(plat->mdio_node,
 						    "snps,dwmac-mdio"))
-			break;
+				break;
 		}
 	}

Julia Lawall

2017-01-12 19:46 UTC #1 lore raw

Quoted 9 lines · Dan Carpenter

The break statement should be indented one more tab.
Signed-off-by Dan Carpenter <[email protected]>
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +0 -0
index ac32f9e..4daa8a3 100644

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c static int stmmac_dt_phy(struct plat_stmmacenet_data *plat, · line 194

Quoted 5 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +1 -1

@@ -191,7 +191,7 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
 		for_each_child_of_node(np, plat->mdio_node) {
 			if (of_device_is_compatible(plat->mdio_node,
 						    "snps,dwmac-mdio"))
-			break;
+				break;

If there is a break, there is probably also a need for an of_node_put?

julia

Quoted 8 lines

drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +0 -1
 		}
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

David Miller

maintainer Applied

2017-01-16 03:14 UTC #2 lore raw

In reply to Dan Carpenter

From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300

Quoted 3 lines

The break statement should be indented one more tab.
Signed-off-by Dan Carpenter <[email protected]>

Applied, but like Julia I think we might have a missing of_node_put() here.

Dan Carpenter

author

2017-01-16 09:19 UTC #3 lore raw

Quoted 9 lines · David Miller

From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 3 lines
The break statement should be indented one more tab.
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put() here.

Of course, sorry for dropping the ball on this. I'll send a patch for that.

regards,
dan carpenter

Dan Carpenter

author

2017-01-16 09:39 UTC #4 lore raw

Quoted 6 lines · Dan Carpenter

Quoted 5 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 2 lines
The break statement should be indented one more tab.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put() here.
Of course, sorry for dropping the ball on this. I'll send a patch for that.

Actually, I've looked at it some more and I think this function is OK. We're supposed to do an of_node_put() later... I can't find where that happens, but presumably that's because I don't know stmmac well. This code here, though, is fine.

regards,
dan carpenter

Julia Lawall

2017-01-16 21:46 UTC #5 lore raw

Quoted 12 lines · Dan Carpenter

Quoted 11 lines · Dan Carpenter
Quoted 9 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 3 lines
The break statement should be indented one more tab.
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put() here.

Quoted 8 lines

Quoted 3 lines
Of course, sorry for dropping the ball on this. I'll send a patch for that.
Actually, I've looked at it some more and I think this function is OK. We're supposed to do an of_node_put() later... I can't find where that happens, but presumably that's because I don't know stmmac well. This code here, though, is fine.

Why do you think it is fine? Does anyone in the calling context know which child would have caused the break? An extra put is only needed on that one. Is there a guarantee that the break is always taken?

julia

Dan Carpenter

author

2017-01-16 21:56 UTC #6 lore raw

Quoted 20 lines · Julia Lawall

Quoted 16 lines · Dan Carpenter
Quoted 14 lines · Dan Carpenter
Quoted 9 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 3 lines
The break statement should be indented one more tab.
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put() here.
Of course, sorry for dropping the ball on this. I'll send a patch for that.

Quoted 7 lines

Quoted 4 lines
Actually, I've looked at it some more and I think this function is OK. We're supposed to do an of_node_put() later... I can't find where that happens, but presumably that's because I don't know stmmac well. This code here, though, is fine.
Why do you think it is fine? Does anyone in the calling context know which child would have caused the break?

Yeah. It's saved in plat->mdio_node and we expect to be holding on either path through the function.

(It would be better if one of the stmmac people were responding here insead of a random fix the indenting weenie like myself.)

regards,
dan caprenter

David Miller

maintainer

2017-01-16 22:00 UTC #7 lore raw

From: Dan Carpenter <[email protected]>
Date: Tue, 17 Jan 2017 00:56:15 +0300

Quoted 2 lines

(It would be better if one of the stmmac people were responding here insead of a random fix the indenting weenie like myself.)

They are all too busy trying to rename the driver, because that's so much more important.

Alexandre Torgue

2017-01-17 08:28 UTC #8 lore raw

Dear David

Quoted 8 lines · David Miller

From: Dan Carpenter <[email protected]>
Date: Tue, 17 Jan 2017 00:56:15 +0300
Quoted 2 lines
(It would be better if one of the stmmac people were responding here insead of a random fix the indenting weenie like myself.)
They are all too busy trying to rename the driver, because that's so much more important.

No, we don't spend all our times to deals with stmmac renaming. Just busy on other topic and we continue to do our best with Peppe to review stmmac patch.

Regards
Alexandre

Quoted 1 line

Julia Lawall

2017-01-16 22:10 UTC #9 lore raw

In reply to Dan Carpenter

Quoted 26 lines · Dan Carpenter

Quoted 25 lines · Julia Lawall
Quoted 20 lines · Dan Carpenter
Quoted 6 lines · Dan Carpenter
Quoted 5 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 2 lines
The break statement should be indented one more tab.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put() here.
Of course, sorry for dropping the ball on this. I'll send a patch for that.
Actually, I've looked at it some more and I think this function is OK. We're supposed to do an of_node_put() later... I can't find where that happens, but presumably that's because I don't know stmmac well. This code here, though, is fine.

Quoted 8 lines

Quoted 2 lines
Why do you think it is fine? Does anyone in the calling context know which child would have caused the break?
Yeah. It's saved in plat->mdio_node and we expect to be holding on either path through the function.
(It would be better if one of the stmmac people were responding here insead of a random fix the indenting weenie like myself.)

OK, I agree that there should not be an of_node_put with the break.

Perhaps there should be an of_node_put on plat->mdio_node in stmmac_remove_config_dt, like there is an of_node_put on plat->phy_node. But it would certainly be helpful to hear from someone who knows the code better.

julia

Alexandre Torgue

2017-01-17 08:25 UTC #10 lore raw

Hi Julia

Quoted 39 lines · Julia Lawall

Quoted 26 lines · Dan Carpenter
Quoted 25 lines · Julia Lawall
Quoted 20 lines · Dan Carpenter
Quoted 6 lines · Dan Carpenter
Quoted 5 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 2 lines
The break statement should be indented one more tab.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put() here.
Of course, sorry for dropping the ball on this. I'll send a patch for that.
Actually, I've looked at it some more and I think this function is OK. We're supposed to do an of_node_put() later... I can't find where that happens, but presumably that's because I don't know stmmac well. This code here, though, is fine.
Quoted 8 lines
Quoted 2 lines
Why do you think it is fine? Does anyone in the calling context know which child would have caused the break?
Yeah. It's saved in plat->mdio_node and we expect to be holding on either path through the function.
(It would be better if one of the stmmac people were responding here insead of a random fix the indenting weenie like myself.)

Quoted 6 lines

OK, I agree that there should not be an of_node_put with the break.
Perhaps there should be an of_node_put on plat->mdio_node in stmmac_remove_config_dt, like there is an of_node_put on plat->phy_node. But it would certainly be helpful to hear from someone who knows the code better.

I also think it's missing! Can you propose a patch ?

br
Alex

Quoted 3 lines

julia

Julia Lawall

2017-01-17 11:45 UTC #11 lore raw

Quoted 45 lines · Alexandre Torgue

Hi Julia
Quoted 42 lines · Julia Lawall
Quoted 37 lines · Dan Carpenter
Quoted 22 lines · Julia Lawall
Quoted 18 lines · Dan Carpenter
Quoted 16 lines · Dan Carpenter
Quoted 10 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Thu, 12 Jan 2017 21:46:32 +0300
Quoted 3 lines
The break statement should be indented one more tab.
Signed-off-by Dan Carpenter <[email protected]>
Applied, but like Julia I think we might have a missing of_node_put()
here.
Of course, sorry for dropping the ball on this. I'll send a patch for
that.
Quoted 8 lines
Quoted 5 lines
Actually, I've looked at it some more and I think this function is OK. We're supposed to do an of_node_put() later... I can't find where that
happens, but presumably that's because I don't know stmmac well. This code here, though, is fine.
Why do you think it is fine? Does anyone in the calling context know which child would have caused the break?
Yeah. It's saved in plat->mdio_node and we expect to be holding on either path through the function.
(It would be better if one of the stmmac people were responding here insead of a random fix the indenting weenie like myself.)

Quoted 8 lines

Quoted 6 lines
OK, I agree that there should not be an of_node_put with the break.
Perhaps there should be an of_node_put on plat->mdio_node in stmmac_remove_config_dt, like there is an of_node_put on plat->phy_node. But it would certainly be helpful to hear from someone who knows the code better.
I also think it's missing! Can you propose a patch ?

Done. Thanks for the clarification.

julia