From: Andrew Halaney <ahalaney@redhat.com>
To: Rob Herring <robh@kernel.org>, Saravana Kannan <saravanak@google.com>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>,
Andrew Lunn <andrew@lunn.ch>,
Abhishek Chauhan <quic_abchauha@quicinc.com>,
Serge Semin <fancer.lancer@gmail.com>,
devicetree@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Andrew Halaney <ahalaney@redhat.com>
Subject: [PATCH RFT] of: property: fw_devlink: Add support for the "phy-handle" binding
Date: Mon, 30 Sep 2024 16:28:38 -0500 [thread overview]
Message-ID: <20240930-phy-handle-fw-devlink-v1-1-4ea46acfcc12@redhat.com> (raw)
Add support for parsing the phy-handle binding so that fw_devlink can
enforce the dependency. This prevents MACs (that use this binding to
claim they're using the corresponding phy) from probing prior to the
phy, unless the phy is a child of the MAC (which results in a
dependency cycle) or similar.
For some motivation, imagine a device topology like so:
ðernet0 {
phy-mode = "sgmii";
phy-handle = <&sgmii_phy0>;
mdio {
compatible = "snps,dwmac-mdio";
sgmii_phy0: phy@8 {
compatible = "ethernet-phy-id0141.0dd4";
reg = <0x8>;
device_type = "ethernet-phy";
};
sgmii_phy1: phy@a {
compatible = "ethernet-phy-id0141.0dd4";
reg = <0xa>;
device_type = "ethernet-phy";
};
};
};
ðernet1 {
phy-mode = "sgmii";
phy-handle = <&sgmii_phy1>;
};
Here ethernet1 depends on sgmii_phy1 to function properly. In the below
link an issue is reported where ethernet1 is probed and used prior to
sgmii_phy1, resulting in a failure to get things working for ethernet1.
With this change in place ethernet1 doesn't probe until sgmii_phy1 is
ready, resulting in ethernet1 functioning properly.
ethernet0 consumes sgmii_phy0, but this dependency isn't enforced
via the device_links backing fw_devlink since ethernet0 is the parent of
sgmii_phy0. Here's a log showing that in action:
[ 7.000432] qcom-ethqos 23040000.ethernet: Fixed dependency cycle(s) with /soc@0/ethernet@23040000/mdio/phy@8
With this change in place ethernet1's dependency is properly described,
and it doesn't probe prior to sgmii_phy1 being available.
Link: https://lore.kernel.org/netdev/7723d4l2kqgrez3yfauvp2ueu6awbizkrq4otqpsqpytzp45q2@rju2nxmqu4ew/
Suggested-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---
I've marked this as an RFT because when looking through old mailing
list discusssions and kernel tech talks on this subject, I was unable
to really understand why in the past phy-handle had been left out. There
were some loose references to circular dependencies (which seem more or
less handled by fw_devlink to me), and the fact that a lot of behavior
happens in ndo_open() (but I couldn't quite grok the concern there).
I'd appreciate more testing by others and some feedback from those who
know this a bit better to indicate whether fw_devlink is ready to handle
this or not.
At least in my narrow point of view, it's working well for me.
Thanks,
Andrew
---
drivers/of/property.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 11b922fde7af..4a2fca75e1c6 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1220,6 +1220,7 @@ DEFINE_SIMPLE_PROP(hwlocks, "hwlocks", "#hwlock-cells")
DEFINE_SIMPLE_PROP(extcon, "extcon", NULL)
DEFINE_SIMPLE_PROP(nvmem_cells, "nvmem-cells", "#nvmem-cell-cells")
DEFINE_SIMPLE_PROP(phys, "phys", "#phy-cells")
+DEFINE_SIMPLE_PROP(phy_handle, "phy-handle", NULL)
DEFINE_SIMPLE_PROP(wakeup_parent, "wakeup-parent", NULL)
DEFINE_SIMPLE_PROP(pinctrl0, "pinctrl-0", NULL)
DEFINE_SIMPLE_PROP(pinctrl1, "pinctrl-1", NULL)
@@ -1366,6 +1367,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
{ .parse_prop = parse_extcon, },
{ .parse_prop = parse_nvmem_cells, },
{ .parse_prop = parse_phys, },
+ { .parse_prop = parse_phy_handle, },
{ .parse_prop = parse_wakeup_parent, },
{ .parse_prop = parse_pinctrl0, },
{ .parse_prop = parse_pinctrl1, },
---
base-commit: cea5425829f77e476b03702426f6b3701299b925
change-id: 20240829-phy-handle-fw-devlink-b829622200ae
Best regards,
--
Andrew Halaney <ahalaney@redhat.com>
next reply other threads:[~2024-09-30 21:28 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-30 21:28 Andrew Halaney [this message]
2024-10-01 0:12 ` [PATCH RFT] of: property: fw_devlink: Add support for the "phy-handle" binding Saravana Kannan
2024-10-01 19:22 ` Andrew Halaney
2024-10-01 20:07 ` Andrew Lunn
2024-10-02 19:34 ` Andrew Halaney
2024-10-03 2:07 ` Saravana Kannan
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=20240930-phy-handle-fw-devlink-v1-1-4ea46acfcc12@redhat.com \
--to=ahalaney@redhat.com \
--cc=andrew@lunn.ch \
--cc=devicetree@vger.kernel.org \
--cc=fancer.lancer@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=quic_abchauha@quicinc.com \
--cc=robh@kernel.org \
--cc=saravanak@google.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).