* Re: [patch net-next] stmmac: indent an if statement
@ 2017-01-16 21:56 ` Dan Carpenter
0 siblings, 0 replies; 24+ 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] 24+ 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
-1 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement
@ 2017-01-16 22:00 ` David Miller
0 siblings, 0 replies; 24+ 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] 24+ 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
-1 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement
@ 2017-01-17 8:28 ` Alexandre Torgue
0 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement
2017-01-16 21:56 ` Dan Carpenter
@ 2017-01-16 22:10 ` Julia Lawall
-1 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement
@ 2017-01-16 22:10 ` Julia Lawall
0 siblings, 0 replies; 24+ 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] 24+ 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
-1 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement
@ 2017-01-17 8:25 ` Alexandre Torgue
0 siblings, 0 replies; 24+ 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] 24+ 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
-1 siblings, 0 replies; 24+ 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] 24+ messages in thread
* Re: [patch net-next] stmmac: indent an if statement
@ 2017-01-17 11:45 ` Julia Lawall
0 siblings, 0 replies; 24+ 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] 24+ messages in thread