linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Sean Anderson <sean.anderson@seco.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Madalin Bucur <madalin.bucur@nxp.com>,
	"David S . Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Frank Rowand <frowand.list@gmail.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Li Yang <leoyang.li@nxp.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Paul Mackerras <paulus@samba.org>,
	Rob Herring <robh+dt@kernel.org>,
	Saravana Kannan <saravanak@google.com>,
	Shawn Guo <shawnguo@kernel.org>,
	UNGLinuxDriver@microchip.com,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [RFC PATCH net-next 0/9] net: pcs: Add support for devices probed in the "usual" manner
Date: Wed, 20 Jul 2022 16:53:14 +0300	[thread overview]
Message-ID: <20220720135314.5cjxiifrq5ig4vjb@skbuf> (raw)
In-Reply-To: <c0a11900-5a31-ca90-220f-74e3380cef8c@seco.com> <c0a11900-5a31-ca90-220f-74e3380cef8c@seco.com>

On Tue, Jul 19, 2022 at 03:34:45PM -0400, Sean Anderson wrote:
> We could do it, but it'd be a pretty big hack. Something like the
> following. Phylink would need to be modified to grab the lock before
> every op and check if the PCS is dead or not. This is of course still
> not optimal, since there's no way to re-attach a PCS once it goes away.

You assume it's just phylink who operates on a PCS structure, but if you
include your search pool to also cover include/linux/pcs/pcs-xpcs.h,
you'll see a bunch of exported functions which are called directly by
the client drivers (stmmac, sja1105). At this stage it gets pretty hard
to validate that drivers won't attempt from any code path to do
something stupid with a dead PCS. All in all it creates an environment
with insanely weak guarantees; that's pretty hard to get behind IMO.

> IMO a better solution is to use devlink and submit a patch to add
> notifications which the MAC driver can register for. That way it can
> find out when the PCS goes away and potentially do something about it
> (or just let itself get removed).

Not sure I understand what connection there is between devlink (device
links) and PCS {de}registration notifications. We could probably add those
notifications without any intervention from the device core: we would
just need to make this new PCS "core" to register an blocking_notifier_call_chain
to which interested drivers could add their notifier blocks. How a
certain phylink user is going to determine that "hey, this PCS is
definitely mine and I can use it" is an open question. In any case, my
expectation is that we have a notifier chain, we can at least continue
operating (avoid unbinding the struct device), but essentially move our
phylink_create/phylink_destroy calls to within those notifier blocks.

Again, retrofitting this model to existing drivers, phylink API (and
maybe even its internal structure) is something that's hard to hop on
board of; I think it's a solution waiting for a problem, and I don't
have an interest to develop or even review it.

  reply	other threads:[~2022-07-20 13:53 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-11 16:05 [RFC PATCH net-next 0/9] net: pcs: Add support for devices probed in the "usual" manner Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 1/9] dt-bindings: net: Add lynx PCS Sean Anderson
2022-07-12  8:47   ` Krzysztof Kozlowski
2022-07-12 14:57     ` Sean Anderson
2022-07-12 15:00       ` Krzysztof Kozlowski
2022-07-12 15:06         ` Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 2/9] dt-bindings: net: Expand pcs-handle to an array Sean Anderson
2022-07-12  8:51   ` Krzysztof Kozlowski
2022-07-12 15:06     ` Sean Anderson
2022-07-12 15:18       ` Krzysztof Kozlowski
2022-07-12 15:23         ` Sean Anderson
2022-07-12 15:59         ` Russell King (Oracle)
2022-07-14 10:45           ` Krzysztof Kozlowski
2022-07-18 19:46             ` Rob Herring
2022-07-11 16:05 ` [RFC PATCH net-next 3/9] net: pcs: Add helpers for registering and finding PCSs Sean Anderson
2022-07-11 19:42   ` Saravana Kannan
2022-07-11 19:53     ` Sean Anderson
2022-07-11 20:59   ` Russell King (Oracle)
2022-07-11 21:47     ` Sean Anderson
2022-07-11 21:55       ` Sean Anderson
2022-07-12 15:51       ` Russell King (Oracle)
2022-07-12 23:15         ` Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 4/9] net: pcs: lynx: Convert to an mdio driver Sean Anderson
2022-07-19 16:01   ` Vladimir Oltean
2022-07-19 16:16     ` Sean Anderson
2022-07-19 16:20       ` Vladimir Oltean
2022-07-11 16:05 ` [RFC PATCH net-next 5/9] net: pcs: lynx: Use pcs_get_by_provider to get PCS Sean Anderson
2022-07-19 17:26   ` Vladimir Oltean
2022-07-19 19:41     ` Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 6/9] net: pcs: lynx: Remove lynx_get_mdio_device and lynx_pcs_destroy Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 7/9] arm64: dts: Add compatible strings for Lynx PCSs Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 8/9] powerpc: " Sean Anderson
2022-07-11 16:05 ` [RFC PATCH net-next 9/9] net: pcs: lynx: Remove remaining users of lynx_pcs_create Sean Anderson
2022-07-18 19:44   ` Rob Herring
2022-07-19 15:25 ` [RFC PATCH net-next 0/9] net: pcs: Add support for devices probed in the "usual" manner Vladimir Oltean
2022-07-19 15:28   ` Sean Anderson
2022-07-19 15:38     ` Vladimir Oltean
2022-07-19 15:46       ` Sean Anderson
2022-07-19 18:11         ` Vladimir Oltean
2022-07-19 19:34           ` Sean Anderson
2022-07-20 13:53             ` Vladimir Oltean [this message]
2022-07-21 21:42               ` Sean Anderson
     [not found]               ` <8622e12e-66c9-e338-27a1-07e53390881e@seco.com>
2022-07-29 22:15                 ` Sean Anderson

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=20220720135314.5cjxiifrq5ig4vjb@skbuf \
    --to=olteanv@gmail.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=benh@kernel.crashing.org \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=frowand.list@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=kuba@kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=madalin.bucur@nxp.com \
    --cc=mpe@ellerman.id.au \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=paulus@samba.org \
    --cc=robh+dt@kernel.org \
    --cc=saravanak@google.com \
    --cc=sean.anderson@seco.com \
    --cc=shawnguo@kernel.org \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.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).