Re: [net-next v2 10/11] bridge: switchdev: cfm: switchdev interface implementation

[Allan W. Nielsen]

Hi Jiri

On 01.10.2020 14:49, Jiri Pirko wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
content is safe

Thu, Oct 01, 2020 at 12:30:18PM CEST, henrik.bjoernl...@microchip.com wrote:

This is the definition of the CFM switchdev interface.

The interface consist of these objects:
   SWITCHDEV_OBJ_ID_MEP_CFM,
   SWITCHDEV_OBJ_ID_MEP_CONFIG_CFM,
   SWITCHDEV_OBJ_ID_CC_CONFIG_CFM,
   SWITCHDEV_OBJ_ID_CC_PEER_MEP_CFM,
   SWITCHDEV_OBJ_ID_CC_CCM_TX_CFM,
   SWITCHDEV_OBJ_ID_MEP_STATUS_CFM,
   SWITCHDEV_OBJ_ID_PEER_MEP_STATUS_CFM

MEP instance add/del
   switchdev_port_obj_add(SWITCHDEV_OBJ_ID_MEP_CFM)
   switchdev_port_obj_del(SWITCHDEV_OBJ_ID_MEP_CFM)

MEP cofigure
   switchdev_port_obj_add(SWITCHDEV_OBJ_ID_MEP_CONFIG_CFM)

MEP CC cofigure
   switchdev_port_obj_add(SWITCHDEV_OBJ_ID_CC_CONFIG_CFM)

Peer MEP add/del
   switchdev_port_obj_add(SWITCHDEV_OBJ_ID_CC_PEER_MEP_CFM)
   switchdev_port_obj_del(SWITCHDEV_OBJ_ID_CC_PEER_MEP_CFM)

Start/stop CCM transmission
   switchdev_port_obj_add(SWITCHDEV_OBJ_ID_CC_CCM_TX_CFM)

Get MEP status
  switchdev_port_obj_get(SWITCHDEV_OBJ_ID_MEP_STATUS_CFM)

Get Peer MEP status
  switchdev_port_obj_get(SWITCHDEV_OBJ_ID_PEER_MEP_STATUS_CFM)

Reviewed-by: Horatiu Vultur  
Signed-off-by: Henrik Bjoernlund  


You have to submit the driver parts as a part of this patchset.
Otherwise it is no good.

Fair enough.

With MRP we did it like this, and after Nik asked for details on what is
being offload, we thought that adding this would help.

The reason why we did not include the implementation of this interface
is that it is for a new SoC which is still not fully available which is
why we have not done the basic SwitchDev driver for it yet. But the
basic functionality clearly needs to come first.

Our preference is to continue fixing the comments we got on the pure SW
implementation and then get back to the SwitchDev offloading.

This will mean dropping the last 2 patches in the serie.

Does that work for you Jiri, and Nik?

/Allan

Re: [PATCH RFC 0/7] net: bridge: cfm: Add support for Connectivity Fault Management(CFM)

[Allan W. Nielsen]

Hi,

On 08.09.2020 11:04, Henrik Bjoernlund - M31679 wrote:

On Sun, 2020-09-06 at 20:21 +0200, Horatiu Vultur wrote:

The 09/04/2020 15:44, Stephen Hemminger wrote:
> On Fri, 4 Sep 2020 09:15:20 + Henrik Bjoernlund
>  wrote:

Hi, I also had the same initial thought - this really doesn't seem to
affect the bridge in any way, it's only collecting and transmitting
information. I get that you'd like to use the bridge as a passthrough
device to switchdev to program your hw, could you share what would be
offloaded more specifically ?

Yes.

The HW will offload the periodic sending of CCM frames, and the
reception.

If CCM frames are not received as expected, it will raise an interrupt.

This means that all the functionality provided in this series will be
offloaded to HW.

The offloading is very important on our HW where we have a small CPU,
serving many ports, with a high frequency of CFM frames.

The HW also support Link-Trace and Loop-back, which we may come back to
later.


All you do - snooping and blocking these packets can easily be done
from user- space with the help of ebtables, but since we need to have
a software implementation/fallback of anything being offloaded via
switchdev we might need this after all, I'd just prefer to push as
much as possible to user-space.

In addition to Henriks comment, it is worth mentioning that we are
trying to push as much of the functionallity to user-space (learnings
from the MRP discussions).

This is why there are currently no in-kernel users of the CCM-lose
singnal. When a CCM-defect is happening the network typically needs to
be re-configured. This we are trying to keep in user-space.


I plan to review the individual patches tomorrow.

Thanks.

/Allan

Re: [EXT] Re: [PATCH v2 net-next 03/10] net: mscc: ocelot: allocated rules to different hardware VCAP TCAMs by chain index

[Allan W. Nielsen]

On 17.07.2020 12:08, Vladimir Oltean wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
content is safe

On Fri, Jul 17, 2020 at 09:34:11AM +0200, Joergen Andreasen wrote:

The 07/16/2020 17:45, Vladimir Oltean wrote:
> Hi Vladimir,
>
> On Thu, Jul 16, 2020 at 10:37:40AM +, Xiaoliang Yang wrote:
> > Hi Joergen,
> >
> >
> > -Original Message-
> > From: Joergen Andreasen 
> > Sent: 2020年7月16日 16:51
> >
> > > >> >> Chain 0:   The default chain - today this is in IS2. If we 
proceed
> > > >> >> with this as is - then this will change.
> > > >> >> Chain 1-:  These are offloaded by "basic" classification.
> > > >> >> Chain 1-1: These are offloaded in IS1
> > > >> >> Chain 1: Lookup-0 in IS1, and here we could 
limit the
> > > >> >>  action to do QoS related stuff 
(priority
> > > >> >>  update)
> > > >> >> Chain 11000: Lookup-1 in IS1, here we could do 
VLAN
> > > >> >>  stuff
> > > >> >> Chain 12000: Lookup-2 in IS1, here we could 
apply the
> > > >> >>  "PAG" which is essentially a GOTO.
> > > >> >>
> > > >> >> Chain 2-2: These are offloaded in IS2
> > > >> >> Chain 2-20255: Lookup-0 in IS2, where 
CHAIN-ID -
> > > >> >>2 is the PAG value.
> > > >> >> Chain 21000-21000: Lookup-1 in IS2.
> > > >> >>
> > > >> >> All these chains should be optional - users should only need to
> > > >> >> configure the chains they need. To make this work, we need to
> > > >> >> configure both the desired actions (could be priority update) and 
the goto action.
> > > >> >> Remember in HW, all packets goes through this process, while in
> > > >> >> SW they only follow the "goto" path.
> > > >> >>
> > >>
> > >> I agree with this chain assignment, following is an example to set rules:
> > >>
> > >> 1. Set a matchall rule for each chain, the last chain do not need goto 
chain action.
> > >> # tc filter add dev swp0 chain 0 flower skip_sw action goto chain 1
> > >> # tc filter add dev swp0 chain 1 flower skip_sw action goto chain 
21000
> > >> In driver, use these rules to register the chain.
> > >>
> > >> 2. Set normal rules.
> > >> # tc filter add dev swp0 chain 1 protocol 802.1Q parent : flower 
skip_sw vlan_id 1 vlan_prio 1 action skbedit priority 1 action goto chain 21000
> > >> # tc filter add dev swp0 chain 21000 protocol 802.1Q parent : flower 
skip_sw vlan_id 1 vlan_prio 1 action drop
> > >>
> > >> In driver, we check if the chain ID has been registered, and goto
> > >> chain is the same as first matchall rule, if is not, then return
> > >> error. Each rule need has goto action except last chain.
> > >>
> > >> I also have check about chain template, it can not set an action
> > >> template for each chain, so I think it's no use for our case. If
> > >> this way to set rules is OK, I will update the patch to do as this.
> > >>
> > >> Thanks,
> > >> Xiaoliang Yang
> > >
> >
> > > I agree that you cannot set an action template for each chain but
> > > you can set a match template which for example can be used for
> > > setting up which IS1 key to generate for the device/port.
> > > The template ensures that you cannot add an illegal match.
> > > I have attached a snippet from a testcase I wrote in order to test these 
ideas.
> > > Note that not all actions are valid for the hardware.
> > >
> > > SMAC   = "00:00:00:11:11:11"
> > > DMAC   = "00:00:00:dd:dd:dd"
> > > VID1   = 0x10
> > > VID2   = 0x20
> > > PCP1   = 3
> > > PCP2   = 5
> > > DEI= 1
> > > SIP= "10.10.0.1"
> > > DIP= "10.10.0.2"
> > >
> > > IS1_L0 = 1 # IS1 lookup 0
> > > IS1_L1 = 11000 # IS1 lookup 1
> > > IS1_L2 = 12000 # IS1 lookup 2
> > >
> > > IS2_L0 = 2 # IS2 lookup 0 # IS2 2 - 20255 -> pag 0-255
> > > IS2_L0_P1  = 20001 # IS2 lookup 0 pag 1
> > > IS2_L0_P2  = 20002 # IS2 lookup 0 pag 2
> > >
> > > IS2_L1 = 21000 # IS2 lookup 1
> > >
> > > $skip = "skip_hw" # or "skip_sw"
> > >
> > > test "Chain templates and goto" do
> > > t_i "'prio #' sets the sequence of filters. Lowest number = highest 
priority = checked first. 0..0x"
> > > t_i "'handle #' is a reference to the filter. Use this is if you need to 
reference the filter later. 0..0x"
> > > t_i "'chain #' is the chain to use. Chain 0 is the default. Different 
chains can have different templates. 0..0x"
> > > $ts.dut.run "tc qdisc add dev #{$dp[0]} clsact"
> > >
> > > t_i "Add templates"
> > > t_i "Configure the VCAP port configuration to match the shortest key that 
fulfill the purpose"
> >
> > > t_i "Create a template that sets IS1 lookup 0 to generate S1_NORMAL with 
S1_DMAC_DIP_ENA"
> > > t

Re: [PATCH v2 net-next 03/10] net: mscc: ocelot: allocated rules to different hardware VCAP TCAMs by chain index

[Allan W. Nielsen]

On 09.06.2020 15:55, Vladimir Oltean wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
content is safe

Hi Allan,

On Mon, 8 Jun 2020 at 16:56, Allan W. Nielsen
 wrote:


On 03.06.2020 13:04, Vladimir Oltean wrote:
>On Tue, 2 Jun 2020 at 11:38, Allan W. Nielsen
> wrote:
>>
>> Hi Xiaoliang,
>>
>> Happy to see that you are moving in the directions of multi chain - this
>> seems ilke a much better fit to me.
>>
>>
>> On 02.06.2020 13:18, Xiaoliang Yang wrote:
>> >There are three hardware TCAMs for ocelot chips: IS1, IS2 and ES0. Each
>> >one supports different actions. The hardware flow order is: IS1->IS2->ES0.
>> >
>> >This patch add three blocks to store rules according to chain index.
>> >chain 0 is offloaded to IS1, chain 1 is offloaded to IS2, and egress chain
>> >0 is offloaded to ES0.
>>
>> Using "static" allocation to to say chain-X goes to TCAM Y, also seems
>> like the right approach to me. Given the capabilities of the HW, this
>> will most likely be the easiest scheme to implement and to explain to
>> the end-user.
>>
>> But I think we should make some adjustments to this mapping schema.
>>
>> Here are some important "things" I would like to consider when defining
>> this schema:
>>
>> - As you explain, we have 3 TCAMs (IS1, IS2 and ES0), but we have 3
>>parallel lookups in IS1 and 2 parallel lookups in IS2 - and also these
>>TCAMs has a wide verity of keys.
>>
>> - We can utilize these multiple parallel lookups such that it seems like
>>they are done in serial (that is if they do not touch the same
>>actions), but as they are done in parallel they can not influence each
>>other.
>>
>> - We can let IS1 influence the IS2 lookup (like the GOTO actions was
>>intended to be used).
>>
>> - The chip also has other QoS classification facilities which sits
>>before the TCAM (take a look at 3.7.3 QoS, DP, and DSCP Classification
>>in vsc7514 datasheet). It we at some point in time want to enable
>>this, then I think we need to do that in the same tc-flower framework.
>>
>> Here is my initial suggestion for an alternative chain-schema:
>>
>> Chain 0:   The default chain - today this is in IS2. If we proceed
>> with this as is - then this will change.
>> Chain 1-:  These are offloaded by "basic" classification.
>> Chain 1-1: These are offloaded in IS1
>> Chain 1: Lookup-0 in IS1, and here we could limit the
>>  action to do QoS related stuff (priority
>>  update)
>> Chain 11000: Lookup-1 in IS1, here we could do VLAN
>>  stuff
>> Chain 12000: Lookup-2 in IS1, here we could apply the
>>  "PAG" which is essentially a GOTO.
>>
>> Chain 2-2: These are offloaded in IS2
>> Chain 2-20255: Lookup-0 in IS2, where CHAIN-ID -
>>2 is the PAG value.
>> Chain 21000-21000: Lookup-1 in IS2.
>>
>> All these chains should be optional - users should only need to
>> configure the chains they need. To make this work, we need to configure
>> both the desired actions (could be priority update) and the goto action.
>> Remember in HW, all packets goes through this process, while in SW they
>> only follow the "goto" path.
>>
>> An example could be (I have not tested this yet - sorry):
>>
>> tc qdisc add dev eth0 ingress
>>
>> # Activate lookup 11000. We can not do any other rules in chain 0, also
>> # this implicitly means that we do not want any chains <11000.
>> tc filter add dev eth0 parent : chain 0
>> action
>> matchall goto 11000
>>
>> tc filter add dev eth0 parent : chain 11000 \
>> flower src_mac 00:01:00:00:00:00/00:ff:00:00:00:00 \
>> action \
>> vlan modify id 1234 \
>> pipe \
>> goto 20001
>>
>> tc filter add dev eth0 parent : chain 20001 ...
>>
>> Maybe it would be an idea to create some use-cases, implement them in a
>> test which can pass with today's SW, and then once we have a common
>> understanding of what we want, we can implement it?
>>
>> /Allan
>>
>> >Using action goto chain to express flow order as follows:
>> >tc fil

Re: [PATCH v2 net-next 03/10] net: mscc: ocelot: allocated rules to different hardware VCAP TCAMs by chain index

[Allan W. Nielsen]

On 03.06.2020 13:04, Vladimir Oltean wrote:

On Tue, 2 Jun 2020 at 11:38, Allan W. Nielsen
 wrote:


Hi Xiaoliang,

Happy to see that you are moving in the directions of multi chain - this
seems ilke a much better fit to me.


On 02.06.2020 13:18, Xiaoliang Yang wrote:
>There are three hardware TCAMs for ocelot chips: IS1, IS2 and ES0. Each
>one supports different actions. The hardware flow order is: IS1->IS2->ES0.
>
>This patch add three blocks to store rules according to chain index.
>chain 0 is offloaded to IS1, chain 1 is offloaded to IS2, and egress chain
>0 is offloaded to ES0.

Using "static" allocation to to say chain-X goes to TCAM Y, also seems
like the right approach to me. Given the capabilities of the HW, this
will most likely be the easiest scheme to implement and to explain to
the end-user.

But I think we should make some adjustments to this mapping schema.

Here are some important "things" I would like to consider when defining
this schema:

- As you explain, we have 3 TCAMs (IS1, IS2 and ES0), but we have 3
   parallel lookups in IS1 and 2 parallel lookups in IS2 - and also these
   TCAMs has a wide verity of keys.

- We can utilize these multiple parallel lookups such that it seems like
   they are done in serial (that is if they do not touch the same
   actions), but as they are done in parallel they can not influence each
   other.

- We can let IS1 influence the IS2 lookup (like the GOTO actions was
   intended to be used).

- The chip also has other QoS classification facilities which sits
   before the TCAM (take a look at 3.7.3 QoS, DP, and DSCP Classification
   in vsc7514 datasheet). It we at some point in time want to enable
   this, then I think we need to do that in the same tc-flower framework.

Here is my initial suggestion for an alternative chain-schema:

Chain 0:   The default chain - today this is in IS2. If we proceed
with this as is - then this will change.
Chain 1-:  These are offloaded by "basic" classification.
Chain 1-1: These are offloaded in IS1
Chain 1: Lookup-0 in IS1, and here we could limit the
 action to do QoS related stuff (priority
 update)
Chain 11000: Lookup-1 in IS1, here we could do VLAN
 stuff
Chain 12000: Lookup-2 in IS1, here we could apply the
 "PAG" which is essentially a GOTO.

Chain 2-2: These are offloaded in IS2
Chain 2-20255: Lookup-0 in IS2, where CHAIN-ID -
   2 is the PAG value.
Chain 21000-21000: Lookup-1 in IS2.

All these chains should be optional - users should only need to
configure the chains they need. To make this work, we need to configure
both the desired actions (could be priority update) and the goto action.
Remember in HW, all packets goes through this process, while in SW they
only follow the "goto" path.

An example could be (I have not tested this yet - sorry):

tc qdisc add dev eth0 ingress

# Activate lookup 11000. We can not do any other rules in chain 0, also
# this implicitly means that we do not want any chains <11000.
tc filter add dev eth0 parent : chain 0
action
matchall goto 11000

tc filter add dev eth0 parent : chain 11000 \
flower src_mac 00:01:00:00:00:00/00:ff:00:00:00:00 \
action \
vlan modify id 1234 \
pipe \
goto 20001

tc filter add dev eth0 parent : chain 20001 ...

Maybe it would be an idea to create some use-cases, implement them in a
test which can pass with today's SW, and then once we have a common
understanding of what we want, we can implement it?

/Allan

>Using action goto chain to express flow order as follows:
>tc filter add dev swp0 chain 0 parent : flower skip_sw \
>action goto chain 1
>
>Signed-off-by: Xiaoliang Yang 
>---
> drivers/net/ethernet/mscc/ocelot_ace.c| 51 +++
> drivers/net/ethernet/mscc/ocelot_ace.h|  7 ++--
> drivers/net/ethernet/mscc/ocelot_flower.c | 46 +---
> include/soc/mscc/ocelot.h |  2 +-
> include/soc/mscc/ocelot_vcap.h|  4 +-
> 5 files changed, 81 insertions(+), 29 deletions(-)



/Allan


What would be the advantage, from a user perspective, in exposing the
3 IS1 lookups as separate chains with orthogonal actions?
If the user wants to add an IS1 action that performs QoS
classification, VLAN classification and selects a custom PAG, they
would have to install 3 separate filters with the same key, each into
its own chain. Then the driver would be smart enough to figure out
that the 3 keys are actually the same, so it could merge them.
In comparison, we could just add a single filter to the IS1 chai

Re: [PATCH v2 net-next 03/10] net: mscc: ocelot: allocated rules to different hardware VCAP TCAMs by chain index

[Allan W. Nielsen]

Hi Xiaoliang,

Happy to see that you are moving in the directions of multi chain - this
seems ilke a much better fit to me.


On 02.06.2020 13:18, Xiaoliang Yang wrote:

There are three hardware TCAMs for ocelot chips: IS1, IS2 and ES0. Each
one supports different actions. The hardware flow order is: IS1->IS2->ES0.

This patch add three blocks to store rules according to chain index.
chain 0 is offloaded to IS1, chain 1 is offloaded to IS2, and egress chain
0 is offloaded to ES0.


Using "static" allocation to to say chain-X goes to TCAM Y, also seems
like the right approach to me. Given the capabilities of the HW, this
will most likely be the easiest scheme to implement and to explain to
the end-user.

But I think we should make some adjustments to this mapping schema.

Here are some important "things" I would like to consider when defining
this schema:

- As you explain, we have 3 TCAMs (IS1, IS2 and ES0), but we have 3
  parallel lookups in IS1 and 2 parallel lookups in IS2 - and also these
  TCAMs has a wide verity of keys.

- We can utilize these multiple parallel lookups such that it seems like
  they are done in serial (that is if they do not touch the same
  actions), but as they are done in parallel they can not influence each
  other.

- We can let IS1 influence the IS2 lookup (like the GOTO actions was
  intended to be used).

- The chip also has other QoS classification facilities which sits
  before the TCAM (take a look at 3.7.3 QoS, DP, and DSCP Classification
  in vsc7514 datasheet). It we at some point in time want to enable
  this, then I think we need to do that in the same tc-flower framework.

Here is my initial suggestion for an alternative chain-schema:

Chain 0:   The default chain - today this is in IS2. If we proceed
   with this as is - then this will change.
Chain 1-:  These are offloaded by "basic" classification.
Chain 1-1: These are offloaded in IS1
   Chain 1: Lookup-0 in IS1, and here we could limit the
action to do QoS related stuff (priority
update)
   Chain 11000: Lookup-1 in IS1, here we could do VLAN
stuff
   Chain 12000: Lookup-2 in IS1, here we could apply the
"PAG" which is essentially a GOTO.

Chain 2-2: These are offloaded in IS2
   Chain 2-20255: Lookup-0 in IS2, where CHAIN-ID -
  2 is the PAG value.
   Chain 21000-21000: Lookup-1 in IS2.

All these chains should be optional - users should only need to
configure the chains they need. To make this work, we need to configure
both the desired actions (could be priority update) and the goto action.
Remember in HW, all packets goes through this process, while in SW they
only follow the "goto" path.

An example could be (I have not tested this yet - sorry):

tc qdisc add dev eth0 ingress

# Activate lookup 11000. We can not do any other rules in chain 0, also
# this implicitly means that we do not want any chains <11000.
tc filter add dev eth0 parent : chain 0 
   action

   matchall goto 11000

tc filter add dev eth0 parent : chain 11000 \
   flower src_mac 00:01:00:00:00:00/00:ff:00:00:00:00 \
   action \
   vlan modify id 1234 \
   pipe \
   goto 20001

tc filter add dev eth0 parent : chain 20001 ...

Maybe it would be an idea to create some use-cases, implement them in a
test which can pass with today's SW, and then once we have a common
understanding of what we want, we can implement it?

/Allan


Using action goto chain to express flow order as follows:
   tc filter add dev swp0 chain 0 parent : flower skip_sw \
   action goto chain 1

Signed-off-by: Xiaoliang Yang 
---
drivers/net/ethernet/mscc/ocelot_ace.c| 51 +++
drivers/net/ethernet/mscc/ocelot_ace.h|  7 ++--
drivers/net/ethernet/mscc/ocelot_flower.c | 46 +---
include/soc/mscc/ocelot.h |  2 +-
include/soc/mscc/ocelot_vcap.h|  4 +-
5 files changed, 81 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c 
b/drivers/net/ethernet/mscc/ocelot_ace.c
index 748c618db7d8..b76593b40097 100644
--- a/drivers/net/ethernet/mscc/ocelot_ace.c
+++ b/drivers/net/ethernet/mscc/ocelot_ace.c
@@ -341,6 +341,8 @@ static void is2_action_set(struct ocelot *ocelot, struct 
vcap_data *data,
   vcap_action_set(vcap, data, VCAP_IS2_ACT_CPU_QU_NUM, 0);
   vcap_action_set(vcap, data, VCAP_IS2_ACT_CPU_COPY_ENA, 0);
   break;
+   default:
+   break;
   }
}

@@ -644,9 +646,9 @@ static void is2_entry_set(struct ocelot *ocelot, int ix,
}

