netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	netdev@vger.kernel.org, davem@davemlof.net
Subject: Re: [PATCH net-next v2 5/5] net: dsa: bcm_sf2: Register our slave MDIO bus
Date: Tue, 7 Jun 2016 19:00:50 +0200	[thread overview]
Message-ID: <20160607170050.GA28994@lunn.ch> (raw)
In-Reply-To: <87r3c9vtay.fsf@ketchup.mtl.sfl>

On Tue, Jun 07, 2016 at 12:48:37PM -0400, Vivien Didelot wrote:
> Hi Florian, Andrew,
> 
> Vivien Didelot <vivien.didelot@savoirfairelinux.com> writes:
> 
> > Hum reviewing that again, I see that if one of the 2 subsequent calls to
> > request_irq fails, you end up with an unregistered MDIO bus.
> >
> > We have the same issue in the mv88e6xxx legacy probe code if dsa.c fails
> > to allocate the dsa_switch structure. I'm moving the MDIO register code
> > to the setup function like you are doing here (good idea!) to fix that.
> 
> In fact it doesn't fix the issue because dsa_switch_driver doesn't
> provide any remove/teardown function, in which mv88e6xxx and sf2 could
> unregister their MDIO bus on switch removal.
> 
> Would it be worth it to add such optional function to DSA drivers?

It is not needed with DSA2 binding. The driver is always in control,
and it performs the unregister from the core. So it knows when to
unregister the mdio bus, either because probe has failed for some
reason, or it is being unloaded. That is also why i register the mdio
bus in probe, and unregister it in remove. Normal practice for a
driver.

With the legacy interface it is tricky. When would you call such a
remove/tairdown function when using the old binding?

	Andrew

  reply	other threads:[~2016-06-07 17:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-06 23:14 [PATCH net-next v2 0/5] net: dsa: misc improvements Florian Fainelli
2016-06-06 23:14 ` [PATCH net-next v2 1/5] net: dsa: Provide unique DSA slave MII bus names Florian Fainelli
2016-06-06 23:14 ` [PATCH net-next v2 2/5] net: dsa: Initialize ds->enabled_port_mask and ds->phys_mii_mask Florian Fainelli
2016-06-07  0:22   ` Andrew Lunn
2016-06-07 13:05     ` Andrew Lunn
2016-06-06 23:14 ` [PATCH net-next v2 3/5] net: dsa: Add initialization helper for CPU port ethtool_ops Florian Fainelli
2016-06-07 13:05   ` Vivien Didelot
2016-06-06 23:14 ` [PATCH net-next v2 4/5] net: dsa: Initialize CPU port ethtool ops per tree Florian Fainelli
2016-06-07  0:34   ` Andrew Lunn
2016-06-06 23:14 ` [PATCH net-next v2 5/5] net: dsa: bcm_sf2: Register our slave MDIO bus Florian Fainelli
2016-06-07  0:39   ` Andrew Lunn
2016-06-07 13:23   ` Vivien Didelot
2016-06-07 14:46   ` Vivien Didelot
2016-06-07 16:48     ` Vivien Didelot
2016-06-07 17:00       ` Andrew Lunn [this message]
2016-06-07 17:33         ` Vivien Didelot
2016-06-07 18:44           ` Andrew Lunn
2016-06-07 19:11             ` Vivien Didelot
2016-06-07 19:29               ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160607170050.GA28994@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=davem@davemlof.net \
    --cc=f.fainelli@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@savoirfairelinux.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).