| Message ID | 1483893663-15673-4-git-send-email-ivan.khoronzhuk@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/08/2017 10:41 AM, Ivan Khoronzhuk wrote: > No need to create additional vars to identify if interface is running. > So simplify code by removing redundant var and checking usage counter > instead. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > --- > drivers/net/ethernet/ti/cpsw.c | 14 ++++---------- > 1 file changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c > index 40d7fc9..daae87f 100644 > --- a/drivers/net/ethernet/ti/cpsw.c > +++ b/drivers/net/ethernet/ti/cpsw.c > @@ -357,7 +357,6 @@ struct cpsw_slave { > struct phy_device *phy; > struct net_device *ndev; > u32 port_vlan; > - u32 open_stat; > }; > > static inline u32 slave_read(struct cpsw_slave *slave, u32 offset) > @@ -1241,7 +1240,7 @@ static int cpsw_common_res_usage_state(struct cpsw_common *cpsw) > u32 usage_count = 0; > > for (i = 0; i < cpsw->data.slaves; i++) > - if (cpsw->slaves[i].open_stat) > + if (netif_running(cpsw->slaves[i].ndev)) > usage_count++; Not sure this will work as you expected, but may be I've missed smth :( code in static int __dev_open(struct net_device *dev) .. set_bit(__LINK_STATE_START, &dev->state); if (ops->ndo_validate_addr) ret = ops->ndo_validate_addr(dev); if (!ret && ops->ndo_open) ret = ops->ndo_open(dev); netpoll_poll_enable(dev); if (ret) clear_bit(__LINK_STATE_START, &dev->state); .. so, netif_running(ndev) will start returning true before calling ops->ndo_open(dev); > > return usage_count; > @@ -1502,7 +1501,7 @@ static int cpsw_ndo_open(struct net_device *ndev) > CPSW_RTL_VERSION(reg)); > > /* initialize host and slave ports */ > - if (!cpsw_common_res_usage_state(cpsw)) > + if (cpsw_common_res_usage_state(cpsw) < 2) Ah. You've changed the condition here. I think it might be reasonable to hide this inside cpsw_common_res_usage_state() and seems it can be renamed to smth like cpsw_is_running(). > cpsw_init_host_port(priv); > for_each_slave(priv, cpsw_slave_open, priv); > > @@ -1513,7 +1512,7 @@ static int cpsw_ndo_open(struct net_device *ndev) > cpsw_ale_add_vlan(cpsw->ale, cpsw->data.default_vlan, > ALE_ALL_PORTS, ALE_ALL_PORTS, 0, 0); > > - if (!cpsw_common_res_usage_state(cpsw)) { > + if (cpsw_common_res_usage_state(cpsw) < 2) { > /* disable priority elevation */ > __raw_writel(0, &cpsw->regs->ptype); > > @@ -1556,9 +1555,6 @@ static int cpsw_ndo_open(struct net_device *ndev) > cpdma_ctlr_start(cpsw->dma); > cpsw_intr_enable(cpsw); > > - if (cpsw->data.dual_emac) > - cpsw->slaves[priv->emac_port].open_stat = true; > - > return 0; > > err_cleanup: > @@ -1578,7 +1574,7 @@ static int cpsw_ndo_stop(struct net_device *ndev) > netif_tx_stop_all_queues(priv->ndev); > netif_carrier_off(priv->ndev); > > - if (cpsw_common_res_usage_state(cpsw) <= 1) { > + if (!cpsw_common_res_usage_state(cpsw)) { and here __LINK_STATE_START will be cleared before calling ops->ndo_stop(dev); So, from one side netif_running(ndev) usage will simplify cpsw_common_res_usage_state() internals, but from another side - it will make places where it's used even more entangled :( as for me, because when cpsw_common_res_usage_state() will return 1 in cpsw_ndo_open() it will mean "no interfaces is really running yet", but the same value 1 in cpsw_ndo_stop() will mean "there are still one is running". > napi_disable(&cpsw->napi_rx); > napi_disable(&cpsw->napi_tx); > cpts_unregister(cpsw->cpts); > @@ -1592,8 +1588,6 @@ static int cpsw_ndo_stop(struct net_device *ndev) > cpsw_split_res(ndev); > > pm_runtime_put_sync(cpsw->dev); > - if (cpsw->data.dual_emac) > - cpsw->slaves[priv->emac_port].open_stat = false; > return 0; > } > >
On Mon, Jan 09, 2017 at 11:25:38AM -0600, Grygorii Strashko wrote: > > > On 01/08/2017 10:41 AM, Ivan Khoronzhuk wrote: > > No need to create additional vars to identify if interface is running. > > So simplify code by removing redundant var and checking usage counter > > instead. > > > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > > --- > > drivers/net/ethernet/ti/cpsw.c | 14 ++++---------- > > 1 file changed, 4 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c > > index 40d7fc9..daae87f 100644 > > --- a/drivers/net/ethernet/ti/cpsw.c > > +++ b/drivers/net/ethernet/ti/cpsw.c > > @@ -357,7 +357,6 @@ struct cpsw_slave { > > struct phy_device *phy; > > struct net_device *ndev; > > u32 port_vlan; > > - u32 open_stat; > > }; > > > > static inline u32 slave_read(struct cpsw_slave *slave, u32 offset) > > @@ -1241,7 +1240,7 @@ static int cpsw_common_res_usage_state(struct cpsw_common *cpsw) > > u32 usage_count = 0; > > > > for (i = 0; i < cpsw->data.slaves; i++) > > - if (cpsw->slaves[i].open_stat) > > + if (netif_running(cpsw->slaves[i].ndev)) > > usage_count++; > > Not sure this will work as you expected, but may be I've missed smth :( I've changed conditions, will work. > > code in static int __dev_open(struct net_device *dev) > .. > set_bit(__LINK_STATE_START, &dev->state); > > if (ops->ndo_validate_addr) > ret = ops->ndo_validate_addr(dev); > > if (!ret && ops->ndo_open) > ret = ops->ndo_open(dev); > > netpoll_poll_enable(dev); > > if (ret) > clear_bit(__LINK_STATE_START, &dev->state); > .. > > so, netif_running(ndev) will start returning true before calling ops->ndo_open(dev); Yes, It's done bearing it in mind of course. > > > > > return usage_count; > > @@ -1502,7 +1501,7 @@ static int cpsw_ndo_open(struct net_device *ndev) > > CPSW_RTL_VERSION(reg)); > > > > /* initialize host and slave ports */ > > - if (!cpsw_common_res_usage_state(cpsw)) > > + if (cpsw_common_res_usage_state(cpsw) < 2) > > Ah. You've changed the condition here. > > I think it might be reasonable to hide this inside cpsw_common_res_usage_state() > and seems it can be renamed to smth like cpsw_is_running(). It probably needs to be renamed to smth a little different, like cpsw_get_usage_count ...or cpsw_get_open_ndev_count > > > > cpsw_init_host_port(priv); > > for_each_slave(priv, cpsw_slave_open, priv); > > > > @@ -1513,7 +1512,7 @@ static int cpsw_ndo_open(struct net_device *ndev) > > cpsw_ale_add_vlan(cpsw->ale, cpsw->data.default_vlan, > > ALE_ALL_PORTS, ALE_ALL_PORTS, 0, 0); > > > > - if (!cpsw_common_res_usage_state(cpsw)) { > > + if (cpsw_common_res_usage_state(cpsw) < 2) { > > /* disable priority elevation */ > > __raw_writel(0, &cpsw->regs->ptype); > > > > @@ -1556,9 +1555,6 @@ static int cpsw_ndo_open(struct net_device *ndev) > > cpdma_ctlr_start(cpsw->dma); > > cpsw_intr_enable(cpsw); > > > > - if (cpsw->data.dual_emac) > > - cpsw->slaves[priv->emac_port].open_stat = true; > > - > > return 0; > > > > err_cleanup: > > @@ -1578,7 +1574,7 @@ static int cpsw_ndo_stop(struct net_device *ndev) > > netif_tx_stop_all_queues(priv->ndev); > > netif_carrier_off(priv->ndev); > > > > - if (cpsw_common_res_usage_state(cpsw) <= 1) { > > + if (!cpsw_common_res_usage_state(cpsw)) { > > and here __LINK_STATE_START will be cleared before calling ops->ndo_stop(dev); Actually it's changed because of it. > So, from one side netif_running(ndev) usage will simplify cpsw_common_res_usage_state() internals, > but from another side - it will make places where it's used even more entangled :( as for me, > because when cpsw_common_res_usage_state() will return 1 in cpsw_ndo_open() it will mean > "no interfaces is really running yet", but the same value 1 in cpsw_ndo_stop() why not? no interfaces running, except the one excuting ndo_open now. It's more clear then duplicating it and using two different ways in different places for identifing running devices. Current way more close to some testing code, not final version. Just to be consistent better to change it. Yes, it returns different results when it's called from ndo_close and ndo_open. Maybe name for the function is not very close to an action it's doing, it declares more intention, and even not for every case. What about to rename it to some cpsw_get_open_ndev_count and add comments in several places explaining what it actually do. > will mean "there are still one is running". > > > napi_disable(&cpsw->napi_rx); > > napi_disable(&cpsw->napi_tx); > > cpts_unregister(cpsw->cpts); > > @@ -1592,8 +1588,6 @@ static int cpsw_ndo_stop(struct net_device *ndev) > > cpsw_split_res(ndev); > > > > pm_runtime_put_sync(cpsw->dev); > > - if (cpsw->data.dual_emac) > > - cpsw->slaves[priv->emac_port].open_stat = false; > > return 0; > > } > > > > > > -- > regards, > -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/10/2017 07:56 PM, Ivan Khoronzhuk wrote: > On Mon, Jan 09, 2017 at 11:25:38AM -0600, Grygorii Strashko wrote: >> >> >> On 01/08/2017 10:41 AM, Ivan Khoronzhuk wrote: >>> No need to create additional vars to identify if interface is running. >>> So simplify code by removing redundant var and checking usage counter >>> instead. >>> >>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> >>> --- >>> drivers/net/ethernet/ti/cpsw.c | 14 ++++---------- >>> 1 file changed, 4 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c >>> index 40d7fc9..daae87f 100644 >>> --- a/drivers/net/ethernet/ti/cpsw.c >>> +++ b/drivers/net/ethernet/ti/cpsw.c >>> @@ -357,7 +357,6 @@ struct cpsw_slave { >>> struct phy_device *phy; >>> struct net_device *ndev; >>> u32 port_vlan; >>> - u32 open_stat; >>> }; >>> >>> static inline u32 slave_read(struct cpsw_slave *slave, u32 offset) >>> @@ -1241,7 +1240,7 @@ static int cpsw_common_res_usage_state(struct cpsw_common *cpsw) >>> u32 usage_count = 0; >>> >>> for (i = 0; i < cpsw->data.slaves; i++) >>> - if (cpsw->slaves[i].open_stat) >>> + if (netif_running(cpsw->slaves[i].ndev)) >>> usage_count++; >> >> Not sure this will work as you expected, but may be I've missed smth :( > I've changed conditions, will work. > >> >> code in static int __dev_open(struct net_device *dev) >> .. >> set_bit(__LINK_STATE_START, &dev->state); >> >> if (ops->ndo_validate_addr) >> ret = ops->ndo_validate_addr(dev); >> >> if (!ret && ops->ndo_open) >> ret = ops->ndo_open(dev); >> >> netpoll_poll_enable(dev); >> >> if (ret) >> clear_bit(__LINK_STATE_START, &dev->state); >> .. >> >> so, netif_running(ndev) will start returning true before calling ops->ndo_open(dev); > Yes, It's done bearing it in mind of course. > >> >>> >>> return usage_count; >>> @@ -1502,7 +1501,7 @@ static int cpsw_ndo_open(struct net_device *ndev) >>> CPSW_RTL_VERSION(reg)); >>> >>> /* initialize host and slave ports */ >>> - if (!cpsw_common_res_usage_state(cpsw)) >>> + if (cpsw_common_res_usage_state(cpsw) < 2) >> >> Ah. You've changed the condition here. >> >> I think it might be reasonable to hide this inside cpsw_common_res_usage_state() >> and seems it can be renamed to smth like cpsw_is_running(). > It probably needs to be renamed to smth a little different, > like cpsw_get_usage_count ...or cpsw_get_open_ndev_count cpsw_get_usage_count () sounds good > >> >> >>> cpsw_init_host_port(priv); >>> for_each_slave(priv, cpsw_slave_open, priv); >>> >>> @@ -1513,7 +1512,7 @@ static int cpsw_ndo_open(struct net_device *ndev) >>> cpsw_ale_add_vlan(cpsw->ale, cpsw->data.default_vlan, >>> ALE_ALL_PORTS, ALE_ALL_PORTS, 0, 0); >>> >>> - if (!cpsw_common_res_usage_state(cpsw)) { >>> + if (cpsw_common_res_usage_state(cpsw) < 2) { >>> /* disable priority elevation */ >>> __raw_writel(0, &cpsw->regs->ptype); >>> >>> @@ -1556,9 +1555,6 @@ static int cpsw_ndo_open(struct net_device *ndev) >>> cpdma_ctlr_start(cpsw->dma); >>> cpsw_intr_enable(cpsw); >>> >>> - if (cpsw->data.dual_emac) >>> - cpsw->slaves[priv->emac_port].open_stat = true; >>> - >>> return 0; >>> >>> err_cleanup: >>> @@ -1578,7 +1574,7 @@ static int cpsw_ndo_stop(struct net_device *ndev) >>> netif_tx_stop_all_queues(priv->ndev); >>> netif_carrier_off(priv->ndev); >>> >>> - if (cpsw_common_res_usage_state(cpsw) <= 1) { >>> + if (!cpsw_common_res_usage_state(cpsw)) { >> >> and here __LINK_STATE_START will be cleared before calling ops->ndo_stop(dev); > Actually it's changed because of it. > >> So, from one side netif_running(ndev) usage will simplify cpsw_common_res_usage_state() internals, >> but from another side - it will make places where it's used even more entangled :( as for me, >> because when cpsw_common_res_usage_state() will return 1 in cpsw_ndo_open() it will mean >> "no interfaces is really running yet", but the same value 1 in cpsw_ndo_stop() > why not? no interfaces running, except the one excuting ndo_open now. > It's more clear then duplicating it and using two different ways in > different places for identifing running devices. Current way more > close to some testing code, not final version. Just to be consistent > better to change it. > > Yes, it returns different results when it's called from ndo_close and > ndo_open. Maybe name for the function is not very close to an action > it's doing, it declares more intention, and even not for every case. > What about to rename it to some cpsw_get_open_ndev_count and add > comments in several places explaining what it actually do. yes. please. comments are required at least. its actually a question why __LINK_STATE_START is managed this way in ./net/core/dev.c __dev_open() set_bit(__LINK_STATE_START, &dev->state); <---- before .ndo_open() if (!ret && ops->ndo_open) ret = ops->ndo_open(dev); <---- shouldn't set_bit(__LINK_STATE_START, &dev->state) be after calling .ndo_open() ?? __dev_close_many() clear_bit(__LINK_STATE_START, &dev->state); <-stop sequence is differ from open. if (ops->ndo_stop) ops->ndo_stop(dev);
On Thu, Jan 12, 2017 at 11:34:47AM -0600, Grygorii Strashko wrote: Hi Grygorii, Sorry for late reply. > > > On 01/10/2017 07:56 PM, Ivan Khoronzhuk wrote: > > On Mon, Jan 09, 2017 at 11:25:38AM -0600, Grygorii Strashko wrote: > >> > >> > >> On 01/08/2017 10:41 AM, Ivan Khoronzhuk wrote: > >>> No need to create additional vars to identify if interface is running. > >>> So simplify code by removing redundant var and checking usage counter > >>> instead. > >>> > >>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> > >>> --- > >>> drivers/net/ethernet/ti/cpsw.c | 14 ++++---------- > >>> 1 file changed, 4 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c > >>> index 40d7fc9..daae87f 100644 > >>> --- a/drivers/net/ethernet/ti/cpsw.c > >>> +++ b/drivers/net/ethernet/ti/cpsw.c > >>> @@ -357,7 +357,6 @@ struct cpsw_slave { > >>> struct phy_device *phy; > >>> struct net_device *ndev; > >>> u32 port_vlan; > >>> - u32 open_stat; > >>> }; > >>> > >>> static inline u32 slave_read(struct cpsw_slave *slave, u32 offset) > >>> @@ -1241,7 +1240,7 @@ static int cpsw_common_res_usage_state(struct cpsw_common *cpsw) > >>> u32 usage_count = 0; > >>> > >>> for (i = 0; i < cpsw->data.slaves; i++) > >>> - if (cpsw->slaves[i].open_stat) > >>> + if (netif_running(cpsw->slaves[i].ndev)) > >>> usage_count++; > >> > >> Not sure this will work as you expected, but may be I've missed smth :( > > I've changed conditions, will work. > > > >> > >> code in static int __dev_open(struct net_device *dev) > >> .. > >> set_bit(__LINK_STATE_START, &dev->state); > >> > >> if (ops->ndo_validate_addr) > >> ret = ops->ndo_validate_addr(dev); > >> > >> if (!ret && ops->ndo_open) > >> ret = ops->ndo_open(dev); > >> > >> netpoll_poll_enable(dev); > >> > >> if (ret) > >> clear_bit(__LINK_STATE_START, &dev->state); > >> .. > >> > >> so, netif_running(ndev) will start returning true before calling ops->ndo_open(dev); > > Yes, It's done bearing it in mind of course. > > > >> > >>> > >>> return usage_count; > >>> @@ -1502,7 +1501,7 @@ static int cpsw_ndo_open(struct net_device *ndev) > >>> CPSW_RTL_VERSION(reg)); > >>> > >>> /* initialize host and slave ports */ > >>> - if (!cpsw_common_res_usage_state(cpsw)) > >>> + if (cpsw_common_res_usage_state(cpsw) < 2) > >> > >> Ah. You've changed the condition here. > >> > >> I think it might be reasonable to hide this inside cpsw_common_res_usage_state() > >> and seems it can be renamed to smth like cpsw_is_running(). > > It probably needs to be renamed to smth a little different, > > like cpsw_get_usage_count ...or cpsw_get_open_ndev_count > > cpsw_get_usage_count () sounds good Like it more also. Will change it. > > > > >> > >> > >>> cpsw_init_host_port(priv); > >>> for_each_slave(priv, cpsw_slave_open, priv); > >>> > >>> @@ -1513,7 +1512,7 @@ static int cpsw_ndo_open(struct net_device *ndev) > >>> cpsw_ale_add_vlan(cpsw->ale, cpsw->data.default_vlan, > >>> ALE_ALL_PORTS, ALE_ALL_PORTS, 0, 0); > >>> > >>> - if (!cpsw_common_res_usage_state(cpsw)) { > >>> + if (cpsw_common_res_usage_state(cpsw) < 2) { > >>> /* disable priority elevation */ > >>> __raw_writel(0, &cpsw->regs->ptype); > >>> > >>> @@ -1556,9 +1555,6 @@ static int cpsw_ndo_open(struct net_device *ndev) > >>> cpdma_ctlr_start(cpsw->dma); > >>> cpsw_intr_enable(cpsw); > >>> > >>> - if (cpsw->data.dual_emac) > >>> - cpsw->slaves[priv->emac_port].open_stat = true; > >>> - > >>> return 0; > >>> > >>> err_cleanup: > >>> @@ -1578,7 +1574,7 @@ static int cpsw_ndo_stop(struct net_device *ndev) > >>> netif_tx_stop_all_queues(priv->ndev); > >>> netif_carrier_off(priv->ndev); > >>> > >>> - if (cpsw_common_res_usage_state(cpsw) <= 1) { > >>> + if (!cpsw_common_res_usage_state(cpsw)) { > >> > >> and here __LINK_STATE_START will be cleared before calling ops->ndo_stop(dev); > > Actually it's changed because of it. > > > >> So, from one side netif_running(ndev) usage will simplify cpsw_common_res_usage_state() internals, > >> but from another side - it will make places where it's used even more entangled :( as for me, > >> because when cpsw_common_res_usage_state() will return 1 in cpsw_ndo_open() it will mean > >> "no interfaces is really running yet", but the same value 1 in cpsw_ndo_stop() > > why not? no interfaces running, except the one excuting ndo_open now. > > It's more clear then duplicating it and using two different ways in > > different places for identifing running devices. Current way more > > close to some testing code, not final version. Just to be consistent > > better to change it. > > > > Yes, it returns different results when it's called from ndo_close and > > ndo_open. Maybe name for the function is not very close to an action > > it's doing, it declares more intention, and even not for every case. > > What about to rename it to some cpsw_get_open_ndev_count and add > > comments in several places explaining what it actually do. > > yes. please. comments are required at least. > > its actually a question why __LINK_STATE_START is managed this way in ./net/core/dev.c > > __dev_open() > set_bit(__LINK_STATE_START, &dev->state); <---- before .ndo_open() > > if (!ret && ops->ndo_open) > ret = ops->ndo_open(dev); > > <---- shouldn't set_bit(__LINK_STATE_START, &dev->state) be after calling .ndo_open() ?? By logic yes, but in another way it looks like it 's done intentionally. Some code can be based on it, some that can be executed while .ndo_open and after. And it should act the same in both cases. In case of cpsw, at least cpsw_adjust_link() can be called while .ndo_open and also after, but in both cases it's supposed that flag is set. In case of cpsw_adjust_link() there is no way to predict when it will be called, so here only one way - set __LINK_STATE_START before .ndo_open(), result - flag is set in any case already. Maybe that is one of the reasons of such sequence. Changing the logic can bring a lot of headache, don't want to touch it here. > > __dev_close_many() > > clear_bit(__LINK_STATE_START, &dev->state); <-stop sequence is differ from open. Yes, and it doens't have postponed tasks like .ndo_open. > > if (ops->ndo_stop) > ops->ndo_stop(dev); > > > -- > regards, > -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 40d7fc9..daae87f 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -357,7 +357,6 @@ struct cpsw_slave { struct phy_device *phy; struct net_device *ndev; u32 port_vlan; - u32 open_stat; }; static inline u32 slave_read(struct cpsw_slave *slave, u32 offset) @@ -1241,7 +1240,7 @@ static int cpsw_common_res_usage_state(struct cpsw_common *cpsw) u32 usage_count = 0; for (i = 0; i < cpsw->data.slaves; i++) - if (cpsw->slaves[i].open_stat) + if (netif_running(cpsw->slaves[i].ndev)) usage_count++; return usage_count; @@ -1502,7 +1501,7 @@ static int cpsw_ndo_open(struct net_device *ndev) CPSW_RTL_VERSION(reg)); /* initialize host and slave ports */ - if (!cpsw_common_res_usage_state(cpsw)) + if (cpsw_common_res_usage_state(cpsw) < 2) cpsw_init_host_port(priv); for_each_slave(priv, cpsw_slave_open, priv); @@ -1513,7 +1512,7 @@ static int cpsw_ndo_open(struct net_device *ndev) cpsw_ale_add_vlan(cpsw->ale, cpsw->data.default_vlan, ALE_ALL_PORTS, ALE_ALL_PORTS, 0, 0); - if (!cpsw_common_res_usage_state(cpsw)) { + if (cpsw_common_res_usage_state(cpsw) < 2) { /* disable priority elevation */ __raw_writel(0, &cpsw->regs->ptype); @@ -1556,9 +1555,6 @@ static int cpsw_ndo_open(struct net_device *ndev) cpdma_ctlr_start(cpsw->dma); cpsw_intr_enable(cpsw); - if (cpsw->data.dual_emac) - cpsw->slaves[priv->emac_port].open_stat = true; - return 0; err_cleanup: @@ -1578,7 +1574,7 @@ static int cpsw_ndo_stop(struct net_device *ndev) netif_tx_stop_all_queues(priv->ndev); netif_carrier_off(priv->ndev); - if (cpsw_common_res_usage_state(cpsw) <= 1) { + if (!cpsw_common_res_usage_state(cpsw)) { napi_disable(&cpsw->napi_rx); napi_disable(&cpsw->napi_tx); cpts_unregister(cpsw->cpts); @@ -1592,8 +1588,6 @@ static int cpsw_ndo_stop(struct net_device *ndev) cpsw_split_res(ndev); pm_runtime_put_sync(cpsw->dev); - if (cpsw->data.dual_emac) - cpsw->slaves[priv->emac_port].open_stat = false; return 0; }
No need to create additional vars to identify if interface is running. So simplify code by removing redundant var and checking usage counter instead. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- drivers/net/ethernet/ti/cpsw.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)