static void vcap_entry_get(struct ocelot *ocelot, struct ocelot_ace_rule *rule,
-  int ix)
+  int ix, int block_id)
{
-   const

Re: [PATCH RFC net-next 10/13] net: bridge: add port flags for host flooding

[Allan W. Nielsen]

On 22.05.2020 16:13, Vladimir Oltean wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
content is safe

On Fri, 22 May 2020 at 15:38, Nikolay Aleksandrov
 wrote:


On 22/05/2020 00:10, Vladimir Oltean wrote:
> From: Vladimir Oltean 
>
> In cases where the bridge is offloaded by a switchdev, there are
> situations where we can optimize RX filtering towards the host. To be
> precise, the host only needs to do termination, which it can do by
> responding at the MAC addresses of the slave ports and of the bridge
> interface itself. But most notably, it doesn't need to do forwarding,
> so there is no need to see packets with unknown destination address.
>
> But there are, however, cases when a switchdev does need to flood to the
> CPU. Such an example is when the switchdev is bridged with a foreign
> interface, and since there is no offloaded datapath, packets need to
> pass through the CPU. Currently this is the only identified case, but it
> can be extended at any time.
>
> So far, switchdev implementers made driver-level assumptions, such as:
> this chip is never integrated in SoCs where it can be bridged with a
> foreign interface, so I'll just disable host flooding and save some CPU
> cycles. Or: I can never know what else can be bridged with this
> switchdev port, so I must leave host flooding enabled in any case.
>
> Let the bridge drive the host flooding decision, and pass it to
> switchdev via the same mechanism as the external flooding flags.
>
> Signed-off-by: Vladimir Oltean 
> ---
>  include/linux/if_bridge.h |  3 +++
>  net/bridge/br_if.c| 40 +++
>  net/bridge/br_switchdev.c |  4 +++-
>  3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index b3a8d3054af0..6891a432862d 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -49,6 +49,9 @@ struct br_ip_list {
>  #define BR_ISOLATED  BIT(16)
>  #define BR_MRP_AWARE BIT(17)
>  #define BR_MRP_LOST_CONT BIT(18)
> +#define BR_HOST_FLOODBIT(19)
> +#define BR_HOST_MCAST_FLOOD  BIT(20)
> +#define BR_HOST_BCAST_FLOOD  BIT(21)
>
>  #define BR_DEFAULT_AGEING_TIME   (300 * HZ)
>
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index a0e9a7937412..aae59d1e619b 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -166,6 +166,45 @@ void br_manage_promisc(struct net_bridge *br)
>   }
>  }
>
> +static int br_manage_host_flood(struct net_bridge *br)
> +{
> + const unsigned long mask = BR_HOST_FLOOD | BR_HOST_MCAST_FLOOD |
> +BR_HOST_BCAST_FLOOD;
> + struct net_bridge_port *p, *q;
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + unsigned long flags = p->flags;
> + bool sw_bridging = false;
> + int err;
> +
> + list_for_each_entry(q, &br->port_list, list) {
> + if (p == q)
> + continue;
> +
> + if (!netdev_port_same_parent_id(p->dev, q->dev)) {
> + sw_bridging = true;
> + break;
> + }
> + }
> +
> + if (sw_bridging)
> + flags |= mask;
> + else
> + flags &= ~mask;
> +
> + if (flags == p->flags)
> + continue;
> +
> + err = br_switchdev_set_port_flag(p, flags, mask);
> + if (err)
> + return err;
> +
> + p->flags = flags;
> + }
> +
> + return 0;
> +}
> +
>  int nbp_backup_change(struct net_bridge_port *p,
> struct net_device *backup_dev)
>  {
> @@ -231,6 +270,7 @@ static void nbp_update_port_count(struct net_bridge *br)
>   br->auto_cnt = cnt;
>   br_manage_promisc(br);
>   }
> + br_manage_host_flood(br);
>  }
>

Can we do this only at port add/del ?
Right now it will be invoked also by br_port_flags_change() upon BR_AUTO_MASK 
flag change.



Yes, we can do that.
Actually I have some doubts about BR_HOST_BCAST_FLOOD. We can't
disable that in the no-foreign-interface case, can we? For IPv6, it
looks like the stack does take care of installing dev_mc addresses for
the neighbor discovery protocol, but for IPv4 I guess the assumption
is that broadcast ARP should always be processed?


Ideally this should be per VLAN. In case of IPv4, you only need to be
part of the broadcast domain on VLANs with an associated vlan-interface.


>  static void nbp_delete_promisc(struct net_bridge_port *p)
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 015209bf44aa..360806ac7463 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -56,7 +56,9 @@ bool nbp_switchdev_allowed_egress(const struct 
net_bridge_port *p,
>
>  /* Flags that can be offlo

Re: [PATCH RFC net-next 00/13] RX filtering for DSA switches

[Allan W. Nielsen]

Hi Vladimir,

I'm very happy to see that you started working on this. Let me know if
you need help to update the Ocelot/Felix driver to support this.

/Allan

On 22.05.2020 00:10, Vladimir Oltean wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
content is safe

From: Vladimir Oltean 

This is a WIP series whose stated goal is to allow DSA and switchdev
drivers to flood less traffic to the CPU while keeping the same level of
functionality.

The strategy is to whitelist towards the CPU only the {DMAC, VLAN} pairs
that the operating system has expressed its interest in, either due to
those being the MAC addresses of one of the switch ports, or addresses
added to our device's RX filter via calls to dev_uc_add/dev_mc_add.
Then, the traffic which is not explicitly whitelisted is not sent by the
hardware to the CPU, under the assumption that the CPU didn't ask for it
and would have dropped it anyway.

The ground for these patches were the discussions surrounding RX
filtering with switchdev in general, as well as with DSA in particular:

"[PATCH net-next 0/4] DSA: promisc on master, generic flow dissector code":
https://www.spinics.net/lists/netdev/msg651922.html
"[PATCH v3 net-next 2/2] net: dsa: felix: Allow unknown unicast traffic towards the 
CPU port module":
https://www.spinics.net/lists/netdev/msg634859.html
"[PATCH v3 0/2] net: core: Notify on changes to dev->promiscuity":
https://lkml.org/lkml/2019/8/29/255
LPC2019 - SwitchDev offload optimizations:
https://www.youtube.com/watch?v=B1HhxEcU7Jg

Unicast filtering comes to me as most important, and this includes
termination of MAC addresses corresponding to the network interfaces in
the system (DSA switch ports, VLAN sub-interfaces, bridge interface).
The first 4 patches use Ivan Khoronzhuk's IVDF framework for extending
network interface addresses with a Virtual ID (typically VLAN ID). This
matches DSA switches perfectly because their FDB already contains keys
of the {DMAC, VID} form.

Multicast filtering was taken and reworked from Florian Fainelli's
previous attempts, according to my own understanding of multicast
forwarding requirements of an IGMP snooping switch. This is the part
that needs the most extra work, not only in the DSA core but also in
drivers. For this reason, I've left out of this patchset anything that
has to do with driver-level configuration (since the audience is a bit
larger than usual), as I'm trying to focus more on policy for now, and
the series is already pretty huge.

Florian Fainelli (3):
 net: bridge: multicast: propagate br_mc_disabled_update() return
 net: dsa: add ability to program unicast and multicast filters for CPU
   port
 net: dsa: wire up multicast IGMP snooping attribute notification

Ivan Khoronzhuk (4):
 net: core: dev_addr_lists: add VID to device address
 net: 8021q: vlan_dev: add vid tag to addresses of uc and mc lists
 net: 8021q: vlan_dev: add vid tag for vlan device own address
 ethernet: eth: add default vid len for all ethernet kind devices

Vladimir Oltean (6):
 net: core: dev_addr_lists: export some raw __hw_addr helpers
 net: dsa: don't use switchdev_notifier_fdb_info in
   dsa_switchdev_event_work
 net: dsa: mroute: don't panic the kernel if called without the prepare
   phase
 net: bridge: add port flags for host flooding
 net: dsa: deal with new flooding port attributes from bridge
 net: dsa: treat switchdev notifications for multicast router connected
   to port

include/linux/if_bridge.h |   3 +
include/linux/if_vlan.h   |   2 +
include/linux/netdevice.h |  11 ++
include/net/dsa.h |  17 +++
net/8021q/Kconfig |  12 ++
net/8021q/vlan.c  |   3 +
net/8021q/vlan.h  |   2 +
net/8021q/vlan_core.c |  25 
net/8021q/vlan_dev.c  | 102 +++---
net/bridge/br_if.c|  40 ++
net/bridge/br_multicast.c |  21 ++-
net/bridge/br_switchdev.c |   4 +-
net/core/dev_addr_lists.c | 144 +++
net/dsa/Kconfig   |   1 +
net/dsa/dsa2.c|   6 +
net/dsa/dsa_priv.h|  27 +++-
net/dsa/port.c| 155 
net/dsa/slave.c   | 288 +++---
net/dsa/switch.c  |  36 +
net/ethernet/eth.c|  12 +-
20 files changed, 780 insertions(+), 131 deletions(-)

--
2.25.1


/Allan

Re: [EXT] Re: [PATCH v1 net-next 4/6] net: mscc: ocelot: VCAP IS1 support

[Allan W. Nielsen]

On 07.05.2020 11:23, Xiaoliang Yang wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
content is safe

Hi Allan,



Hi Vladimir,

On 06.05.2020 13:53, Vladimir Oltean wrote:

[snip]

>At the moment, the driver does not support more than 1 action. We might
>need to change that, but we can still install more filters with the
>same key and still be fine (see more below). When there is more than 1
>action, the IS1 stuff will be combined into a single rule programmed
>into IS1, and the IS2 stuff will be combined into a single new rule
>with the same keys installed into VCAP IS2. Would that not work?
>
>> The SW model have these two rules in the same table, and can stop
>> process at the first match. SW will do the action of the first frame
>> matching.
>>
>
>Actually I think this is an incorrect assumption - software stops at
>the first action only if told to do so. Let me copy-paste a text from a
>different email thread.

I'm still not able to see how this proposal will give us the same behavioral in 
SW and in HW.

A simple example:

tc qdisc add dev enp0s3 ingress
tc filter add dev enp0s3 protocol 802.1Q parent : \
 prio 10 flower vlan_id 5 action vlan modify id 10 tc filter add dev enp0s3 
protocol 802.1Q parent : \
 prio 20 flower src_mac 00:00:00:00:00:08 action drop

We can then inject a frame with VID 5 and smac ::08:
$ ef tx tap0 eth smac 00:00:00:00:00:08 ctag vid 5

We can then check the filter and see that it only hit the first rule:

$ tc -s filter show dev enp0s3 ingress
filter protocol 802.1Q pref 10 flower chain 0 filter protocol 802.1Q pref 10 
flower chain 0 handle 0x1
   vlan_id 5
   not_in_hw
 action order 1: vlan  modify id 10 protocol 802.1Q priority 0 pipe
  index 1 ref 1 bind 1 installed 19 sec used 6 sec
 Action statistics:
 Sent 42 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

filter protocol 802.1Q pref 20 flower chain 0 filter protocol 802.1Q pref 20 
flower chain 0 handle 0x1
  src_mac 00:00:00:00:00:08
  not_in_hw
action order 1: gact action drop
 random type none pass val 0
 index 1 ref 1 bind 1 installed 11 sec used 11 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

If this was done with the proposed HW offload, then both rules would have been 
hit and we would have a different behavioral.

This can be fixed by adding the "continue" action to the first rule:



tc filter add dev enp0s3 protocol 802.1Q parent : \
 prio 10 flower vlan_id 5 action vlan modify id 10 continue tc filter add 
dev enp0s3 protocol 802.1Q parent : \
 prio 20 flower src_mac 00:00:00:00:00:08 action drop

But that would again break if we add 2 rules manipulating the VLAN (as the HW 
does not continue with in a single TCAM).

My point is: I do not think we can hide the fact that this is done in 
independent TCAMs in the silicon.

I think it is possible to do this with the chain feature (even though it is not 
a perfect match), but it would require more analysis.

/Allan


Do you mean it's better to set vlan modify filters in a different chain, and 
write the filter entries with a same chain in the same VCAP TCAM?
For example:
   tc filter add dev enp0s3 protocol 802.1Q chain 11 parent : prio 10 
flower skip_sw vlan_id 5 action vlan modify id 10
   tc filter add dev enp0s3 protocol 802.1Q chain 22 parent : prio 20 
flower skip_sw src_mac 00:00:00:00:00:08 action drop
for this usage, we only need to ensure a chain corresponding to a VCAP in 
ocelot ace driver. I'm not sure is my understanding right?


I still have not found a satisfying solution to this. As I understand
the chains, they require the "goto" action to be used to tie them
together.

We could use that to represent a single lookup in is1 and link that to a
lookup in is2. Not sure if we should, it will also require
(non-backwards compatible) changes in how the existing IS2 support is
working.

Again, I do not have the answer (I'm also looking for it), but I think
we need something where it is clear to the user that this end up in
different lists.

/Allan

Re: [PATCH v1 net-next 4/6] net: mscc: ocelot: VCAP IS1 support

[Allan W. Nielsen]

Hi Vladimir,

On 06.05.2020 13:53, Vladimir Oltean wrote:

On Wed, 6 May 2020 at 12:45, Allan W. Nielsen
 wrote:


Hi Xiaoliang,

On 06.05.2020 15:48, Xiaoliang Yang wrote:
>VCAP IS1 is a VCAP module which can filter MAC, IP, VLAN, protocol, and
>TCP/UDP ports keys, and do Qos and VLAN retag actions.
>This patch added VCAP IS1 support in ocelot ace driver, which can supports
>vlan modify action of tc filter.
>Usage:
>tc qdisc add dev swp0 ingress
>tc filter add dev swp0 protocol 802.1Q parent : flower \
>skip_sw vlan_id 1 vlan_prio 1 action vlan modify id 2 priority 2
I skimmed skimmed through the patch serie, and the way I understood it
is that you look at the action, and if it is a VLAN operation, then you
put it in IS1 and if it is one of the other then put it in IS2.

This is how the HW is designed - I'm aware of that.

But how will this work if you have 2 rules, 1 modifying the VLAN and
another rule dropping certain packets?



At the moment, the driver does not support more than 1 action. We
might need to change that, but we can still install more filters with
the same key and still be fine (see more below). When there is more
than 1 action, the IS1 stuff will be combined into a single rule
programmed into IS1, and the IS2 stuff will be combined into a single
new rule with the same keys installed into VCAP IS2. Would that not
work?


The SW model have these two rules in the same table, and can stop
process at the first match. SW will do the action of the first frame
matching.



Actually I think this is an incorrect assumption - software stops at
the first action only if told to do so. Let me copy-paste a text from
a different email thread.


I'm still not able to see how this proposal will give us the same
behavioral in SW and in HW.

A simple example:

tc qdisc add dev enp0s3 ingress
tc filter add dev enp0s3 protocol 802.1Q parent : \
prio 10 flower vlan_id 5 action vlan modify id 10
tc filter add dev enp0s3 protocol 802.1Q parent : \
prio 20 flower src_mac 00:00:00:00:00:08 action drop

We can then inject a frame with VID 5 and smac ::08:
$ ef tx tap0 eth smac 00:00:00:00:00:08 ctag vid 5 


We can then check the filter and see that it only hit the first rule:

$ tc -s filter show dev enp0s3 ingress
filter protocol 802.1Q pref 10 flower chain 0
filter protocol 802.1Q pref 10 flower chain 0 handle 0x1
  vlan_id 5
  not_in_hw
action order 1: vlan  modify id 10 protocol 802.1Q priority 0 pipe
 index 1 ref 1 bind 1 installed 19 sec used 6 sec
Action statistics:
Sent 42 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

filter protocol 802.1Q pref 20 flower chain 0
filter protocol 802.1Q pref 20 flower chain 0 handle 0x1
  src_mac 00:00:00:00:00:08
  not_in_hw
action order 1: gact action drop
 random type none pass val 0
 index 1 ref 1 bind 1 installed 11 sec used 11 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0

If this was done with the proposed HW offload, then both rules would
have been hit and we would have a different behavioral.

This can be fixed by adding the "continue" action to the first rule:

tc filter add dev enp0s3 protocol 802.1Q parent : \
prio 10 flower vlan_id 5 action vlan modify id 10 continue
tc filter add dev enp0s3 protocol 802.1Q parent : \
prio 20 flower src_mac 00:00:00:00:00:08 action drop

But that would again break if we add 2 rules manipulating the VLAN (as
the HW does not continue with in a single TCAM).

My point is: I do not think we can hide the fact that this is done
in independent TCAMs in the silicon.

I think it is possible to do this with the chain feature (even though it
is not a perfect match), but it would require more analysis.

/Allan

Re: [PATCH v1 net-next 4/6] net: mscc: ocelot: VCAP IS1 support

[Allan W. Nielsen]

Hi Xiaoliang,

On 06.05.2020 15:48, Xiaoliang Yang wrote:

VCAP IS1 is a VCAP module which can filter MAC, IP, VLAN, protocol, and
TCP/UDP ports keys, and do Qos and VLAN retag actions.
This patch added VCAP IS1 support in ocelot ace driver, which can supports
vlan modify action of tc filter.
Usage:
   tc qdisc add dev swp0 ingress
   tc filter add dev swp0 protocol 802.1Q parent : flower \
   skip_sw vlan_id 1 vlan_prio 1 action vlan modify id 2 priority 2

I skimmed skimmed through the patch serie, and the way I understood it
is that you look at the action, and if it is a VLAN operation, then you
put it in IS1 and if it is one of the other then put it in IS2.

This is how the HW is designed - I'm aware of that.

But how will this work if you have 2 rules, 1 modifying the VLAN and
another rule dropping certain packets?

The SW model have these two rules in the same table, and can stop
process at the first match. SW will do the action of the first frame
matching.

The HW will how-ever do both, as they are in independent TCAMs.

If we want to enable all the TCAM lookups in Ocelot/Felix, then we need
to find a way where we will get the same results when doing the
operation in HW and in SW.

/Allan

Re: [v3] ocelot_ace: fix action of trap

[Allan W . Nielsen]

Hi,

This is fixing a bug introduced in b596229448dd2

Acked-by: Allan W. Nielsen 

/Allan


The 08/20/2019 12:20, Yangbo Lu wrote:
> External E-Mail
> 
> 
> The trap action should be copying the frame to CPU and
> dropping it for forwarding, but current setting was just
> copying frame to CPU.
> 
> Signed-off-by: Yangbo Lu 
> ---
>  drivers/net/ethernet/mscc/ocelot_ace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c 
> b/drivers/net/ethernet/mscc/ocelot_ace.c
> index 39aca1a..86fc6e6 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> @@ -317,7 +317,7 @@ static void is2_action_set(struct vcap_data *data,
>   break;
>   case OCELOT_ACL_ACTION_TRAP:
>   VCAP_ACT_SET(PORT_MASK, 0x0);
> - VCAP_ACT_SET(MASK_MODE, 0x0);
> + VCAP_ACT_SET(MASK_MODE, 0x1);
>   VCAP_ACT_SET(POLICE_ENA, 0x0);
>   VCAP_ACT_SET(POLICE_IDX, 0x0);
>   VCAP_ACT_SET(CPU_QU_NUM, 0x0);
> -- 
> 2.7.4
> 
> 

-- 
/Allan

Re: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP Ethernet frames

[Allan W . Nielsen]

The 08/14/2019 04:56, Y.b. Lu wrote:
> > -Original Message-
> > From: Allan W . Nielsen 
> > Sent: Tuesday, August 13, 2019 2:25 PM
> > To: Y.b. Lu 
> > Cc: netdev@vger.kernel.org; David S . Miller ;
> > Alexandre Belloni ; Microchip Linux Driver
> > Support 
> > Subject: Re: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP Ethernet frames
> > 
> > The 08/13/2019 10:52, Yangbo Lu wrote:
> > > All the PTP messages over Ethernet have etype 0x88f7 on them.
> > > Use etype as the key to trap PTP messages.
> > >
> > > Signed-off-by: Yangbo Lu 
> > > ---
> > > Changes for v2:
> > >   - Added this patch.
> > > ---
> > >  drivers/net/ethernet/mscc/ocelot.c | 28 
> > >  1 file changed, 28 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/mscc/ocelot.c
> > > b/drivers/net/ethernet/mscc/ocelot.c
> > > index 6932e61..40f4e0d 100644
> > > --- a/drivers/net/ethernet/mscc/ocelot.c
> > > +++ b/drivers/net/ethernet/mscc/ocelot.c
> > > @@ -1681,6 +1681,33 @@ int ocelot_probe_port(struct ocelot *ocelot, u8
> > > port,  }  EXPORT_SYMBOL(ocelot_probe_port);
> > >
> > > +static int ocelot_ace_add_ptp_rule(struct ocelot *ocelot) {
> > > + struct ocelot_ace_rule *rule;
> > > +
> > > + rule = kzalloc(sizeof(*rule), GFP_KERNEL);
> > > + if (!rule)
> > > + return -ENOMEM;
> > > +
> > > + /* Entry for PTP over Ethernet (etype 0x88f7)
> > > +  * Action: trap to CPU port
> > > +  */
> > > + rule->ocelot = ocelot;
> > > + rule->prio = 1;
> > > + rule->type = OCELOT_ACE_TYPE_ETYPE;
> > > + /* Available on all ingress port except CPU port */
> > > + rule->ingress_port = ~BIT(ocelot->num_phys_ports);
> > > + rule->dmac_mc = OCELOT_VCAP_BIT_1;
> > > + rule->frame.etype.etype.value[0] = 0x88;
> > > + rule->frame.etype.etype.value[1] = 0xf7;
> > > + rule->frame.etype.etype.mask[0] = 0xff;
> > > + rule->frame.etype.etype.mask[1] = 0xff;
> > > + rule->action = OCELOT_ACL_ACTION_TRAP;
> > > +
> > > + ocelot_ace_rule_offload_add(rule);
> > > + return 0;
> > > +}
> > > +
> > >  int ocelot_init(struct ocelot *ocelot)  {
> > >   u32 port;
> > > @@ -1708,6 +1735,7 @@ int ocelot_init(struct ocelot *ocelot)
> > >   ocelot_mact_init(ocelot);
> > >   ocelot_vlan_init(ocelot);
> > >   ocelot_ace_init(ocelot);
> > > + ocelot_ace_add_ptp_rule(ocelot);
> > >
> > >   for (port = 0; port < ocelot->num_phys_ports; port++) {
> > >   /* Clear all counters (5 groups) */
> > This seems really wrong to me, and much too hard-coded...
> > 
> > What if I want to forward the PTP frames to be forwarded like a normal
> > non-aware PTP switch?
> 
> [Y.b. Lu] As Andrew said, other switches could identify PTP messages and 
> forward to CPU for processing.
> https://patchwork.ozlabs.org/patch/1145627/
Yes, it would be good to see some exampels to understand this better.

> I'm also wondering whether there is common method in linux to address your 
> questions.
Me too.

> If no, I think trapping all PTP messages on all ports to CPU could be used 
> for now.
> If users require PTP synchronization, they actually don’t want a non-aware 
> PTP switch.
Can we continue this discussion in the other thread where I listed the 3
scenarios?

> I once see other ocelot code configure ptp trap rules in ioctl timestamping
> setting. But I don’t think it's proper either.  Enable timestamping doesn’t
> mean we want to trap PTP messages.
Where did you see this?

The effort in [1] is just about the time-stamping and does not really consider
the bridge part of it, and it should not be installing any TCAM rules (I believe
it did in earlier versions, but this has been changed).

[1] https://patchwork.ozlabs.org/patch/1145777/

> > What if do not want this on all ports?
> [Y.b. Lu] Actually I don’t think there should be difference of handling PTP 
> messages on each port.
> You don’t need to run PTP protocol application on the specific port if you 
> don’t want.
What if you want some vlans or some ports to be PTP unaware, and other to be PTP
aware.

> > If you do not have an application behind this implementing a boundary or
> > transparent clock, then you are breaking PTP on the network.
> [Y.b. Lu] You're right. But actually for PTP network, all PTP devices should 
> run PTP protocol on it.
> Of course, it's better to have a way to configure it as non-aware PTP switch.
I think we agree.

In my point of view, it is the PTP daemon who should configure frames to be
trapped. Then the switch will be PTP unaware until the PTP daemon starts up and
is ready to make it aware.

If we put it in the init function, then it will be of PTP broken until the PTP
daemon starts.

/Allan

Re: [PATCH 3/3] ocelot_ace: fix action of trap

[Allan W. Nielsen]

Hi Y.b. and Andrew,

The 08/14/2019 04:28, Y.b. Lu wrote:
> > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype
> > 0x88f7.
> > 
> > Is this the correct way to handle PTP for this switch? For other switches we
> > don't need such traps. The switch itself identifies PTP frames and forwards
> > them to the CPU so it can process them.
> > 
> > I'm just wondering if your general approach is wrong?
> 
> [Y.b. Lu] PTP messages over Ethernet will use two multicast addresses.
> 01-80-C2-00-00-0E for peer delay messages.
Yes, and as you write, this is a BPDU which must not be forwarded (and they are
not).

> 01-1B-19-00-00-00 for other messages.
Yes, this is a normal L2 multicast address, which by default are broadcastet.

> But only 01-80-C2-00-00-0E could be handled by hardware filter for BPDU frames
> (01-80-C2-00-00-0x).  For PTP messages handling, trapping them to CPU through
> VCAP IS2 is the suggested way by Ocelot/Felix.
As I see it there are at least 3 scenarios which needs to be considered and
ideally supported:

1) Operate as a PTP-unaware switch. This means that Peer-delays messages are
   trapped to the CPU and not handled. End-to-End PTP sessions can still run on
   the network as 01-1B-19-00-00-00 frames are forwarded normally.

   This is what we have by default today.

2) A "passive" PTP switch (in MSCC/MCHP we call this an end-to-end transparent
   clock) where 01-80-C2-00-00-0E frames are still trapped to the CPU (and not
   handled), 01-1B-19-00-00-00 frames are forwarded, but we use the TCAM to add
   the residence time to the correction field in Sync and Delay request
   messages.

   This is a simple mechanism which allow end-to-end PTP sessions to synchronize
   their time, and compensate for the variable delay caused by the switch.

   Compared to implement a complete boundary clock, this is much simpler, and
   cause a much lower work load on the CPU (even small switches may be serving
   many many PTP sessions).

