diff mbox

[1/2] of_mdio: add new DT property 'link' for fixed-link

Message ID 559EB1A6.1090008@list.ru
State New, archived
Headers show

Commit Message

Stas Sergeev July 9, 2015, 5:38 p.m. UTC
Currently for fixed-link the link state is always set to UP.
This patch introduces the new property 'link' that accepts the
following string arguments: "up", "down" and "auto".
"down" may be needed if the link is physically unconnected.
"auto" is needed to enable the link paramaters auto-negotiation,
that is built into some MII protocols, namely SGMII.
The appropriate documentation is added and explicitly states that
"auto" is very specific (protocol, HW and driver-specific), and
is therefore should be used with care.

Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>

CC: Rob Herring <robh+dt@kernel.org>
CC: Pawel Moll <pawel.moll@arm.com>
CC: Mark Rutland <mark.rutland@arm.com>
CC: Ian Campbell <ijc+devicetree@hellion.org.uk>
CC: Kumar Gala <galak@codeaurora.org>
CC: Florian Fainelli <f.fainelli@gmail.com>
CC: Grant Likely <grant.likely@linaro.org>
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: netdev@vger.kernel.org
---
 .../devicetree/bindings/net/fixed-link.txt         |  8 +++-
 drivers/of/of_mdio.c                               | 48 ++++++++++++++++++++--
 include/linux/of_mdio.h                            |  5 +++
 3 files changed, 57 insertions(+), 4 deletions(-)

Comments

Florian Fainelli July 9, 2015, 6:24 p.m. UTC | #1
(there is no such thing as linux-net@vger.kernel.org, please remove it
from your future submissions).

On 09/07/15 10:38, Stas Sergeev wrote:
> 
> Currently for fixed-link the link state is always set to UP.

Not quite true, this is always a driver decision to make.

> This patch introduces the new property 'link' that accepts the
> following string arguments: "up", "down" and "auto".
> "down" may be needed if the link is physically unconnected.

In which case you probably do not even care about inserting such a
property in the first place, do you? What would be the value of forcibly
having a link permanently down (not counting loopback)?

> "auto" is needed to enable the link paramaters auto-negotiation,
> that is built into some MII protocols, namely SGMII.

RGMII also has an in-band status FWIW.

> The appropriate documentation is added and explicitly states that
> "auto" is very specific (protocol, HW and driver-specific), and
> is therefore should be used with care.

And therefore probably be made a device (and driver) specific decision
whether this is the right thing to do.

I do no think this addition to the "fixed-link" property is desirable
the way you have defined it.

More comments below.

