[PATCH net-next 0/3] Adding PHY Loopback tunable

[PATCH net-next 0/3] Adding PHY Loopback tunable

[Allan W. Nielsen]

Hi All,

This series add support for PHY Loopback tunable, and implement it
for MSCC phys.

To configure loopback, the ethtool_tunable structure is used. 'id' must be
set to 'ETHTOOL_PHY_LOOPBACK' and 'data' specifies the loopback type:
ETHTOOL_PHY_LOOPBACK_* (DISABLE, NEAR, FAR or EXTN).

Here is how to configure loopback using ethtool:

Ethtool Help: ethtool -h for PHY tunables
    ethtool --set-phy-tunable DEVNAME      Set PHY tunable
                [ loopback off|near|far|extn ]
    ethtool --get-phy-tunable DEVNAME      Get PHY tunable
                [ loopback ]

Ethtool ex:
  ethtool --set-phy-tunable eth0 loopback near
  ethtool --set-phy-tunable eth0 loopback far
  ethtool --set-phy-tunable eth0 loopback extn
  ethtool --set-phy-tunable eth0 loopback off
  ethtool --get-phy-tunable eth0 loopback

Patches to ethtool will follow shortly.

The feature is tested on Beaglebone Black with VSC 8531 PHY.

Please review.

Best regards
Allan and Raju

Raju Lakkaraju (3):
  ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables
  ethtool: Core impl for ETHTOOL_PHY_LOOPBACK tunable.
  net: phy: Add Loopback support in Microsemi PHYs driver

 drivers/net/phy/mscc.c       | 118 +++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/ethtool.h |   8 +++
 net/core/ethtool.c           |   2 +
 3 files changed, 128 insertions(+)

-- 
2.7.3

[PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Allan W. Nielsen]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

3 types of PHY loopback are supported.
i.e. Near-End Loopback, Far-End Loopback and External Loopback.

Near-End Loopback:
Transmitted data (TXD) is looped back in the PCS block onto the receive
data signal (RXD). When Near-End loopback enable, no data is transmitted
over the network. no data receive from the network.

Far-End Loopback:
This loopback is a special test mode to allow testing the PHY from link
partner side. In this mode data that is received from the link partner pass
through the PHY's receiver, looped back on the MII and transmitted back to
the link partner.
Data present on the transmit data pins of the MAC interface is ignored when
using this test.