3) Full PTP aware switch. In this mode we need all PTP frames trapped (on the
   ports where PTP are running) to the CPU, and we need a PTP daemon in
   user-space to process them in-order for things to work.

   I guess this is what you are trying to achieve.

Eventually, I hope we can get to a point where all (and maybe more) scenarios
are supported.

Lets consider them case by case:

1) This is what we have today.

2) To support this, we need a SW implementation of this, and then we can add
   hooks to offload this in HW.

   We would certainly be interested in supporting this in both SW and HW.

3) It can be done via 'tc' using the trap action, but I do not know if this is
   the desired way of doing it. Ocelot will be using a TCAM rule to do this,
   which align nicely with the 'tc' approach, but other chips may be have
   dedicated HW for doing this.

   Also, in the current implementation we will be using a rule per port, and
   ideally we could have done it with a single rule (this is what Y.B. prepared
   in this patch series).

I'm very much against configuring option 3 in the driver initialization, as it
will prevent us from having a conforming switch if a PTP daemon is not running
in user-space, and it give us very little room for supporting other ways in the
future without breaking backwards compatibility.

> I have a question since you are experts.
I'm not really an expert on this, but I have access to some good guidance from
collages knowing PTP very well :-D

> For other switches, whether they are always trapping PTP messages to CPU?
Good question, I could not find anything in the SW bridge forcing option 3.

My understanding is that the SW bridge is implementing option 1, but I could be
wrong.

> Is there any common method in linux to configure switch to select trapping or
> just forwarding PTP messages?
You should be able to use TC for this. But due to the port vs port-mask
limitation you will need to install a rule per port.

I do not know if this is what others are doing, but would like to learn about
that.

/Allan

Re: [PATCH 3/3] ocelot_ace: fix action of trap

[Allan W. Nielsen]

> > -Original Message-
> > From: Allan W. Nielsen 
> > Sent: Tuesday, August 13, 2019 2:16 PM
> > To: Y.b. Lu 
> > Cc: netdev@vger.kernel.org; David S . Miller ;
> > Alexandre Belloni ; Microchip Linux Driver
> > Support 
> > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap
> > 
> > The 08/13/2019 02:12, Y.b. Lu wrote:
> > > > -Original Message-
> > > > From: Allan W. Nielsen 
> > > > Sent: Monday, August 12, 2019 8:32 PM
> > > > To: Y.b. Lu 
> > > > Cc: netdev@vger.kernel.org; David S . Miller ;
> > > > Alexandre Belloni ; Microchip Linux
> > > > Driver Support 
> > > > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap
> > > >
> > > > The 08/12/2019 18:48, Yangbo Lu wrote:
> > > > > The trap action should be copying the frame to CPU and dropping it
> > > > > for forwarding, but current setting was just copying frame to CPU.
> > > >
> > > > Are there any actions which do a "copy-to-cpu" and still forward the
> > > > frame in HW?
> > >
> > > [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by
> > upstream.
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > >
> > hwork.ozlabs.org%2Fproject%2Fnetdev%2Flist%2F%3Fseries%3D115399%26s
> > tat
> > >
> > e%3D*&data=02%7C01%7Cyangbo.lu%40nxp.com%7C42cd202cb17b45
> > 69821708d
> > >
> > 71fb5c5de%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6370127
> > 37899910
> > >
> > 736&sdata=QnsDaWPHK9rb0XWg%2BduYEha6fuYSlv4YZdsu5f4kbfc%3D
> > &res
> > > erved=0
> > >
> > > I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype
> > 0x88f7.
> > > When I used current TRAP option, I found the frames were not only copied 
> > > to
> > CPU, but also forwarded to other ports.
> > > So I just made the TRAP option same with DROP option except enabling
> > CPU_COPY_ENA in the patch.
> > This is still wrong to do - and it will not work for Ocelot (and I doubt it 
> > will
> > work for your Felix target).
> > 
> > The policer setting in the drop action ensure that the frame is dropped 
> > even if
> > other pipe-line steps in the switch has set the copy-to-cpu flag.
> > 
> > I think you can fix this patch my just clearing the port mask, and not set 
> > the
> > policer.
> 
> [Y.b. Lu] Sorry. I missed your previous comments on the TRAP action.
> With my configuration in the patch, it indeed worked. Maybe it was because 
> "the CPU port is not touched by MASK_MODE" which I saw in RM.
Okay. If this is working, then you should properly test and see if the DROP
action is working on your target. I do not have access to the SoC which incldues
Felix, so I cannot check.

> I will try your suggestion too. It sound more proper.
Sounds good - thanks

/Allan

Re: [v2, 3/4] ocelot_ace: fix action of trap

[Allan W . Nielsen]

The 08/13/2019 08:16, Allan W . Nielsen wrote:
> The 08/13/2019 10:52, Yangbo Lu wrote:
> > The trap action should be copying the frame to CPU and
> > dropping it for forwarding, but current setting was just
> > copying frame to CPU.
> > 
> > Signed-off-by: Yangbo Lu 
> > ---
> > Changes for v2:
> > - None.
> > ---
> >  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c 
> > b/drivers/net/ethernet/mscc/ocelot_ace.c
> > index 91250f3..59ad590 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> > +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> > @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
> > break;
> > case OCELOT_ACL_ACTION_TRAP:
> > VCAP_ACT_SET(PORT_MASK, 0x0);
> > -   VCAP_ACT_SET(MASK_MODE, 0x0);
> > -   VCAP_ACT_SET(POLICE_ENA, 0x0);
> > -   VCAP_ACT_SET(POLICE_IDX, 0x0);
> > +   VCAP_ACT_SET(MASK_MODE, 0x1);
> > +   VCAP_ACT_SET(POLICE_ENA, 0x1);
> > +   VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
> > VCAP_ACT_SET(CPU_QU_NUM, 0x0);
> > VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
> > break;
> 
> This is still wrong, please see the comments provided the first time you
> submitted this.
> 
> /Allan

I believe this will make it work - but I have not tested it:

case OCELOT_ACL_ACTION_TRAP:
VCAP_ACT_SET(PORT_MASK, 0x0);
-   VCAP_ACT_SET(MASK_MODE, 0x0);
+   VCAP_ACT_SET(MASK_MODE, 0x1);
VCAP_ACT_SET(CPU_QU_NUM, 0x0);
VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
break;

-- 
/Allan

Re: [v2, 4/4] ocelot: add VCAP IS2 rule to trap PTP Ethernet frames

[Allan W . Nielsen]

The 08/13/2019 10:52, Yangbo Lu wrote:
> All the PTP messages over Ethernet have etype 0x88f7 on them.
> Use etype as the key to trap PTP messages.
> 
> Signed-off-by: Yangbo Lu 
> ---
> Changes for v2:
>   - Added this patch.
> ---
>  drivers/net/ethernet/mscc/ocelot.c | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot.c 
> b/drivers/net/ethernet/mscc/ocelot.c
> index 6932e61..40f4e0d 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -1681,6 +1681,33 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
>  }
>  EXPORT_SYMBOL(ocelot_probe_port);
>  
> +static int ocelot_ace_add_ptp_rule(struct ocelot *ocelot)
> +{
> + struct ocelot_ace_rule *rule;
> +
> + rule = kzalloc(sizeof(*rule), GFP_KERNEL);
> + if (!rule)
> + return -ENOMEM;
> +
> + /* Entry for PTP over Ethernet (etype 0x88f7)
> +  * Action: trap to CPU port
> +  */
> + rule->ocelot = ocelot;
> + rule->prio = 1;
> + rule->type = OCELOT_ACE_TYPE_ETYPE;
> + /* Available on all ingress port except CPU port */
> + rule->ingress_port = ~BIT(ocelot->num_phys_ports);
> + rule->dmac_mc = OCELOT_VCAP_BIT_1;
> + rule->frame.etype.etype.value[0] = 0x88;
> + rule->frame.etype.etype.value[1] = 0xf7;
> + rule->frame.etype.etype.mask[0] = 0xff;
> + rule->frame.etype.etype.mask[1] = 0xff;
> + rule->action = OCELOT_ACL_ACTION_TRAP;
> +
> + ocelot_ace_rule_offload_add(rule);
> + return 0;
> +}
> +
>  int ocelot_init(struct ocelot *ocelot)
>  {
>   u32 port;
> @@ -1708,6 +1735,7 @@ int ocelot_init(struct ocelot *ocelot)
>   ocelot_mact_init(ocelot);
>   ocelot_vlan_init(ocelot);
>   ocelot_ace_init(ocelot);
> + ocelot_ace_add_ptp_rule(ocelot);
>  
>   for (port = 0; port < ocelot->num_phys_ports; port++) {
>   /* Clear all counters (5 groups) */
This seems really wrong to me, and much too hard-coded...

What if I want to forward the PTP frames to be forwarded like a normal non-aware
PTP switch?

What if do not want this on all ports?

If you do not have an application behind this implementing a boundary or
transparent clock, then you are breaking PTP on the network.

/Allan

Re: [v2, 3/4] ocelot_ace: fix action of trap

[Allan W . Nielsen]

The 08/13/2019 10:52, Yangbo Lu wrote:
> The trap action should be copying the frame to CPU and
> dropping it for forwarding, but current setting was just
> copying frame to CPU.
> 
> Signed-off-by: Yangbo Lu 
> ---
> Changes for v2:
>   - None.
> ---
>  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c 
> b/drivers/net/ethernet/mscc/ocelot_ace.c
> index 91250f3..59ad590 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
>   break;
>   case OCELOT_ACL_ACTION_TRAP:
>   VCAP_ACT_SET(PORT_MASK, 0x0);
> - VCAP_ACT_SET(MASK_MODE, 0x0);
> - VCAP_ACT_SET(POLICE_ENA, 0x0);
> - VCAP_ACT_SET(POLICE_IDX, 0x0);
> + VCAP_ACT_SET(MASK_MODE, 0x1);
> + VCAP_ACT_SET(POLICE_ENA, 0x1);
> + VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
>   VCAP_ACT_SET(CPU_QU_NUM, 0x0);
>   VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
>   break;

This is still wrong, please see the comments provided the first time you
submitted this.

/Allan

Re: [PATCH 3/3] ocelot_ace: fix action of trap

[Allan W. Nielsen]

The 08/13/2019 02:12, Y.b. Lu wrote:
> > -Original Message-
> > From: Allan W. Nielsen 
> > Sent: Monday, August 12, 2019 8:32 PM
> > To: Y.b. Lu 
> > Cc: netdev@vger.kernel.org; David S . Miller ;
> > Alexandre Belloni ; Microchip Linux Driver
> > Support 
> > Subject: Re: [PATCH 3/3] ocelot_ace: fix action of trap
> > 
> > The 08/12/2019 18:48, Yangbo Lu wrote:
> > > The trap action should be copying the frame to CPU and dropping it for
> > > forwarding, but current setting was just copying frame to CPU.
> > 
> > Are there any actions which do a "copy-to-cpu" and still forward the frame 
> > in
> > HW?
> 
> [Y.b. Lu] We're using Felix switch whose code hadn't been accepted by 
> upstream.
> https://patchwork.ozlabs.org/project/netdev/list/?series=115399&state=*
> 
> I'd like to trap all IEEE 1588 PTP Ethernet frames to CPU through etype 
> 0x88f7.
> When I used current TRAP option, I found the frames were not only copied to 
> CPU, but also forwarded to other ports.
> So I just made the TRAP option same with DROP option except enabling 
> CPU_COPY_ENA in the patch.
This is still wrong to do - and it will not work for Ocelot (and I doubt it will
work for your Felix target).

The policer setting in the drop action ensure that the frame is dropped even if
other pipe-line steps in the switch has set the copy-to-cpu flag.

I think you can fix this patch my just clearing the port mask, and not set the
policer.

/Allan

Re: [PATCH 2/3] ocelot_ace: fix ingress ports setting for rule

[Allan W. Nielsen]

The 08/12/2019 18:48, Yangbo Lu wrote:
> The ingress ports setting of rule should support covering all ports.
> This patch is to use u16 ingress_port for ingress port mask setting
> for ace rule. One bit corresponds one port.
That is how the HW is working, and it would be nice if we could operate on a
port masks/lists instead. But how can this be used?

Can you please explain how/when this will make a difference?

> Signed-off-by: Yangbo Lu 
> ---
>  drivers/net/ethernet/mscc/ocelot_ace.c| 2 +-
>  drivers/net/ethernet/mscc/ocelot_ace.h| 2 +-
>  drivers/net/ethernet/mscc/ocelot_flower.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c 
> b/drivers/net/ethernet/mscc/ocelot_ace.c
> index 5580a58..91250f3 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> @@ -352,7 +352,7 @@ static void is2_entry_set(struct ocelot *ocelot, int ix,
>   data.type = IS2_ACTION_TYPE_NORMAL;
>  
>   VCAP_KEY_ANY_SET(PAG);
> - VCAP_KEY_SET(IGR_PORT_MASK, 0, ~BIT(ace->chip_port));
> + VCAP_KEY_SET(IGR_PORT_MASK, 0, ~ace->ingress_port);
>   VCAP_KEY_BIT_SET(FIRST, OCELOT_VCAP_BIT_1);
>   VCAP_KEY_BIT_SET(HOST_MATCH, OCELOT_VCAP_BIT_ANY);
>   VCAP_KEY_BIT_SET(L2_MC, ace->dmac_mc);
> diff --git a/drivers/net/ethernet/mscc/ocelot_ace.h 
> b/drivers/net/ethernet/mscc/ocelot_ace.h
> index ce72f02..0fe23e0 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ace.h
> +++ b/drivers/net/ethernet/mscc/ocelot_ace.h
> @@ -193,7 +193,7 @@ struct ocelot_ace_rule {
>  
>   enum ocelot_ace_action action;
>   struct ocelot_ace_stats stats;
> - int chip_port;
> + u16 ingress_port;
>  
>   enum ocelot_vcap_bit dmac_mc;
>   enum ocelot_vcap_bit dmac_bc;
> diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c 
> b/drivers/net/ethernet/mscc/ocelot_flower.c
> index 7c60e8c..bfddc50 100644
> --- a/drivers/net/ethernet/mscc/ocelot_flower.c
> +++ b/drivers/net/ethernet/mscc/ocelot_flower.c
> @@ -184,7 +184,7 @@ struct ocelot_ace_rule *ocelot_ace_rule_create(struct 
> flow_cls_offload *f,
>   return NULL;
>  
>   rule->ocelot = block->port->ocelot;
> - rule->chip_port = block->port->chip_port;
> + rule->ingress_port = BIT(block->port->chip_port);
>   return rule;
>  }

-- Allan

Re: [PATCH 3/3] ocelot_ace: fix action of trap

[Allan W. Nielsen]

The 08/12/2019 18:48, Yangbo Lu wrote:
> The trap action should be copying the frame to CPU and
> dropping it for forwarding, but current setting was just
> copying frame to CPU.

Are there any actions which do a "copy-to-cpu" and still forward the frame in
HW?

> Signed-off-by: Yangbo Lu 
> ---
>  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c 
> b/drivers/net/ethernet/mscc/ocelot_ace.c
> index 91250f3..59ad590 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
>   break;
>   case OCELOT_ACL_ACTION_TRAP:
>   VCAP_ACT_SET(PORT_MASK, 0x0);
> - VCAP_ACT_SET(MASK_MODE, 0x0);
> - VCAP_ACT_SET(POLICE_ENA, 0x0);
> - VCAP_ACT_SET(POLICE_IDX, 0x0);
> + VCAP_ACT_SET(MASK_MODE, 0x1);
> + VCAP_ACT_SET(POLICE_ENA, 0x1);
> + VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
This seems wrong. The policer is used to ensure that traffic are discarded, even
in the case where other users of the code has requested it to go to the CPU.

Are you sure this is working? If it is working, then I fear we have an issue
with the DROP action which uses this to discard frames.

>   VCAP_ACT_SET(CPU_QU_NUM, 0x0);
>   VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
>   break;
> -- 
> 2.7.4

-- 
/Allan

Re: [PATCH net-next 0/7] add support for VSC8584 and VSC8574 Microsemi quad-port PHYs

[Allan W. Nielsen]

Hi Linus,

I'm Allan, working for Microchip, who acquired Microsemi, who acquired Vitesse.

Alexandre pointed me to your comments on the VSC7384 switch done by Vitesse ~15
years ago. BTW: the chip is not being sold any more, and is unlikely to turn up
in new products.

I managed to find the complete datasheet of this device, and convincing people
that it can be "opened-up" meaning that it is available without any NDA or
similar. It is a 1.2mb/200 page pdf, it is not avialable at any web page (that I
know of), but if you are interested in having it then I can send it in a mail to
you.

The 09/20/2018 14:38, Linus Walleij wrote:
> Just as a drive-by comment this seems vaguely related to the Vitesse
> DSA switch I merged in drivers/net/dsa/vitesse-vsc73xx.c
> The VSC* product name handily gives away the origin in Vitesse's
> product line.
> 
> The VSC73xx also have the 8051 CPU and internal RAM, but are
> accessed (typically) over SPI, and AFAICT this thing is talking over
> MDIO.
> 
> The Vitesse 73xx however also supports a WAN port and VLANs
> which makes it significantly different, falling into switch class I
> guess.
That is right, here are the list of feature from page 1 in the datasheet.

Features:
- 12 Gigabit Ethernet ports with nonblocking wire- speed performance
- IEEE802.1Q-in-Q nested VLAN support
- Tri-speed (10/100/1000 Mbps) RGMII interfaces
- Full duplex flow control (IEEE802.3x) and half duplex back pressure
- Support for both wire-speed automatic learning, and CPU-based learning
- Flexible link aggregation compliant with IEEE802.3ad
- 208 kB on-chip frame buffer
- Spanning Tree Protocol support (IEEE802.1D)
- Jumbo frame support
- Multiple Spanning Tree support (IEEE802.1s)
- Programmable classifier for QoS (Layer 4/Multimedia) into four classes of
  service
- Port-based Access Control (IEEE802.1X)
- IGMP, GARP, GMRP, and GVRP support
- 8192 MAC addresses and 4,096 VLAN support (IEEE802.1Q)
- Cost effective 4-pin serial CPU interface
- Per-port shaping, policing, and Broadcast and Multicast Storm Control
- Selection between on-chip 8051 CPU, or off-chip 8-bit or 16-bit CPU for SNMP
  and Web-based management

/Allan

Re: [PATCH net-next ] net: mscc: Add SPDX identifier

[Allan W. Nielsen]

On 17/05/18 21:23, Alexandre Belloni wrote:
> ocelot_qsys.h is missing the SPDX identfier, fix that.
> 
> Signed-off-by: Alexandre Belloni 
> ---
>  drivers/net/ethernet/mscc/ocelot_qsys.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_qsys.h 
> b/drivers/net/ethernet/mscc/ocelot_qsys.h
> index aa7267d5ca77..d8c63aa761be 100644
> --- a/drivers/net/ethernet/mscc/ocelot_qsys.h
> +++ b/drivers/net/ethernet/mscc/ocelot_qsys.h
> @@ -1,7 +1,7 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>  /*
>   * Microsemi Ocelot Switch driver
>   *
> - * License: Dual MIT/GPL
>   * Copyright (c) 2017 Microsemi Corporation
>   */
> 
> --
> 2.17.0
> 

Reviewed-by: Allan W. Nielsen 

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 
> > > > 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:  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 .
> 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:  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 dire

Re: [PATCH ethtool 2/2] Ethtool: Implements PHY Loopback

[Allan W. Nielsen]

On 28/11/16 09:56, Florian Fainelli wrote:
> On 11/28/2016 05:25 AM, Allan W. Nielsen wrote:
> > From: Raju Lakkaraju 
> >
> > Add loopback in ethtool tunables to access PHY drivers.
> >
> > 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
> >
> > Signed-off-by: Raju Lakkaraju 
> > Signed-off-by: Allan W. Nielsen 
> > ---
> 
> > +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.
> 
> This is also known as the local loopback test mode, right?
Yes - Traffic transmitted/generated by the host list loopback to the
host instead of transmitting it on the wire.

> > +
> > +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.
> 
> And this is the remote loopback mode.
Yes - Traffic receiwed on the "wire" is transmitted back on the wire.

> > +
> > +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 and 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.
> 
> OK, this name makes sense to me, but for the two other names, we need to
> use a terminology that is clearer to the reader and/or people familiar
> and targeted at using this feature (e.g: in a lab or during manufacturing).
Sure, we can find better names and/or improve the documentation. But
before jumping to that, then it is properly a good idea to agree on
the overall concept.

We will get back to the naming when we agree on the other parts.

/Allan

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 
> >
> > 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

[PATCH ethtool 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

From: Raju Lakkaraju 

This covers kernel changes upto:

commit ef05e4c22648c141298f51aab00eb22066838b47
Author: Raju Lakkaraju 
Date:   Wed Nov 23 15:20:28 2016 +0530

ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHYtunables

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 and 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 
Signed-off-by: Allan W. Nielsen 
---
 ethtool-copy.h | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..1ebdc72 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -247,6 +247,24 @@ struct ethtool_tunable {
void*data[0];
 };
 
+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
+*/
+   __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
@@ -547,6 +565,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -557,6 +576,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1332,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

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

[Allan W. Nielsen]

From: Raju Lakkaraju 

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

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 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 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables

[Allan W. Nielsen]

From: Raju Lakkaraju 

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 
Signed-off-by: Allan W. Nielsen 
---
 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

[PATCH ethtool 2/2] Ethtool: Implements PHY Loopback

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add loopback in ethtool tunables to access PHY drivers.

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

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 ethtool.8.in |  49 +
 ethtool.c| 139 +++
 2 files changed, 188 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..83e6b97 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,15 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.B4 loopback off near far extn
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ loopback ]
+.HP
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +956,46 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A4 loopback off near far extn
+Specifies whether the type of loopback should be enabled
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B loopback
+PHY supports 3 types of the Loopbacks. i.e near, far and external.
+
+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.
+
+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 and 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.
+
+Gets the PHY Loopback status.
+.TS
+.PD
+.RE
+.TE
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 62f86ef..a4e6c0d 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4522,6 +4522,141 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err = 0, i;
+   u8 lpbk_changed = 0;
+   struct ethtool_tunable tunable;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "loopback")) {
+   lpbk_changed = 1;
+   i += 1;
+   if (i < argc)
+   exit_bad_args();
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (lpbk_changed) {
+   u8 type;
+
+   tunable.cmd = ETHTOOL_PHY_GTUNABLE;
+   tunable.id = ETHTOOL_PHY_LOOPBACK;
+   tunable.type_id = ETHTOOL_TUNABLE_U8;
+   tunable.len = 1;
+   tunable.data[0] = &type;
+   err = send_ioctl(ctx, &tunable);
+   if (err < 0) {
+   perror("Cannot Get PHY Loopback status");
+   return 87;
+   }
+   type = *((u8 *)&tunable.data[0]);
+   switch (type) {
+   case ETHTOOL_PHY_LOOPBACK_NEAR:
+   fprintf(stdout, "Near-end Loopback enabled\n");
+   break;
+   case ETHTOOL_PHY_LOOPBACK_FAR:
+   fprintf(stdout, "Far-end Loopback enabled\n");
+   break;
+   case ETHTOOL_PHY_LOOPBACK_EXTN:
+   fprintf(stdout, "External Loopback enabled\n");
+   break;
+   case ETHTOOL_PHY_LOOPBACK_DISABLE:
+   fprintf(stdout, "All PHY Loopbacks disabled\n");
+   break;
+   default:
+   fprintf(stdout, "Invalid Loopback type\n");
+   break;
+   }
+   }
+
+   return err;
+}
+
+s

[PATCH ethtool 0/2] Adding PHY Loopback tunable

[Allan W. Nielsen]

Hi All,

This patch implements for support for PHY Loopback using PHY-Tunables.

The patch will add the following options:

ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ loopback off|near|far|extn ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ loopback ]

Please review.

Best regards
Allan and Raju

Raju Lakkaraju (2):
  ethtool-copy.h:sync with net
  Ethtool: Implements PHY Loopback

 ethtool-copy.h |  23 +-
 ethtool.8.in   |  49 
 ethtool.c  | 139 +
 3 files changed, 210 insertions(+), 1 deletion(-)

-- 
2.7.3

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

[Allan W. Nielsen]

From: Raju Lakkaraju 

Implements the loopback functionality for MSCC PHYs.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 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_MASK0x0008
 
+#define MSCC_PHY_EXT_PHY_CNTL_2  24
+#define CONNECTOR_LOOPBACK_MASK  0x0001
+
 #define MII_VSC85XX_INT_MASK 25
 #define MII_VSC85XX_INT_MASK_MASK0xa000
 #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 */
+   bre

[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 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

Re: [PATCH net-next v2] ethtool: Protect {get,set}_phy_tunable with PHY device mutex

[Allan W. Nielsen]

On 22/11/16 13:55, Florian Fainelli wrote:
> EXTERNAL EMAIL
> 
> 
> PHY drivers should be able to rely on the caller of {get,set}_tunable to
> have acquired the PHY device mutex, in order to both serialize against
> concurrent calls of these functions, but also against PHY state machine
> changes. All ethtool PHY-level functions do this, except
> {get,set}_tunable, so we make them consistent here as well.
> 
> We need to update the Microsemi PHY driver in the same commit to avoid
> introducing either deadlocks, or lack of proper locking.
> 
> Fixes: 968ad9da7e0e ("ethtool: Implements 
> ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE")
> Fixes: 310d9ad57ae0 ("net: phy: Add downshift get/set support in Microsemi 
> PHYs driver")
> Signed-off-by: Florian Fainelli 
Reviewed-by: Allan W. Nielsen 

[PATCH ethtool v5 1/2] ethtool-copy.h:sync with net-next

[Allan W. Nielsen]

This covers kernel changes upto:

commit 607c7029146790201e90b58c4235ddff0304d6e0
Author: Raju Lakkaraju 
Date:   Thu Nov 17 13:07:22 2016 +0100

ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

For operation in cabling environments that are incompatible with
1000BASE-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BASE-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BASE-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller 

Signed-off-by: Allan W. Nielsen 
---
 ethtool-copy.h | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..3d299e3 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -117,8 +117,7 @@ struct ethtool_cmd {
 static __inline__ void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
 __u32 speed)
 {
-
-   ep->speed = (__u16)speed;
+   ep->speed = (__u16)(speed & 0x);
ep->speed_hi = (__u16)(speed >> 16);
 }
 
@@ -247,6 +246,19 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -547,6 +559,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -557,6 +570,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1326,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH ethtool v5 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ downshift on|off [count N] ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ downshift ]

Ethtool ex:
  ethtool --set-phy-tunable eth0 downshift on
  ethtool --set-phy-tunable eth0 downshift off
  ethtool --set-phy-tunable eth0 downshift on count 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
Acked-by: Florian Fainelli 
Tested-by: Florian Fainelli 
---
 ethtool.8.in |  40 +
 ethtool.c| 144 +++
 2 files changed, 184 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..5c36c06 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,18 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.RB [
+.B downshift
+.A1 on off
+.BN count
+.RB ]
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +959,34 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TS
+nokeep;
+lB l.
+.BI count \ N
+Sets the PHY downshift re-tries count.
+.TE
+.PD
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+For operation in cabling environments that are incompatible with 1000BASE-T,
+PHY device provides an automatic link speed downshift operation.
+Link speed downshift after N failed 1000BASE-T auto-negotiation attempts.
+Downshift is useful where cable does not have the 4 pairs instance.
+
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..7dcd005 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,146 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed = 0;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   if (i < argc)
+   exit_bad_args();
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count = 0;
+
+   ds.cmd = ETHTOOL_PHY_GTUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Get PHY downshift count");
+   return 87;
+   }
+   count = *((u8 *)&ds.data[0]);
+   if (count)
+   fprintf(stdout, "Downshift count: %d\n", count);
+   else
+   fprintf(stdout, "Downshift disabled\n");
+   }
+
+   return err;
+}
+
+static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   if (!strcmp(*(ctx->argp + 1), "on")) {
+   *on = 1;
+   } else if (!strcmp(*(ctx->argp + 1), "off")) {
+   *on = 0;
+   } else {
+   fprintf(stderr, "Invalid boolean\n");
+   exit_bad_args();
+   }
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+   int err = 0;
+   u8 ds_cnt = DOWNSHIFT_DEV_DEFAULT_COUNT;
+   u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 