> 
> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net>
> 
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Pawel Moll <pawel.moll@arm.com>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: Ian Campbell <ijc+devicetree@hellion.org.uk>
> CC: Kumar Gala <galak@codeaurora.org>
> CC: Florian Fainelli <f.fainelli@gmail.com>
> CC: Grant Likely <grant.likely@linaro.org>
> CC: devicetree@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: netdev@vger.kernel.org
> ---
>  .../devicetree/bindings/net/fixed-link.txt         |  8 +++-
>  drivers/of/of_mdio.c                               | 48 ++++++++++++++++++++--
>  include/linux/of_mdio.h                            |  5 +++
>  3 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
> index 82bf7e0..070f554 100644
> --- a/Documentation/devicetree/bindings/net/fixed-link.txt
> +++ b/Documentation/devicetree/bindings/net/fixed-link.txt
> @@ -9,8 +9,14 @@ Such a fixed link situation is described by creating a 'fixed-link'
>  sub-node of the Ethernet MAC device node, with the following
>  properties:
> 
> +* 'link' (string, optional), to indicate the link state. Accepted
> +  values are "up", "down" and "auto". "auto" means auto-negotiation of
> +  link parameters. Auto-negotiation is MII protocol, HW and driver-specific
> +  and is not supported in many cases, so use it only when you know what
> +  you do.
>  * 'speed' (integer, mandatory), to indicate the link speed. Accepted
> -  values are 10, 100 and 1000
> +  values are 10, 100 and 1000. If the 'link' property is set to 'auto',
> +  'speed' may not be set. It will then be auto-negotiated, if possible.
>  * 'full-duplex' (boolean, optional), to indicate that full duplex is
>    used. When absent, half duplex is assumed.
>  * 'pause' (boolean, optional), to indicate that pause should be
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 1bd4305..2152cf8 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -280,6 +280,26 @@ bool of_phy_is_fixed_link(struct device_node *np)
>  }
>  EXPORT_SYMBOL(of_phy_is_fixed_link);
> 
> +bool of_phy_is_autoneg_link(struct device_node *np)
> +{
> +	struct device_node *dn;
> +	const char *link_str;
> +	int rc;
> +	bool ret = false;
> +
> +	dn = of_get_child_by_name(np, "fixed-link");
> +	if (!dn)
> +		return false;
> +
> +	rc = of_property_read_string(dn, "link", &link_str);
> +	if (rc == 0 && strcmp(link_str, "auto") == 0)
> +		ret = true;
> +
> +	of_node_put(dn);
> +	return ret;
> +}
> +EXPORT_SYMBOL(of_phy_is_autoneg_link);
> +
>  int of_phy_register_fixed_link(struct device_node *np)
>  {
>  	struct fixed_phy_status status = {};
> @@ -291,11 +311,33 @@ int of_phy_register_fixed_link(struct device_node *np)
>  	/* New binding */
>  	fixed_link_node = of_get_child_by_name(np, "fixed-link");
>  	if (fixed_link_node) {
> -		status.link = 1;
> +		const char *link_str;
> +		int ret;
> +		bool link_auto = false;
> +
> +		ret = of_property_read_string(fixed_link_node, "link",
> +					      &link_str);
> +		if (ret == 0) {
> +			if (strcmp(link_str, "up") == 0)
> +				status.link = 1;
> +			else
> +				status.link = 0;
> +			if (strcmp(link_str, "auto") == 0)
> +				link_auto = true;
> +		} else {
> +			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))
> -			return -EINVAL;
> +		if (of_property_read_u32(fixed_link_node, "speed",
> +					 &status.speed) != 0) {
> +			/* in auto mode just set to some sane value:
> +			 * it will be changed by MAC later */
> +			if (link_auto)
> +				status.speed = 1000;

This is a completely arbitrary speed, that does not more or less sense
than defaulting to 100 or anything else, a driver should be able to set
the speed it wants, based on the parsing of a 'phy-mode' property for
instance. 1000 does not make sense on e.g: MII links.
Stas Sergeev July 9, 2015, 8:43 p.m. UTC | #2
09.07.2015 21:24, Florian Fainelli пишет:
> (there is no such thing as linux-net@vger.kernel.org, please remove it
> from your future submissions).
>
> On 09/07/15 10:38, Stas Sergeev wrote:
>> Currently for fixed-link the link state is always set to UP.
> Not quite true, this is always a driver decision to make.
But what about this part of of_mdio.c:of_phy_register_fixed_link():
---

  	fixed_link_node = of_get_child_by_name(np, "fixed-link");
  	if (fixed_link_node) {
		status.link = 1

---

>> This patch introduces the new property 'link' that accepts the
>> following string arguments: "up", "down" and "auto".
>> "down" may be needed if the link is physically unconnected.
> In which case you probably do not even care about inserting such a
> property in the first place, do you? What would be the value of forcibly
> having a link permanently down (not counting loopback)?
The DTs have a common parts that are included by other
parts. So if you include the definition of your SoC that have
all ethernets defined, and you only set up the external things
like PHYs, then I would see a potential use for "down".
Other than that, it is probably not a big deal.
Please note that I haven't even hard-coded it anywhere:
whatever is not "up" or "auto", is down.
I can remove it from the description if you think that way,
but I'd rather leave it for consistency and for a small but
possible use. Eg my board has 4 ethernets and only 2 are
connected. I feel its right to include the SoC definition and
set the unconnected ones to "down", but other approaches
are possible too.
Should I remove it?

>> "auto" is needed to enable the link paramaters auto-negotiation,
>> that is built into some MII protocols, namely SGMII.
> RGMII also has an in-band status FWIW.
Thanks, will take that into account in v2.

>> The appropriate documentation is added and explicitly states that
>> "auto" is very specific (protocol, HW and driver-specific), and
>> is therefore should be used with care.
> And therefore probably be made a device (and driver) specific decision
> whether this is the right thing to do.
This doesn't work.
It appears even if the driver supports it and wants to use it, the
PHY HW may simply not generate the inband status. This is actually
the whole point why we have a regression now. It is _currently_
a driver decision, and that doesn't work for some people.
The point of this patch set is to make it a DT decision instead.

>> -			return -EINVAL;
>> +		if (of_property_read_u32(fixed_link_node, "speed",
>> +					 &status.speed) != 0) {
>> +			/* in auto mode just set to some sane value:
>> +			 * it will be changed by MAC later */
>> +			if (link_auto)
>> +				status.speed = 1000;
> This is a completely arbitrary speed, that does not more or less sense
> than defaulting to 100 or anything else,
Exactly.
But if I leave it to 0, then fixed-phy driver will return an error,
so I took an arbitrary value.
But if it obscures the code, I'll hack fixed-phy to accept 0 instead,
to get something cleaner. So in v2.

>   a driver should be able to set
> the speed it wants, based on the parsing of a 'phy-mode' property for
> instance.
It actually does, that value is just to "cheat" fixed-phy.
I'll make things more obvious next time.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli July 9, 2015, 9:15 p.m. UTC | #3
On 09/07/15 13:43, Stas Sergeev wrote:
> 09.07.2015 21:24, Florian Fainelli пишет:
>> (there is no such thing as linux-net@vger.kernel.org, please remove it
>> from your future submissions).
>>
>> On 09/07/15 10:38, Stas Sergeev wrote:
>>> Currently for fixed-link the link state is always set to UP.
>> Not quite true, this is always a driver decision to make.
> But what about this part of of_mdio.c:of_phy_register_fixed_link():
> ---
> 
>      fixed_link_node = of_get_child_by_name(np, "fixed-link");
>      if (fixed_link_node) {
>         status.link = 1
> 
> ---

This seems like a logical consequence of finding a "fixed-link" property
for the DT node of interest. If no such property exist, then we do not
set anything.

> 
>>> This patch introduces the new property 'link' that accepts the
>>> following string arguments: "up", "down" and "auto".
>>> "down" may be needed if the link is physically unconnected.
>> In which case you probably do not even care about inserting such a
>> property in the first place, do you? What would be the value of forcibly
>> having a link permanently down (not counting loopback)?
> The DTs have a common parts that are included by other
> parts. So if you include the definition of your SoC that have
> all ethernets defined, and you only set up the external things
> like PHYs, then I would see a potential use for "down".

"down" is equivalent to using a status = "disabled", in fact the latter
is much better since you can even conserve energy and resources by not
enabling something which is not usable.

> Other than that, it is probably not a big deal.
> Please note that I haven't even hard-coded it anywhere:
> whatever is not "up" or "auto", is down.
> I can remove it from the description if you think that way,
> but I'd rather leave it for consistency and for a small but
> possible use. Eg my board has 4 ethernets and only 2 are
> connected. I feel its right to include the SoC definition and
> set the unconnected ones to "down", but other approaches
> are possible too.
> Should I remove it?

What you describe about your board is the perfect example of how a
"status" property should be used.

> 
>>> "auto" is needed to enable the link paramaters auto-negotiation,
>>> that is built into some MII protocols, namely SGMII.
>> RGMII also has an in-band status FWIW.
> Thanks, will take that into account in v2.
> 
>>> The appropriate documentation is added and explicitly states that
>>> "auto" is very specific (protocol, HW and driver-specific), and
>>> is therefore should be used with care.
>> And therefore probably be made a device (and driver) specific decision
>> whether this is the right thing to do.
> This doesn't work.
> It appears even if the driver supports it and wants to use it, the
> PHY HW may simply not generate the inband status. This is actually
> the whole point why we have a regression now. It is _currently_
> a driver decision, and that doesn't work for some people.
> The point of this patch set is to make it a DT decision instead.

Then, if the in-band status indication is not reliable (which really
should be completely understood), you can just ignore the in-band status
and use all the parameter in a 'fixed-link' property, should not we?

If in-band status can be used, then you can decide this with a separate
property which is not in 'fixed-link', would that seem reasonable?

> 
>>> -            return -EINVAL;
>>> +        if (of_property_read_u32(fixed_link_node, "speed",
>>> +                     &status.speed) != 0) {
>>> +            /* in auto mode just set to some sane value:
>>> +             * it will be changed by MAC later */
>>> +            if (link_auto)
>>> +                status.speed = 1000;
>> This is a completely arbitrary speed, that does not more or less sense
>> than defaulting to 100 or anything else,
> Exactly.
> But if I leave it to 0, then fixed-phy driver will return an error,
> so I took an arbitrary value.
> But if it obscures the code, I'll hack fixed-phy to accept 0 instead,
> to get something cleaner. So in v2.
> 
>>   a driver should be able to set
>> the speed it wants, based on the parsing of a 'phy-mode' property for
>> instance.
> It actually does, that value is just to "cheat" fixed-phy.
> I'll make things more obvious next time.
Stas Sergeev July 9, 2015, 9:43 p.m. UTC | #4
10.07.2015 00:15, Florian Fainelli пишет:
> On 09/07/15 13:43, Stas Sergeev wrote:
>> 09.07.2015 21:24, Florian Fainelli пишет:
>>> (there is no such thing as linux-net@vger.kernel.org, please remove it
>>> from your future submissions).
>>>
>>> On 09/07/15 10:38, Stas Sergeev wrote:
>>>> Currently for fixed-link the link state is always set to UP.
>>> Not quite true, this is always a driver decision to make.
>> But what about this part of of_mdio.c:of_phy_register_fixed_link():
>> ---
>>
>>       fixed_link_node = of_get_child_by_name(np, "fixed-link");
>>       if (fixed_link_node) {
>>          status.link = 1
>>
>> ---
> This seems like a logical consequence of finding a "fixed-link" property
> for the DT node of interest. If no such property exist, then we do not
> set anything.
>
>>>> This patch introduces the new property 'link' that accepts the
>>>> following string arguments: "up", "down" and "auto".
>>>> "down" may be needed if the link is physically unconnected.
>>> In which case you probably do not even care about inserting such a
>>> property in the first place, do you? What would be the value of forcibly
>>> having a link permanently down (not counting loopback)?
>> The DTs have a common parts that are included by other
>> parts. So if you include the definition of your SoC that have
>> all ethernets defined, and you only set up the external things
>> like PHYs, then I would see a potential use for "down".
> "down" is equivalent to using a status = "disabled", in fact the latter
> is much better since you can even conserve energy and resources by not
> enabling something which is not usable.
OK, agree.
So I'll probably go for something like
autoneg = 1 | 0;

>> This doesn't work.
>> It appears even if the driver supports it and wants to use it, the
>> PHY HW may simply not generate the inband status. This is actually
>> the whole point why we have a regression now. It is _currently_
>> a driver decision, and that doesn't work for some people.
>> The point of this patch set is to make it a DT decision instead.
> Then, if the in-band status indication is not reliable (which really
> should be completely understood),
Agree!
But this is not something I can help with.
Sebastien Rannou reports the problem, please ask him whatever
you see fits to get a better understanding of a problem.
The fact that his HW does not generate the inband status, is
_my own guess_.

>   you can just ignore the in-band status
> and use all the parameter in a 'fixed-link' property, should not we?
I don't think there is any way at all to find out if the inband stat is
usable or not. I think only the user (or manufacturer) can decide
on that. If there is any way to do a guess-work, that would be an
entirely different story.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastien Rannou July 10, 2015, 8:46 a.m. UTC | #5
On Fri, 10 Jul 2015, Stas Sergeev wrote:

> 10.07.2015 00:15, Florian Fainelli пишет:
> > Then, if the in-band status indication is not reliable (which really
> > should be completely understood),
> Agree!
> But this is not something I can help with.
> Sebastien Rannou reports the problem, please ask him whatever
> you see fits to get a better understanding of a problem.
> The fact that his HW does not generate the inband status, is
> _my own guess_.

Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
inband status) is connected to the switch through QSGMII, and in this context
we are on the media side of the PHY.
Stas Sergeev July 10, 2015, 11:20 a.m. UTC | #6
10.07.2015 11:46, Sebastien Rannou пишет:
> On Fri, 10 Jul 2015, Stas Sergeev wrote:
> 
>> 10.07.2015 00:15, Florian Fainelli пишет:
>>> Then, if the in-band status indication is not reliable (which really
>>> should be completely understood),
>> Agree!
>> But this is not something I can help with.
>> Sebastien Rannou reports the problem, please ask him whatever
>> you see fits to get a better understanding of a problem.
>> The fact that his HW does not generate the inband status, is
>> _my own guess_.
> 
> Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
> a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
> inband status) is connected to the switch through QSGMII, and in this context
> we are on the media side of the PHY.
Hmm, interesting.
So if I parse the above correctly, you have something like 88E1340S set
up into a mode when SGMII is used as media interface and QSGMII as system
interface (terms are from datasheet page 5), then you connect the media
interface to armada-xp and system interface to the switch.

I wonder if it is the right thing to do.
AFAIK you could as well set up armada-xp into QSGMII mode and connect
that to switch. The driver would then disable the use of inband status
and everything would be fine.
Either way, your use-case proves that only DT can decide the use of an
inband status.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli July 10, 2015, 6:22 p.m. UTC | #7
On 10/07/15 04:20, Stas Sergeev wrote:
> 10.07.2015 11:46, Sebastien Rannou пишет:
>> On Fri, 10 Jul 2015, Stas Sergeev wrote:
>>
>>> 10.07.2015 00:15, Florian Fainelli пишет:
>>>> Then, if the in-band status indication is not reliable (which really
>>>> should be completely understood),
>>> Agree!
>>> But this is not something I can help with.
>>> Sebastien Rannou reports the problem, please ask him whatever
>>> you see fits to get a better understanding of a problem.
>>> The fact that his HW does not generate the inband status, is
>>> _my own guess_.
>>
>> Yes, I confirm that my HW does not generate an in-band status. AFAIK, it's
>> a PHY that aggregates 4xSGMIIs to 1xQSGMII ; the MAC side of the PHY (with
>> inband status) is connected to the switch through QSGMII, and in this context
>> we are on the media side of the PHY.
> Hmm, interesting.
> So if I parse the above correctly, you have something like 88E1340S set
> up into a mode when SGMII is used as media interface and QSGMII as system
> interface (terms are from datasheet page 5), then you connect the media
> interface to armada-xp and system interface to the switch.
> 
> I wonder if it is the right thing to do.
> AFAIK you could as well set up armada-xp into QSGMII mode and connect
> that to switch. The driver would then disable the use of inband status
> and everything would be fine.
> Either way, your use-case proves that only DT can decide the use of an
> inband status.

I do not think there is any debate around the need for a property that
defines whether in-band-status is both reliable and usable, the debate
is about *where* to put it.

I still think this does not belong in the fixed-link property, but now
that you have explained a bit more in the other patch what your
understanding of "fixed-link" is, I can see the confusion.

Instead of having a link = "auto", property, how about just something
like this:

fixed-link {
	speed = <1000>;
	full-duplex;
	use-in-band-status;
};

or event this:

fixed-link {
	use-in-band-status;
};

If you parse the 'use-in-band-status' which means that it is reliable
information, then you can override whatever was defined in the DT under
the 'fixed-link' property?
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
index 82bf7e0..070f554 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -9,8 +9,14 @@  Such a fixed link situation is described by creating a 'fixed-link'
 sub-node of the Ethernet MAC device node, with the following
 properties:

+* 'link' (string, optional), to indicate the link state. Accepted
+  values are "up", "down" and "auto". "auto" means auto-negotiation of
+  link parameters. Auto-negotiation is MII protocol, HW and driver-specific
+  and is not supported in many cases, so use it only when you know what
+  you do.
 * 'speed' (integer, mandatory), to indicate the link speed. Accepted
-  values are 10, 100 and 1000
+  values are 10, 100 and 1000. If the 'link' property is set to 'auto',
+  'speed' may not be set. It will then be auto-negotiated, if possible.
 * 'full-duplex' (boolean, optional), to indicate that full duplex is
   used. When absent, half duplex is assumed.
 * 'pause' (boolean, optional), to indicate that pause should be
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1bd4305..2152cf8 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -280,6 +280,26 @@  bool of_phy_is_fixed_link(struct device_node *np)
 }
 EXPORT_SYMBOL(of_phy_is_fixed_link);

+bool of_phy_is_autoneg_link(struct device_node *np)
+{
+	struct device_node *dn;
+	const char *link_str;
+	int rc;
+	bool ret = false;
+
+	dn = of_get_child_by_name(np, "fixed-link");
+	if (!dn)
+		return false;
+
+	rc = of_property_read_string(dn, "link", &link_str);
+	if (rc == 0 && strcmp(link_str, "auto") == 0)
+		ret = true;
+
+	of_node_put(dn);
+	return ret;
+}
+EXPORT_SYMBOL(of_phy_is_autoneg_link);
+
 int of_phy_register_fixed_link(struct device_node *np)
 {
 	struct fixed_phy_status status = {};
@@ -291,11 +311,33 @@  int of_phy_register_fixed_link(struct device_node *np)
 	/* New binding */
 	fixed_link_node = of_get_child_by_name(np, "fixed-link");
 	if (fixed_link_node) {
-		status.link = 1;
+		const char *link_str;
+		int ret;
+		bool link_auto = false;
+
+		ret = of_property_read_string(fixed_link_node, "link",
+					      &link_str);
+		if (ret == 0) {
+			if (strcmp(link_str, "up") == 0)
+				status.link = 1;
+			else
+				status.link = 0;
+			if (strcmp(link_str, "auto") == 0)
+				link_auto = true;
+		} else {
+			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))
-			return -EINVAL;
+		if (of_property_read_u32(fixed_link_node, "speed",
+					 &status.speed) != 0) {
+			/* in auto mode just set to some sane value:
+			 * it will be changed by MAC later */
+			if (link_auto)
+				status.speed = 1000;
+			else
+				return -EINVAL;
+		}
 		status.pause = of_property_read_bool(fixed_link_node, "pause");
 		status.asym_pause = of_property_read_bool(fixed_link_node,
 							  "asym-pause");
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index d449018..647f348 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -65,6 +65,7 @@  static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
 #if defined(CONFIG_OF) && defined(CONFIG_FIXED_PHY)
 extern int of_phy_register_fixed_link(struct device_node *np);
 extern bool of_phy_is_fixed_link(struct device_node *np);
+extern bool of_phy_is_autoneg_link(struct device_node *np);
 #else
 static inline int of_phy_register_fixed_link(struct device_node *np)
 {
@@ -74,6 +75,10 @@  static inline bool of_phy_is_fixed_link(struct device_node *np)
 {
 	return false;
 }
+static inline bool of_phy_is_autoneg_link(struct device_node *np)
+{
+	return false;
+}
 #endif