External Loopback:
An RJ45 loopback cable can be used to route the transmit signals an the
output of the trnsformer back to the receiver inputs. This loopback will
work at either 10 or 100 or 1000 Mbps speed.
RJ45 Loopback cable created by conncting pin 1 to pin 3 and connecting pin
2 to pin 6.

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
---
 include/uapi/linux/ethtool.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index f0db778..59629f5 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -254,6 +254,7 @@ struct ethtool_tunable {
 enum phy_tunable_id {
 	ETHTOOL_PHY_ID_UNSPEC,
 	ETHTOOL_PHY_DOWNSHIFT,
+	ETHTOOL_PHY_LOOPBACK,
 	/*
 	 * Add your fresh new phy tunable attribute above and remember to update
 	 * phy_tunable_strings[] in net/core/ethtool.c
@@ -261,6 +262,13 @@ enum phy_tunable_id {
 	__ETHTOOL_PHY_TUNABLE_COUNT,
 };
 
+enum phy_loopback_type {
+	ETHTOOL_PHY_LOOPBACK_DISABLE,
+	ETHTOOL_PHY_LOOPBACK_NEAR,
+	ETHTOOL_PHY_LOOPBACK_FAR,
+	ETHTOOL_PHY_LOOPBACK_EXTN
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
-- 
2.7.3

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Andrew Lunn]

On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> 
> 3 types of PHY loopback are supported.
> i.e. Near-End Loopback, Far-End Loopback and External Loopback.

Hi Allan

Is this integrated with ethtool --test? You only want the PHY to go
into loopback mode when running ethtool --test external_lb or maybe
ethtool --test offline.

What i think should happen is that this tunable sets the mode the PHY
will go into when loopback is enabled. It does not however enable
loopback. It is running ethtool --test which actually enables the
loopback, probably by setting BMCR_LOOPBACK. Once the test is
finished, the bit is cleared and the PHY goes back into normal
operation.

	Andrew

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Florian Fainelli]

On 11/28/2016 06:14 AM, Andrew Lunn wrote:
> On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
>> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>>
>> 3 types of PHY loopback are supported.
>> i.e. Near-End Loopback, Far-End Loopback and External Loopback.
> 
> Hi Allan
> 
> Is this integrated with ethtool --test? You only want the PHY to go
> into loopback mode when running ethtool --test external_lb or maybe
> ethtool --test offline.
> 
> What i think should happen is that this tunable sets the mode the PHY
> will go into when loopback is enabled. It does not however enable
> loopback. It is running ethtool --test which actually enables the
> loopback, probably by setting BMCR_LOOPBACK. Once the test is
> finished, the bit is cleared and the PHY goes back into normal
> operation.

Agreed with Andrew, there needs to be a tight coupling between the
existing ethtool test infrastructure, and how to perform PHY loopback
testing.

It makes sense for the PHY drivers to be able to implement specific
loopback/test mode, but we need to clarify how these get called from
ethtool --test for instance, and if the Ethernet MAC driver needs to do
something specific as well.
-- 
Florian

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Allan W. Nielsen]

Hi Andrew and Florian,

On 28/11/16 15:14, Andrew Lunn wrote:
> On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
> > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> >
> > 3 types of PHY loopback are supported.
> > i.e. Near-End Loopback, Far-End Loopback and External Loopback.
> Is this integrated with ethtool --test? You only want the PHY to go
> into loopback mode when running ethtool --test external_lb or maybe
> ethtool --test offline.
There are other use-cases for enabling PHY loopback:

1) If the PHY is connected to a switch then a loop-port is sometime
   used to "force/enable" one or more extra pass through the switch
   core. This "hack" can sometime be used to achieve new functionality
   with existing silicon.

2) Existing user-space application may expect to be able to do the
   testing on its own (generate/validate the test traffic).

> What i think should happen is that this tunable sets the mode the
> PHY will go into when loopback is enabled. It does not however
> enable loopback.
That does not make a lot of sense with the "FAR" loopback (it is
looping received traffic back into the wire).

> It is running ethtool --test which actually enables
> the loopback, probably by setting BMCR_LOOPBACK. Once the test is
> finished, the bit is cleared and the PHY goes back into normal
> operation.
We are always happy to integrate with any existing functionality, but
as I understand "ethtool --test" then intention is to perform a test
and then bring back the PHY in to a "normal" state (I may be
wrong...).

The idea with this patch is to allow configuring loopback more
"permanently" (userspace can decide when to activate and when to
de-activate). I should properly have made that clear in the cover
letter.

Please let me know what you think.

/Allan

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Andrew Lunn]

On Mon, Nov 28, 2016 at 08:23:06PM +0100, Allan W. Nielsen wrote:
> Hi Andrew and Florian,
> 
> On 28/11/16 15:14, Andrew Lunn wrote:
> > On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
> > > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> > >
> > > 3 types of PHY loopback are supported.
> > > i.e. Near-End Loopback, Far-End Loopback and External Loopback.
> > Is this integrated with ethtool --test? You only want the PHY to go
> > into loopback mode when running ethtool --test external_lb or maybe
> > ethtool --test offline.
> There are other use-cases for enabling PHY loopback:
> 
> 1) If the PHY is connected to a switch then a loop-port is sometime
>    used to "force/enable" one or more extra pass through the switch
>    core. This "hack" can sometime be used to achieve new functionality
>    with existing silicon.

With Linux, switches are managed via switchdev, or DSA. You will have
to teach this infrastructure that something really odd is going on
with one of its ports before you do anything like this in the PHY
layer. I suggest you leave this use case alone until somebody
really-really wants it. From my knowledge of the Marvell DSA driver,
this is not easy.

> 2) Existing user-space application may expect to be able to do the
>    testing on its own (generate/validate the test traffic).

Please can you reference some existing user-space application and the
kernel API it uses to put the PHY into loopback mode?

> We are always happy to integrate with any existing functionality, but
> as I understand "ethtool --test" then intention is to perform a test
> and then bring back the PHY in to a "normal" state (I may be
> wrong...).

Correct.

> The idea with this patch is to allow configuring loopback more
> "permanently" (userspace can decide when to activate and when to
> de-activate). I should properly have made that clear in the cover
> letter.

Leaving it in loopback is a really bad idea. I've spent days once
working out why an Ethernet did not work. Turned out the PHY powered
up in loopback mode, and the embedded OS running on it did not
initialise it to sensible defaults on probe. Packets we going out,
dhcp server was replying but all incoming packets were discarded.

It is really not obvious when everything looks O.K, but nothing works,
because the PHY is in loopback. There needs to be a big red flag to
warn you.

If you really do what to do this, you should look at NETIF_F_LOOPBACK
and consider how this concept can be applied at the PHY, not the MAC.
But you need the full concept, so you see things like:

2: eth0: <PHY_LOOPBACK,BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
    link/ether 80:ee:73:83:60:27 brd ff:ff:ff:ff:ff:ff

Humm, i've no idea how you actually enable the MAC loopback
NETIF_F_LOOPBACK represents. I don't see anything with ip link set.

     Andrew

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Florian Fainelli]

On 11/28/2016 12:21 PM, Andrew Lunn wrote:
> On Mon, Nov 28, 2016 at 08:23:06PM +0100, Allan W. Nielsen wrote:
>> Hi Andrew and Florian,
>>
>> On 28/11/16 15:14, Andrew Lunn wrote:
>>> On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
>>>> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>>>>
>>>> 3 types of PHY loopback are supported.
>>>> i.e. Near-End Loopback, Far-End Loopback and External Loopback.
>>> Is this integrated with ethtool --test? You only want the PHY to go
>>> into loopback mode when running ethtool --test external_lb or maybe
>>> ethtool --test offline.
>> There are other use-cases for enabling PHY loopback:
>>
>> 1) If the PHY is connected to a switch then a loop-port is sometime
>>    used to "force/enable" one or more extra pass through the switch
>>    core. This "hack" can sometime be used to achieve new functionality
>>    with existing silicon.
> 
> With Linux, switches are managed via switchdev, or DSA. You will have
> to teach this infrastructure that something really odd is going on
> with one of its ports before you do anything like this in the PHY
> layer. I suggest you leave this use case alone until somebody
> really-really wants it. From my knowledge of the Marvell DSA driver,
> this is not easy.

Agree with Andrew here, this particular use case with switches does not
need to be solved now, but if we imagine we need to support that,
chances are that we will want the network device as a configuration
entry point, more than the PHY device itself.

> 
>> 2) Existing user-space application may expect to be able to do the
>>    testing on its own (generate/validate the test traffic).
> 
> Please can you reference some existing user-space application and the
> kernel API it uses to put the PHY into loopback mode?
> 
>> We are always happy to integrate with any existing functionality, but
>> as I understand "ethtool --test" then intention is to perform a test
>> and then bring back the PHY in to a "normal" state (I may be
>> wrong...).
> 
> Correct.
> 
>> The idea with this patch is to allow configuring loopback more
>> "permanently" (userspace can decide when to activate and when to
>> de-activate). I should properly have made that clear in the cover
>> letter.
> 
> Leaving it in loopback is a really bad idea. I've spent days once
> working out why an Ethernet did not work. Turned out the PHY powered
> up in loopback mode, and the embedded OS running on it did not
> initialise it to sensible defaults on probe. Packets we going out,
> dhcp server was replying but all incoming packets were discarded.
> 
> It is really not obvious when everything looks O.K, but nothing works,
> because the PHY is in loopback. There needs to be a big red flag to
> warn you.
> 
> If you really do what to do this, you should look at NETIF_F_LOOPBACK
> and consider how this concept can be applied at the PHY, not the MAC.
> But you need the full concept, so you see things like:
> 
> 2: eth0: <PHY_LOOPBACK,BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
>     link/ether 80:ee:73:83:60:27 brd ff:ff:ff:ff:ff:ff
> 
> Humm, i've no idea how you actually enable the MAC loopback
> NETIF_F_LOOPBACK represents. I don't see anything with ip link set.

I am afraid you lost me on this, NETIF_F_LOOPBACK is a netdev_features_t
bit, so it tells ethtool that this is a potential feature to be turned
on with ethtool -K <iface>. The semantics of this loopack feature are
not defined AFAICT, but a reasonable behavior from the driver is to put
itself in a mode where packets send by a socket-level application are
looped through the Ethernet adapter itself. Whether this happens at the
DMA level, the MII signals, or somewhere in the PHY, is driver specific
unfortunately.

Now, there is another way to toggle a loopback for a given Ethernet
adapter which is to actually set IFF_LOOPBACK in dev->flags for this
interface. Some drivers seem to be able to properly react to that as
well, although I see no way this can be done looking at the iproute2 or
ifconfig man pages..
-- 
Florian

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Andrew Lunn]

> > If you really do what to do this, you should look at NETIF_F_LOOPBACK
> > and consider how this concept can be applied at the PHY, not the MAC.
> > But you need the full concept, so you see things like:
> > 
> > 2: eth0: <LOOPBACK,BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
> >     link/ether 80:ee:73:83:60:27 brd ff:ff:ff:ff:ff:ff
> > 
> > Humm, i've no idea how you actually enable the MAC loopback
> > NETIF_F_LOOPBACK represents. I don't see anything with ip link set.
> 
> I am afraid you lost me on this, NETIF_F_LOOPBACK is a netdev_features_t
> bit, so it tells ethtool that this is a potential feature to be turned
> on with ethtool -K <iface>. 

Yep, i'm talking rubbish after jumping to a wrong conclusion.

> The semantics of this loopack feature are
> not defined AFAICT, but a reasonable behavior from the driver is to put
> itself in a mode where packets send by a socket-level application are
> looped through the Ethernet adapter itself. Whether this happens at the
> DMA level, the MII signals, or somewhere in the PHY, is driver specific
> unfortunately.

Yes, the interesting one might be forcedeth which writes to the PHY
BMCR_LOOPBACK | BMCR_FULLDPLX | BMCR_SPEED1000;
when the NETIF_F_LOOPBACK feature is set.

Maybe this could be generalised and made available for all MACs which
don't support MAC loopback?

What needs considering is the correct duplex and speed. We need to
ensure the MAC and PHY agree on this.
 
> Now, there is another way to toggle a loopback for a given Ethernet
> adapter which is to actually set IFF_LOOPBACK in dev->flags for this
> interface. Some drivers seem to be able to properly react to that as
> well, although I see no way this can be done looking at the iproute2 or
> ifconfig man pages..

I prefer this one, since i expect this will set LOOPBACK in

2: eth0: <LOOPBACK,BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000

making it lot more obvious something funny is going on.

       Andrew

Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Allan W. Nielsen]

On 28/11/16 21:21, Andrew Lunn wrote:
> On Mon, Nov 28, 2016 at 08:23:06PM +0100, Allan W. Nielsen wrote:
> > On 28/11/16 15:14, Andrew Lunn wrote:
> > > On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
> > > > From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> > > > 3 types of PHY loopback are supported.
> > > > i.e. Near-End Loopback, Far-End Loopback and External Loopback.
> > > Is this integrated with ethtool --test? You only want the PHY to go
> > > into loopback mode when running ethtool --test external_lb or maybe
> > > ethtool --test offline.
> > There are other use-cases for enabling PHY loopback:
> > 1) If the PHY is connected to a switch then a loop-port is sometime
> >    used to "force/enable" one or more extra pass through the switch
> >    core. This "hack" can sometime be used to achieve new functionality
> >    with existing silicon.
> With Linux, switches are managed via switchdev, or DSA. You will have
> to teach this infrastructure that something really odd is going on
> with one of its ports before you do anything like this in the PHY
> layer. I suggest you leave this use case alone until somebody
> really-really wants it. From my knowledge of the Marvell DSA driver,
> this is not easy.
> > 2) Existing user-space application may expect to be able to do the
> >    testing on its own (generate/validate the test traffic).
> Please can you reference some existing user-space application and the
> kernel API it uses to put the PHY into loopback mode?
The application I had in mind, is the switch application that I'm normally
working on (a product of MSCC). This application was originally written for
eCos, but is now moved to Linux. The application is currently doing almost
all drivers in user-space (reaching the HW through a UIO driver). This
means that we have an existing set of APIs and associated applications which
among many other things tests the PHYs using loopback facilities. There are
also cases where the loopports are being used as I described earlier.

We are looking at moving some of the drivers into the kernel, and that
require us to find a solution to such issues (nothing have been decided,
and nothing will be decided anytime soon).

I also understand your point of view, you have presented pretty good
arguments, and I do not expect this to change your view on this topic.

On 29/11/16 01:46, Andrew Lunn wrote:
> > > If you really do what to do this, you should look at NETIF_F_LOOPBACK
> > > and consider how this concept can be applied at the PHY, not the MAC.
> > > But you need the full concept, so you see things like:
> > >
> > > 2: eth0: <LOOPBACK,BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
> > >     link/ether 80:ee:73:83:60:27 brd ff:ff:ff:ff:ff:ff
> > >
> > > Humm, i've no idea how you actually enable the MAC loopback
> > > NETIF_F_LOOPBACK represents. I don't see anything with ip link set.
> >
> > I am afraid you lost me on this, NETIF_F_LOOPBACK is a netdev_features_t
> > bit, so it tells ethtool that this is a potential feature to be turned
> > on with ethtool -K <iface>.
> Yep, i'm talking rubbish after jumping to a wrong conclusion.
> > The semantics of this loopack feature are
> > not defined AFAICT, but a reasonable behavior from the driver is to put
> > itself in a mode where packets send by a socket-level application are
> > looped through the Ethernet adapter itself. Whether this happens at the
> > DMA level, the MII signals, or somewhere in the PHY, is driver specific
> > unfortunately.
> 
> Yes, the interesting one might be forcedeth which writes to the PHY
> BMCR_LOOPBACK | BMCR_FULLDPLX | BMCR_SPEED1000;
> when the NETIF_F_LOOPBACK feature is set.
> 
> Maybe this could be generalised and made available for all MACs which
> don't support MAC loopback?
> 
> What needs considering is the correct duplex and speed. We need to
> ensure the MAC and PHY agree on this.
> 
> > Now, there is another way to toggle a loopback for a given Ethernet
> > adapter which is to actually set IFF_LOOPBACK in dev->flags for this
> > interface. Some drivers seem to be able to properly react to that as
> > well, although I see no way this can be done looking at the iproute2 or
> > ifconfig man pages..
> 
> I prefer this one, since i expect this will set LOOPBACK in
> 
> 2: eth0: <LOOPBACK,BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
> 
> making it lot more obvious something funny is going on.
Raju and I will need to dive deeper into this to understand the details of
what you are suggesting. But I think it points in a different direction...

The approach you are describing is to use existing flags (which will make
the loop-back state visible to the user, which is a good thing) to set the
loopback state. Where/how the frames are looped depends on the drivers. The
suggested tunable could be kept, but it will only allow the user to say,
"if the frames are looped in the PHY, then this is how I want them to be
looped".

Also, it does not make a lot of sense for the "FAR" loopback patch (which I
admit is a bit strange...).

The facility we are seeking is much more specific: "Allow the user to bring
the PHY in and out of specific loopback modes" assuming that the user have
reasons to do so.

I'm not sure if your main dislike with this feature is the lack of
transparency/visibility to the end-user, or if is the concept of allowing
the user to control where and how frames are looped (or both).

Best regards
Allan W. Nielsen

[PATCH net-next 2/3] ethtool: Core impl for ETHTOOL_PHY_LOOPBACK tunable.

[Allan W. Nielsen]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

Adding validation support for the ETHTOOL_PHY_LOOPBACK. Functional
implementation needs to be done in the individual PHY drivers.

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
---
 net/core/ethtool.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index e23766c..0542467 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -123,6 +123,7 @@ static const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
 	[ETHTOOL_ID_UNSPEC]     = "Unspec",
 	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
+	[ETHTOOL_PHY_LOOPBACK]	= "phy-loopback",
 };
 
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2437,6 +2438,7 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 {
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
+	case ETHTOOL_PHY_LOOPBACK:
 		if (tuna->len != sizeof(u8) ||
 		    tuna->type_id != ETHTOOL_TUNABLE_U8)
 			return -EINVAL;
-- 
2.7.3

[PATCH net-next 3/3] net: phy: Add Loopback support in Microsemi PHYs driver

[Allan W. Nielsen]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

Implements the loopback functionality for MSCC PHYs.

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
---
 drivers/net/phy/mscc.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 7a3740c..1f9bc72 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -27,6 +27,9 @@ enum rgmii_rx_clock_delay {
 
 /* Microsemi VSC85xx PHY registers */
 /* IEEE 802. Std Registers */
+#define MSCC_PHY_BYPASS_CONTROL		  18
+#define DISABLE_PAIR_SWAP_CORR_MASK	  0x0020
+
 #define MSCC_PHY_EXT_PHY_CNTL_1           23
 #define MAC_IF_SELECTION_MASK             0x1800
 #define MAC_IF_SELECTION_GMII             0
@@ -35,6 +38,9 @@ enum rgmii_rx_clock_delay {
 #define MAC_IF_SELECTION_POS              11
 #define FAR_END_LOOPBACK_MODE_MASK        0x0008
 
+#define MSCC_PHY_EXT_PHY_CNTL_2		  24
+#define CONNECTOR_LOOPBACK_MASK		  0x0001
+
 #define MII_VSC85XX_INT_MASK		  25
 #define MII_VSC85XX_INT_MASK_MASK	  0xa000
 #define MII_VSC85XX_INT_MASK_WOL	  0x0040
@@ -110,6 +116,114 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, u8 page)
 	return rc;
 }
 
+static int vsc85xx_loopback_get(struct phy_device *phydev, u8 *type)
+{
+	u16 reg_val;
+
+	reg_val = phy_read(phydev, MII_BMCR);
+	if (BMCR_LOOPBACK & reg_val) {
+		*type = ETHTOOL_PHY_LOOPBACK_NEAR;
+		goto out;
+	}
+
+	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
+	if (FAR_END_LOOPBACK_MODE_MASK & reg_val) {
+		*type = ETHTOOL_PHY_LOOPBACK_FAR;
+		goto out;
+	}
+
+	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_2);
+	if (CONNECTOR_LOOPBACK_MASK & reg_val) {
+		*type = ETHTOOL_PHY_LOOPBACK_EXTN;
+		goto out;
+	}
+	*type = ETHTOOL_PHY_LOOPBACK_DISABLE;
+
+out:
+	return 0;
+}
+
+static int vsc85xx_loopback_set(struct phy_device *phydev, u8 type)
+{
+	int rc;
+	u16 reg_val;
+
+	/* Clear/Disable all Loopbacks first */
+	/* Disable Near End Loopback */
+	reg_val = phy_read(phydev, MII_BMCR);
+	if (reg_val & BMCR_LOOPBACK && type != ETHTOOL_PHY_LOOPBACK_NEAR) {
+		reg_val &= ~BMCR_LOOPBACK;
+		rc = phy_write(phydev, MII_BMCR, reg_val);
+		if (rc != 0)
+			goto out;
+	}
+
+	/* Disable Far End Loopback */
+	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
+	if (reg_val & FAR_END_LOOPBACK_MODE_MASK &&
+	    type != ETHTOOL_PHY_LOOPBACK_FAR) {
+		reg_val &= ~FAR_END_LOOPBACK_MODE_MASK;
+		rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
+		if (rc != 0)
+			goto out;
+	}
+
+	/* Disable Connector End Loopback */
+	reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_2);
+	if (reg_val & CONNECTOR_LOOPBACK_MASK &&
+	    type != ETHTOOL_PHY_LOOPBACK_EXTN) {
+		reg_val &= ~CONNECTOR_LOOPBACK_MASK;
+		rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_2, reg_val);
+		if (rc != 0)
+			goto out;
+		reg_val = phy_read(phydev, MSCC_PHY_BYPASS_CONTROL);
+		reg_val &= ~DISABLE_PAIR_SWAP_CORR_MASK;
+		rc = phy_write(phydev, MSCC_PHY_BYPASS_CONTROL, reg_val);
+		if (rc != 0)
+			goto out;
+	}
+
+	switch (type) {
+	case ETHTOOL_PHY_LOOPBACK_NEAR:
+		reg_val = phy_read(phydev, MII_BMCR);
+		reg_val |= BMCR_LOOPBACK;
+		rc = phy_write(phydev, MII_BMCR, reg_val);
+		if (rc != 0)
+			goto out;
+		break;
+	case ETHTOOL_PHY_LOOPBACK_FAR:
+		reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
+		reg_val |= FAR_END_LOOPBACK_MODE_MASK;
+		rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
+		if (rc != 0)
+			goto out;
+		break;
+	case ETHTOOL_PHY_LOOPBACK_EXTN:
+		reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_2);
+		reg_val |= CONNECTOR_LOOPBACK_MASK;
+		rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_2, reg_val);
+		if (rc != 0)
+			goto out;
+		reg_val = phy_read(phydev, MSCC_PHY_BYPASS_CONTROL);
+		reg_val |= DISABLE_PAIR_SWAP_CORR_MASK;
+		rc = phy_write(phydev, MSCC_PHY_BYPASS_CONTROL, reg_val);
+		if (rc != 0)
+			goto out;
+		break;
+	case ETHTOOL_PHY_LOOPBACK_DISABLE:
+		/* Already disable all Loopbacks */
+		break;
+	default:
+		phydev_err(phydev, "Invalid Loopback Type (valid only off|near|far|extn)\n");
+		rc = -ERANGE;
+		break;
+	}
+
+out:
+
+	return rc;
+}
+
 static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
 {
 	int rc;
@@ -398,6 +512,8 @@ static int vsc85xx_get_tunable(struct phy_device *phydev,
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
 		return vsc85xx_downshift_get(phydev, (u8 *)data);
+	case ETHTOOL_PHY_LOOPBACK:
+		return vsc85xx_loopback_get(phydev, (u8 *)data);
 	default:
 		return -EINVAL;
 	}
@@ -410,6 +526,8 @@ static int vsc85xx_set_tunable(struct phy_device *phydev,
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
 		return vsc85xx_downshift_set(phydev, *(u8 *)data);
+	case ETHTOOL_PHY_LOOPBACK:
+		return vsc85xx_loopback_set(phydev, *(u8 *)data);
 	default:
 		return -EINVAL;
 	}
-- 
2.7.3

[PATCH net-next 0/3] Adding PHY Loopback tunable

[Allan W. Nielsen]

Hi All,

This series add support for PHY Loopback tunable, and implement it
for MSCC phys.

To configure loopback, the ethtool_tunable structure is used. 'id' must be
set to 'ETHTOOL_PHY_LOOPBACK' and 'data' specifies the loopback type:
ETHTOOL_PHY_LOOPBACK_* (DISABLE, NEAR, FAR or EXTN).

Here is how to configure loopback using ethtool:

Ethtool Help: ethtool -h for PHY tunables
    ethtool --set-phy-tunable DEVNAME      Set PHY tunable
                [ loopback off|near|far|extn ]
    ethtool --get-phy-tunable DEVNAME      Get PHY tunable
                [ loopback ]

Ethtool ex:
  ethtool --set-phy-tunable eth0 loopback near
  ethtool --set-phy-tunable eth0 loopback far
  ethtool --set-phy-tunable eth0 loopback extn
  ethtool --set-phy-tunable eth0 loopback off
  ethtool --get-phy-tunable eth0 loopback

Patches to ethtool will follow shortly.

The feature is tested on Beaglebone Black with VSC 8531 PHY.

Please review.

Best regards
Allan and Raju

Raju Lakkaraju (3):
  ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables
  ethtool: Core impl for ETHTOOL_PHY_LOOPBACK tunable.
  net: phy: Add Loopback support in Microsemi PHYs driver

 drivers/net/phy/mscc.c       | 118 +++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/ethtool.h |   8 +++
 net/core/ethtool.c           |   2 +
 3 files changed, 128 insertions(+)

-- 
2.7.3