* [patch net-next] stmmac: indent an if statement @ 2017-01-12 18:46 Dan Carpenter 2017-01-12 19:46 ` Julia Lawall 2017-01-16 3:14 ` David Miller 0 siblings, 2 replies; 12+ messages in thread From: Dan Carpenter @ 2017-01-12 18:46 UTC (permalink / raw) To: Giuseppe Cavallaro, jpinto Cc: Alexandre Torgue, netdev, linux-kernel, kernel-janitors The break statement should be indented one more tab. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index ac32f9e..4daa8a3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -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; } } ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-12 18:46 [patch net-next] stmmac: indent an if statement Dan Carpenter @ 2017-01-12 19:46 ` Julia Lawall 2017-01-16 3:14 ` David Miller 1 sibling, 0 replies; 12+ messages in thread From: Julia Lawall @ 2017-01-12 19:46 UTC (permalink / raw) To: Dan Carpenter Cc: Giuseppe Cavallaro, jpinto, Alexandre Torgue, netdev, linux-kernel, kernel-janitors On Thu, 12 Jan 2017, Dan Carpenter wrote: > The break statement should be indented one more tab. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > index ac32f9e..4daa8a3 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > @@ -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 > } > } > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-12 18:46 [patch net-next] stmmac: indent an if statement Dan Carpenter 2017-01-12 19:46 ` Julia Lawall @ 2017-01-16 3:14 ` David Miller 2017-01-16 9:19 ` Dan Carpenter 1 sibling, 1 reply; 12+ messages in thread From: David Miller @ 2017-01-16 3:14 UTC (permalink / raw) To: dan.carpenter Cc: peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors From: Dan Carpenter <dan.carpenter@oracle.com> Date: Thu, 12 Jan 2017 21:46:32 +0300 > The break statement should be indented one more tab. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, but like Julia I think we might have a missing of_node_put() here. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 3:14 ` David Miller @ 2017-01-16 9:19 ` Dan Carpenter 2017-01-16 9:39 ` Dan Carpenter 0 siblings, 1 reply; 12+ messages in thread From: Dan Carpenter @ 2017-01-16 9:19 UTC (permalink / raw) To: David Miller Cc: peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: > From: Dan Carpenter <dan.carpenter@oracle.com> > Date: Thu, 12 Jan 2017 21:46:32 +0300 > > > The break statement should be indented one more tab. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 9:19 ` Dan Carpenter @ 2017-01-16 9:39 ` Dan Carpenter 2017-01-16 21:46 ` Julia Lawall 0 siblings, 1 reply; 12+ messages in thread From: Dan Carpenter @ 2017-01-16 9:39 UTC (permalink / raw) To: David Miller Cc: peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: > > From: Dan Carpenter <dan.carpenter@oracle.com> > > Date: Thu, 12 Jan 2017 21:46:32 +0300 > > > > > The break statement should be indented one more tab. > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 9:39 ` Dan Carpenter @ 2017-01-16 21:46 ` Julia Lawall 2017-01-16 21:56 ` Dan Carpenter 0 siblings, 1 reply; 12+ messages in thread From: Julia Lawall @ 2017-01-16 21:46 UTC (permalink / raw) To: Dan Carpenter Cc: David Miller, peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors On Mon, 16 Jan 2017, Dan Carpenter wrote: > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: > > > From: Dan Carpenter <dan.carpenter@oracle.com> > > > Date: Thu, 12 Jan 2017 21:46:32 +0300 > > > > > > > The break statement should be indented one more tab. > > > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > 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. 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 21:46 ` Julia Lawall @ 2017-01-16 21:56 ` Dan Carpenter 2017-01-16 22:00 ` David Miller 2017-01-16 22:10 ` Julia Lawall 0 siblings, 2 replies; 12+ messages in thread From: Dan Carpenter @ 2017-01-16 21:56 UTC (permalink / raw) To: Julia Lawall Cc: David Miller, peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote: > > > On Mon, 16 Jan 2017, Dan Carpenter wrote: > > > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: > > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: > > > > From: Dan Carpenter <dan.carpenter@oracle.com> > > > > Date: Thu, 12 Jan 2017 21:46:32 +0300 > > > > > > > > > The break statement should be indented one more tab. > > > > > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > > > 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. > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 21:56 ` Dan Carpenter @ 2017-01-16 22:00 ` David Miller 2017-01-17 8:28 ` Alexandre Torgue 2017-01-16 22:10 ` Julia Lawall 1 sibling, 1 reply; 12+ messages in thread From: David Miller @ 2017-01-16 22:00 UTC (permalink / raw) To: dan.carpenter Cc: julia.lawall, peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors From: Dan Carpenter <dan.carpenter@oracle.com> Date: Tue, 17 Jan 2017 00:56:15 +0300 > (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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 22:00 ` David Miller @ 2017-01-17 8:28 ` Alexandre Torgue 0 siblings, 0 replies; 12+ messages in thread From: Alexandre Torgue @ 2017-01-17 8:28 UTC (permalink / raw) To: David Miller, dan.carpenter Cc: julia.lawall, peppe.cavallaro, Joao.Pinto, netdev, linux-kernel, kernel-janitors Dear David On 01/16/2017 11:00 PM, David Miller wrote: > From: Dan Carpenter <dan.carpenter@oracle.com> > Date: Tue, 17 Jan 2017 00:56:15 +0300 > >> (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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 21:56 ` Dan Carpenter 2017-01-16 22:00 ` David Miller @ 2017-01-16 22:10 ` Julia Lawall 2017-01-17 8:25 ` Alexandre Torgue 1 sibling, 1 reply; 12+ messages in thread From: Julia Lawall @ 2017-01-16 22:10 UTC (permalink / raw) To: Dan Carpenter Cc: David Miller, peppe.cavallaro, Joao.Pinto, alexandre.torgue, netdev, linux-kernel, kernel-janitors On Tue, 17 Jan 2017, Dan Carpenter wrote: > On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote: > > > > > > On Mon, 16 Jan 2017, Dan Carpenter wrote: > > > > > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: > > > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: > > > > > From: Dan Carpenter <dan.carpenter@oracle.com> > > > > > Date: Thu, 12 Jan 2017 21:46:32 +0300 > > > > > > > > > > > The break statement should be indented one more tab. > > > > > > > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > > > > > 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. > > > > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-16 22:10 ` Julia Lawall @ 2017-01-17 8:25 ` Alexandre Torgue 2017-01-17 11:45 ` Julia Lawall 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Torgue @ 2017-01-17 8:25 UTC (permalink / raw) To: Julia Lawall, Dan Carpenter Cc: David Miller, peppe.cavallaro, Joao.Pinto, netdev, linux-kernel, kernel-janitors Hi Julia On 01/16/2017 11:10 PM, Julia Lawall wrote: > > > On Tue, 17 Jan 2017, Dan Carpenter wrote: > >> On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote: >>> >>> >>> On Mon, 16 Jan 2017, Dan Carpenter wrote: >>> >>>> On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: >>>>> On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: >>>>>> From: Dan Carpenter <dan.carpenter@oracle.com> >>>>>> Date: Thu, 12 Jan 2017 21:46:32 +0300 >>>>>> >>>>>>> The break statement should be indented one more tab. >>>>>>> >>>>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>>>>> >>>>>> 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. >>> >>> 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. I also think it's missing! Can you propose a patch ? br Alex > > julia > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement 2017-01-17 8:25 ` Alexandre Torgue @ 2017-01-17 11:45 ` Julia Lawall 0 siblings, 0 replies; 12+ messages in thread From: Julia Lawall @ 2017-01-17 11:45 UTC (permalink / raw) To: Alexandre Torgue Cc: Dan Carpenter, David Miller, peppe.cavallaro, Joao.Pinto, netdev, linux-kernel, kernel-janitors On Tue, 17 Jan 2017, Alexandre Torgue wrote: > Hi Julia > > On 01/16/2017 11:10 PM, Julia Lawall wrote: > > > > > > On Tue, 17 Jan 2017, Dan Carpenter wrote: > > > > > On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote: > > > > > > > > > > > > On Mon, 16 Jan 2017, Dan Carpenter wrote: > > > > > > > > > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote: > > > > > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote: > > > > > > > From: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > > Date: Thu, 12 Jan 2017 21:46:32 +0300 > > > > > > > > > > > > > > > The break statement should be indented one more tab. > > > > > > > > > > > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > > > > > > > > > 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. > > > > > > > > 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. > > I also think it's missing! Can you propose a patch ? Done. Thanks for the clarification. julia ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-01-17 11:45 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-12 18:46 [patch net-next] stmmac: indent an if statement Dan Carpenter 2017-01-12 19:46 ` Julia Lawall 2017-01-16 3:14 ` David Miller 2017-01-16 9:19 ` Dan Carpenter 2017-01-16 9:39 ` Dan Carpenter 2017-01-16 21:46 ` Julia Lawall 2017-01-16 21:56 ` Dan Carpenter 2017-01-16 22:00 ` David Miller 2017-01-17 8:28 ` Alexandre Torgue 2017-01-16 22:10 ` Julia Lawall 2017-01-17 8:25 ` Alexandre Torgue 2017-01-17 11:45 ` Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).