Re: [PATCH 02/10] net: ax88796: Attach MII bus only when open
Hi Andrew, thank you for reviewing this series! On Tue, Apr 17, 2018 at 10:59 AM, Andrew Lunn wrote: > On Tue, Apr 17, 2018 at 10:04:37AM +1200, Michael Schmitz wrote: >> From: Michael Karcher >> >> Call ax_mii_init in ax_open(), and unregister/remove mdiobus resources >> in ax_close(). >> >> This is needed to be able to unload the module, as the module is busy >> while the MII bus is attached. >> >> Signed-off-by: Michael Karcher >> Signed-off-by: Michael Schmitz >> --- >> drivers/net/ethernet/8390/ax88796.c | 13 + >> 1 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/8390/ax88796.c >> b/drivers/net/ethernet/8390/ax88796.c >> index 2a256aa..f7b8911 100644 >> --- a/drivers/net/ethernet/8390/ax88796.c >> +++ b/drivers/net/ethernet/8390/ax88796.c >> @@ -79,6 +79,8 @@ >> >> static u32 ax_msg_enable; >> >> +static int ax_mii_init(struct net_device *dev); > > Hi Michael > > We try to avoid forward declarations. Please can you move > ax_mii_init() so this is not needed. Done that - had to move the entire bitbang stuff along with ax_mii_init() though. >> + >> /* device private data */ >> >> struct ax_device { >> @@ -396,6 +398,10 @@ static int ax_open(struct net_device *dev) >> >> netdev_dbg(dev, "open\n"); >> >> + ret = ax_mii_init(dev); >> + if (ret) >> + goto failed_request_irq; >> + >> ret = request_irq(dev->irq, ax_ei_interrupt, ax->irqflags, >> dev->name, dev); >> if (ret) > > You are missing some cleanup on error at the end of ax_open(). > It was also missing before. Yep, that's addressed in patch 7 of this series. I'll fold that into this one for clarity. Cheers, Michael > >Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] net: ax88796: Attach MII bus only when open
On Tue, Apr 17, 2018 at 10:04:37AM +1200, Michael Schmitz wrote: > From: Michael Karcher > > Call ax_mii_init in ax_open(), and unregister/remove mdiobus resources > in ax_close(). > > This is needed to be able to unload the module, as the module is busy > while the MII bus is attached. > > Signed-off-by: Michael Karcher > Signed-off-by: Michael Schmitz > --- > drivers/net/ethernet/8390/ax88796.c | 13 + > 1 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/8390/ax88796.c > b/drivers/net/ethernet/8390/ax88796.c > index 2a256aa..f7b8911 100644 > --- a/drivers/net/ethernet/8390/ax88796.c > +++ b/drivers/net/ethernet/8390/ax88796.c > @@ -79,6 +79,8 @@ > > static u32 ax_msg_enable; > > +static int ax_mii_init(struct net_device *dev); Hi Michael We try to avoid forward declarations. Please can you move ax_mii_init() so this is not needed. > + > /* device private data */ > > struct ax_device { > @@ -396,6 +398,10 @@ static int ax_open(struct net_device *dev) > > netdev_dbg(dev, "open\n"); > > + ret = ax_mii_init(dev); > + if (ret) > + goto failed_request_irq; > + > ret = request_irq(dev->irq, ax_ei_interrupt, ax->irqflags, > dev->name, dev); > if (ret) You are missing some cleanup on error at the end of ax_open(). It was also missing before. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/10] net: ax88796: Attach MII bus only when open
From: Michael Karcher Call ax_mii_init in ax_open(), and unregister/remove mdiobus resources in ax_close(). This is needed to be able to unload the module, as the module is busy while the MII bus is attached. Signed-off-by: Michael Karcher Signed-off-by: Michael Schmitz --- drivers/net/ethernet/8390/ax88796.c | 13 + 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/8390/ax88796.c b/drivers/net/ethernet/8390/ax88796.c index 2a256aa..f7b8911 100644 --- a/drivers/net/ethernet/8390/ax88796.c +++ b/drivers/net/ethernet/8390/ax88796.c @@ -79,6 +79,8 @@ static u32 ax_msg_enable; +static int ax_mii_init(struct net_device *dev); + /* device private data */ struct ax_device { @@ -396,6 +398,10 @@ static int ax_open(struct net_device *dev) netdev_dbg(dev, "open\n"); + ret = ax_mii_init(dev); + if (ret) + goto failed_request_irq; + ret = request_irq(dev->irq, ax_ei_interrupt, ax->irqflags, dev->name, dev); if (ret) @@ -442,6 +448,9 @@ static int ax_close(struct net_device *dev) phy_disconnect(dev->phydev); free_irq(dev->irq, dev); + + mdiobus_unregister(ax->mii_bus); + free_mdio_bitbang(ax->mii_bus); return 0; } @@ -758,10 +767,6 @@ static int ax_init_dev(struct net_device *dev) dev->netdev_ops = &ax_netdev_ops; dev->ethtool_ops = &ax_ethtool_ops; - ret = ax_mii_init(dev); - if (ret) - goto err_out; - ax_NS8390_init(dev, 0); ret = register_netdev(dev); -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html