[PATCH ethtool v5 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

(downshift feature is applied in the net-next tree - d3c19c0a72)

This series adds support for downshift (using phy-tunables).

Downshifting can either be turned on/off, or it can be configured to a
specifc count. "count" is optional.

Change set:
v1:
- Initial version of set/get phy tunable with downshift feature.
v2:
- (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
  to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
  Andrew.
v3:
- Fixed Spelling in "ethtool-copy.h:sync with net" 
- Fixed "if send_ioctl() returns an error, print the error message and then
  still print th value of count".
v4:
- Fixing spelling in the example included in the commit message
- Improve the description in the man-page
v5:
- re-sync ethtool.h from the net-next tree.


Allan W. Nielsen (1):
  ethtool-copy.h:sync with net-next

Raju Lakkaraju (1):
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
downshift

 ethtool-copy.h |  21 +++--
 ethtool.8.in   |  40 
 ethtool.c  | 144 +
 3 files changed, 202 insertions(+), 3 deletions(-)

-- 
2.7.3

Re: [PATCH net-next 4/5] net: phy: bcm7xxx: Add support for downshift/Wirespeed

[Allan W. Nielsen]

Hi,

On 22/11/16 12:07, Florian Fainelli wrote:
> On 11/22/2016 12:02 PM, Andrew Lunn wrote:
> >> +static int bcm7xxx_28nm_set_tunable(struct phy_device *phydev,
> >> +struct ethtool_tunable *tuna,
> >> +const void *data)
> >> +{
> >> +u8 count = *(u8 *)data;
> >> +int ret;
> >> +
> >> +switch (tuna->id) {
> >> +case ETHTOOL_PHY_DOWNSHIFT:
> >> +ret = bcm_phy_downshift_set(phydev, count);
> >> +break;
> >> +default:
> >> +return -EOPNOTSUPP;
> >> +}
> >> +
> >> +if (ret)
> >> +return ret;
> >> +
> >> +/* Disable EEE advertisment since this prevents the PHY
> >> + * from successfully linking up, trigger auto-negotiation restart
> >> + * to let the MAC decide what to do.
> >> + */
> >> +ret = bcm_phy_set_eee(phydev, count == DOWNSHIFT_DEV_DISABLE);
> >> +if (ret)
> >> +return ret;
> >> +
> >> +return genphy_restart_aneg(phydev);
> >> +}
> >
> > Hi Florian
> >
> > Is the locking O.K. here? The core code does not take the phy lock.
> > But i think your shadow register accesses at least need to be
> > protected by the lock?
> 
> There should be some kind of protection, but I was expecting it to be
> done at the caller level, so that when {get,set}_tunable run, they are
> serialized with respect to each other, clearly, by looking at the code,
> this is not the case.
> 
> >
> > Maybe we should think about this locking a bit. It is normal for the
> > lock to be held when using ops in the phy driver structure. The
> > exception is suspend/resume. Maybe we should also take the lock before
> > calling the phydev->drv->get_tunable() and phydev->drv->set_tunable()?
> 
> Yes, that certainly seems like a good approach to me, let me cook a
> patch doing that.

Just for my understanding (such that I will not make the same mistake again)...

Why is it that phy functions such as get_wol needs to take the phy_lock and
others like get_tunable does not.

I do understand the arguments on why the lock should be held by the caller of
get_tunable, but I do not understand why the same argument does not apply for
get_wol.

/Allan

[PATCH ethtool v4 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ downshift on|off [count N] ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ downshift ]

Ethtool ex:
  ethtool --set-phy-tunable eth0 downshift on
  ethtool --set-phy-tunable eth0 downshift off
  ethtool --set-phy-tunable eth0 downshift on count 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
Acked-by: Florian Fainelli 
Tested-by: Florian Fainelli 
---
 ethtool.8.in |  40 +
 ethtool.c| 144 +++
 2 files changed, 184 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..5c36c06 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,18 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.RB [
+.B downshift
+.A1 on off
+.BN count
+.RB ]
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +959,34 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TS
+nokeep;
+lB l.
+.BI count \ N
+Sets the PHY downshift re-tries count.
+.TE
+.PD
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+For operation in cabling environments that are incompatible with 1000BASE-T,
+PHY device provides an automatic link speed downshift operation.
+Link speed downshift after N failed 1000BASE-T auto-negotiation attempts.
+Downshift is useful where cable does not have the 4 pairs instance.
+
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..7dcd005 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,146 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed = 0;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   if (i < argc)
+   exit_bad_args();
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count = 0;
+
+   ds.cmd = ETHTOOL_PHY_GTUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Get PHY downshift count");
+   return 87;
+   }
+   count = *((u8 *)&ds.data[0]);
+   if (count)
+   fprintf(stdout, "Downshift count: %d\n", count);
+   else
+   fprintf(stdout, "Downshift disabled\n");
+   }
+
+   return err;
+}
+
+static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   if (!strcmp(*(ctx->argp + 1), "on")) {
+   *on = 1;
+   } else if (!strcmp(*(ctx->argp + 1), "off")) {
+   *on = 0;
+   } else {
+   fprintf(stderr, "Invalid boolean\n");
+   exit_bad_args();
+   }
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+   int err = 0;
+   u8 ds_cnt = DOWNSHIFT_DEV_DEFAULT_COUNT;
+   u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 

[PATCH ethtool v4 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

From: Raju Lakkaraju 

This covers kernel changes upto:

commit f5a4732f85613b3fb43f8bc33a017e3db3b3605a
Author: Raju Lakkaraju 
Date:   Wed Nov 9 16:33:09 2016 +0530

ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

For operation in cabling environments that are incompatible with
1000BASE-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BASE-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BASE-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 

Signed-off-by: Allan W. Nielsen 
Acked-by: Florian Fainelli 
---
 ethtool-copy.h | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..2e2448f 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -247,6 +247,19 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -547,6 +560,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -557,6 +571,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1327,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH ethtool v4 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

(downshift feature is applied in the net-next tree - d3c19c0a72)

This series adds support for downshift (using phy-tunables).

Downshifting can either be turned on/off, or it can be configured to a
specifc count.

"count" is optional.

Change set:
v1:
- Initial version of set/get phy tunable with downshift feature.
v2:
- (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
  to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
  Andrew.
v3:
- Fixed Spelling in "ethtool-copy.h:sync with net" 
- Fixed "if send_ioctl() returns an error, print the error message and then
  still print th value of count".
v4:
- Fixing spelling in the example included in the commit message
- Improve the description in the man-page

Raju Lakkaraju (2):
  ethtool-copy.h:sync with net
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
downshift

 ethtool-copy.h |  18 +++-
 ethtool.8.in   |  40 
 ethtool.c  | 144 +
 3 files changed, 201 insertions(+), 1 deletion(-)

-- 
2.7.3

[PATCH net-next v3 1/5] ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Defines a generic API to get/set phy tunables. The API is using the
existing ethtool_tunable/tunable_type_id types which is already being used
for mac level tunables.

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 include/uapi/linux/ethtool.h | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 8e54723..42f696f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,6 +248,16 @@ struct ethtool_tunable {
void*data[0];
 };
 
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -548,6 +558,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -558,6 +569,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1313,7 +1325,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH ethtool v3 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ downshift on|off [count N] ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ downshift ]

Ethtool ex:
  ethtool --set-phy-tuanble eth0 downshift on
  ethtool --set-phy-tuanble eth0 downshift off
  ethtool --set-phy-tuanble eth0 downshift on count 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 ethtool.8.in |  39 
 ethtool.c| 144 +++
 2 files changed, 183 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..337d0cf 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,18 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.RB [
+.B downshift
+.A1 on off
+.BN count
+.RB ]
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +959,33 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TS
+nokeep;
+lB l.
+.BI count \ N
+Sets the PHY downshift re-tries count.
+.TE
+.PD
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+For operation in cabling environments that are incompatible with 1000BASE-T,
+PHY device provides an automatic link speed downshift operation.
+Link speed downshift after N failed 1000BASE-T auto-negotiation attempts.
+
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..7dcd005 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,146 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed = 0;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   if (i < argc)
+   exit_bad_args();
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count = 0;
+
+   ds.cmd = ETHTOOL_PHY_GTUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Get PHY downshift count");
+   return 87;
+   }
+   count = *((u8 *)&ds.data[0]);
+   if (count)
+   fprintf(stdout, "Downshift count: %d\n", count);
+   else
+   fprintf(stdout, "Downshift disabled\n");
+   }
+
+   return err;
+}
+
+static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   if (!strcmp(*(ctx->argp + 1), "on")) {
+   *on = 1;
+   } else if (!strcmp(*(ctx->argp + 1), "off")) {
+   *on = 0;
+   } else {
+   fprintf(stderr, "Invalid boolean\n");
+   exit_bad_args();
+   }
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+   int err = 0;
+   u8 ds_cnt = DOWNSHIFT_DEV_DEFAULT_COUNT;
+   u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 0;
+
+   if (ctx->argc == 0)
+   exit_bad_args();
+
+   /* Parse arguments */
+   while (ctx->ar

[PATCH net-next v3 4/5] ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable

[Allan W. Nielsen]

From: Raju Lakkaraju 

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

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 net/core/ethtool.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 61aebdf..e9b45567 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -122,6 +122,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = 
{
 static const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
[ETHTOOL_ID_UNSPEC] = "Unspec",
+   [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift",
 };
 
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2435,6 +2436,11 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
 static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 {
switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   if (tuna->len != sizeof(u8) ||
+   tuna->type_id != ETHTOOL_TUNABLE_U8)
+   return -EINVAL;
+   break;
default:
return -EINVAL;
}
-- 
2.7.3

[PATCH net-next v3 2/5] ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Adding get_tunable/set_tunable function pointer to the phy_driver
structure, and uses these function pointers to implement the
ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE ioctls.

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 include/linux/phy.h |  7 +
 net/core/ethtool.c  | 87 +
 2 files changed, 94 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 9880d73..3d35c36 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -611,6 +611,13 @@ struct phy_driver {
void (*get_strings)(struct phy_device *dev, u8 *data);
void (*get_stats)(struct phy_device *dev,
  struct ethtool_stats *stats, u64 *data);
+
+   /* Get and Set PHY tunables */
+   int (*get_tunable)(struct phy_device *dev,
+  struct ethtool_tunable *tuna, void *data);
+   int (*set_tunable)(struct phy_device *dev,
+   struct ethtool_tunable *tuna,
+   const void *data);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),
\
  struct phy_driver, mdiodrv)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..61aebdf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -119,6 +119,11 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] 
= {
[ETHTOOL_TX_COPYBREAK]  = "tx-copybreak",
 };
 
