| Message ID | 1469586048-15697-5-git-send-email-peter.chen@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Peter Chen <peter.chen@nxp.com> Date: Wed, 27 Jul 2016 10:20:37 +0800 > diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c > index 0d4ea92..d15d96b 100644 > --- a/drivers/net/ethernet/aurora/nb8800.c > +++ b/drivers/net/ethernet/aurora/nb8800.c > @@ -1006,6 +1006,7 @@ static int nb8800_stop(struct net_device *dev) > > netif_stop_queue(dev); > napi_disable(&priv->napi); > + of_node_put(priv->phy_node); > > nb8800_dma_stop(dev); > nb8800_mac_rx(dev, false); This is broken. The priv->phy_node reference is taken in the probe function, therefore you have to drop that reference in the "remove" function not the "stop" function. I'm just looking over basic details of this patch series, and along with some other feedback you've received, it looks like you really didn't put a lot of auditing into the changes you are making. Please go over this series one more time and resubmit the entire thing after everything is sorted out and double-checked. Thank you.
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c >> b/drivers/net/ethernet/aurora/nb8800.c >> index 0d4ea92..d15d96b 100644 >> --- a/drivers/net/ethernet/aurora/nb8800.c >> +++ b/drivers/net/ethernet/aurora/nb8800.c >> @@ -1006,6 +1006,7 @@ static int nb8800_stop(struct net_device *dev) >> >> netif_stop_queue(dev); >> napi_disable(&priv->napi); >> + of_node_put(priv->phy_node); >> >> nb8800_dma_stop(dev); >> nb8800_mac_rx(dev, false); > >This is broken. The priv->phy_node reference is taken in the probe function, >therefore you have to drop that reference in the "remove" function not the "stop" >function. > >I'm just looking over basic details of this patch series, and along with some other >feedback you've received, it looks like you really didn't put a lot of auditing into the >changes you are making. > >Please go over this series one more time and resubmit the entire thing after >everything is sorted out and double-checked. > Thanks for your comments, David. I do not familiar with Ethernet driver, and thought if the node is used up, it can be freed like other drivers. But for phy_node, it will be used in whole driver's life cycle, it is my careless. I will review each patch in this series again. Best regards, Peter
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index 0d4ea92..d15d96b 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -1006,6 +1006,7 @@ static int nb8800_stop(struct net_device *dev) netif_stop_queue(dev); napi_disable(&priv->napi); + of_node_put(priv->phy_node); nb8800_dma_stop(dev); nb8800_mac_rx(dev, false); @@ -1504,6 +1505,7 @@ static int nb8800_probe(struct platform_device *pdev) err_free_dma: nb8800_dma_free(dev); err_free_bus: + of_node_put(priv->phy_node); mdiobus_unregister(bus); err_disable_clk: clk_disable_unprepare(priv->clk);
of_node_put needs to be called when the device node which is got from of_parse_phandle has finished using. Signed-off-by: Peter Chen <peter.chen@nxp.com> --- drivers/net/ethernet/aurora/nb8800.c | 2 ++ 1 file changed, 2 insertions(+)