All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Geert Uytterhoeven <geert@linux-m68k.org>,
	David Bauer <mail@david-bauer.net>
Cc: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Marek Behún" <marek.behun@nic.cz>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"David Bauer" <mail@david-bauer.net>,
	"David S. Miller" <davem@davemloft.net>,
	"Sergei Shtylyov" <sergei.shtylyov@cogentembedded.com>,
	netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH net 1/1] mdio_bus: fix mdio_register_device when RESET_CONTROLLER is disabled
Date: Thu, 21 Nov 2019 03:08:22 +0100	[thread overview]
Message-ID: <20191121020822.GD18325@lunn.ch> (raw)
In-Reply-To: <alpine.DEB.2.21.1911201053330.25420@ramsan.of.borg>

> The difference with the non-optional case is that
> __devm_reset_control_get() registers a cleanup function if there's
> no error condition, even for NULL (which is futile, will send a patch).
> 
> However, more importantly, mdiobus_register_reset() calls a devm_*()
> function on "&mdiodev->dev" ("mdio_bus ee700000.ethernet-ffffffff:01"),
> which is a different device than the one being probed
> (("ee700000.ethernet"), see also the callstack below).
> In fact "&mdiodev->dev" hasn't been probed yet, leading to the WARNING
> when it is probed later.
> 
>     [<c0582de8>] (mdiobus_register_device) from [<c05810e0>] (phy_device_register+0xc/0x74)
>     [<c05810e0>] (phy_device_register) from [<c0675ef4>] (of_mdiobus_register_phy+0x144/0x17c)
>     [<c0675ef4>] (of_mdiobus_register_phy) from [<c06760f0>] (of_mdiobus_register+0x1c4/0x2d0)
>     [<c06760f0>] (of_mdiobus_register) from [<c0589f0c>] (sh_eth_drv_probe+0x778/0x8ac)
>     [<c0589f0c>] (sh_eth_drv_probe) from [<c0516ce8>] (platform_drv_probe+0x48/0x94)
> 
> Has commit 71dd6c0dff51b5f1 ("net: phy: add support for
> reset-controller") been tested with an actual reset present?
> 
> Are Ethernet drivers not (no longer) allowed to register MDIO busses?

That is not good. The devm_reset_control_get() call need replaces with
an unmanaged version, and a call to reset_control_put() added to
mdiobus_unregister_device().

David, could you look at this, it was a patch from you that added
this.

	Andrew

  reply	other threads:[~2019-11-21  2:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-18 18:15 [PATCH net 1/1] mdio_bus: fix mdio_register_device when RESET_CONTROLLER is disabled Marek Behún
2019-11-18 23:38 ` Andrew Lunn
2019-11-19  2:00 ` David Miller
2019-11-19 10:27 ` Andy Shevchenko
2019-11-20 10:36   ` Geert Uytterhoeven
2019-11-20 10:42     ` Geert Uytterhoeven
2019-11-21  2:08     ` Andrew Lunn [this message]
2019-11-21  8:38       ` David Bauer
2019-11-21  8:40         ` Geert Uytterhoeven

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=20191121020822.GD18325@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mail@david-bauer.net \
    --cc=marek.behun@nic.cz \
    --cc=netdev@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.