+static const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+   [ETHTOOL_ID_UNSPEC] = "Unspec",
+};
+
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
 {
struct ethtool_gfeatures cmd = {
@@ -227,6 +232,9 @@ static int __ethtool_get_sset_count(struct net_device *dev, 
int sset)
if (sset == ETH_SS_TUNABLES)
return ARRAY_SIZE(tunable_strings);
 
+   if (sset == ETH_SS_PHY_TUNABLES)
+   return ARRAY_SIZE(phy_tunable_strings);
+
if (sset == ETH_SS_PHY_STATS) {
if (dev->phydev)
return phy_get_sset_count(dev->phydev);
@@ -253,6 +261,8 @@ static void __ethtool_get_strings(struct net_device *dev,
   sizeof(rss_hash_func_strings));
else if (stringset == ETH_SS_TUNABLES)
memcpy(data, tunable_strings, sizeof(tunable_strings));
+   else if (stringset == ETH_SS_PHY_TUNABLES)
+   memcpy(data, phy_tunable_strings, sizeof(phy_tunable_strings));
else if (stringset == ETH_SS_PHY_STATS) {
struct phy_device *phydev = dev->phydev;
 
@@ -2422,6 +2432,76 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
};
 }
 
+static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
+{
+   switch (tuna->id) {
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int get_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->get_tunable))
+   return -EOPNOTSUPP;
+
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   ret = phydev->drv->get_tunable(phydev, &tuna, data);
+   if (ret)
+   goto out;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_to_user(useraddr, data, tuna.len))
+   goto out;
+   ret = 0;
+
+out:
+   kfree(data);
+   return ret;
+}
+
+static int set_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->set_tunable))
+   return -EOPNOTSUPP;
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_from_user(data, useraddr, tuna.len))
+   goto out;
+   ret = phydev->drv->set_tunable(phydev, &tuna, data);
+
+out:
+   kfree(data);
+   return ret;
+}
+
 /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
 
 int dev_ethtool(struct net *net, s

[PATCH net-next v3 5/5] net: phy: Add downshift get/set support in Microsemi PHYs driver

[Allan W. Nielsen]

From: Raju Lakkaraju 

Implements the phy tunable function pointers and implement downshift
functionality for MSCC PHYs.

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 drivers/net/phy/mscc.c | 100 +
 1 file changed, 100 insertions(+)

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index d0026ab..92018ba 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -46,8 +46,15 @@ enum rgmii_rx_clock_delay {
 
 #define MSCC_EXT_PAGE_ACCESS 31
 #define MSCC_PHY_PAGE_STANDARD   0x /* Standard registers */
+#define MSCC_PHY_PAGE_EXTENDED   0x0001 /* Extended registers */
 #define MSCC_PHY_PAGE_EXTENDED_2 0x0002 /* Extended reg - page 2 */
 
+/* Extended Page 1 Registers */
+#define MSCC_PHY_ACTIPHY_CNTL20
+#define DOWNSHIFT_CNTL_MASK  0x001C
+#define DOWNSHIFT_EN 0x0010
+#define DOWNSHIFT_CNTL_POS   2
+
 /* Extended Page 2 Registers */
 #define MSCC_PHY_RGMII_CNTL  20
 #define RGMII_RX_CLK_DELAY_MASK  0x0070
@@ -75,6 +82,8 @@ enum rgmii_rx_clock_delay {
 #define MSCC_VDDMAC_2500 2500
 #define MSCC_VDDMAC_3300 3300
 
+#define DOWNSHIFT_COUNT_MAX  5
+
 struct vsc8531_private {
int rate_magic;
 };
@@ -101,6 +110,66 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, 
u8 page)
return rc;
 }
 
+static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
+{
+   int rc;
+   u16 reg_val;
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= DOWNSHIFT_CNTL_MASK;
+   if (!(reg_val & DOWNSHIFT_EN))
+   *count = DOWNSHIFT_DEV_DISABLE;
+   else
+   *count = ((reg_val & ~DOWNSHIFT_EN) >> DOWNSHIFT_CNTL_POS) + 2;
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
+static int vsc85xx_downshift_set(struct phy_device *phydev, u8 count)
+{
+   int rc;
+   u16 reg_val;
+
+   if (count == DOWNSHIFT_DEV_DEFAULT_COUNT) {
+   /* Default downshift count 3 (i.e. Bit3:2 = 0b01) */
+   count = ((1 << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   } else if (count > DOWNSHIFT_COUNT_MAX || count == 1) {
+   phydev_err(phydev, "Downshift count should be 2,3,4 or 5\n");
+   return -ERANGE;
+   } else if (count) {
+   /* Downshift count is either 2,3,4 or 5 */
+   count = (((count - 2) << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   }
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= ~(DOWNSHIFT_CNTL_MASK);
+   reg_val |= count;
+   rc = phy_write(phydev, MSCC_PHY_ACTIPHY_CNTL, reg_val);
+   if (rc != 0)
+   goto out_unlock;
+
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
 static int vsc85xx_wol_set(struct phy_device *phydev,
   struct ethtool_wolinfo *wol)
 {
@@ -329,6 +398,29 @@ static int vsc85xx_default_config(struct phy_device 
*phydev)
return rc;
 }
 
+static int vsc85xx_get_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna, void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_get(phydev, (u8 *)data);
+   default:
+   return -EINVAL;
+   }
+}
+
+static int vsc85xx_set_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna,
+  const void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_set(phydev, *(u8 *)data);
+   default:
+   return -EINVAL;
+   }
+}
+
 static int vsc85xx_config_init(struct phy_device *phydev)
 {
int rc;
@@ -418,6 +510,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &vsc85xx_wol_set,
.get_wol= &vsc85xx_wol_get,
+   .get_tunable= &vsc85xx_get_tunable,
+   .set_tunable= &vsc85xx_set_tunable,
 },
 {
.phy_id = PHY_ID_VSC8531,
@@ -437,6 +531,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &am

[PATCH net-next v3 3/5] ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

[Allan W. Nielsen]

From: Raju Lakkaraju 

For operation in cabling environments that are incompatible with
1000BASE-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BASE-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BASE-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
Reviewed-by: Andrew Lunn 
---
 include/uapi/linux/ethtool.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 42f696f..f0db778 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,9 +248,12 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
 enum phy_tunable_id {
ETHTOOL_PHY_ID_UNSPEC,
-
+   ETHTOOL_PHY_DOWNSHIFT,
/*
 * Add your fresh new phy tunable attribute above and remember to update
 * phy_tunable_strings[] in net/core/ethtool.c
-- 
2.7.3

[PATCH ethtool v3 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

From: Raju Lakkaraju 

This covers kernel changes upto:

commit f5a4732f85613b3fb43f8bc33a017e3db3b3605a
Author: Raju Lakkaraju 
Date:   Wed Nov 9 16:33:09 2016 +0530

ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

For operation in cabling environments that are incompatible with
1000BASE-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BASE-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BASE-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 ethtool-copy.h | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..2e2448f 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -247,6 +247,19 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -547,6 +560,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -557,6 +571,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1327,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH net-next v3 0/5] Adding PHY-Tunables and downshift support

[Allan W. Nielsen]

Hi All,

(This is a re-post of the v3 patch set with a new cover letter - I was not
aware that the cover letters was used a commit comments in merge commits).

This series add support for PHY tunables, and uses this facility to
configure downshifting. The downshifting mechanism is implemented for MSCC
phys.

This series tries to address the comments provided back in mid October when
this feature was posted along with fast-link-failure. Fast-link-failure has
been separated out, but we would like to pick continue on that if/when we
agree on how the phy-tunables and downshifting should be done.

The proposed generic interface is similar to
ETHTOOL_GTUNABLE/ETHTOOL_STUNABLE, it uses the same type
(ethtool_tunable/tunable_type_id) but a new enum (phy_tunable_id) is added
to reflect the PHY tunable.

The implementation just call the newly added function pointers in
get_tunable/set_tunable phy_device structure.

To configure downshifting, the ethtool_tunable structure is used. 'id' must
be set to 'ETHTOOL_PHY_DOWNSHIFT', 'type_id' must be set to
'ETHTOOL_TUNABLE_U8' and 'data' value configure the amount of downshift
re-tries.

If configured to DOWNSHIFT_DEV_DISABLE, then downshift is disabled If
configured to DOWNSHIFT_DEV_DEFAULT_COUNT, then it is up to the device to
choose a device-specific re-try count.

Tested on Beaglebone Black with VSC 8531 PHY.

Change set:
v0:

- Link Speed downshift and Fast Link failure-2 features coded by using
  Device tree.
v1: 
- Split the Downshift and FLF2 features in different set of patches.
- Removed DT access and implemented IOCTL access suggested by Andrew.
- Added function pointers in get_tunable/set_tunable phy_device structure
v2:
- Added trace message with a hist is printed when downshifting clould not
  be eanbled with the requested count
- (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
  to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
  Andrew.
v3:
- Fixed Spelling in "net: phy: Add downshift get/set support in Microsemi
  PHYs driver"

Raju Lakkaraju (5):
  ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE
  ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE
  ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables
  ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable
  net: phy: Add downshift get/set support in Microsemi PHYs driver

 drivers/net/phy/mscc.c   | 100 +++
 include/linux/phy.h  |   7 +++
 include/uapi/linux/ethtool.h |  18 +++-
 net/core/ethtool.c   |  93 
 4 files changed, 217 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH ethtool v3 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

Hi All,

(This is a re-post of the v3 patch set with a new cover letter - I was not
aware that the cover letters was used as commit comments in merge commits).

This patch implements for set/get downshifting.

Downshifting can either be turned on/off, or it can be configured to a
specifc count.

"count" is optional.

Tested on Beaglebone Black with VSC 8531 PHY.

Change set:
v1:
- Initial version of set/get phy tunable with downshift feature.
v2:
- (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
  to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
  Andrew.
v3:
- Fixed Spelling in "ethtool-copy.h:sync with net" 
- Fixed "if send_ioctl() returns an error, print the error message and then
  still print th value of count".

Raju Lakkaraju (2):
  ethtool-copy.h:sync with net
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
downshift

 ethtool-copy.h |  18 +++-
 ethtool.8.in   |  39 
 ethtool.c  | 144 +
 3 files changed, 200 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH net-next v3 2/5] ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Adding get_tunable/set_tunable function pointer to the phy_driver
structure, and uses these function pointers to implement the
ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE ioctls.

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 include/linux/phy.h |  7 +
 net/core/ethtool.c  | 87 +
 2 files changed, 94 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 9880d73..3d35c36 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -611,6 +611,13 @@ struct phy_driver {
void (*get_strings)(struct phy_device *dev, u8 *data);
void (*get_stats)(struct phy_device *dev,
  struct ethtool_stats *stats, u64 *data);
+
+   /* Get and Set PHY tunables */
+   int (*get_tunable)(struct phy_device *dev,
+  struct ethtool_tunable *tuna, void *data);
+   int (*set_tunable)(struct phy_device *dev,
+   struct ethtool_tunable *tuna,
+   const void *data);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),
\
  struct phy_driver, mdiodrv)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..61aebdf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -119,6 +119,11 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] 
= {
[ETHTOOL_TX_COPYBREAK]  = "tx-copybreak",
 };
 
+static const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+   [ETHTOOL_ID_UNSPEC] = "Unspec",
+};
+
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
 {
struct ethtool_gfeatures cmd = {
@@ -227,6 +232,9 @@ static int __ethtool_get_sset_count(struct net_device *dev, 
int sset)
if (sset == ETH_SS_TUNABLES)
return ARRAY_SIZE(tunable_strings);
 
+   if (sset == ETH_SS_PHY_TUNABLES)
+   return ARRAY_SIZE(phy_tunable_strings);
+
if (sset == ETH_SS_PHY_STATS) {
if (dev->phydev)
return phy_get_sset_count(dev->phydev);
@@ -253,6 +261,8 @@ static void __ethtool_get_strings(struct net_device *dev,
   sizeof(rss_hash_func_strings));
else if (stringset == ETH_SS_TUNABLES)
memcpy(data, tunable_strings, sizeof(tunable_strings));
+   else if (stringset == ETH_SS_PHY_TUNABLES)
+   memcpy(data, phy_tunable_strings, sizeof(phy_tunable_strings));
else if (stringset == ETH_SS_PHY_STATS) {
struct phy_device *phydev = dev->phydev;
 
@@ -2422,6 +2432,76 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
};
 }
 
+static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
+{
+   switch (tuna->id) {
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int get_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->get_tunable))
+   return -EOPNOTSUPP;
+
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   ret = phydev->drv->get_tunable(phydev, &tuna, data);
+   if (ret)
+   goto out;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_to_user(useraddr, data, tuna.len))
+   goto out;
+   ret = 0;
+
+out:
+   kfree(data);
+   return ret;
+}
+
+static int set_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->set_tunable))
+   return -EOPNOTSUPP;
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_from_user(data, useraddr, tuna.len))
+   goto out;
+   ret = phydev->drv->set_tunable(phydev, &tuna, data);
+
+out:
+   kfree(data);
+   return ret;
+}
+
 /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
 
 int dev_ethtool(struct net *net, s

[PATCH ethtool v3 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ downshift on|off [count N] ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ downshift ]

Ethtool ex:
  ethtool --set-phy-tuanble eth0 downshift on
  ethtool --set-phy-tuanble eth0 downshift off
  ethtool --set-phy-tuanble eth0 downshift on count 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 ethtool.8.in |  39 
 ethtool.c| 144 +++
 2 files changed, 183 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..337d0cf 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,18 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.RB [
+.B downshift
+.A1 on off
+.BN count
+.RB ]
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +959,33 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TS
+nokeep;
+lB l.
+.BI count \ N
+Sets the PHY downshift re-tries count.
+.TE
+.PD
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+For operation in cabling environments that are incompatible with 1000BASE-T,
+PHY device provides an automatic link speed downshift operation.
+Link speed downshift after N failed 1000BASE-T auto-negotiation attempts.
+
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..7dcd005 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,146 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed = 0;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   if (i < argc)
+   exit_bad_args();
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count = 0;
+
+   ds.cmd = ETHTOOL_PHY_GTUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Get PHY downshift count");
+   return 87;
+   }
+   count = *((u8 *)&ds.data[0]);
+   if (count)
+   fprintf(stdout, "Downshift count: %d\n", count);
+   else
+   fprintf(stdout, "Downshift disabled\n");
+   }
+
+   return err;
+}
+
+static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   if (!strcmp(*(ctx->argp + 1), "on")) {
+   *on = 1;
+   } else if (!strcmp(*(ctx->argp + 1), "off")) {
+   *on = 0;
+   } else {
+   fprintf(stderr, "Invalid boolean\n");
+   exit_bad_args();
+   }
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+   int err = 0;
+   u8 ds_cnt = DOWNSHIFT_DEV_DEFAULT_COUNT;
+   u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 0;
+
+   if (ctx->argc == 0)
+   exit_bad_args();
+
+   /* Parse arguments */
+   while (ctx->ar

[PATCH ethtool v3 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

From: Raju Lakkaraju 

This covers kernel changes upto:

commit f5a4732f85613b3fb43f8bc33a017e3db3b3605a
Author: Raju Lakkaraju 
Date:   Wed Nov 9 16:33:09 2016 +0530

ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

For operation in cabling environments that are incompatible with
1000BASE-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BASE-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BASE-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 ethtool-copy.h | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..2e2448f 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -247,6 +247,19 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -547,6 +560,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -557,6 +571,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1327,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH ethtool v3 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

Hi All,

This is a follow-up on the v2 patch series.

Following is updated:

- Spelling
- consistent spaces
- error handling of send_ioctl()

Old cover letters included below.

Please review.

Best regards
Allan and Raju


Raju Lakkaraju (2):
  ethtool-copy.h:sync with net
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
downshift

 ethtool-copy.h |  18 +++-
 ethtool.8.in   |  39 
 ethtool.c  | 144 +
 3 files changed, 200 insertions(+), 1 deletion(-)

> From c235004654ee0212b6cf6ca7af14eb6ef8a04dee Mon Sep 17 00:00:00 2001
> From: "Allan W. Nielsen" 
> Date: Mon, 14 Nov 2016 10:22:03 +0100
> Subject: [PATCH ethtool v2 0/2] Adding downshift support to ethtool
> 
> This is a follow-up on the patch series posted at Fri, 4 Nov 2016 11:27:31 
> +0100
> (old cover letter included below).
> 
> The following is changed/added/addressed:
> - Syntax is changed from "--set-phy-tunable downshift on|off|%d" to
>   "--set-phy-tunable [downshift on|off [count N]]" - as requested by Andrew.
> - Short description of downshifting is added to the man page.
>
> Old cover letter:
> > From 5d926ca56f13e283aaa98e820d4720305be4da8f Mon Sep 17 00:00:00 2001
> > From: "Allan W. Nielsen" 
> > Date: Fri, 4 Nov 2016 11:27:31 +0100
> > Subject: [PATCH ethtool 0/2] Adding downshift support to ethtool
> > 
> > Hi All,
> > 
> > This patch implements for set/get downshifting.
> > 
> > Downshifting can either be turned on/off, or it can be configured to a
> > specifc count.
> > 
> > If no "count" are provided, then it is up to the individual PHY driver to
> > configure a default count.

-- 
2.7.3

[PATCH net-next v3 5/5] net: phy: Add downshift get/set support in Microsemi PHYs driver

[Allan W. Nielsen]

From: Raju Lakkaraju 

Implements the phy tunable function pointers and implement downshift
functionality for MSCC PHYs.

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 drivers/net/phy/mscc.c | 100 +
 1 file changed, 100 insertions(+)

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index d0026ab..92018ba 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -46,8 +46,15 @@ enum rgmii_rx_clock_delay {
 
 #define MSCC_EXT_PAGE_ACCESS 31
 #define MSCC_PHY_PAGE_STANDARD   0x /* Standard registers */
+#define MSCC_PHY_PAGE_EXTENDED   0x0001 /* Extended registers */
 #define MSCC_PHY_PAGE_EXTENDED_2 0x0002 /* Extended reg - page 2 */
 
+/* Extended Page 1 Registers */
+#define MSCC_PHY_ACTIPHY_CNTL20
+#define DOWNSHIFT_CNTL_MASK  0x001C
+#define DOWNSHIFT_EN 0x0010
+#define DOWNSHIFT_CNTL_POS   2
+
 /* Extended Page 2 Registers */
 #define MSCC_PHY_RGMII_CNTL  20
 #define RGMII_RX_CLK_DELAY_MASK  0x0070
@@ -75,6 +82,8 @@ enum rgmii_rx_clock_delay {
 #define MSCC_VDDMAC_2500 2500
 #define MSCC_VDDMAC_3300 3300
 
+#define DOWNSHIFT_COUNT_MAX  5
+
 struct vsc8531_private {
int rate_magic;
 };
@@ -101,6 +110,66 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, 
u8 page)
return rc;
 }
 
+static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
+{
+   int rc;
+   u16 reg_val;
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= DOWNSHIFT_CNTL_MASK;
+   if (!(reg_val & DOWNSHIFT_EN))
+   *count = DOWNSHIFT_DEV_DISABLE;
+   else
+   *count = ((reg_val & ~DOWNSHIFT_EN) >> DOWNSHIFT_CNTL_POS) + 2;
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
+static int vsc85xx_downshift_set(struct phy_device *phydev, u8 count)
+{
+   int rc;
+   u16 reg_val;
+
+   if (count == DOWNSHIFT_DEV_DEFAULT_COUNT) {
+   /* Default downshift count 3 (i.e. Bit3:2 = 0b01) */
+   count = ((1 << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   } else if (count > DOWNSHIFT_COUNT_MAX || count == 1) {
+   phydev_err(phydev, "Downshift count should be 2,3,4 or 5\n");
+   return -ERANGE;
+   } else if (count) {
+   /* Downshift count is either 2,3,4 or 5 */
+   count = (((count - 2) << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   }
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= ~(DOWNSHIFT_CNTL_MASK);
+   reg_val |= count;
+   rc = phy_write(phydev, MSCC_PHY_ACTIPHY_CNTL, reg_val);
+   if (rc != 0)
+   goto out_unlock;
+
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
 static int vsc85xx_wol_set(struct phy_device *phydev,
   struct ethtool_wolinfo *wol)
 {
@@ -329,6 +398,29 @@ static int vsc85xx_default_config(struct phy_device 
*phydev)
return rc;
 }
 
+static int vsc85xx_get_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna, void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_get(phydev, (u8 *)data);
+   default:
+   return -EINVAL;
+   }
+}
+
+static int vsc85xx_set_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna,
+  const void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_set(phydev, *(u8 *)data);
+   default:
+   return -EINVAL;
+   }
+}
+
 static int vsc85xx_config_init(struct phy_device *phydev)
 {
int rc;
@@ -418,6 +510,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &vsc85xx_wol_set,
.get_wol= &vsc85xx_wol_get,
+   .get_tunable= &vsc85xx_get_tunable,
+   .set_tunable= &vsc85xx_set_tunable,
 },
 {
.phy_id = PHY_ID_VSC8531,
@@ -437,6 +531,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &am

[PATCH net-next v3 3/5] ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

[Allan W. Nielsen]

From: Raju Lakkaraju 

For operation in cabling environments that are incompatible with
1000BASE-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BASE-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BASE-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 include/uapi/linux/ethtool.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 42f696f..f0db778 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,9 +248,12 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
 enum phy_tunable_id {
ETHTOOL_PHY_ID_UNSPEC,
-
+   ETHTOOL_PHY_DOWNSHIFT,
/*
 * Add your fresh new phy tunable attribute above and remember to update
 * phy_tunable_strings[] in net/core/ethtool.c
-- 
2.7.3

[PATCH net-next v3 1/5] ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Defines a generic API to get/set phy tunables. The API is using the
existing ethtool_tunable/tunable_type_id types which is already being used
for mac level tunables.

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 include/uapi/linux/ethtool.h | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 8e54723..42f696f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,6 +248,16 @@ struct ethtool_tunable {
void*data[0];
 };
 
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -548,6 +558,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -558,6 +569,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1313,7 +1325,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH net-next v3 0/5] Adding PHY-Tunables and downshift support

[Allan W. Nielsen]

Hi All,

This follow fixes the comments provided on the v2 series.

Following is the list of changes.
- Spelling in "net: phy: Add downshift get/set support in Microsemi PHYs driver"

Ethtool patches will follow shortly.

Old cover letters included below.

Please review.

Best regards
Allan and Raju


Raju Lakkaraju (5):
  ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE
  ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE
  ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables
  ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable
  net: phy: Add downshift get/set support in Microsemi PHYs driver

 drivers/net/phy/mscc.c   | 100 +++
 include/linux/phy.h  |   7 +++
 include/uapi/linux/ethtool.h |  18 +++-
 net/core/ethtool.c   |  93 
 4 files changed, 217 insertions(+), 1 deletion(-)

> From 5ca4dac36f45e097b93ae4cfdf35a8922e5d975e Mon Sep 17 00:00:00 2001
> prom: "Allan W. Nielsen" 
> Date: Mon, 14 Nov 2016 10:04:07 +0100
> Subject: [PATCH net-next v2 0/5] Adding PHY-Tunables and downshift support
> 
> Hi,
> 
> This is a follow-up on the patch series posted at Fri, 4 Nov 2016 10:55:25
> +0100 (old cover letter included below).
> 
> The following is changed/added/addressed:
> - Support for __ethtool_get_strings()/phy_tunables is added
> - Using DOWNSHIFT_DEV_DISABLE define instead of '0' when disabling
> - If downshifting clould not be eanbled with the requested count, then a
>   trace message with a hist is printed.
> - Using ERANGE as requested by Andrew.
> - Andrew suggested to use ENOSUPP when a tunable is not implemented We kept
>   the EINVAL because ENOSUPP does not exists, and the existing MAC-level
>   tunable is also using EINVAL in the same scenario.
> - Andrew suggested that the driver should accept a count of 1 and "just"
>   round it up to 2 (the nearest allowed value). We discussed it a bit (Raju
>   and I) and preferred to return EINVAL when a configuration value could
>   not be accommodated. People which wants "help" to choose a supported
>   value should be using the DOWNSHIFT_DEV_DEFAULT_COUNT value.
> - (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
>   to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
>   Andrew.
> 
> Ethtool patches will follow shortly.
>
> Old cover letter:
> > From 1a318266822f5d74c58b5219ebbdf5a6a5f567dc Mon Sep 17 00:00:00 2001
> > From: "Allan W. Nielsen" 
> > Date: Fri, 4 Nov 2016 10:55:25 +0100
> > Subject: [PATCH net-next 0/5] Adding PHY-Tunables and downshift support
> > 
> > Hi All,
> > 
> > This series add support for PHY tunables, and uses this facility to 
> > configure
> > downshifting. The downshifting mechanism is implemented for MSCC phys.
> > 
> > This series tries to address the comments provided back in mid October when 
> > this
> > feature was posted along with fast-link-failure. Fast-link-failure has been
> > separated out, but we would like to pick continue on that if/when we agree 
> > on
> > how the phy-tunables and downshifting should be done.
> > 
> > The proposed generic interface is similar to 
> > ETHTOOL_GTUNABLE/ETHTOOL_STUNABLE,
> > it uses the same type (ethtool_tunable/tunable_type_id) but a new enum
> > (phy_tunable_id) is added to reflect the PHY tunable.
> > 
> > The implementation just call the newly added function pointers in
> > get_tunable/set_tunable phy_device structure.
> > 
> > To configure downshifting, the ethtool_tunable structure is used. 'id' must 
> > be
> > set to 'ETHTOOL_PHY_DOWNSHIFT', 'type_id' must be set to 
> > 'ETHTOOL_TUNABLE_U8'
> > and 'data' value configure the amount of downshift re-tries.
> > 
> > If configured to DOWNSHIFT_DEV_DISABLE, then downshift is disabled
> > If configured to DOWNSHIFT_DEV_DEFAULT_COUNT, then it is up to the device to
> > choose a device-specific re-try count.
> > 
> > Patches to implement this in ethtool will follow in a few minutes.

-- 
2.7.3

[PATCH net-next v3 4/5] ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable

[Allan W. Nielsen]

From: Raju Lakkaraju 

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

Signed-off-by: Raju Lakkaraju 
Reviewed-by: Andrew Lunn 
Signed-off-by: Allan W. Nielsen 
---
 net/core/ethtool.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 61aebdf..e9b45567 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -122,6 +122,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = 
{
 static const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
[ETHTOOL_ID_UNSPEC] = "Unspec",
+   [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift",
 };
 
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2435,6 +2436,11 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
 static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 {
switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   if (tuna->len != sizeof(u8) ||
+   tuna->type_id != ETHTOOL_TUNABLE_U8)
+   return -EINVAL;
+   break;
default:
return -EINVAL;
}
-- 
2.7.3

[PATCH net-next v2 4/5] ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable

[Allan W. Nielsen]

From: Raju Lakkaraju 

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

Signed-off-by: Raju Lakkaraju 
---
 net/core/ethtool.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 61aebdf..e9b45567 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -122,6 +122,7 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] = 
{
 static const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
[ETHTOOL_ID_UNSPEC] = "Unspec",
+   [ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift",
 };
 
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2435,6 +2436,11 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
 static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 {
switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   if (tuna->len != sizeof(u8) ||
+   tuna->type_id != ETHTOOL_TUNABLE_U8)
+   return -EINVAL;
+   break;
default:
return -EINVAL;
}
-- 
2.7.3

[PATCH ethtool v2 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

From: Raju Lakkaraju 

This covers kernel changes upto:

commit f5a4732f85613b3fb43f8bc33a017e3db3b3605a
Author: Raju Lakkaraju 
Date:   Wed Nov 9 16:33:09 2016 +0530

ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE

For operation in cabling environments that are incompatible with 1000BAST-T,
PHY device may provide an automatic link speed downshift operation. When
enabled, the device automatically changes its 1000BAST-T auto-negotiation
to the next slower speed after a configured number of failed attempts at
1000BAST-T.
This feature is useful in setting up in networks using older cable
installations that include only pairs A and B, and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
---
 ethtool-copy.h | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..2e2448f 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -247,6 +247,19 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -547,6 +560,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -557,6 +571,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1312,7 +1327,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH net-next v2 2/5] ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Adding get_tunable/set_tunable function pointer to the phy_driver
structure, and uses these function pointers to implement the
ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE ioctls.

Signed-off-by: Raju Lakkaraju 
---
 include/linux/phy.h |  7 +
 net/core/ethtool.c  | 87 +
 2 files changed, 94 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 9880d73..3d35c36 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -611,6 +611,13 @@ struct phy_driver {
void (*get_strings)(struct phy_device *dev, u8 *data);
void (*get_stats)(struct phy_device *dev,
  struct ethtool_stats *stats, u64 *data);
+
+   /* Get and Set PHY tunables */
+   int (*get_tunable)(struct phy_device *dev,
+  struct ethtool_tunable *tuna, void *data);
+   int (*set_tunable)(struct phy_device *dev,
+   struct ethtool_tunable *tuna,
+   const void *data);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),
\
  struct phy_driver, mdiodrv)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..61aebdf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -119,6 +119,11 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN] 
= {
[ETHTOOL_TX_COPYBREAK]  = "tx-copybreak",
 };
 
+static const char
+phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
+   [ETHTOOL_ID_UNSPEC] = "Unspec",
+};
+
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
 {
struct ethtool_gfeatures cmd = {
@@ -227,6 +232,9 @@ static int __ethtool_get_sset_count(struct net_device *dev, 
int sset)
if (sset == ETH_SS_TUNABLES)
return ARRAY_SIZE(tunable_strings);
 
+   if (sset == ETH_SS_PHY_TUNABLES)
+   return ARRAY_SIZE(phy_tunable_strings);
+
if (sset == ETH_SS_PHY_STATS) {
if (dev->phydev)
return phy_get_sset_count(dev->phydev);
@@ -253,6 +261,8 @@ static void __ethtool_get_strings(struct net_device *dev,
   sizeof(rss_hash_func_strings));
else if (stringset == ETH_SS_TUNABLES)
memcpy(data, tunable_strings, sizeof(tunable_strings));
+   else if (stringset == ETH_SS_PHY_TUNABLES)
+   memcpy(data, phy_tunable_strings, sizeof(phy_tunable_strings));
else if (stringset == ETH_SS_PHY_STATS) {
struct phy_device *phydev = dev->phydev;
 
@@ -2422,6 +2432,76 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
};
 }
 
+static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
+{
+   switch (tuna->id) {
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int get_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->get_tunable))
+   return -EOPNOTSUPP;
+
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   ret = phydev->drv->get_tunable(phydev, &tuna, data);
+   if (ret)
+   goto out;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_to_user(useraddr, data, tuna.len))
+   goto out;
+   ret = 0;
+
+out:
+   kfree(data);
+   return ret;
+}
+
+static int set_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->set_tunable))
+   return -EOPNOTSUPP;
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_from_user(data, useraddr, tuna.len))
+   goto out;
+   ret = phydev->drv->set_tunable(phydev, &tuna, data);
+
+out:
+   kfree(data);
+   return ret;
+}
+
 /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -2479,6 +2559,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_GET_TS_INFO:
