[net-next:,v4,2/8] net: mdio: switch fixed-link PHYs API to fwnode_
Commit Message
fixed-link PHYs API is used by DSA and a number of drivers
and was depending on of_. Switch to fwnode_ so to make it
hardware description agnostic and allow to be used in ACPI
world as well.
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
include/linux/fwnode_mdio.h | 19 ++++
drivers/net/mdio/fwnode_mdio.c | 96 ++++++++++++++++++++
drivers/net/mdio/of_mdio.c | 79 +---------------
3 files changed, 118 insertions(+), 76 deletions(-)
Comments
On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> fixed-link PHYs API is used by DSA and a number of drivers
> and was depending on of_. Switch to fwnode_ so to make it
> hardware description agnostic and allow to be used in ACPI
> world as well.
Would it be better to let the fixed-link PHY die, and have everyone use
the more flexible fixed link implementation in phylink?
On Mon, Jan 16, 2023 at 05:50:13PM +0000, Russell King (Oracle) wrote:
> On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> > fixed-link PHYs API is used by DSA and a number of drivers
> > and was depending on of_. Switch to fwnode_ so to make it
> > hardware description agnostic and allow to be used in ACPI
> > world as well.
>
> Would it be better to let the fixed-link PHY die, and have everyone use
> the more flexible fixed link implementation in phylink?
Would it be even better if DSA had some driver-level prerequisites to
impose for ACPI support - like phylink support rather than adjust_link -
and we would simply branch off to a dsa_shared_port_link_register_acpi()
function, leaving the current dsa_shared_port_link_register_of() alone,
with all its workarounds and hacks? I don't believe that carrying all
that logic over to a common fwnode based API is the proper way forward.
On Mon, Jan 16, 2023 at 08:16:18PM +0200, Vladimir Oltean wrote:
> On Mon, Jan 16, 2023 at 05:50:13PM +0000, Russell King (Oracle) wrote:
> > On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> > > fixed-link PHYs API is used by DSA and a number of drivers
> > > and was depending on of_. Switch to fwnode_ so to make it
> > > hardware description agnostic and allow to be used in ACPI
> > > world as well.
> >
> > Would it be better to let the fixed-link PHY die, and have everyone use
> > the more flexible fixed link implementation in phylink?
>
> Would it be even better if DSA had some driver-level prerequisites to
> impose for ACPI support - like phylink support rather than adjust_link -
> and we would simply branch off to a dsa_shared_port_link_register_acpi()
> function, leaving the current dsa_shared_port_link_register_of() alone,
> with all its workarounds and hacks? I don't believe that carrying all
> that logic over to a common fwnode based API is the proper way forward.
I agree with you there, here is little attempt to make a clean ACPI
binding. Most of the attempts to add ACPI support seem to try to take
the short cut for just search/replace of_ with fwnode_. And we then
have to push back and say no, and generally it then goes quiet.
Marcin, please approach this from the other end. Please document in
Documentation/firmware-guide/acpi/dsd what a clean binding should look
like, and then try to implement it.
Andrew
> +int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
> +{
> + struct fixed_phy_status status = {};
> + struct fwnode_handle *fixed_link_node;
> + u32 fixed_link_prop[5];
> + const char *managed;
> + int rc;
> +
> + if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
> + strcmp(managed, "in-band-status") == 0) {
> + /* status is zeroed, namely its .link member */
> + goto register_phy;
> + }
> +
> + /* New binding */
> + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
> + if (fixed_link_node) {
> + status.link = 1;
> + status.duplex = fwnode_property_present(fixed_link_node,
> + "full-duplex");
> + rc = fwnode_property_read_u32(fixed_link_node, "speed",
> + &status.speed);
> + if (rc) {
> + fwnode_handle_put(fixed_link_node);
> + return rc;
> + }
> + status.pause = fwnode_property_present(fixed_link_node, "pause");
> + status.asym_pause = fwnode_property_present(fixed_link_node,
> + "asym-pause");
> + fwnode_handle_put(fixed_link_node);
> +
> + goto register_phy;
> + }
> +
> + /* Old binding */
> + rc = fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
> + ARRAY_SIZE(fixed_link_prop));
> + if (rc)
> + return rc;
> +
> + status.link = 1;
> + status.duplex = fixed_link_prop[1];
> + status.speed = fixed_link_prop[2];
> + status.pause = fixed_link_prop[3];
> + status.asym_pause = fixed_link_prop[4];
This is one example of the issue i just pointed out. The "Old binding"
has been deprecated for years. Maybe a decade? There is no reason it
should be used in ACPI.
Andrew
Hi Marcin,
On Mon, 2023-01-16 at 18:34 +0100, Marcin Wojtas wrote:
> fixed-link PHYs API is used by DSA and a number of drivers
> and was depending on of_. Switch to fwnode_ so to make it
> hardware description agnostic and allow to be used in ACPI
> world as well.
>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> include/linux/fwnode_mdio.h | 19 ++++
> drivers/net/mdio/fwnode_mdio.c | 96 ++++++++++++++++++++
> drivers/net/mdio/of_mdio.c | 79 +---------------
> 3 files changed, 118 insertions(+), 76 deletions(-)
>
> #endif /* __LINUX_FWNODE_MDIO_H */
> diff --git a/drivers/net/mdio/fwnode_mdio.c
> b/drivers/net/mdio/fwnode_mdio.c
> index b782c35c4ac1..56f57381ae69 100644
> --- a/drivers/net/mdio/fwnode_mdio.c
> +++ b/drivers/net/mdio/fwnode_mdio.c
> @@ -10,6 +10,7 @@
> #include <linux/fwnode_mdio.h>
> #include <linux/of.h>
> #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
> #include <linux/pse-pd/pse.h>
>
> MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
> @@ -185,3 +186,98 @@ int fwnode_mdiobus_register_phy(struct mii_bus
> *bus,
> return rc;
> }
> EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
> +
> +/*
> + * fwnode_phy_is_fixed_link() and fwnode_phy_register_fixed_link()
> must
> + * support two bindings:
> + * - the old binding, where 'fixed-link' was a property with 5
> + * cells encoding various information about the fixed PHY
> + * - the new binding, where 'fixed-link' is a sub-node of the
> + * Ethernet device.
> + */
networking comment style
> +bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *fixed_link_node;
> + const char *managed;
> +
> + /* New binding */
> + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-
> link");
> + if (fixed_link_node) {
> + fwnode_handle_put(fixed_link_node);
> + return true;
> + }
> +
> + if (fwnode_property_read_string(fwnode, "managed", &managed) ==
> 0 &&
> + strcmp(managed, "auto") != 0)
> + return true;
> +
> + /* Old binding */
> + return fwnode_property_count_u32(fwnode, "fixed-link") == 5;
> +}
> +EXPORT_SYMBOL(fwnode_phy_is_fixed_link);
> +
> +int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
> +{
> + struct fixed_phy_status status = {};
> + struct fwnode_handle *fixed_link_node;
Reverse christmas tree
> + u32 fixed_link_prop[5];
> + const char *managed;
> + int rc;
> +
> + if (fwnode_property_read_string(fwnode, "managed", &managed) ==
> 0 &&
> + strcmp(managed, "in-band-status") == 0) {
> + /* status is zeroed, namely its .link member */
> + goto register_phy;
> + }
> +
> + /* New binding */
> + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-
> link");
> + if (fixed_link_node) {
> + status.link = 1;
> + status.duplex =
> fwnode_property_present(fixed_link_node,
> + "full-duplex");
> + rc = fwnode_property_read_u32(fixed_link_node, "speed",
> + &status.speed);
> + if (rc) {
> + fwnode_handle_put(fixed_link_node);
> + return rc;
> + }
> + status.pause = fwnode_property_present(fixed_link_node,
> "pause");
> + status.asym_pause =
> fwnode_property_present(fixed_link_node,
> + "asym-
> pause");
> + fwnode_handle_put(fixed_link_node);
> +
> + goto register_phy;
> + }
> +
> + /* Old binding */
> + rc = fwnode_property_read_u32_array(fwnode, "fixed-link",
> fixed_link_prop,
> + ARRAY_SIZE(fixed_link_prop)
> );
> + if (rc)
> + return rc;
> +
> + status.link = 1;
> + status.duplex = fixed_link_prop[1];
> + status.speed = fixed_link_prop[2];
> + status.pause = fixed_link_prop[3];
> + status.asym_pause = fixed_link_prop[4];
> +
> +register_phy:
> + return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status,
> fwnode));
> +}
> +EXPORT_SYMBOL(fwnode_phy_register_fixed_link);
> +
>
>
Hi Andrew and Vladimir,
pon., 16 sty 2023 o 23:04 Andrew Lunn <andrew@lunn.ch> napisał(a):
>
> On Mon, Jan 16, 2023 at 08:16:18PM +0200, Vladimir Oltean wrote:
> > On Mon, Jan 16, 2023 at 05:50:13PM +0000, Russell King (Oracle) wrote:
> > > On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> > > > fixed-link PHYs API is used by DSA and a number of drivers
> > > > and was depending on of_. Switch to fwnode_ so to make it
> > > > hardware description agnostic and allow to be used in ACPI
> > > > world as well.
> > >
> > > Would it be better to let the fixed-link PHY die, and have everyone use
> > > the more flexible fixed link implementation in phylink?
> >
> > Would it be even better if DSA had some driver-level prerequisites to
> > impose for ACPI support - like phylink support rather than adjust_link -
> > and we would simply branch off to a dsa_shared_port_link_register_acpi()
> > function, leaving the current dsa_shared_port_link_register_of() alone,
> > with all its workarounds and hacks? I don't believe that carrying all
> > that logic over to a common fwnode based API is the proper way forward.
In the past couple of years, a number of subsystems have migrated to a
more generic HW description abstraction (e.g. a big chunk of network,
pinctrl, gpio). ACPI aside, with this patchset one can even try to
describe the switch topology with the swnode (I haven't tried that
though). I fully agree that there should be no 0-day baggage in the
DSA ACPI binding (FYI the more fwnode- version of the
dsa_shared_port_validate_of() cought one issue in the WIP ACPI
description in my setup). On the other hand, I find fwnode_/device_
APIs really helpful for most of the cases - ACPI/OF/swnode differences
can be hidden to a generic layer and the need of maintaining separate
code paths related to the hardware description on the driver/subsystem
level is minimized. An example could be found in v1 of this series,
the last 4 patches in [1] show that it can be done in a simple /
seamless way, especially given the ACPI (fwnode) PHY description in
phylink is already settled and widely used. I am aware at the end of
the day, after final review all this can be more complex.
I expect that the actual DSA ACPI support acceptance will require a
lot of discussions and decisions, on whether certain solutions are
worth migrating from OF world or require spec modification. For now my
goal was to migrate to a more generic HW description API, and so to
allow possible follow-up ACPI-related modifications, and additions to
be extracted and better tracked.
>
> I agree with you there, here is little attempt to make a clean ACPI
> binding. Most of the attempts to add ACPI support seem to try to take
> the short cut for just search/replace of_ with fwnode_. And we then
> have to push back and say no, and generally it then goes quiet.
In most cases, the devices' description is pretty straightforward:
* a node (single or with some children), resources (mem, irqs), mmio
address space, optionally address on a bus and a couple of properties
The DSDT/SSDT tables are very well suited for this. In case of
separate, contained drivers that is also really easy to maintain.
However, I fully understand your concerns and caution before blessing
any change related to subsystem/generic code. Therefore ACPI support
addition was split after v1 (refer to discussion in [1]) and will
require ACPI maintainers' input and guidelines.
>
> Marcin, please approach this from the other end. Please document in
> Documentation/firmware-guide/acpi/dsd what a clean binding should look
> like, and then try to implement it.
>
This is how I initially approached this (original submission: [2]; a
bit updated version, working on top of the current patchset: [3]). We
then agreed that in order to remove a bit hacky mitigation of the
double ACPI scan problem, an MDIOSerialBus _CRS method should be
defined in the ACPI spec, similar to the
I2CSerialBus/SPISerialBus/UARTSerialBus. I am going to submit the
first version for review in the coming days. The DSA purely
ACPI-related changes would be updated and submitted, once the method
is accepted.
Best regards,
Marcin
[1] https://www.spinics.net/lists/netdev/msg827337.html
[2] https://www.spinics.net/lists/netdev/msg827345.html
[3] https://github.com/semihalf-wojtas-marcin/Linux-Kernel/commit/e017e69c0eda18747029bfe0c335df204670ba59
On Tue, Jan 17, 2023 at 05:05:53PM +0100, Marcin Wojtas wrote:
> In the past couple of years, a number of subsystems have migrated to a
> more generic HW description abstraction (e.g. a big chunk of network,
> pinctrl, gpio). ACPI aside, with this patchset one can even try to
> describe the switch topology with the swnode (I haven't tried that
> though). I fully agree that there should be no 0-day baggage in the
> DSA ACPI binding (FYI the more fwnode- version of the
> dsa_shared_port_validate_of() cought one issue in the WIP ACPI
> description in my setup). On the other hand, I find fwnode_/device_
> APIs really helpful for most of the cases - ACPI/OF/swnode differences
> can be hidden to a generic layer and the need of maintaining separate
> code paths related to the hardware description on the driver/subsystem
> level is minimized. An example could be found in v1 of this series,
> the last 4 patches in [1] show that it can be done in a simple /
> seamless way, especially given the ACPI (fwnode) PHY description in
> phylink is already settled and widely used. I am aware at the end of
> the day, after final review all this can be more complex.
>
> I expect that the actual DSA ACPI support acceptance will require a
> lot of discussions and decisions, on whether certain solutions are
> worth migrating from OF world or require spec modification. For now my
> goal was to migrate to a more generic HW description API, and so to
> allow possible follow-up ACPI-related modifications, and additions to
> be extracted and better tracked.
I have a simple question.
If you expect that the DSA ACPI bindings will require a lot of
discussions, then how do you know that what you convert to fwnode now
will be needed later, and why do you insist to mechanically convert
everything to fwnode without having that discussion first?
You see the lack of a need to maintain separate code paths between OF
and ACPI as useful. Yet the DSA maintainers don't, and in some cases
this is specifically what they want to avoid. So a mechanical conversion
will end up making absolutely no progress.
Hi Marcin,
On Tue, Jan 17, 2023 at 05:20:01PM +0100, Marcin Wojtas wrote:
> Hi Russell,
>
>
> pon., 16 sty 2023 o 18:50 Russell King (Oracle) <linux@armlinux.org.uk>
> napisał(a):
> >
> > On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> > > fixed-link PHYs API is used by DSA and a number of drivers
> > > and was depending on of_. Switch to fwnode_ so to make it
> > > hardware description agnostic and allow to be used in ACPI
> > > world as well.
> >
> > Would it be better to let the fixed-link PHY die, and have everyone use
> > the more flexible fixed link implementation in phylink?
> >
> ,
> This patchset did not intend to introduce any functional change, simply
> switch to a more generic HW description abstraction. Killing
> of/fwnode_phy_(de)register_fixed_link entirely seems to be a challenge, as
> there are a lot of users beyond the DSA. Otoh I see a value in having
> of_/fwnode_phy_is_fixed_link check, afaik there is no equivalent in
> phylink...
Phylink provides a much improved implementation of fixed-link that is
way more flexible than the phylib approach - it can implement speeds
in excess of 1G. DSA already supports phylink with modern updated
drivers that do not use the "adjust_link" implementation.
What I'm proposing is that we don't bring the baggage of the phylib
based fixed link forwards into fwnode, and leave this to be DT-only.
I think this is what Andrew and Vladimir have also said.
Thanks.
Hi,
wt., 17 sty 2023 o 17:34 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Tue, Jan 17, 2023 at 05:05:53PM +0100, Marcin Wojtas wrote:
> > In the past couple of years, a number of subsystems have migrated to a
> > more generic HW description abstraction (e.g. a big chunk of network,
> > pinctrl, gpio). ACPI aside, with this patchset one can even try to
> > describe the switch topology with the swnode (I haven't tried that
> > though). I fully agree that there should be no 0-day baggage in the
> > DSA ACPI binding (FYI the more fwnode- version of the
> > dsa_shared_port_validate_of() cought one issue in the WIP ACPI
> > description in my setup). On the other hand, I find fwnode_/device_
> > APIs really helpful for most of the cases - ACPI/OF/swnode differences
> > can be hidden to a generic layer and the need of maintaining separate
> > code paths related to the hardware description on the driver/subsystem
> > level is minimized. An example could be found in v1 of this series,
> > the last 4 patches in [1] show that it can be done in a simple /
> > seamless way, especially given the ACPI (fwnode) PHY description in
> > phylink is already settled and widely used. I am aware at the end of
> > the day, after final review all this can be more complex.
> >
> > I expect that the actual DSA ACPI support acceptance will require a
> > lot of discussions and decisions, on whether certain solutions are
> > worth migrating from OF world or require spec modification. For now my
> > goal was to migrate to a more generic HW description API, and so to
> > allow possible follow-up ACPI-related modifications, and additions to
> > be extracted and better tracked.
>
> I have a simple question.
>
> If you expect that the DSA ACPI bindings will require a lot of
> discussions, then how do you know that what you convert to fwnode now
> will be needed later, and why do you insist to mechanically convert
> everything to fwnode without having that discussion first?
>
Ok, let me clarify. From the technical standpoint, I think it is
fairly easy and to a very big extent, we should be able to reuse, what
is already existing - I made it work with a really minimal set of
changes, using a standard nodes' hierarchy and generic methods in the
ACPI tables. As more difficult, I consider getting this solution
accepted by the ACPI and the network subsystem maintainers, also given
the OF quirks/legacy stuff, that apparently needs to be ruled out in
such circumstances. However, I perceive a preparation step, with
migrating to the more generic HW description API in the generic
net/dsa, as a sort of improvement, but I get your point and I will
wait with resubmitting these changes again.
> You see the lack of a need to maintain separate code paths between OF
> and ACPI as useful. Yet the DSA maintainers don't, and in some cases
> this is specifically what they want to avoid. So a mechanical conversion
> will end up making absolutely no progress.
Fair enough. I'll keep it on hold until MDIOSerialBus gets accepted
and repost a bigger, combined patchset with all changes like in the
v1.
Best regards,
Marcin
wt., 17 sty 2023 o 18:58 Russell King (Oracle) <linux@armlinux.org.uk>
napisał(a):
>
> Hi Marcin,
>
> On Tue, Jan 17, 2023 at 05:20:01PM +0100, Marcin Wojtas wrote:
> > Hi Russell,
> >
> >
> > pon., 16 sty 2023 o 18:50 Russell King (Oracle) <linux@armlinux.org.uk>
> > napisał(a):
> > >
> > > On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> > > > fixed-link PHYs API is used by DSA and a number of drivers
> > > > and was depending on of_. Switch to fwnode_ so to make it
> > > > hardware description agnostic and allow to be used in ACPI
> > > > world as well.
> > >
> > > Would it be better to let the fixed-link PHY die, and have everyone use
> > > the more flexible fixed link implementation in phylink?
> > >
> > ,
> > This patchset did not intend to introduce any functional change, simply
> > switch to a more generic HW description abstraction. Killing
> > of/fwnode_phy_(de)register_fixed_link entirely seems to be a challenge, as
> > there are a lot of users beyond the DSA. Otoh I see a value in having
> > of_/fwnode_phy_is_fixed_link check, afaik there is no equivalent in
> > phylink...
>
> Phylink provides a much improved implementation of fixed-link that is
> way more flexible than the phylib approach - it can implement speeds
> in excess of 1G. DSA already supports phylink with modern updated
> drivers that do not use the "adjust_link" implementation.
>
> What I'm proposing is that we don't bring the baggage of the phylib
> based fixed link forwards into fwnode, and leave this to be DT-only.
> I think this is what Andrew and Vladimir have also said.
>
Ok, thanks for clarifying.
Best regards,
Marcin
On Tue, Jan 17, 2023 at 05:05:53PM +0100, Marcin Wojtas wrote:
> Hi Andrew and Vladimir,
>
> pon., 16 sty 2023 o 23:04 Andrew Lunn <andrew@lunn.ch> napisał(a):
> >
> > On Mon, Jan 16, 2023 at 08:16:18PM +0200, Vladimir Oltean wrote:
> > > On Mon, Jan 16, 2023 at 05:50:13PM +0000, Russell King (Oracle) wrote:
> > > > On Mon, Jan 16, 2023 at 06:34:14PM +0100, Marcin Wojtas wrote:
> > > > > fixed-link PHYs API is used by DSA and a number of drivers
> > > > > and was depending on of_. Switch to fwnode_ so to make it
> > > > > hardware description agnostic and allow to be used in ACPI
> > > > > world as well.
> > > >
> > > > Would it be better to let the fixed-link PHY die, and have everyone use
> > > > the more flexible fixed link implementation in phylink?
> > >
> > > Would it be even better if DSA had some driver-level prerequisites to
> > > impose for ACPI support - like phylink support rather than adjust_link -
> > > and we would simply branch off to a dsa_shared_port_link_register_acpi()
> > > function, leaving the current dsa_shared_port_link_register_of() alone,
> > > with all its workarounds and hacks? I don't believe that carrying all
> > > that logic over to a common fwnode based API is the proper way forward.
>
> In the past couple of years, a number of subsystems have migrated to a
> more generic HW description abstraction (e.g. a big chunk of network,
> pinctrl, gpio). ACPI aside, with this patchset one can even try to
> describe the switch topology with the swnode (I haven't tried that
> though). I fully agree that there should be no 0-day baggage in the
> DSA ACPI binding (FYI the more fwnode- version of the
> dsa_shared_port_validate_of() cought one issue in the WIP ACPI
> description in my setup). On the other hand, I find fwnode_/device_
> APIs really helpful for most of the cases - ACPI/OF/swnode differences
> can be hidden to a generic layer and the need of maintaining separate
> code paths related to the hardware description on the driver/subsystem
> level is minimized.
It looks like we are heading towards three different descriptions. OF,
ACPI and swnode. Each is likely to be different. OF has a lot of
history in it, deprecated things etc, which should not appear in the
others. So i see a big ugly block of code for the OF binding, and
hopefully clean and tidy code for ACPI binding and a clean and tidy
bit of code for swmode.,
It would be nice if the results of that parsing could be presented to
the drivers in a uniform way, so the driver itself does not need to
care where the information came from. But to me it is clear that this
uniform layer has no direct access to the databases, since the
database as are different.
Andrew
@@ -16,6 +16,11 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
int fwnode_mdiobus_register_phy(struct mii_bus *bus,
struct fwnode_handle *child, u32 addr);
+int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode);
+
+void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode);
+
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode);
#else /* CONFIG_FWNODE_MDIO */
int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
struct phy_device *phy,
@@ -30,6 +35,20 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
{
return -EINVAL;
}
+
+static inline int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
+{
+ return -ENODEV;
+}
+
+static inline void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode)
+{
+}
+
+static inline bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+ return false;
+}
#endif
#endif /* __LINUX_FWNODE_MDIO_H */
@@ -10,6 +10,7 @@
#include <linux/fwnode_mdio.h>
#include <linux/of.h>
#include <linux/phy.h>
+#include <linux/phy_fixed.h>
#include <linux/pse-pd/pse.h>
MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
@@ -185,3 +186,98 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
return rc;
}
EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
+/*
+ * fwnode_phy_is_fixed_link() and fwnode_phy_register_fixed_link() must
+ * support two bindings:
+ * - the old binding, where 'fixed-link' was a property with 5
+ * cells encoding various information about the fixed PHY
+ * - the new binding, where 'fixed-link' is a sub-node of the
+ * Ethernet device.
+ */
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *fixed_link_node;
+ const char *managed;
+
+ /* New binding */
+ fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+ if (fixed_link_node) {
+ fwnode_handle_put(fixed_link_node);
+ return true;
+ }
+
+ if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
+ strcmp(managed, "auto") != 0)
+ return true;
+
+ /* Old binding */
+ return fwnode_property_count_u32(fwnode, "fixed-link") == 5;
+}
+EXPORT_SYMBOL(fwnode_phy_is_fixed_link);
+
+int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct fixed_phy_status status = {};
+ struct fwnode_handle *fixed_link_node;
+ u32 fixed_link_prop[5];
+ const char *managed;
+ int rc;
+
+ if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
+ strcmp(managed, "in-band-status") == 0) {
+ /* status is zeroed, namely its .link member */
+ goto register_phy;
+ }
+
+ /* New binding */
+ fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+ if (fixed_link_node) {
+ status.link = 1;
+ status.duplex = fwnode_property_present(fixed_link_node,
+ "full-duplex");
+ rc = fwnode_property_read_u32(fixed_link_node, "speed",
+ &status.speed);
+ if (rc) {
+ fwnode_handle_put(fixed_link_node);
+ return rc;
+ }
+ status.pause = fwnode_property_present(fixed_link_node, "pause");
+ status.asym_pause = fwnode_property_present(fixed_link_node,
+ "asym-pause");
+ fwnode_handle_put(fixed_link_node);
+
+ goto register_phy;
+ }
+
+ /* Old binding */
+ rc = fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
+ ARRAY_SIZE(fixed_link_prop));
+ if (rc)
+ return rc;
+
+ status.link = 1;
+ status.duplex = fixed_link_prop[1];
+ status.speed = fixed_link_prop[2];
+ status.pause = fixed_link_prop[3];
+ status.asym_pause = fixed_link_prop[4];
+
+register_phy:
+ return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, fwnode));
+}
+EXPORT_SYMBOL(fwnode_phy_register_fixed_link);
+
+void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct phy_device *phydev;
+
+ phydev = fwnode_phy_find_device(fwnode);
+ if (!phydev)
+ return;
+
+ fixed_phy_unregister(phydev);
+
+ put_device(&phydev->mdio.dev); /* fwnode_phy_find_device() */
+ phy_device_free(phydev); /* fixed_phy_register() */
+}
+EXPORT_SYMBOL(fwnode_phy_deregister_fixed_link);
@@ -353,91 +353,18 @@ EXPORT_SYMBOL(of_phy_get_and_connect);
*/
bool of_phy_is_fixed_link(struct device_node *np)
{
- struct device_node *dn;
- int len, err;
- const char *managed;
-
- /* New binding */
- dn = of_get_child_by_name(np, "fixed-link");
- if (dn) {
- of_node_put(dn);
- return true;
- }
-
- err = of_property_read_string(np, "managed", &managed);
- if (err == 0 && strcmp(managed, "auto") != 0)
- return true;
-
- /* Old binding */
- if (of_get_property(np, "fixed-link", &len) &&
- len == (5 * sizeof(__be32)))
- return true;
-
- return false;
+ return fwnode_phy_is_fixed_link(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_phy_is_fixed_link);
int of_phy_register_fixed_link(struct device_node *np)
{
- struct fixed_phy_status status = {};
- struct device_node *fixed_link_node;
- u32 fixed_link_prop[5];
- const char *managed;
-
- if (of_property_read_string(np, "managed", &managed) == 0 &&
- strcmp(managed, "in-band-status") == 0) {
- /* status is zeroed, namely its .link member */
- goto register_phy;
- }
-
- /* New binding */
- fixed_link_node = of_get_child_by_name(np, "fixed-link");
- if (fixed_link_node) {
- status.link = 1;
- status.duplex = of_property_read_bool(fixed_link_node,
- "full-duplex");
- if (of_property_read_u32(fixed_link_node, "speed",
- &status.speed)) {
- of_node_put(fixed_link_node);
- return -EINVAL;
- }
- status.pause = of_property_read_bool(fixed_link_node, "pause");
- status.asym_pause = of_property_read_bool(fixed_link_node,
- "asym-pause");
- of_node_put(fixed_link_node);
-
- goto register_phy;
- }
-
- /* Old binding */
- if (of_property_read_u32_array(np, "fixed-link", fixed_link_prop,
- ARRAY_SIZE(fixed_link_prop)) == 0) {
- status.link = 1;
- status.duplex = fixed_link_prop[1];
- status.speed = fixed_link_prop[2];
- status.pause = fixed_link_prop[3];
- status.asym_pause = fixed_link_prop[4];
- goto register_phy;
- }
-
- return -ENODEV;
-
-register_phy:
- return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, of_fwnode_handle(np)));
+ return fwnode_phy_register_fixed_link(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_phy_register_fixed_link);
void of_phy_deregister_fixed_link(struct device_node *np)
{
- struct phy_device *phydev;
-
- phydev = of_phy_find_device(np);
- if (!phydev)
- return;
-
- fixed_phy_unregister(phydev);
-
- put_device(&phydev->mdio.dev); /* of_phy_find_device() */
- phy_device_free(phydev); /* fixed_phy_register() */
+ fwnode_phy_deregister_fixed_link(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_phy_deregister_fixed_link);