netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Eric Woudstra <ericwouds@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Frank Wunderlich <frank-w@public-files.de>,
	Daniel Golle <daniel@makrotopia.org>,
	Lucien Jheng <lucien.jheng@airoha.com>,
	Zhi-Jun You <hujy652@protonmail.com>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 net-next 2/2] net: phy: air_en8811h: Add the Airoha EN8811H PHY driver
Date: Tue, 5 Mar 2024 14:24:25 +0000	[thread overview]
Message-ID: <ZecrGTsBZ9VgsGZ+@shell.armlinux.org.uk> (raw)
In-Reply-To: <0184291e-a3c7-4e54-8c75-5b8654d582b4@lunn.ch>

On Tue, Mar 05, 2024 at 03:06:45PM +0100, Andrew Lunn wrote:
> > The only way I can see around this problem would be to look up the
> > PHY in order to get a pointer to the struct phy_device in the network
> > device's probe function, and attach the PHY there _before_ you register
> > the network device. You can then return EPROBE_DEFER and, because you
> > are returning it in a .probe function, the probe will be retried once
> > other probes in the system (such as your PHY driver) have finished.
> > This also means that userspace doesn't see the appearance of the
> > non-functional network device until it's ready, and thus can use
> > normal hotplug mechanisms to notice the network device.
> 
> What i'm thinking is we add another op to phy_driver dedicated to
> firmware download. We let probe run as is, so the PHY is registered
> and available. But if the firmware op is set, we start a thread and
> call the op in it. Once the op exits, we signal a completion event.
> phy_attach_direct() would then wait on the completion.

That's really not good, because phy_attach_direct() can be called
from .ndo_open, which will result in the rtnl lock being held while
we wait - so this is not much better than having the firmware load
in .config_init.

If we drop the lock, then we need to audit what the effect of that
would be - for example, if the nic is being opened, it may mean
that another attempt to open the nic could be started. Or it may
mean that an attempt to configure the nic down could be started.
Then the original open proceeds and state is now messed up.

I do get the feeling that trying to work around "I don't want the
firmware in the initramfs" is creating more problems and pain than
it's worth.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2024-03-05 14:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-02 18:38 [PATCH v2 net-next 0/2] Add en8811h phy driver and devicetree binding doc Eric Woudstra
2024-03-02 18:38 ` [PATCH v2 net-next 1/2] dt-bindings: net: airoha,en8811h: Add en8811h Eric Woudstra
2024-03-04  8:09   ` Krzysztof Kozlowski
2024-03-02 18:38 ` [PATCH v2 net-next 2/2] net: phy: air_en8811h: Add the Airoha EN8811H PHY driver Eric Woudstra
2024-03-03  2:37   ` Daniel Golle
2024-03-03 14:22     ` Eric Woudstra
2024-03-03 16:47     ` Andrew Lunn
2024-03-03 17:03       ` Daniel Golle
2024-03-03 17:08         ` Andrew Lunn
2024-03-05  8:24     ` Eric Woudstra
2024-03-03 17:29   ` Andrew Lunn
2024-03-05  8:13     ` Eric Woudstra
2024-03-05 13:54       ` Andrew Lunn
2024-03-07 16:48         ` Eric Woudstra
2024-03-07 17:40           ` Daniel Golle
2024-03-07 18:47             ` Andrew Lunn
2024-03-03 17:51   ` Andrew Lunn
2024-03-05  8:19     ` Eric Woudstra
2024-03-05  9:03       ` Russell King (Oracle)
2024-03-05 14:06         ` Andrew Lunn
2024-03-05 14:24           ` Russell King (Oracle) [this message]
2024-03-05 15:59             ` Andrew Lunn
2024-03-07 16:50             ` Eric Woudstra
2024-03-04 18:05   ` Simon Horman
2024-03-05  8:39     ` Eric Woudstra

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=ZecrGTsBZ9VgsGZ+@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=ericwouds@gmail.com \
    --cc=frank-w@public-files.de \
    --cc=hkallweit1@gmail.com \
    --cc=hujy652@protonmail.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=kuba@kernel.org \
    --cc=lucien.jheng@airoha.com \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=robh+dt@kernel.org \
    /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).