case ETHTOOL_GEEE:
case ETHTOOL_GTUNABLE:
+

[PATCH net-next v2 0/5] Adding PHY-Tunables and downshift support

[Allan W. Nielsen]

Hi,

This is a follow-up on the patch series posted at Fri, 4 Nov 2016 10:55:25
+0100 (old cover letter included below).

The following is changed/added/addressed:
- Support for __ethtool_get_strings()/phy_tunables is added
- Using DOWNSHIFT_DEV_DISABLE define instead of '0' when disabling
- If downshifting clould not be eanbled with the requested count, then a
  trace message with a hist is printed.
- Using ERANGE as requested by Andrew.
- Andrew suggested to use ENOSUPP when a tunable is not implemented We kept
  the EINVAL because ENOSUPP does not exists, and the existing MAC-level
  tunable is also using EINVAL in the same scenario.
- Andrew suggested that the driver should accept a count of 1 and "just"
  round it up to 2 (the nearest allowed value). We discussed it a bit (Raju
  and I) and preferred to return EINVAL when a configuration value could
  not be accommodated. People which wants "help" to choose a supported
  value should be using the DOWNSHIFT_DEV_DEFAULT_COUNT value.
- (ethtool) Syntax is changed from "--set-phy-tunable downshift on|off|%d"
  to "--set-phy-tunable [downshift on|off [count N]]" - as requested by
  Andrew.

Ethtool patches will follow shortly.

Please review

Best regards
Allan and Raju


Raju Lakkaraju (5):
  ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE
  ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE
  ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables
  ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable
  net: phy: Add downshift get/set support in Microsemi PHYs driver

 drivers/net/phy/mscc.c   | 100 +++
 include/linux/phy.h  |   7 +++
 include/uapi/linux/ethtool.h |  18 +++-
 net/core/ethtool.c   |  93 
 4 files changed, 217 insertions(+), 1 deletion(-)

-- 
2.7.3


Old cover letter:
> From 1a318266822f5d74c58b5219ebbdf5a6a5f567dc Mon Sep 17 00:00:00 2001
> From: "Allan W. Nielsen" 
> Date: Fri, 4 Nov 2016 10:55:25 +0100
> Subject: [PATCH net-next 0/5] Adding PHY-Tunables and downshift support
> 
> Hi All,
> 
> This series add support for PHY tunables, and uses this facility to configure
> downshifting. The downshifting mechanism is implemented for MSCC phys.
> 
> This series tries to address the comments provided back in mid October when 
> this
> feature was posted along with fast-link-failure. Fast-link-failure has been
> separated out, but we would like to pick continue on that if/when we agree on
> how the phy-tunables and downshifting should be done.
> 
> The proposed generic interface is similar to 
> ETHTOOL_GTUNABLE/ETHTOOL_STUNABLE,
> it uses the same type (ethtool_tunable/tunable_type_id) but a new enum
> (phy_tunable_id) is added to reflect the PHY tunable.
> 
> The implementation just call the newly added function pointers in
> get_tunable/set_tunable phy_device structure.
> 
> To configure downshifting, the ethtool_tunable structure is used. 'id' must be
> set to 'ETHTOOL_PHY_DOWNSHIFT', 'type_id' must be set to 'ETHTOOL_TUNABLE_U8'
> and 'data' value configure the amount of downshift re-tries.
> 
> If configured to DOWNSHIFT_DEV_DISABLE, then downshift is disabled
> If configured to DOWNSHIFT_DEV_DEFAULT_COUNT, then it is up to the device to
> choose a device-specific re-try count.
> 
> Patches to implement this in ethtool will follow in a few minutes.

[PATCH net-next v2 1/5] ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Defines a generic API to get/set phy tunables. The API is using the
existing ethtool_tunable/tunable_type_id types which is already being used
for mac level tunables.

