netdev - Re: [PATCH net-next v2 5/5] net: dsa: bcm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160607170050.GA28994@lunn.ch>
Date:	Tue, 7 Jun 2016 19:00:50 +0200
From:	Andrew Lunn <andrew@...n.ch>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:	Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
	davem@...emlof.net
Subject: Re: [PATCH net-next v2 5/5] net: dsa: bcm_sf2: Register our slave
 MDIO bus

On Tue, Jun 07, 2016 at 12:48:37PM -0400, Vivien Didelot wrote:
> Hi Florian, Andrew,
> 
> Vivien Didelot <vivien.didelot@...oirfairelinux.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