Signed-off-by: Raju Lakkaraju 
---
 include/uapi/linux/ethtool.h | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 8e54723..42f696f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,6 +248,16 @@ struct ethtool_tunable {
void*data[0];
 };
 
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+
+   /*
+* Add your fresh new phy tunable attribute above and remember to update
+* phy_tunable_strings[] in net/core/ethtool.c
+*/
+   __ETHTOOL_PHY_TUNABLE_COUNT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -548,6 +558,7 @@ struct ethtool_pauseparam {
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
  * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
+ * @ETH_SS_PHY_TUNABLES: PHY tunable names
  */
 enum ethtool_stringset {
ETH_SS_TEST = 0,
@@ -558,6 +569,7 @@ enum ethtool_stringset {
ETH_SS_RSS_HASH_FUNCS,
ETH_SS_TUNABLES,
ETH_SS_PHY_STATS,
+   ETH_SS_PHY_TUNABLES,
 };
 
 /**
@@ -1313,7 +1325,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH ethtool v2 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

This is a follow-up on the patch series posted at Fri, 4 Nov 2016 11:27:31 +0100
(old cover letter included below).


The following is changed/added/addressed:
- Syntax is changed from "--set-phy-tunable downshift on|off|%d" to
  "--set-phy-tunable [downshift on|off [count N]]" - as requested by Andrew.
- Short description of downshifting is added to the man page.


Please review

Best regards
Allan and Raju

Raju Lakkaraju (2):
  ethtool-copy.h:sync with net
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
downshift

 ethtool-copy.h |  18 +++-
 ethtool.8.in   |  39 
 ethtool.c  | 144 +
 3 files changed, 200 insertions(+), 1 deletion(-)

Old cover letter:
> From 5d926ca56f13e283aaa98e820d4720305be4da8f Mon Sep 17 00:00:00 2001
> From: "Allan W. Nielsen" 
> Date: Fri, 4 Nov 2016 11:27:31 +0100
> Subject: [PATCH ethtool 0/2] Adding downshift support to ethtool
> 
> Hi All,
> 
> This patch implements for set/get downshifting.
> 
> Downshifting can either be turned on/off, or it can be configured to a
> specifc count.
> 
> If no "count" are provided, then it is up to the individual PHY driver to
> configure a default count.

-- 
2.7.3

[PATCH net-next v2 5/5] net: phy: Add downshift get/set support in Microsemi PHYs driver

[Allan W. Nielsen]

From: Raju Lakkaraju 

Implements the phy tunable function pointers and implement downshift
functionality for MSCC PHYs.

Signed-off-by: Raju Lakkaraju 
---
 drivers/net/phy/mscc.c | 100 +
 1 file changed, 100 insertions(+)

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index d0026ab..92018ba 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -46,8 +46,15 @@ enum rgmii_rx_clock_delay {
 
 #define MSCC_EXT_PAGE_ACCESS 31
 #define MSCC_PHY_PAGE_STANDARD   0x /* Standard registers */
+#define MSCC_PHY_PAGE_EXTENDED   0x0001 /* Extended registers */
 #define MSCC_PHY_PAGE_EXTENDED_2 0x0002 /* Extended reg - page 2 */
 
+/* Extended Page 1 Registers */
+#define MSCC_PHY_ACTIPHY_CNTL20
+#define DOWNSHIFT_CNTL_MASK  0x001C
+#define DOWNSHIFT_EN 0x0010
+#define DOWNSHIFT_CNTL_POS   2
+
 /* Extended Page 2 Registers */
 #define MSCC_PHY_RGMII_CNTL  20
 #define RGMII_RX_CLK_DELAY_MASK  0x0070
@@ -75,6 +82,8 @@ enum rgmii_rx_clock_delay {
 #define MSCC_VDDMAC_2500 2500
 #define MSCC_VDDMAC_3300 3300
 
+#define DOWNSHIFT_COUNT_MAX  5
+
 struct vsc8531_private {
int rate_magic;
 };
@@ -101,6 +110,66 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, 
u8 page)
return rc;
 }
 
+static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
+{
+   int rc;
+   u16 reg_val;
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= DOWNSHIFT_CNTL_MASK;
+   if (!(reg_val & DOWNSHIFT_EN))
+   *count = DOWNSHIFT_DEV_DISABLE;
+   else
+   *count = ((reg_val & ~DOWNSHIFT_EN) >> DOWNSHIFT_CNTL_POS) + 2;
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
+static int vsc85xx_downshift_set(struct phy_device *phydev, u8 count)
+{
+   int rc;
+   u16 reg_val;
+
+   if (count == DOWNSHIFT_DEV_DEFAULT_COUNT) {
+   /* Default downshift count 3 (i.e. Bit3:2 = 0b01) */
+   count = ((1 << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   } else if (count > DOWNSHIFT_COUNT_MAX || count == 1) {
+   phydev_err(phydev, "Downshift count should be 2,3,4 or 5\n");
+   return -ERANGE;
+   } else if (count) {
+   /* Downshift count is either 2,3,4 or 5 */
+   count = (((count - 2) << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   }
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= ~(DOWNSHIFT_CNTL_MASK);
+   reg_val |= count;
+   rc = phy_write(phydev, MSCC_PHY_ACTIPHY_CNTL, reg_val);
+   if (rc != 0)
+   goto out_unlock;
+
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
 static int vsc85xx_wol_set(struct phy_device *phydev,
   struct ethtool_wolinfo *wol)
 {
@@ -329,6 +398,29 @@ static int vsc85xx_default_config(struct phy_device 
*phydev)
return rc;
 }
 
+static int vsc85xx_get_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna, void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_get(phydev, (u8 *)data);
+   default:
+   return -EINVAL;
+   }
+}
+
+static int vsc85xx_set_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna,
+  const void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_set(phydev, *(u8 *)data);
+   default:
+   return -EINVAL;
+   }
+}
+
 static int vsc85xx_config_init(struct phy_device *phydev)
 {
int rc;
@@ -418,6 +510,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &vsc85xx_wol_set,
.get_wol= &vsc85xx_wol_get,
+   .get_tunable= &vsc85xx_get_tunable,
+   .set_tunable= &vsc85xx_set_tunable,
 },
 {
.phy_id = PHY_ID_VSC8531,
@@ -437,6 +531,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &vsc85xx_wol_set,
.get_wol= &vsc85xx_wol_get,
+   .get_tunable= &vsc85xx_get_tunable,
+   .set_tunable= &vsc85xx_set_tunable,
 },
 {

[PATCH net-next v2 3/5] ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

[Allan W. Nielsen]

From: Raju Lakkaraju 

For operation in cabling environments that are incompatible with
1000BAST-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BAST-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BAST-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
---
 include/uapi/linux/ethtool.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 42f696f..f0db778 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,9 +248,12 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
 enum phy_tunable_id {
ETHTOOL_PHY_ID_UNSPEC,
-
+   ETHTOOL_PHY_DOWNSHIFT,
/*
 * Add your fresh new phy tunable attribute above and remember to update
 * phy_tunable_strings[] in net/core/ethtool.c
-- 
2.7.3

[PATCH ethtool v2 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ downshift on|off [count N] ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ downshift ]

Ethtool ex:
  ethtool --set-phy-tuanble eth0 downshift on
  ethtool --set-phy-tuanble eth0 downshift off
  ethtool --set-phy-tuanble eth0 downshift on count 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju 
---
 ethtool.8.in |  39 
 ethtool.c| 144 +++
 2 files changed, 183 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..2242e90 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,18 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.RB [
+.B downshift
+.A1 on off
+.BN count
+.RB ]
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +959,33 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TS
+nokeep;
+lB l.
+.BI count \ N
+Sets the PHY downshift re-tries count.
+.TE
+.PD
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+For operation in cabling environments that are incompatible with 1000BAST-T,
+PHY device provides an automatic link speed downshift operation.
+Link speed downshift after N failed 1000BASE-T auto-negotiation attempts.
+
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..3b7ea4e 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,146 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed = 0;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count = 0;
+
+   ds.cmd = ETHTOOL_PHY_GTUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Get PHY downshift count");
+   err = 87;
+   }
+   count = *((u8 *)&ds.data[0]);
+   if (count)
+   fprintf(stdout, "  Downshift count: %d\n",
+   count);
+   else
+   fprintf(stdout, " Downshift disabled\n");
+   }
+
+   return err;
+}
+
+static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   if (!strcmp(*(ctx->argp + 1), "on")) {
+   *on = 1;
+   } else if (!strcmp(*(ctx->argp + 1), "off")) {
+   *on = 0;
+   } else {
+   fprintf(stderr, "Invalid boolean\n");
+   exit_bad_args();
+   }
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+   if (ctx->argc < 2)
+   return 0;
+
+   if (strcmp(*ctx->argp, name))
+   return 0;
+
+   *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+
+   ctx->argc -= 2;
+   ctx->argp += 2;
+
+   return 1;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+   int err = 0;
+   u8 ds_cnt = DOWNSHIFT_DEV_DEFAULT_COUNT;
+   u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 0;
+
+   if (ctx->argc == 0)
+   exit_bad_args();
+
+   /* Parse arguments */
+   while (ctx->argc) {
+   if (parse_named_bool(ctx, "downshift", &ds_enable)) {
+   ds_changed = 1;
+   ds_has_cnt = parse_named_u8(ctx, "count", &ds_cnt);
+   } else {
+   exi

Re: [lkp] [net] af1fee9821: BUG:spinlock_trylock_failure_on_UP_on_CPU

[Allan W. Nielsen]

Hi Ye Xiaolong,

On 08/11/16 10:01, Ye Xiaolong wrote:
> Could you tell us what troubles you have met when trying the "lkp qemu"
> tool, it would be better if you could paste some log so we can help to
> improve it.

Sure:

anielsen@lx-anielsen ~/work/opensource-phy/lkp-tests (master) $ bin/lkp qemu -k 
../net-next/bug-build/arch/x86_64/boot/bzImage 
/home/anielsen/Downloads/job-script
/home/anielsen/Downloads/job-script: line 2: $'\r': command not found
/home/anielsen/Downloads/job-script: line 3: syntax error near unexpected token
`$'\r''
'home/anielsen/Downloads/job-script: line 3: `export_top_env()
mkdir: cannot create directory ‘/lkp’: Permission denied
mv: cannot stat ‘/home/anielsen/.lkp/cache/lkp-x86_64.cgz’: No such file or
directory
find: ‘/boot/lost+found’: Permission denied
^C

Looks like it expect unix-style line endings (not sure where it got converted 
to dos-style):

So I tried that (and created some of the missing folders):

$ dos2unix /home/anielsen/Downloads/job-script
$ sudo mkdir /lkp
$ sudo chmod 777 /lkp
$ bin/lkp qemu -k ../net-next/bug-build/arch/x86_64/boot/bzImage 
/home/anielsen/Downloads/job-script
make: Entering directory 
'/home/anielsen/work/opensource-phy/lkp-tests/bin/event'
klcc-c -o wakeup.o wakeup.c
make: klcc: Command not found
: recipe for target 'wakeup.o' failed
make: *** [wakeup.o] Error 127
make: Leaving directory '/home/anielsen/work/opensource-phy/lkp-tests/bin/event'
mv: cannot stat ‘/home/anielsen/.lkp/cache/lkp-x86_64.cgz’: No such file or 
directory
downloading initrds ...
/usr/bin/wget -q --local-encoding=UTF-8 --retry-connrefused --waitretry 1000 
--tries 1000 
https://github.com/0day-ci/lkp-qemu/raw/master/osimage/debian/debian-x86_64-2016-08-31.cgz
 -N -P /home/anielsen/.lkp/cache/osimage/debian
/usr/bin/wget -q --local-encoding=UTF-8 --retry-connrefused --waitretry 1000 
--tries 1000 
https://github.com/0day-ci/lkp-qemu/raw/master/osimage/deps/debian-x86_64-2016-08-31.cgz/lkp_2016-11-02.cgz
 -N -P /home/anielsen/.lkp/cache/osimage/deps/debian-x86_64-2016-08-31.cgz
Failed to download osimage/deps/debian-x86_64-2016-08-31.cgz/lkp_2016-11-02.cgz

Okay, so klcc is missing:
$ sudo emerge -av dev-libs/klibc
...
make -j8 prepare CC=x86_64-pc-linux-gnu-gcc HOSTCC=x86_64-pc-linux-gnu-gcc
scripts/kconfig/conf --silentoldconfig Kconfig
  CHK include/linux/version.h
  UPD include/linux/version.h
  CHK include/generated/utsrelease.h
  UPD include/generated/utsrelease.h
  CC  kernel/bounds.s
In file included from include/linux/compiler.h:48:0,
 from include/linux/stddef.h:4,
 from include/linux/posix_types.h:4,
 from include/linux/types.h:17,
 from include/linux/page-flags.h:8,
 from kernel/bounds.c:9:
include/linux/compiler-gcc.h:90:30: fatal error: linux/compiler-gcc5.h: No such 
file or directory
compilation terminated.
...

And this is where I gave up (at least for now) - it is because I have not
installed my kernel using the packet system - that is my fault and I will get it
fixed.

I did see that you wrote in the readme.md that this needs to run as root - but
it did not look like a permission issue...

So, not very usefull feedback - I know - sorry.

I will get this fixed, give it another try and the get back to you.

/Allan

Re: [lkp] [net] af1fee9821: BUG:spinlock_trylock_failure_on_UP_on_CPU

[Allan W. Nielsen]

Hi,

I'm quite surprised too.

I asked Raju to see if he can re-produce, and then try again with the
microsemi_phy disabled and see it that makes a difference.

If Raju gets stuck on this, then I will try to help our during the after-noon.

/Allan

On 07/11/16 10:31, Andrew Lunn wrote:
> EXTERNAL EMAIL
> 
> 
> On Mon, Nov 07, 2016 at 10:26:28AM +0800, kernel test robot wrote:
> >
> > FYI, we noticed the following commit:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> > commit af1fee98219992ba2c12441a447719652ed7e983 ("net: phy: Add support for 
> > Microsemi VSC 8530/40 Fast Ethernet PHY")
> 
> Humm, interesting. I've no idea how this patch can trigger such a
> warning. The USB gadget does not have an Ethernet PHY, let alone a
> Microsemi 8530/40 Fast Ethernet PHY this patch adds.
> 
> However, it seems nicely documented how to reproduce this. Allan, can
> you setup such a system? Reproduce it, then revert the change and see
> if it still happens.
> 
>Andrew

Re: [PATCH net-next 5/5] net: phy: Add downshift get/set support in Microsemi PHYs driver

[Allan W. Nielsen]

On 04/11/16 13:27, Andrew Lunn wrote:
> > + } else if (count) {
> > + /* Downshift count is either 2,3,4 or 5 */
> > + count = (((count - 2) << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
> 
> Ah, now i see why + 2. But this means it never does what you ask it to
> do. It would be better to round up < 2 to 2, and leave all the others
> as is.
Not sure I understand what you mean...

If the user configure "count == 1", then you want that to be rounded up to
"count == 2", because the HW does not support a count of 1???

If the user configure count to 6, 7, 8 etc. would you also like to round it down
to 5?

I'm okay with that but not sure it is want you mean... (and it will eliminate
your comment on ERANGE - because all values will be accepted, they are just
rounded to "closet" supported value).


(The other comments is understood, and we will get them fixed).


/Allan

Re: [PATCH net-next 2/5] ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

On 04/11/16 13:13, Andrew Lunn wrote:
> On Fri, Nov 04, 2016 at 11:35:39AM +0100, Allan W. Nielsen wrote:
> > From: Raju Lakkaraju 
> >
> > Adding get_tunable/set_tunable function pointer to the phy_driver
> > structure, and uses these function pointers to implement the
> > ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE ioctls.
> For consistency, it would also be nice to add code in
> __ethtool_get_strings().
Okay, we add add that, and I assume you also want ethtool to use it when the
"-k|--show-features" options is invoked?

Are there other cases where this should be used?

/Allan

Re: [PATCH ethtool 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

On 04/11/16 12:59, Andrew Lunn wrote:
> > +++ b/ethtool.8.in
> > @@ -340,6 +340,14 @@ ethtool \- query or control network driver and 
> > hardware settings
> >  .B2 tx-lpi on off
> >  .BN tx-timer
> >  .BN advertise
> > +.HP
> > +.B ethtool \-\-set\-phy\-tunable
> > +.I devname
> > +.B3 downshift on off N
> > +.HP
> 
> I don't think there is any other option which is on|off|N. The general
> pattern would be
> 
> --set-phy-tunable downshift on|off [count N]
> 
> With count being optional.
> 
> This also allows avoiding the ambiguity of what
> 
> --set-phy-tunable downshift 0
> 
> means. To me, that means downshift after 0 retries. But it actually
> seems to mean never downshift. You can then error out when somebody
> does
> 
> --set-phy-tunable downshift on count 0
> or
> --set-phy-tunable downshift off count 42
Okay, I do not have any strong feelings about the syntax here.

We could not find any other signature which had the same need, so we have been
looking for things that was "close" to...

What is being proposed is similar to "-s [ mdix auto|on|off ]".

What you request is similar to "-e|--eeprom-dump [ raw on|off ] [ offset N ]".

So, unless there are other comments, then we will change this in the next
version.

> Please also add a few sentences about what downshift is, and what N
> means. Is it tries, or retries?
Sure.

We will wait a few days to see what other comments we get.

Thanks for reviewing.

/Allan

Re: [PATCH net-next 1/5] ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

Hi,

On 04/11/16 13:03, Andrew Lunn wrote:
> On Fri, Nov 04, 2016 at 11:35:38AM +0100, Allan W. Nielsen wrote:
> > From: Raju Lakkaraju 
> >
> > Defines a generic API to get/set phy tunables. The API is using the
> > existing ethtool_tunable/tunable_type_id types which is already being used
> > for mac level tunables.
> >
> > Signed-off-by: Raju Lakkaraju 
> > Signed-off-by: Allan W. Nielsen 
> > ---
> >  include/uapi/linux/ethtool.h | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index 8e54723..fd0bd36 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -248,6 +248,10 @@ struct ethtool_tunable {
> >   void*data[0];
> >  };
> >
> > +enum phy_tunable_id {
> > + ETHTOOL_PHY_ID_UNSPEC,
> > +};
> 
> Do you have any idea what this is for? A grep for
> ETHTOOL_TUNABLE_UNSPEC does not turn up anything.
It is not used...

It was "just" to mimic how "tunable_type_id/ETHTOOL_TUNABLE_UNSPEC" (and other)
is done.

The thinking was that we did not want an "ID" of zero do to anything - because
that could mean the programmer had forgot to set the field...

I have on strong feelings about this, please let us know if you would like this
done in an other way.

/Allan

[PATCH net-next 4/5] ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable

[Allan W. Nielsen]

From: Raju Lakkaraju 

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

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 net/core/ethtool.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 75f19ab..1a66faa 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2425,6 +2425,11 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
 static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 {
switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   if (tuna->len != sizeof(u8) ||
+   tuna->type_id != ETHTOOL_TUNABLE_U8)
+   return -EINVAL;
+   break;
default:
return -EINVAL;
}
-- 
2.7.3

[PATCH net-next 5/5] net: phy: Add downshift get/set support in Microsemi PHYs driver

[Allan W. Nielsen]

From: Raju Lakkaraju 

Implements the phy tunable function pointers and implement downshift
functionality for MSCC PHYs.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 drivers/net/phy/mscc.c | 102 +
 1 file changed, 102 insertions(+)

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index d0026ab..7edd32d 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -46,8 +46,15 @@ enum rgmii_rx_clock_delay {
 
 #define MSCC_EXT_PAGE_ACCESS 31
 #define MSCC_PHY_PAGE_STANDARD   0x /* Standard registers */
+#define MSCC_PHY_PAGE_EXTENDED   0x0001 /* Extended registers */
 #define MSCC_PHY_PAGE_EXTENDED_2 0x0002 /* Extended reg - page 2 */
 
+/* Extended Page 1 Registers */
+#define MSCC_PHY_ACTIPHY_CNTL20
+#define DOWNSHIFT_CNTL_MASK  0x001C
+#define DOWNSHIFT_EN 0x0010
+#define DOWNSHIFT_CNTL_POS   2
+
 /* Extended Page 2 Registers */
 #define MSCC_PHY_RGMII_CNTL  20
 #define RGMII_RX_CLK_DELAY_MASK  0x0070
@@ -75,6 +82,8 @@ enum rgmii_rx_clock_delay {
 #define MSCC_VDDMAC_2500 2500
 #define MSCC_VDDMAC_3300 3300
 
+#define DOWNSHIFT_COUNT_MAX  5
+
 struct vsc8531_private {
int rate_magic;
 };
@@ -101,6 +110,66 @@ static int vsc85xx_phy_page_set(struct phy_device *phydev, 
u8 page)
return rc;
 }
 
+static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
+{
+   int rc;
+   u16 reg_val;
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= DOWNSHIFT_CNTL_MASK;
+   if (!(reg_val & DOWNSHIFT_EN))
+   *count = 0;
+   else
+   *count = ((reg_val & ~DOWNSHIFT_EN) >> DOWNSHIFT_CNTL_POS) + 2;
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
+static int vsc85xx_downshift_set(struct phy_device *phydev, u8 count)
+{
+   int rc;
+   u16 reg_val;
+
+   if (count == DOWNSHIFT_DEV_DEFAULT_COUNT) {
+   /* Default downshift count 3 (i.e. Bit3:2 = 0b01) */
+   count = ((1 << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   } else if (count > DOWNSHIFT_COUNT_MAX || count == 1) {
+   phydev_err(phydev, "Invalid downshift count\n");
+   return -EINVAL;
+   } else if (count) {
+   /* Downshift count is either 2,3,4 or 5 */
+   count = (((count - 2) << DOWNSHIFT_CNTL_POS) | DOWNSHIFT_EN);
+   }
+
+   mutex_lock(&phydev->lock);
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
+   if (rc != 0)
+   goto out_unlock;
+
+   reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
+   reg_val &= ~(DOWNSHIFT_CNTL_MASK);
+   reg_val |= count;
+   rc = phy_write(phydev, MSCC_PHY_ACTIPHY_CNTL, reg_val);
+   if (rc != 0)
+   goto out_unlock;
+
+   rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
+
+out_unlock:
+   mutex_unlock(&phydev->lock);
+
+   return rc;
+}
+
 static int vsc85xx_wol_set(struct phy_device *phydev,
   struct ethtool_wolinfo *wol)
 {
@@ -329,6 +398,31 @@ static int vsc85xx_default_config(struct phy_device 
*phydev)
return rc;
 }
 
+static int vsc85xx_get_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna, void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_get(phydev, (u8 *)data);
+   default:
+   phydev_err(phydev, "Unsupported PHY tunable id\n");
+   return -EINVAL;
+   }
+}
+
+static int vsc85xx_set_tunable(struct phy_device *phydev,
+  struct ethtool_tunable *tuna,
+  const void *data)
+{
+   switch (tuna->id) {
+   case ETHTOOL_PHY_DOWNSHIFT:
+   return vsc85xx_downshift_set(phydev, *(u8 *)data);
+   default:
+   phydev_err(phydev, "Unsupported PHY tunable id\n");
+   return -EINVAL;
+   }
+}
+
 static int vsc85xx_config_init(struct phy_device *phydev)
 {
int rc;
@@ -418,6 +512,8 @@ static struct phy_driver vsc85xx_driver[] = {
.probe  = &vsc85xx_probe,
.set_wol= &vsc85xx_wol_set,
.get_wol= &vsc85xx_wol_get,
+   .get_tunable= &vsc85xx_get_tunable,
+   .set_tunable= &vsc85xx_set_tunable,
 },
 {
.phy_id = PHY_ID_VSC8531,
@@ -437,6 +533,8 @@ static struct phy_driver vs

[PATCH ethtool 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

This covers kernel changes upto:

commit 67168af82f30bacbd734a4472670cba6b3d6fd36
Author: Raju Lakkaraju 
Date:   Wed Nov 2 11:35:12 2016 +0530

ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

For operation in cabling environments that are incompatible with 
1000BAST-T, PHY
device may provide an automatic link speed downshift operation. When 
enabled,
the device automatically changes its 1000BAST-T auto-negotiation to the next
slower speed after a configured number of failed attempts at 1000BAST-T.  
This
feature is useful in setting up in networks using older cable installations 
that
include only pairs A and B, and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 

Signed-off-by: Allan W. Nielsen 
---
 ethtool-copy.h | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..040c5b5 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -10,14 +10,16 @@
  * Portions Copyright (C) Sun Microsystems 2008
  */
 
-#ifndef _LINUX_ETHTOOL_H
-#define _LINUX_ETHTOOL_H
+#ifndef _UAPI_LINUX_ETHTOOL_H
+#define _UAPI_LINUX_ETHTOOL_H
 
 #include 
 #include 
 #include 
 
+#ifndef __KERNEL__
 #include  /* for INT_MAX */
+#endif
 
 /* All structures exposed to userland should be defined such that they
  * have the same layout for 32-bit and 64-bit userland.
@@ -114,15 +116,14 @@ struct ethtool_cmd {
__u32   reserved[2];
 };
 
-static __inline__ void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
+static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
 __u32 speed)
 {
-
-   ep->speed = (__u16)speed;
+   ep->speed = (__u16)(speed & 0x);
ep->speed_hi = (__u16)(speed >> 16);
 }
 
-static __inline__ __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
+static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
 {
return (ep->speed_hi << 16) | ep->speed;
 }
@@ -247,6 +248,14 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -878,12 +887,12 @@ struct ethtool_rx_flow_spec {
 #define ETHTOOL_RX_FLOW_SPEC_RING  0xLL
 #define ETHTOOL_RX_FLOW_SPEC_RING_VF   0x00FFLL
 #define ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF 32
-static __inline__ __u64 ethtool_get_flow_spec_ring(__u64 ring_cookie)
+static inline __u64 ethtool_get_flow_spec_ring(__u64 ring_cookie)
 {
return ETHTOOL_RX_FLOW_SPEC_RING & ring_cookie;
 };
 
-static __inline__ __u64 ethtool_get_flow_spec_ring_vf(__u64 ring_cookie)
+static inline __u64 ethtool_get_flow_spec_ring_vf(__u64 ring_cookie)
 {
return (ETHTOOL_RX_FLOW_SPEC_RING_VF & ring_cookie) >>
ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF;
@@ -1312,7 +1321,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
@@ -1483,7 +1493,7 @@ enum ethtool_link_mode_bit_indices {
 
 #define SPEED_UNKNOWN  -1
 
-static __inline__ int ethtool_validate_speed(__u32 speed)
+static inline int ethtool_validate_speed(__u32 speed)
 {
return speed <= INT_MAX || speed == SPEED_UNKNOWN;
 }
@@ -1493,7 +1503,7 @@ static __inline__ int ethtool_validate_speed(__u32 speed)
 #define DUPLEX_FULL0x01
 #define DUPLEX_UNKNOWN 0xff
 
-static __inline__ int ethtool_validate_duplex(__u8 duplex)
+static inline int ethtool_validate_duplex(__u8 duplex)
 {
switch (duplex) {
case DUPLEX_HALF:
@@ -1743,4 +1753,4 @@ struct ethtool_link_settings {
 * __u32 map_lp_advertising[link_mode_masks_nwords];
 */
 };
-#endif /* _LINUX_ETHTOOL_H */
+#endif /* _UAPI_LINUX_ETHTOOL_H */
-- 
2.7.3

[PATCH ethtool 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

Hi All,

This patch implements for set/get downshifting.

Downshifting can either be turned on/off, or it can be configured to a
specifc count.

If no "count" are provided, then it is up to the individual PHY driver to
configure a default count.

Best regards
Allan and Raju

Allan W. Nielsen (1):
  ethtool-copy.h:sync with net

Raju Lakkaraju (1):
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
downshift

 ethtool-copy.h | 34 ++
 ethtool.8.in   | 27 +
 ethtool.c  | 92 ++
 3 files changed, 141 insertions(+), 12 deletions(-)

-- 
2.7.3

[PATCH ethtool 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju 

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
ethtool --set-phy-tunable DEVNAME  Set PHY tunable
[ downshift on|off|%d ]
ethtool --get-phy-tunable DEVNAME  Get PHY tunable
[ downshift ]

Ethtool ex:
  ethtool --set-phy-tuanble eth0 downshift on
  ethtool --set-phy-tuanble eth0 downshift off
  ethtool --set-phy-tuanble eth0 downshift 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 ethtool.8.in | 27 ++
 ethtool.c| 92 
 2 files changed, 119 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..e1fd51f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,14 @@ ethtool \- query or control network driver and hardware 
settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.B3 downshift on off N
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +955,25 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TP
+.BI downshift \ N
+Sets the PHY downshift count.
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..c9a0a1d 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,94 @@ static int do_seee(struct cmd_context *ctx)
return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed;
+
+   if (argc < 1)
+   exit_bad_args();
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count = 0;
+
+   ds.cmd = ETHTOOL_PHY_GTUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Get PHY downshift count");
+   err = 87;
+   }
+   count = *((u8 *)&ds.data[0]);
+   if (count)
+   fprintf(stdout, "  Downshift count: %d\n",
+   count);
+   else
+   fprintf(stdout, " Downshift disabled\n");
+   }
+
+   return err;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+   int argc = ctx->argc;
+   char **argp = ctx->argp;
+   int err, i;
+   u8 downshift_changed, downshift_wanted = 0;
+
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argp[i], "downshift")) {
+   downshift_changed = 1;
+   i += 1;
+   if (i >= argc)
+   exit_bad_args();
+   if (!strcmp(argp[i], "on"))
+   downshift_wanted = DOWNSHIFT_DEV_DEFAULT_COUNT;
+   else if (!strcmp(argp[i], "off"))
+   downshift_wanted = DOWNSHIFT_DEV_DISABLE;
+   else
+   downshift_wanted =
+   get_uint_range(argp[i], 0, 0xff);
+   } else  {
+   exit_bad_args();
+   }
+   }
+
+   if (downshift_changed) {
+   struct ethtool_tunable ds;
+   u8 count;
+
+   ds.cmd = ETHTOOL_PHY_STUNABLE;
+   ds.id = ETHTOOL_PHY_DOWNSHIFT;
+   ds.type_id = ETHTOOL_TUNABLE_U8;
+   ds.len = 1;
+   ds.data[0] = &count;
+   *((u8 *)&ds.data[0]) = downshift_wanted;
+   err = send_ioctl(ctx, &ds);
+   if (err < 0) {
+   perror("Cannot Set PHY downshift count");
+   err = 87;
+   }
+   }
+
+   retu

[PATCH net-next 2/5] ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Adding get_tunable/set_tunable function pointer to the phy_driver
structure, and uses these function pointers to implement the
ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE ioctls.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 include/linux/phy.h |  7 +
 net/core/ethtool.c  | 77 +
 2 files changed, 84 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index e7e1fd3..d30daf8 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -611,6 +611,13 @@ struct phy_driver {
void (*get_strings)(struct phy_device *dev, u8 *data);
void (*get_stats)(struct phy_device *dev,
  struct ethtool_stats *stats, u64 *data);
+
+   /* Get and Set PHY tunables */
+   int (*get_tunable)(struct phy_device *dev,
+  struct ethtool_tunable *tuna, void *data);
+   int (*set_tunable)(struct phy_device *dev,
+   struct ethtool_tunable *tuna,
+   const void *data);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),
\
  struct phy_driver, mdiodrv)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 9774898..75f19ab 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2422,6 +2422,76 @@ static int ethtool_set_per_queue(struct net_device *dev, 
void __user *useraddr)
};
 }
 
+static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
+{
+   switch (tuna->id) {
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int get_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->get_tunable))
+   return -EOPNOTSUPP;
+
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   ret = phydev->drv->get_tunable(phydev, &tuna, data);
+   if (ret)
+   goto out;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_to_user(useraddr, data, tuna.len))
+   goto out;
+   ret = 0;
+
+out:
+   kfree(data);
+   return ret;
+}
+
+static int set_phy_tunable(struct net_device *dev, void __user *useraddr)
+{
+   int ret;
+   struct ethtool_tunable tuna;
+   struct phy_device *phydev = dev->phydev;
+   void *data;
+
+   if (!(phydev && phydev->drv && phydev->drv->set_tunable))
+   return -EOPNOTSUPP;
+   if (copy_from_user(&tuna, useraddr, sizeof(tuna)))
+   return -EFAULT;
+   ret = ethtool_phy_tunable_valid(&tuna);
+   if (ret)
+   return ret;
+   data = kmalloc(tuna.len, GFP_USER);
+   if (!data)
+   return -ENOMEM;
+   useraddr += sizeof(tuna);
+   ret = -EFAULT;
+   if (copy_from_user(data, useraddr, tuna.len))
+   goto out;
+   ret = phydev->drv->set_tunable(phydev, &tuna, data);
+
+out:
+   kfree(data);
+   return ret;
+}
+
 /* The main entry point in this file.  Called from net/core/dev_ioctl.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -2479,6 +2549,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_GET_TS_INFO:
case ETHTOOL_GEEE:
case ETHTOOL_GTUNABLE:
+   case ETHTOOL_PHY_GTUNABLE:
break;
default:
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
@@ -2684,6 +2755,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_SLINKSETTINGS:
rc = ethtool_set_link_ksettings(dev, useraddr);
break;
+   case ETHTOOL_PHY_GTUNABLE:
+   rc = get_phy_tunable(dev, useraddr);
+   break;
+   case ETHTOOL_PHY_STUNABLE:
+   rc = set_phy_tunable(dev, useraddr);
+   break;
default:
rc = -EOPNOTSUPP;
}
-- 
2.7.3

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

[Allan W. Nielsen]

From: Raju Lakkaraju 

For operation in cabling environments that are incompatible with
1000BAST-T, PHY device may provide an automatic link speed downshift
operation. When enabled, the device automatically changes its 1000BAST-T
auto-negotiation to the next slower speed after a configured number of
failed attempts at 1000BAST-T.  This feature is useful in setting up in
networks using older cable installations that include only pairs A and B,
and not pairs C and D.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 include/uapi/linux/ethtool.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index fd0bd36..040c5b5 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,8 +248,12 @@ struct ethtool_tunable {
void*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT0xff
+#define DOWNSHIFT_DEV_DISABLE  0
+
 enum phy_tunable_id {
ETHTOOL_PHY_ID_UNSPEC,
+   ETHTOOL_PHY_DOWNSHIFT,
 };
 
 /**
-- 
2.7.3

[PATCH net-next 0/5] Adding PHY-Tunables and downshift support

[Allan W. Nielsen]

Hi All,

This series add support for PHY tunables, and uses this facility to configure
downshifting. The downshifting mechanism is implemented for MSCC phys.

This series tries to address the comments provided back in mid October when this
feature was posted along with fast-link-failure. Fast-link-failure has been
separated out, but we would like to pick continue on that if/when we agree on
how the phy-tunables and downshifting should be done.

The proposed generic interface is similar to ETHTOOL_GTUNABLE/ETHTOOL_STUNABLE,
it uses the same type (ethtool_tunable/tunable_type_id) but a new enum
(phy_tunable_id) is added to reflect the PHY tunable.

The implementation just call the newly added function pointers in
get_tunable/set_tunable phy_device structure.

To configure downshifting, the ethtool_tunable structure is used. 'id' must be
set to 'ETHTOOL_PHY_DOWNSHIFT', 'type_id' must be set to 'ETHTOOL_TUNABLE_U8'
and 'data' value configure the amount of downshift re-tries.

If configured to DOWNSHIFT_DEV_DISABLE, then downshift is disabled
If configured to DOWNSHIFT_DEV_DEFAULT_COUNT, then it is up to the device to
choose a device-specific re-try count.

Patches to implement this in ethtool will follow in a few minutes.

Please review.

Best regards
Allan and Raju

Raju Lakkaraju (5):
  ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE
  ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE
  ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables
  ethtool: Core impl for ETHTOOL_PHY_DOWNSHIFT tunable
  net: phy: Add downshift get/set support in Microsemi PHYs driver

 drivers/net/phy/mscc.c   | 102 +++
 include/linux/phy.h  |   7 +++
 include/uapi/linux/ethtool.h |  11 -
 net/core/ethtool.c   |  82 ++
 4 files changed, 201 insertions(+), 1 deletion(-)

-- 
2.7.3

[PATCH net-next 1/5] ethtool: (uapi) Add ETHTOOL_PHY_GTUNABLE and ETHTOOL_PHY_STUNABLE

[Allan W. Nielsen]

From: Raju Lakkaraju 

Defines a generic API to get/set phy tunables. The API is using the
existing ethtool_tunable/tunable_type_id types which is already being used
for mac level tunables.

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 include/uapi/linux/ethtool.h | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 8e54723..fd0bd36 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -248,6 +248,10 @@ struct ethtool_tunable {
void*data[0];
 };
 
+enum phy_tunable_id {
+   ETHTOOL_PHY_ID_UNSPEC,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -1313,7 +1317,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS  0x004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS  0x004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE   0x004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE   0x004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET ETHTOOL_GSET
-- 
2.7.3

[PATCH net-next 1/1] net: phy: Add support for Microsemi VSC 8530/40 Fast Ethernet PHY

[Allan W. Nielsen]

From: Raju Lakkaraju 

Signed-off-by: Raju Lakkaraju 
Signed-off-by: Allan W. Nielsen 
---
 drivers/net/phy/Kconfig |  2 +-
 drivers/net/phy/mscc.c  | 42 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 45f68ea..ff31c10 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -290,7 +290,7 @@ config MICROCHIP_PHY
 config MICROSEMI_PHY
tristate "Microsemi PHYs"
---help---
- Currently supports the VSC8531 and VSC8541 PHYs
+ Currently supports VSC8530, VSC8531, VSC8540 and VSC8541 PHYs
 
 config NATIONAL_PHY
tristate "National Semiconductor PHYs"
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 113616b..d0026ab 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -65,7 +65,9 @@ enum rgmii_rx_clock_delay {
 #define SECURE_ON_PASSWD_LEN_4   0x4000
 
 /* Microsemi PHY ID's */
+#define PHY_ID_VSC8530   0x00070560
 #define PHY_ID_VSC8531   0x00070570
+#define PHY_ID_VSC8540   0x00070760
 #define PHY_ID_VSC8541   0x00070770
 
 #define MSCC_VDDMAC_1500 1500
@@ -399,6 +401,25 @@ static int vsc85xx_probe(struct phy_device *phydev)
 /* Microsemi VSC85xx PHYs */
 static struct phy_driver vsc85xx_driver[] = {
 {
+   .phy_id = PHY_ID_VSC8530,
+   .name   = "Microsemi FE VSC8530",
+   .phy_id_mask= 0xfff0,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .soft_reset = &genphy_soft_reset,
+   .config_init= &vsc85xx_config_init,
+   .config_aneg= &genphy_config_aneg,
+   .aneg_done  = &genphy_aneg_done,
+   .read_status= &genphy_read_status,
+   .ack_interrupt  = &vsc85xx_ack_interrupt,
+   .config_intr= &vsc85xx_config_intr,
+   .suspend= &genphy_suspend,
+   .resume = &genphy_resume,
+   .probe  = &vsc85xx_probe,
+   .set_wol= &vsc85xx_wol_set,
+   .get_wol= &vsc85xx_wol_get,
+},
+{
.phy_id = PHY_ID_VSC8531,
.name   = "Microsemi VSC8531",
.phy_id_mask= 0xfff0,
@@ -418,6 +439,25 @@ static struct phy_driver vsc85xx_driver[] = {
.get_wol= &vsc85xx_wol_get,
 },
 {
+   .phy_id = PHY_ID_VSC8540,
+   .name   = "Microsemi FE VSC8540 SyncE",
+   .phy_id_mask= 0xfff0,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .soft_reset = &genphy_soft_reset,
+   .config_init= &vsc85xx_config_init,
+   .config_aneg= &genphy_config_aneg,
+   .aneg_done  = &genphy_aneg_done,
+   .read_status= &genphy_read_status,
+   .ack_interrupt  = &vsc85xx_ack_interrupt,
+   .config_intr= &vsc85xx_config_intr,
+   .suspend= &genphy_suspend,
+   .resume = &genphy_resume,
+   .probe  = &vsc85xx_probe,
+   .set_wol= &vsc85xx_wol_set,
+   .get_wol= &vsc85xx_wol_get,
+},
+{
.phy_id = PHY_ID_VSC8541,
.name   = "Microsemi VSC8541 SyncE",
.phy_id_mask= 0xfff0,
@@ -442,7 +482,9 @@ static struct phy_driver vsc85xx_driver[] = {
 module_phy_driver(vsc85xx_driver);
 
 static struct mdio_device_id __maybe_unused vsc85xx_tbl[] = {
+   { PHY_ID_VSC8530, 0xfff0, },
{ PHY_ID_VSC8531, 0xfff0, },
+   { PHY_ID_VSC8540, 0xfff0, },
{ PHY_ID_VSC8541, 0xfff0, },
{ }
 };
-- 
2.7.3

[PATCH net-next 0/1] net: phy: Add support for Microsemi VSC 8530/40 Fast Ethernet PHY

[Allan W. Nielsen]

Hi All,

This patch adds support for VSC 8530/40 which is 10/100 Mbps variants of VSC
8531/41.

Please review.

Best regards
Allan and Raju


Raju Lakkaraju (1):
  net: phy: Add support for Microsemi VSC 8530/40 Fast Ethernet PHY

 drivers/net/phy/Kconfig |  2 +-
 drivers/net/phy/mscc.c  | 42 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

-- 
2.7.3

Re: [PATCH net-next v11 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Hi David,

I'm really sorry if I messed up, or is not following the protocol...

But you have applied it already:
http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=4f58e6dceb0e44ca8f21568ed81e1df24e55964c

> commit 4f58e6dceb0e44ca8f21568ed81e1df24e55964c
> Author:     Allan W. Nielsen 
> AuthorDate: Wed Oct 12 15:47:51 2016 +0200
> Commit: David S. Miller 
> CommitDate: Fri Oct 14 10:06:13 2016 -0400
> 
> net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.
> 
> Edge-Rate cleanup include the following:
> - Updated device tree bindings documentation for edge-rate
> - The edge-rate is now specified as a "slowdown", meaning that it is now
>   being specified as positive values instead of negative (both
>   documentation and implementation wise).
> - Only explicitly documented values for "vsc8531,vddmac" and
>   "vsc8531,edge-slowdown" are accepted by the device driver.
> - Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
> - Read/validate devicetree settings in probe instead of init
> 
> Signed-off-by: Allan W. Nielsen 
> Signed-off-by: Raju Lakkaraju 
> Signed-off-by: David S. Miller 

Maybe the misunderstanding was caused by me posting the re-based version in
another thread.

Anyway, thanks a lot for the big effort you put into maintaining this
sub-system. I will be more care full next time to avoid such confusions.

Best regards
Allan W. Nielsen





On 14/10/16 11:05, David Miller wrote:
> EXTERNAL EMAIL
> 
> 
> From: "Allan W. Nielsen" 
> Date: Thu, 13 Oct 2016 20:21:30 +0200
> 
> > Edge-Rate cleanup include the following:
> > - Updated device tree bindings documentation for edge-rate
> > - The edge-rate is now specified as a "slowdown", meaning that it is now
> >   being specified as positive values instead of negative (both
> >   documentation and implementation wise).
> > - Only explicitly documented values for "vsc8531,vddmac" and
> >   "vsc8531,edge-slowdown" are accepted by the device driver.
> > - Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
> > - Read/validate devicetree settings in probe instead of init
> >
> > Signed-off-by: Allan W. Nielsen 
> > Signed-off-by: Raju Lakkaraju 
> 
> This patch does not apply to the net-next tree.
> 
> Take my tree, put this email of your's into a file, and run this:
> 
> bash$ git am file
> 
> and you will get:
> 
> [davem@dhcp-10-15-49-210 net-next]$ git am --signoff 
> net-next-v11-1-1-net-phy-Cleanup-the-Edge-Rate-feature-in-Microsemi-PHYs..patch
> Applying: net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.
> error: patch failed: 
> Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt:6
> error: Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt: patch does 
> not apply
> error: patch failed: drivers/net/phy/mscc.c:12
> error: drivers/net/phy/mscc.c: patch does not apply
> error: include/dt-bindings/net/mscc-phy-vsc8531.h: does not exist in index
> Patch failed at 0001 net: phy: Cleanup the Edge-Rate feature in Microsemi 
> PHYs.
> The copy of the patch that failed is found in:
>/home/davem/src/GIT/net-next/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Please do not resubmit this patch until you can successfully email the
> patch to yourself and apply it cleanly to the net-next tree.
> 
> 

[PATCH net-next v11 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Edge-Rate cleanup include the following:
- Updated device tree bindings documentation for edge-rate
- The edge-rate is now specified as a "slowdown", meaning that it is now
  being specified as positive values instead of negative (both
  documentation and implementation wise).
- Only explicitly documented values for "vsc8531,vddmac" and
  "vsc8531,edge-slowdown" are accepted by the device driver.
- Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
- Read/validate devicetree settings in probe instead of init

Signed-off-by: Allan W. Nielsen 
Signed-off-by: Raju Lakkaraju 
---
 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  51 
 drivers/net/phy/mscc.c | 135 ++---
 include/dt-bindings/net/mscc-phy-vsc8531.h |  21 
 3 files changed, 90 insertions(+), 117 deletions(-)
 delete mode 100644 include/dt-bindings/net/mscc-phy-vsc8531.h

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt 
b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 99c7eb0..bdefefc6 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -6,22 +6,27 @@ Required properties:
  Documentation/devicetree/bindings/net/phy.txt
 
 Optional properties:
-- vsc8531,vddmac   : The vddmac in mV.
+- vsc8531,vddmac   : The vddmac in mV. Allowed values is listed
+ in the first row of Table 1 (below).
+ This property is only used in combination
+ with the 'edge-slowdown' property.
+ Default value is 3300.
 - vsc8531,edge-slowdown: % the edge should be slowed down relative to
- the fastest possible edge time. Native sign
- need not enter.
+ the fastest possible edge time.
  Edge rate sets the drive strength of the MAC
- interface output signals.  Changing the drive
- strength will affect the edge rate of the output
- signal.  The goal of this setting is to help
- reduce electrical emission (EMI) by being able
- to reprogram drive strength and in effect slow
- down the edge rate if desired.  Table 1 shows the
- impact to the edge rate per VDDMAC supply for each
- drive strength setting.
- Ref: Table:1 - Edge rate change below.
-
-Note: see dt-bindings/net/mscc-phy-vsc8531.h for applicable values
+ interface output signals.  Changing the
+ drive strength will affect the edge rate of
+ the output signal.  The goal of this setting
+ is to help reduce electrical emission (EMI)
+ by being able to reprogram drive strength
+ and in effect slow down the edge rate if
+ desired.
+ To adjust the edge-slowdown, the 'vddmac'
+ must be specified. Table 1 lists the
+ supported edge-slowdown values for a given
+ 'vddmac'.
+ Default value is 0%.
+ Ref: Table:1 - Edge rate change (below).
 
 Table: 1 - Edge rate change
 |
@@ -29,23 +34,23 @@ Table: 1 - Edge rate change
 |  |
 | 3300 mV  2500 mV 1800 mV 1500 mV |
 |---|
-| Default  Deafult Default Default |
+| 0%   0%  0%  0%  |
 | (Fastest)(recommended)   (recommended)   |
 |---|
-| -2%  -3% -5% -6% |
+| 2%   3%  5%  6%  |
 |---|
-| -4%  -6% -9% -14%|
+| 4%   6%  9%  14% |
 |---|
-| -7%  -10%-16%-21%|
+| 7%   10% 16% 21% |
 |(recommended) (recommended)   |
 |---|
-| -10% -14%-23%-29%

[PATCH net-next v11 0/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs (rebase)

[Allan W. Nielsen]

Hi,

On 13/10/16 10:01, David Miller wrote:
> This doesn't apply cleanly to net-next, please respin.
Sorry about that... But, I have already send a rebased version (v11) which does
apply cleanly. This time I will try and see if I can get git-send-email to use
the same thread ID.

Anyway, here is a re-post of version 11 (which should apply cleanly on-top of
net-next)

v10 was reviewed by Florian and Andrew:

On 12/10/16 02:14, Florian Fainelli wrote:
> On 10/10/2016 07:13 AM, Allan W. Nielsen wrote:
> > Edge-Rate cleanup include the following:
> > ...
> Reviewed-by: Florian Fainelli 

On 11/10/16 15:09, Andrew Lunn wrote:
> On Mon, Oct 10, 2016 at 04:13:45PM +0200, Allan W. Nielsen wrote:
> > Edge-Rate cleanup include the following:
> > ...
> Reviewed-by: Andrew Lunn 

Best regards
Allan W. Nielsen

[PATCH net-next v11 0/1] net: phy: Rebase Edge-Rate clean up

[Allan W. Nielsen]

Hi,

I can see that the "Add Wake-on-LAN driver for Microsemi PHYs" (0a55c12f97)
patch has been applied to net-next, and that it is causing the Edge-Rate patch
to conflict.

I have therefore rebased v10 of the patch to fit on top of net-next.

/Allan

-- 
2.7.3

[PATCH net-next v11 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Edge-Rate cleanup include the following:
- Updated device tree bindings documentation for edge-rate
- The edge-rate is now specified as a "slowdown", meaning that it is now
  being specified as positive values instead of negative (both
  documentation and implementation wise).
- Only explicitly documented values for "vsc8531,vddmac" and
  "vsc8531,edge-slowdown" are accepted by the device driver.
- Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
- Read/validate devicetree settings in probe instead of init

Signed-off-by: Allan W. Nielsen 
Signed-off-by: Raju Lakkaraju 
---
 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  51 
 drivers/net/phy/mscc.c | 135 ++---
 include/dt-bindings/net/mscc-phy-vsc8531.h |  21 
 3 files changed, 90 insertions(+), 117 deletions(-)
 delete mode 100644 include/dt-bindings/net/mscc-phy-vsc8531.h

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt 
b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 99c7eb0..bdefefc6 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -6,22 +6,27 @@ Required properties:
  Documentation/devicetree/bindings/net/phy.txt
 
 Optional properties:
-- vsc8531,vddmac   : The vddmac in mV.
+- vsc8531,vddmac   : The vddmac in mV. Allowed values is listed
+ in the first row of Table 1 (below).
+ This property is only used in combination
+ with the 'edge-slowdown' property.
+ Default value is 3300.
 - vsc8531,edge-slowdown: % the edge should be slowed down relative to
- the fastest possible edge time. Native sign
- need not enter.
+ the fastest possible edge time.
  Edge rate sets the drive strength of the MAC
- interface output signals.  Changing the drive
- strength will affect the edge rate of the output
- signal.  The goal of this setting is to help
- reduce electrical emission (EMI) by being able
- to reprogram drive strength and in effect slow
- down the edge rate if desired.  Table 1 shows the
- impact to the edge rate per VDDMAC supply for each
- drive strength setting.
- Ref: Table:1 - Edge rate change below.
-
-Note: see dt-bindings/net/mscc-phy-vsc8531.h for applicable values
+ interface output signals.  Changing the
+ drive strength will affect the edge rate of
+ the output signal.  The goal of this setting
+ is to help reduce electrical emission (EMI)
+ by being able to reprogram drive strength
+ and in effect slow down the edge rate if
+ desired.
+ To adjust the edge-slowdown, the 'vddmac'
+ must be specified. Table 1 lists the
+ supported edge-slowdown values for a given
+ 'vddmac'.
+ Default value is 0%.
+ Ref: Table:1 - Edge rate change (below).
 
 Table: 1 - Edge rate change
 |
@@ -29,23 +34,23 @@ Table: 1 - Edge rate change
 |  |
 | 3300 mV  2500 mV 1800 mV 1500 mV |
 |---|
-| Default  Deafult Default Default |
+| 0%   0%  0%  0%  |
 | (Fastest)(recommended)   (recommended)   |
 |---|
-| -2%  -3% -5% -6% |
+| 2%   3%  5%  6%  |
 |---|
-| -4%  -6% -9% -14%|
+| 4%   6%  9%  14% |
 |---|
-| -7%  -10%-16%-21%|
+| 7%   10% 16% 21% |
 |(recommended) (recommended)   |
 |---|
-| -10% -14%-23%-29%

[PATCH net-next v10 0/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Hi All,

Yet another iteration on the edge rate.

Included in this iteration:
- vdd and slowdown is removed from private structure
- rate_magic added to private structure
- devicetree settings is read and validated in probe instead of init
- unused reference in device-tree doc removed

Please review

/Allan

[PATCH net-next v10 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Edge-Rate cleanup include the following:
- Updated device tree bindings documentation for edge-rate
- The edge-rate is now specified as a "slowdown", meaning that it is now
  being specified as positive values instead of negative (both
  documentation and implementation wise).
- Only explicitly documented values for "vsc8531,vddmac" and
  "vsc8531,edge-slowdown" are accepted by the device driver.
- Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
- Read/validate devicetree settings in probe instead of init

Signed-off-by: Allan W. Nielsen 
Signed-off-by: Raju Lakkaraju 
---
 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   |  51 +
 drivers/net/phy/mscc.c | 127 ++---
 include/dt-bindings/net/mscc-phy-vsc8531.h |  21 
 3 files changed, 86 insertions(+), 113 deletions(-)
 delete mode 100644 include/dt-bindings/net/mscc-phy-vsc8531.h

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt 
b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 99c7eb0..bdefefc6 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -6,22 +6,27 @@ Required properties:
  Documentation/devicetree/bindings/net/phy.txt
 
 Optional properties:
-- vsc8531,vddmac   : The vddmac in mV.
+- vsc8531,vddmac   : The vddmac in mV. Allowed values is listed
+ in the first row of Table 1 (below).
+ This property is only used in combination
+ with the 'edge-slowdown' property.
+ Default value is 3300.
 - vsc8531,edge-slowdown: % the edge should be slowed down relative to
- the fastest possible edge time. Native sign
- need not enter.
+ the fastest possible edge time.
  Edge rate sets the drive strength of the MAC
- interface output signals.  Changing the drive
- strength will affect the edge rate of the output
- signal.  The goal of this setting is to help
- reduce electrical emission (EMI) by being able
- to reprogram drive strength and in effect slow
- down the edge rate if desired.  Table 1 shows the
- impact to the edge rate per VDDMAC supply for each
- drive strength setting.
- Ref: Table:1 - Edge rate change below.
-
-Note: see dt-bindings/net/mscc-phy-vsc8531.h for applicable values
+ interface output signals.  Changing the
+ drive strength will affect the edge rate of
+ the output signal.  The goal of this setting
+ is to help reduce electrical emission (EMI)
+ by being able to reprogram drive strength
+ and in effect slow down the edge rate if
+ desired.
+ To adjust the edge-slowdown, the 'vddmac'
+ must be specified. Table 1 lists the
+ supported edge-slowdown values for a given
+ 'vddmac'.
+ Default value is 0%.
+ Ref: Table:1 - Edge rate change (below).
 
 Table: 1 - Edge rate change
 |
@@ -29,23 +34,23 @@ Table: 1 - Edge rate change
 |  |
 | 3300 mV  2500 mV 1800 mV 1500 mV |
 |---|
-| Default  Deafult Default Default |
+| 0%   0%  0%  0%  |
 | (Fastest)(recommended)   (recommended)   |
 |---|
-| -2%  -3% -5% -6% |
+| 2%   3%  5%  6%  |
 |---|
-| -4%  -6% -9% -14%|
+| 4%   6%  9%  14% |
 |---|
-| -7%  -10%-16%-21%|
+| 7%   10% 16% 21% |
 |(recommended) (recommended)   |
 |---|
-| -10% -14%-23%-29%

Re: [PATCH net-next v9 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Hi,

On 07/10/16 22:22, Andrew Lunn wrote:
> Overall, this is much better. I just have a few nitpicks.
It is good to hear that we are getting closer ;-)

> dt-bindings/net/mscc-phy-vsc8531.h is removed by this patch. It would
> be good to also remove the reference.
My bad.

> You don't need edge_slowdown and vddmac in the private structure,
> since they are never used after determining what rate_magic is.
Year... I was actually a bit confused about this... But assumed that you had
some conventions about saving "input" configuration.

Well, clearly not - I will clean this up.

Actually, there is no need to store rate_magic either... It is only used in the
init function. This means that there is no need for private date...

The code could look somthing like this (it is not tested, so just look at see if
you like the idea):

#ifdef CONFIG_OF_MDIO
static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
{
int rc;
u8 sd;
u16 vdd;
struct device *dev = &phydev->mdio.dev;
struct device_node *of_node = dev->of_node;
u8 sd_array_size = ARRAY_SIZE(edge_table[0].slowdown);

if (!of_node)
return -ENODEV;

rc = of_property_read_u16(of_node, "vsc8531,vddmac", &vdd);
if (rc != 0)
vdd = MSCC_VDDMAC_3300;

rc = of_property_read_u8(of_node, "vsc8531,edge-slowdown", &sd);
if (rc != 0)
sd = 0;

for (vdd = 0; vdd < ARRAY_SIZE(edge_table); vdd++)
if (edge_table[vdd].vddmac == vddmac)
for (sd = 0; sd < sd_array_size; sd++)
if (edge_table[vdd].slowdown[sd] == slowdown)
return (sd_array_size - sd - 1);

return -EINVAL;
}
#else
static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
{
return 0;
}
#endif /* CONFIG_OF_MDIO */


static int vsc85xx_config_init(struct phy_device *phydev)
{
int rc, rate_magic;
...
rate_magic = vsc85xx_edge_rate_magic_get(phydev);
if (rate_magic < 0)
return rate_magic;

rc = vsc85xx_edge_rate_cntl_set(phydev, rate_magic);
if (rc)
return rc;
...
}

This is clearly how I would prefere it, as it would simplify the implementation.

It should be no problem to have this tested, and have a new patch avialable
tomorrow.

/Allan

[PATCH net-next v9 0/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs

[Allan W. Nielsen]

Hi Andrew (and other),

Raju and I have been going through all the comments received on the edge-rate
feature, and tried to address them (once again...).

The patch is rebased to fit on top of net-next (it depends on a4cc96d1f).

Following initiatives is covered:

- Updated device tree bindings documentation for edge-rate
- The edge-rate is now specified as a "slowdown", meaning that it is now
  being specified as positive values instead of negative (both
  documentation and implementation wise).
- Only explicitly documented values for "vsc8531,vddmac" and
  "vsc8531,edge-slowdown" are accepted by the device driver.
- Deleting include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
- Initialize to default values when CONFIG_OF_MDIO is not defined
- Using ARRAY_SIZE when iterating through an array.

Please review.

Best regards
Allan W. Nielsen

[PATCH net-next v9 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.

[Allan W. Nielsen]

Edge-Rate cleanup include the following:
- Updated device tree bindings documentation for edge-rate
- The edge-rate is now specified as a "slowdown", meaning that it is now
  being specified as positive values instead of negative (both
  documentation and implementation wise).
- Only explicitly documented values for "vsc8531,vddmac" and
  "vsc8531,edge-slowdown" are accepted by the device driver.
- Deleting include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.

Signed-off-by: Allan W. Nielsen 
Signed-off-by: Raju Lakkaraju 
---
 .../devicetree/bindings/net/mscc-phy-vsc8531.txt   | 49 --
 drivers/net/phy/mscc.c | 79 +-
 include/dt-bindings/net/mscc-phy-vsc8531.h | 21 --
 3 files changed, 75 insertions(+), 74 deletions(-)
 delete mode 100644 include/dt-bindings/net/mscc-phy-vsc8531.h

diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt 
b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 99c7eb0..f552033 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -6,20 +6,27 @@ Required properties:
  Documentation/devicetree/bindings/net/phy.txt
 
 Optional properties:
-- vsc8531,vddmac   : The vddmac in mV.
+- vsc8531,vddmac   : The vddmac in mV. Allowed values is listed
+ in the first row of Table 1 (below).
+ This property is only used in combination
+ with the 'edge-slowdown' property.
+ Default value is 3300.
 - vsc8531,edge-slowdown: % the edge should be slowed down relative to
- the fastest possible edge time. Native sign
- need not enter.
+ the fastest possible edge time.
  Edge rate sets the drive strength of the MAC
- interface output signals.  Changing the drive
- strength will affect the edge rate of the output
- signal.  The goal of this setting is to help
- reduce electrical emission (EMI) by being able
- to reprogram drive strength and in effect slow
- down the edge rate if desired.  Table 1 shows the
- impact to the edge rate per VDDMAC supply for each
- drive strength setting.
- Ref: Table:1 - Edge rate change below.
+ interface output signals.  Changing the
+ drive strength will affect the edge rate of
+ the output signal.  The goal of this setting
+ is to help reduce electrical emission (EMI)
+ by being able to reprogram drive strength
+ and in effect slow down the edge rate if
+ desired.
+ To adjust the edge-slowdown, the 'vddmac'
+ must be specified. Table 1 lists the
+ supported edge-slowdown values for a given
+ 'vddmac'.
+ Default value is 0%.
+ Ref: Table:1 - Edge rate change (below).
 
 Note: see dt-bindings/net/mscc-phy-vsc8531.h for applicable values
 
@@ -29,23 +36,23 @@ Table: 1 - Edge rate change
 |  |
 | 3300 mV  2500 mV 1800 mV 1500 mV |
 |---|
-| Default  Deafult Default Default |
+| 0%   0%  0%  0%  |
 | (Fastest)(recommended)   (recommended)   |
 |---|
-| -2%  -3% -5% -6% |
+| 2%   3%  5%  6%  |
 |---|
-| -4%  -6% -9% -14%|
+| 4%   6%  9%  14% |
 |---|
-| -7%  -10%-16%-21%|
+| 7%   10% 16% 21% |
 |(recommended) (recommended)   |
 |---|
-| -10% -14%-23%-29%|
+| 10%  14% 23% 29% |
 |---|
-| -17% -23%-35%-42%

Re: [PATCH v3 net-next] net: phy: Add Edge-rate driver for Microsemi PHYs.

[Allan W. Nielsen]

Hi Andrew,

> > +Optional properties:
> > +- vsc8531,edge-rate  : Edge rate sets the drive strength of the MAC
> > +   interface output signals.  Changing the drive
> > +   strength will affect the edge rate of the output
> > +   signal.
> 
> Are we specifying a rate or a strength? It is called edge-rate, so it
> expect it to be a rate, mV/pS or something similar.
I do not know - sorry.

If it is a hard requirement to document these settings using physical units then
we need to requist some help from the HW team.

I will forward this to the HW team - but it will properly take a few days to get
a decent answer.

Best regards
Allan W. Nielsen