Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] ARM64: Add Internal PHY support for Meson GXL
From: Neil Armstrong @ 2016-11-04 15:51 UTC (permalink / raw)
  To: f.fainelli, carlo, khilman, andrew
  Cc: Neil Armstrong, netdev, linux-amlogic, linux-arm-kernel,
	linux-kernel

The Amlogic Meson GXL SoCs have an internal RMII PHY that is muxed with the
external RGMII pins.

In order to support switching between the two PHYs links, extended registers
size for mdio-mux-mmioreg must be added.

The DT related patches submitted as RFC in [3] will be sent in a separate
patchset due to multiple patchsets and DTSI migrations.

Changes since v2 RFC patchset at : [3]
 - Change phy Kconfig/Makefile alphabetic order
 - GXL dtsi cleanup

Changes since original RFC patchset at : [2]
 - Remove meson8b experimental phy switching
 - Switch to mdio-mux-mmioreg with extennded size support
 - Add internal phy support for S905x and p231
 - Add external PHY support for p230

[1] http://lkml.kernel.org/r/1477932286-27482-1-git-send-email-narmstrong@baylibre.com
[2] http://lkml.kernel.org/r/1477060838-14164-1-git-send-email-narmstrong@baylibre.com
[3] http://lkml.kernel.org/r/1477932987-27871-1-git-send-email-narmstrong@baylibre.com

Neil Armstrong (2):
  net: mdio-mux-mmioreg: Add support for 16bit and 32bit register sizes
  net: phy: Add Meson GXL Internal PHY driver

 .../devicetree/bindings/net/mdio-mux-mmioreg.txt   |  4 +-
 drivers/net/phy/Kconfig                            |  5 ++
 drivers/net/phy/Makefile                           |  1 +
 drivers/net/phy/mdio-mux-mmioreg.c                 | 60 ++++++++++++----
 drivers/net/phy/meson-gxl.c                        | 81 ++++++++++++++++++++++
 5 files changed, 136 insertions(+), 15 deletions(-)
 create mode 100644 drivers/net/phy/meson-gxl.c

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Sebastian Frias @ 2016-11-04 15:36 UTC (permalink / raw)
  To: Måns Rullgård; +Cc: David S. Miller, netdev, LKML, Mason, Andrew Lunn
In-Reply-To: <yw1xlgwzjmti.fsf@unicorn.mansr.com>

Hi Måns,

On 11/04/2016 04:18 PM, Måns Rullgård wrote:
> Sebastian Frias <sf84@laposte.net> writes:
> 
>> The delay can be applied at PHY or MAC level, but since
>> PHY drivers will apply the delay at PHY level when using
>> one of the "internal delay" declinations of RGMII mode
>> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
>> at MAC level causes issues.
> 
> The Broadcom GENET driver does the same thing.
> 

Well, I don't know who uses that driver, or why they did it that way.

However, with the current code and DT bindings, if one requires
the delay, phy-connection-type="rgmii-txid" must be set.

But when doing so, both the Atheros 8035 and the Aurora NB8800 drivers
will apply the delay.

I think a better way of dealing with this is that both, PHY and MAC
drivers exchange information so that the delay is applied only once.

I can see how to do that in another patch set.

>> Signed-off-by: Sebastian Frias <sf84@laposte.net>
>> ---
>>  drivers/net/ethernet/aurora/nb8800.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>> index b59aa35..d2855c9 100644
>> --- a/drivers/net/ethernet/aurora/nb8800.c
>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>> @@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
>>  		break;
>>
>>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>> -		pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>> +		pad_mode = PAD_MODE_RGMII;
>>  		break;
>>
>>  	default:
>> -- 
>> 1.7.11.2
> 
> If this change is correct (and I'm not convinced it is), that case
> should be merged with the one above it and PHY_INTERFACE_MODE_RGMII_RXID
> added as well.
> 

I can do a single patch.

The reason I made two patches was that it was clear what this patch
does, i.e.: do not apply the delay at MAC level, and what the subsequent
patch does, i.e.: handle all RGMII declinations.

Best regards,

Sebastian

^ permalink raw reply

* Re: [PATCH (net.git) 0/3] stmmac: fix PTP support
From: David Miller @ 2016-11-04 15:30 UTC (permalink / raw)
  To: peppe.cavallaro
  Cc: netdev, alexandre.torgue, rayagond, richardcochran, linux-kernel,
	seraphin.bonnaffe
In-Reply-To: <f215077e-e37d-b59d-763b-370ecc06792e@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Fri, 4 Nov 2016 14:53:09 +0100

> the series have some Acked-by, do you prefer a new
> series (I can rebase them if you ask me) or you can keep
> this one? Or you have some advice or issue to warn?

If it's not in an "Action Required" state in patchwork, you
can safely assume it must be resubmitted in order for me to
consider it.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Sebastian Frias @ 2016-11-04 15:29 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Måns Rullgård, David S. Miller, netdev, LKML, Mason
In-Reply-To: <20161104151151.GF3600@lunn.ch>

Hi Andrew,

On 11/04/2016 04:11 PM, Andrew Lunn wrote:
> On Fri, Nov 04, 2016 at 04:02:24PM +0100, Sebastian Frias wrote:
>> The delay can be applied at PHY or MAC level, but since
>> PHY drivers will apply the delay at PHY level when using
>> one of the "internal delay" declinations of RGMII mode
>> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
>> at MAC level causes issues.
>>
>> Signed-off-by: Sebastian Frias <sf84@laposte.net>
>> ---
>>  drivers/net/ethernet/aurora/nb8800.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>> index b59aa35..d2855c9 100644
>> --- a/drivers/net/ethernet/aurora/nb8800.c
>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>> @@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
>>  		break;
>>  
>>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>> -		pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>> +		pad_mode = PAD_MODE_RGMII;
>>  		break;
> 
> How many boards use this Ethernet driver? How many boards are your
> potentially breaking, because they need this delay?
> 

This part is specific to the Tango architecture, as noted by the
function name "nb8800_tangox_init".

Also the register used here is Sigma-specific (i.e.: not related to the
Aurora VLSI MAC, "au-nb8800")

The thing is that without this patch if we set
phy-connection-type="rgmii-txid" on the DT, then both, the PHY and the
MAC, will apply the delay.

Best regards,

Sebastian

> I guess it is a small number, because doesn't it require the PHY is
> also broken, not adding a delay when it should?
> 
>      Andrew
> 

^ permalink raw reply

* Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Måns Rullgård @ 2016-11-04 15:27 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Sebastian Frias, David S. Miller, netdev, LKML, Mason
In-Reply-To: <20161104151151.GF3600@lunn.ch>

Andrew Lunn <andrew@lunn.ch> writes:

> On Fri, Nov 04, 2016 at 04:02:24PM +0100, Sebastian Frias wrote:
>> The delay can be applied at PHY or MAC level, but since
>> PHY drivers will apply the delay at PHY level when using
>> one of the "internal delay" declinations of RGMII mode
>> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
>> at MAC level causes issues.

If this is correct, most of the PHY drivers are broken.

>> Signed-off-by: Sebastian Frias <sf84@laposte.net>
>> ---
>>  drivers/net/ethernet/aurora/nb8800.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>> index b59aa35..d2855c9 100644
>> --- a/drivers/net/ethernet/aurora/nb8800.c
>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>> @@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
>>  		break;
>>  
>>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>> -		pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>> +		pad_mode = PAD_MODE_RGMII;
>>  		break;
>
> How many boards use this Ethernet driver? How many boards are your
> potentially breaking, because they need this delay?
>
> I guess it is a small number, because doesn't it require the PHY is
> also broken, not adding a delay when it should?

What if the PHY doesn't have that option?

-- 
Måns Rullgård

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Måns Rullgård @ 2016-11-04 15:22 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Mason, netdev, Florian Fainelli, Timur Tabi, Sergei Shtylyov,
	Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
	Sebastian Frias
In-Reply-To: <20161104151744.GG3600@lunn.ch>

Andrew Lunn <andrew@lunn.ch> writes:

> On Fri, Nov 04, 2016 at 03:05:00PM +0000, Måns Rullgård wrote:
>> Andrew Lunn <andrew@lunn.ch> writes:
>> 
>> >> > I agree with you. But fixing it is likely to break boards which
>> >> > currently have "rgmii", but actually need the delay in order to work.
>> >> 
>> >> Does the internal delay here refer to the PHY or the MAC?  It's a
>> >> property of the MAC node after all.
>> >
>> > It is the PHY which applies the delay.
>> 
>> Says who?
>
> The source code.

There's source code that disagrees with that.  The Broadcom GENET
driver, for instance.

>>  Some MACs can do it too.
>
> I'm sure they can. But look at the code. Nearly none do, and those
> that do are potentially broken.

Those few drivers that do anything differently based on these values
enable clock delay in the MAC.  That's why I wrote the NB8800 driver the
way I did.

-- 
Måns Rullgård

^ permalink raw reply

* Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Måns Rullgård @ 2016-11-04 15:18 UTC (permalink / raw)
  To: Sebastian Frias; +Cc: David S. Miller, netdev, LKML, Mason, Andrew Lunn
In-Reply-To: <581CA300.1060609@laposte.net>

Sebastian Frias <sf84@laposte.net> writes:

> The delay can be applied at PHY or MAC level, but since
> PHY drivers will apply the delay at PHY level when using
> one of the "internal delay" declinations of RGMII mode
> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
> at MAC level causes issues.

The Broadcom GENET driver does the same thing.

> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
>  drivers/net/ethernet/aurora/nb8800.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index b59aa35..d2855c9 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
>  		break;
>
>  	case PHY_INTERFACE_MODE_RGMII_TXID:
> -		pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
> +		pad_mode = PAD_MODE_RGMII;
>  		break;
>
>  	default:
> -- 
> 1.7.11.2

If this change is correct (and I'm not convinced it is), that case
should be merged with the one above it and PHY_INTERFACE_MODE_RGMII_RXID
added as well.

-- 
Måns Rullgård

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Andrew Lunn @ 2016-11-04 15:17 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Mason, netdev, Florian Fainelli, Timur Tabi, Sergei Shtylyov,
	Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
	Sebastian Frias
In-Reply-To: <yw1xpombjnfn.fsf@unicorn.mansr.com>

On Fri, Nov 04, 2016 at 03:05:00PM +0000, Måns Rullgård wrote:
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> >> > I agree with you. But fixing it is likely to break boards which
> >> > currently have "rgmii", but actually need the delay in order to work.
> >> 
> >> Does the internal delay here refer to the PHY or the MAC?  It's a
> >> property of the MAC node after all.
> >
> > It is the PHY which applies the delay.
> 
> Says who?

The source code.

>  Some MACs can do it too.

I'm sure they can. But look at the code. Nearly none do, and those
that do are potentially broken.
 
> There is also no way to specify the amount of delay required even though
> many devices support more than one value.

Those PHYs that which support variable delays add there own device
tree properties. See for example Micrel KSZ9021/KSZ9031 Gigabit Ethernet PHY

     Andrew

^ permalink raw reply

* Re: [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Andrew Lunn @ 2016-11-04 15:11 UTC (permalink / raw)
  To: Sebastian Frias
  Cc: Måns Rullgård, David S. Miller, netdev, LKML, Mason
In-Reply-To: <581CA300.1060609@laposte.net>

On Fri, Nov 04, 2016 at 04:02:24PM +0100, Sebastian Frias wrote:
> The delay can be applied at PHY or MAC level, but since
> PHY drivers will apply the delay at PHY level when using
> one of the "internal delay" declinations of RGMII mode
> (like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
> at MAC level causes issues.
> 
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
>  drivers/net/ethernet/aurora/nb8800.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index b59aa35..d2855c9 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
>  		break;
>  
>  	case PHY_INTERFACE_MODE_RGMII_TXID:
> -		pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
> +		pad_mode = PAD_MODE_RGMII;
>  		break;

How many boards use this Ethernet driver? How many boards are your
potentially breaking, because they need this delay?

I guess it is a small number, because doesn't it require the PHY is
also broken, not adding a delay when it should?

     Andrew

^ permalink raw reply

* Re: Coding Style: Reverse XMAS tree declarations ?
From: David Miller @ 2016-11-04 15:07 UTC (permalink / raw)
  To: lsanfil
  Cc: joe, madalin.bucur, akpm, corbet, netdev, linuxppc-dev,
	linux-kernel, oss, ppc, pebolle, joakim.tjernlund
In-Reply-To: <581C6A7D.8030704@marvell.com>

From: Lino Sanfilippo <lsanfil@marvell.com>
Date: Fri, 4 Nov 2016 12:01:17 +0100

> Hi,
> 
> On 04.11.2016 07:53, Joe Perches wrote:
>>
>> CHECK:REVERSE_XMAS_TREE: Prefer ordering declarations longest to
>> shortest
>> #446: FILE: drivers/net/ethernet/ethoc.c:446:
>> +			int size = bd.stat >> 16;
>> +			struct sk_buff *skb;
>>
> 
> should not this case be valid? Optically the longer line is already
> before the shorter.
> I think that the whole point in using this reverse xmas tree ordering
> is to have
> the code optically tidied up and not to enforce ordering between
> variable name lengths.

That's correct.

^ permalink raw reply

* [PATCH 2/2] net: ethernet: nb8800: handle all RGMII declinations
From: Sebastian Frias @ 2016-11-04 15:05 UTC (permalink / raw)
  To: Måns Rullgård, David S. Miller, netdev; +Cc: LKML, Mason, Andrew Lunn

Commit a999589ccaae ("phylib: add RGMII-ID interface mode definition")
and commit 7d400a4c5897 ("phylib: add PHY interface modes for internal
delay for tx and rx only") added several RGMII declinations:
PHY_INTERFACE_MODE_RGMII_ID, PHY_INTERFACE_MODE_RGMII_RXID and
PHY_INTERFACE_MODE_RGMII_TXID to deal with internal delays.

Those are all RGMII modes (1Gbit) and must be considered that way when
setting the MAC Mode or the Pads Mode for the HW to work properly.

Signed-off-by: Sebastian Frias <sf84@laposte.net>
---
 drivers/net/ethernet/aurora/nb8800.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index d2855c9..6230ace 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -609,7 +609,10 @@ static void nb8800_mac_config(struct net_device *dev)
 		mac_mode |= HALF_DUPLEX;
 
 	if (gigabit) {
-		if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
+		if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII ||
+		    priv->phy_mode == PHY_INTERFACE_MODE_RGMII_ID ||
+		    priv->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID ||
+		    priv->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
 			mac_mode |= RGMII_MODE;
 
 		mac_mode |= GMAC_MODE;
@@ -1278,9 +1281,8 @@ static int nb8800_tangox_init(struct net_device *dev)
 		break;
 
 	case PHY_INTERFACE_MODE_RGMII:
-		pad_mode = PAD_MODE_RGMII;
-		break;
-
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		pad_mode = PAD_MODE_RGMII;
 		break;
-- 
1.7.11.2

^ permalink raw reply related

* Re: Ethernet not working on a different SoC with same eth HW
From: Måns Rullgård @ 2016-11-04 15:05 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Mason, netdev, Florian Fainelli, Timur Tabi, Sergei Shtylyov,
	Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
	Sebastian Frias
In-Reply-To: <20161104142223.GD3600@lunn.ch>

Andrew Lunn <andrew@lunn.ch> writes:

>> > I agree with you. But fixing it is likely to break boards which
>> > currently have "rgmii", but actually need the delay in order to work.
>> 
>> Does the internal delay here refer to the PHY or the MAC?  It's a
>> property of the MAC node after all.
>
> It is the PHY which applies the delay.

Says who?  Some MACs can do it too.

> The phy-mode property is in the MAC part of the device tree binding,
> but the delay is not the primary purpose of this property. Its primary
> purpose, RGMII, RMII, SGMII, etc, is applicable to both the MAC and
> the PHY. It probably would of been better to have the delays as a
> separate property, but that is not how it is...

The connection type is obviously the same at both ends, so it doesn't
really matter where that is specified.  The delay can be applied at
either end or not at all, and it's anything but clear what the
properties are supposed to mean.

There is also no way to specify the amount of delay required even though
many devices support more than one value.

-- 
Måns Rullgård

^ permalink raw reply

* [PATCH 1/2] net: ethernet: nb8800: Do not apply TX delay at MAC level
From: Sebastian Frias @ 2016-11-04 15:02 UTC (permalink / raw)
  To: Måns Rullgård, David S. Miller, netdev; +Cc: LKML, Mason, Andrew Lunn

The delay can be applied at PHY or MAC level, but since
PHY drivers will apply the delay at PHY level when using
one of the "internal delay" declinations of RGMII mode
(like PHY_INTERFACE_MODE_RGMII_TXID), applying it again
at MAC level causes issues.

Signed-off-by: Sebastian Frias <sf84@laposte.net>
---
 drivers/net/ethernet/aurora/nb8800.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index b59aa35..d2855c9 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1282,7 +1282,7 @@ static int nb8800_tangox_init(struct net_device *dev)
 		break;
 
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
+		pad_mode = PAD_MODE_RGMII;
 		break;
 
 	default:
-- 
1.7.11.2

^ permalink raw reply related

* [PATCH net] fib_trie: correct /proc/net/route for large read buffer
From: Jason Baron @ 2016-11-04 14:45 UTC (permalink / raw)
  To: davem; +Cc: netdev, Andy Whitcroft, Alexander Duyck

From: Jason Baron <jbaron@akamai.com>

When read() is called on /proc/net/route requesting a size that is one
entry size (128 bytes) less than m->size or greater, the resulting output
has missing and/or duplicate entries. Since m->size is typically PAGE_SIZE,
for a PAGE_SIZE of 4,096 this means that reads requesting more than 3,968
bytes will see bogus output.

For example:

for i in {100..200}; do
	ip route add 192.168.1.$i dev eth0
done
dd if=/proc/net/route of=/tmp/good bs=1024
dd if=/proc/net/route of=/tmp/bad bs=4096

# diff -q /tmp/good /tmp/bad
Files /tmp/good and /tmp/bad differ

I think this has gone unnoticed, since the output of 'netstat -r' and
'route' is generated by reading in 1,024 byte increments and thus not
corrupted. Further, the number of entries in the route table needs to be
sufficiently large in order to trigger the problematic case.

The issue arises because fib_route_get_idx() does not properly handle
the case where pos equals iter->pos. This case only arises when we have
a large read buffer size because we end up re-requesting the last entry
that overflowed m->buf. In the case of a smaller read buffer size,
we don't exceed the size of m->buf, and thus fib_route_get_idx() is called
with pos greater than iter->pos.

Fix by properly handling the iter->pos == pos case.

Fixes: 25b97c016b26 ("ipv4: off-by-one in continuation handling in /proc/net/route")
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 net/ipv4/fib_trie.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 31cef3602585..1017533fc75c 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2411,12 +2411,17 @@ static struct key_vector *fib_route_get_idx(struct fib_route_iter *iter,
 					    loff_t pos)
 {
 	struct key_vector *l, **tp = &iter->tnode;
+	loff_t saved_pos = 0;
 	t_key key;
 
 	/* use cache location of next-to-find key */
 	if (iter->pos > 0 && pos >= iter->pos) {
 		pos -= iter->pos;
 		key = iter->key;
+		if (pos == 0) {
+			saved_pos = iter->pos;
+			key--;
+		}
 	} else {
 		iter->pos = 0;
 		key = 0;
@@ -2436,10 +2441,13 @@ static struct key_vector *fib_route_get_idx(struct fib_route_iter *iter,
 			break;
 	}
 
-	if (l)
+	if (l) {
 		iter->key = key;	/* remember it */
-	else
+		if (saved_pos)
+			iter->pos = saved_pos;
+	} else {
 		iter->pos = 0;		/* forget it */
+	}
 
 	return l;
 }
-- 
2.6.1

^ permalink raw reply related

* Re: Ethernet not working on a different SoC with same eth HW
From: Sebastian Frias @ 2016-11-04 14:01 UTC (permalink / raw)
  To: Andrew Lunn, Mason
  Cc: Mans Rullgard, netdev, Florian Fainelli, Timur Tabi,
	Sergei Shtylyov, Zefir Kurtisi, Martin Blumenstingl,
	Uwe Kleine-Konig, Daniel Mack
In-Reply-To: <20161104135752.GC3600@lunn.ch>

On 11/04/2016 02:57 PM, Andrew Lunn wrote:
>> Considering the ethernet DT bindings:
>>
>> https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt
>>
>> Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid".
>>
>> Assuming that "rxid" (rx internal delay) and "rx clock delay" are
>> in fact the same concept with different names, do you agree that
>> it would be unexpected for "rgmii rx clock delay" to be enabled
>> when a DTB specifies "rgmii" or "rgmii-txid" ?
> 
> I agree with you. But fixing it is likely to break boards which
> currently have "rgmii", but actually need the delay in order to work.
> 
> 	  Andrew
> 

I think you are right, if I disable the RX delay on 'drivers/net/phy/at803x.c'
I get RX errors when using the 'drivers/net/ethernet/aurora/nb8800.c' driver,
errors reported in function nb8800_poll()

if (IS_RX_ERROR(rxd->report))
{
   nb8800_rx_error(dev, rxd->report);
}

(this is on the same board Mason is discussing about)

Another thing to note is that 'drivers/net/ethernet/aurora/nb8800.c' is
currently checking PHY_INTERFACE_MODE_RGMII_TXID to add a TX clock output
delay.

Since a PHY, like the 'drivers/net/phy/at803x.c', will setup a TX delay if
phy-connection-type="rgmii-txid" on DT, the code in 'nb8800.c' may be adding
an additional (and possibly unwanted) delay, right?

Actually, I have a patch for 'nb8800.c' to remove the "additional" TX delay,
and I did not see regressions with it. I can post the patch as RFC if
you want.

One more thing, the "tx" and "rx" concepts are in reference to what?
Is "tx" at PHY driver to be matched by "rx" at Ethernet driver?

Best regards,

Sebastian

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Andrew Lunn @ 2016-11-04 14:22 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Mason, netdev, Florian Fainelli, Timur Tabi, Sergei Shtylyov,
	Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
	Sebastian Frias
In-Reply-To: <yw1xtwbnjq88.fsf@unicorn.mansr.com>

> > I agree with you. But fixing it is likely to break boards which
> > currently have "rgmii", but actually need the delay in order to work.
> 
> Does the internal delay here refer to the PHY or the MAC?  It's a
> property of the MAC node after all.

It is the PHY which applies the delay.

The phy-mode property is in the MAC part of the device tree binding,
but the delay is not the primary purpose of this property. Its primary
purpose, RGMII, RMII, SGMII, etc, is applicable to both the MAC and
the PHY. It probably would of been better to have the delays as a
separate property, but that is not how it is...

	 Andrew

^ permalink raw reply

* Re: [PATCH net-next v4 3/9] ipv6: sr: add support for SRH encapsulation and injection with lwtunnels
From: Thomas Graf @ 2016-11-04 14:21 UTC (permalink / raw)
  To: David Lebrun; +Cc: netdev
In-Reply-To: <1478255388-32213-4-git-send-email-david.lebrun@uclouvain.be>

On 11/04/16 at 11:29am, David Lebrun wrote:
> +/* insert an SRH within an IPv6 packet, just after the IPv6 header */
> +static int seg6_do_srh_inline(struct sk_buff *skb, struct ipv6_sr_hdr *osrh)
> +{
> +	struct ipv6hdr *hdr, *oldhdr;
> +	struct ipv6_sr_hdr *isrh;
> +	int hdrlen, err;
> +
> +	hdrlen = (osrh->hdrlen + 1) << 3;
> +
> +	err = pskb_expand_head(skb, hdrlen, 0, GFP_ATOMIC);
> +	if (unlikely(err))
> +		return err;
> +
> +	oldhdr = ipv6_hdr(skb);
> +
> +	skb_pull(skb, sizeof(struct ipv6hdr));
> +	skb_postpull_rcsum(skb, skb_network_header(skb),
> +			   sizeof(struct ipv6hdr));
> +
> +	skb_push(skb, sizeof(struct ipv6hdr) + hdrlen);
> +	skb_reset_network_header(skb);
> +	skb_mac_header_rebuild(skb);
> +
> +	hdr = ipv6_hdr(skb);
> +
> +	memmove(hdr, oldhdr, sizeof(*hdr));
> +
> +	isrh = (void *)hdr + sizeof(*hdr);
> +	memcpy(isrh, osrh, hdrlen);
> +
> +	isrh->nexthdr = hdr->nexthdr;
> +	hdr->nexthdr = NEXTHDR_ROUTING;
> +
> +	isrh->segments[0] = hdr->daddr;
> +	hdr->daddr = isrh->segments[isrh->first_segment];

Where do you verify that isrh->first_segment is not out of bounds?

> +	skb_postpush_rcsum(skb, hdr, sizeof(struct ipv6hdr) + hdrlen);
> +
> +	return 0;
> +}
> +
> +
> +static int seg6_build_state(struct net_device *dev, struct nlattr *nla,
> +			    unsigned int family, const void *cfg,
> +			    struct lwtunnel_state **ts)
> +{
> +	struct nlattr *tb[SEG6_IPTUNNEL_MAX + 1];
> +	struct seg6_iptunnel_encap *tuninfo;
> +	struct lwtunnel_state *newts;
> +	struct seg6_lwt *slwt;
> +	int tuninfo_len;
> +	int err;
> +
> +	err = nla_parse_nested(tb, SEG6_IPTUNNEL_MAX, nla,
> +			       seg6_iptunnel_policy);
> +
> +	if (err < 0)
> +		return err;
> +
> +	if (!tb[SEG6_IPTUNNEL_SRH])
> +		return -EINVAL;
> +
> +	tuninfo = nla_data(tb[SEG6_IPTUNNEL_SRH]);
> +	tuninfo_len = SEG6_IPTUN_ENCAP_SIZE(tuninfo);

Nothing guarantees the size of the Netlink attribute right now. You
need to add a minimal size requirement to seg6_iptunnel_policy and
then check that the additional len provided in the struct itself does
not exceed the Netlink attribute length.

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Mason @ 2016-11-04 14:13 UTC (permalink / raw)
  To: Mans Rullgard, Andrew Lunn
  Cc: netdev, Florian Fainelli, Timur Tabi, Sergei Shtylyov,
	Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
	Sebastian Frias
In-Reply-To: <yw1xtwbnjq88.fsf@unicorn.mansr.com>

On 04/11/2016 15:04, Måns Rullgård wrote:

> Andrew Lunn <andrew@lunn.ch> writes:
> 
>>> Considering the ethernet DT bindings:
>>>
>>> https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt
>>>
>>> Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid".
>>>
>>> Assuming that "rxid" (rx internal delay) and "rx clock delay" are
>>> in fact the same concept with different names, do you agree that
>>> it would be unexpected for "rgmii rx clock delay" to be enabled
>>> when a DTB specifies "rgmii" or "rgmii-txid" ?
>>
>> I agree with you. But fixing it is likely to break boards which
>> currently have "rgmii", but actually need the delay in order to work.
> 
> Does the internal delay here refer to the PHY or the MAC?  It's a
> property of the MAC node after all.

Isn't there a one-to-one correspondence between MAC and PHY?

Can a MAC be connected to multiple PHYs?
Can a PHY be connected to multiple MACs?

If it is a one-to-one correspondence, then whether the
property is specified in the MAC or PHY node is just a
matter of presentation, IMHO.

Regards.

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Måns Rullgård @ 2016-11-04 14:04 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Mason, netdev, Florian Fainelli, Timur Tabi, Sergei Shtylyov,
	Zefir Kurtisi, Martin Blumenstingl, Uwe Kleine-Konig, Daniel Mack,
	Sebastian Frias
In-Reply-To: <20161104135752.GC3600@lunn.ch>

Andrew Lunn <andrew@lunn.ch> writes:

>> Considering the ethernet DT bindings:
>> 
>> https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt
>> 
>> Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid".
>> 
>> Assuming that "rxid" (rx internal delay) and "rx clock delay" are
>> in fact the same concept with different names, do you agree that
>> it would be unexpected for "rgmii rx clock delay" to be enabled
>> when a DTB specifies "rgmii" or "rgmii-txid" ?
>
> I agree with you. But fixing it is likely to break boards which
> currently have "rgmii", but actually need the delay in order to work.

Does the internal delay here refer to the PHY or the MAC?  It's a
property of the MAC node after all.

-- 
Måns Rullgård

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Andrew Lunn @ 2016-11-04 13:57 UTC (permalink / raw)
  To: Mason
  Cc: Mans Rullgard, netdev, Florian Fainelli, Timur Tabi,
	Sergei Shtylyov, Zefir Kurtisi, Martin Blumenstingl,
	Uwe Kleine-Konig, Daniel Mack, Sebastian Frias
In-Reply-To: <581C9273.906@free.fr>

> Considering the ethernet DT bindings:
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt
> 
> Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid".
> 
> Assuming that "rxid" (rx internal delay) and "rx clock delay" are
> in fact the same concept with different names, do you agree that
> it would be unexpected for "rgmii rx clock delay" to be enabled
> when a DTB specifies "rgmii" or "rgmii-txid" ?

I agree with you. But fixing it is likely to break boards which
currently have "rgmii", but actually need the delay in order to work.

	  Andrew

^ permalink raw reply

* Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower
From: Jes Sorensen @ 2016-11-04 13:56 UTC (permalink / raw)
  To: Joe Perches
  Cc: John Heenan, Kalle Valo, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478162497.1924.16.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> writes:
> On Sun, 2016-10-30 at 19:02 -0400, Jes Sorensen wrote:
>> Code is 80 characters wide, and comments are /* */ never the ugly C++
>> crap.
>
> You might look at the recent Linus Torvalds authored commit
> 5e467652ffef (?printk: re-organize log_output() to be more legible")
> which does both of those: c99 // comments and > 80 columns.
>
> Absolutes are for zealots.

What Linus does in his code, is totally up to him. What I pull into the
driver that *I* maintain, is up to me. It is perfectly normal to expect
submitters to respect the coding style of the piece of code they are
trying to edit.

Jes

^ permalink raw reply

* Re: [PATCH net-next 5/5] net: phy: Add downshift get/set support in Microsemi PHYs driver
From: Andrew Lunn @ 2016-11-04 13:55 UTC (permalink / raw)
  To: Allan W. Nielsen; +Cc: netdev, f.fainelli, raju.lakkaraju, cphealy, robh
In-Reply-To: <20161104134234.GC11277@microsemi.com>

On Fri, Nov 04, 2016 at 02:42:34PM +0100, Allan W. Nielsen wrote:
> 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???

Yes. The other option would be to return ERANGE when 1 is asked
for. The real question is, which is better for the user? Returning
ERANGE and letting the user make a guessing game to figure out what is
valid, or magically turn 1 into 2. I will let you decide which is
best.

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

No. ERANGE. The user has to expect some upper limit, and ERANGE is a
good indication they have reached it. But having a lowered limit of 2
is less obvious.

   Andrew

^ permalink raw reply

* Re: [PATCH (net.git) 0/3] stmmac: fix PTP support
From: Giuseppe CAVALLARO @ 2016-11-04 13:53 UTC (permalink / raw)
  To: netdev, David Miller
  Cc: alexandre.torgue, rayagond, richardcochran, linux-kernel,
	seraphin.bonnaffe
In-Reply-To: <1477464964-5960-1-git-send-email-peppe.cavallaro@st.com>

Hi David,

the series have some Acked-by, do you prefer a new
series (I can rebase them if you ask me) or you can keep
this one? Or you have some advice or issue to warn?

Regards,
peppe

On 10/26/2016 8:56 AM, Giuseppe Cavallaro wrote:
> This subset of patches aim to fix the PTP support
> for the stmmac and especially for 4.x chip series.
> While setting PTP on an ST box with 4.00a Ethernet
> core, the kernel panics due to a broken settings
> of the descriptors. The patches review the
> register configuration, the algo used for configuring
> the protocol, the way to get the timestamp inside
> the RX/TX descriptors and, in the end, the statistics
> displayed by ethtool.
>
> Giuseppe Cavallaro (3):
>   stmmac: update the PTP header file
>   stmmac: fix PTP support for GMAC4
>   stmmac: fix PTP type ethtool stats
>
>  drivers/net/ethernet/stmicro/stmmac/common.h       | 24 +++---
>  drivers/net/ethernet/stmicro/stmmac/descs.h        | 20 +++--
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 95 ++++++++++++++++-----
>  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h |  4 +
>  drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 28 ++++---
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h       |  1 +
>  .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 19 +++--
>  .../net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c  | 43 ++++++++--
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 97 +++++++++++-----------
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c   |  9 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h   | 72 ++++++++--------
>  11 files changed, 260 insertions(+), 152 deletions(-)
>

^ permalink raw reply

* Re: Ethernet not working on a different SoC with same eth HW
From: Mason @ 2016-11-04 13:51 UTC (permalink / raw)
  To: Mans Rullgard
  Cc: netdev, Andrew Lunn, Florian Fainelli, Timur Tabi,
	Sergei Shtylyov, Zefir Kurtisi, Martin Blumenstingl,
	Uwe Kleine-Konig, Daniel Mack, Sebastian Frias
In-Reply-To: <yw1xy40zjrcw.fsf@unicorn.mansr.com>

On 04/11/2016 14:40, Måns Rullgård wrote:

> Mason <slash.tmp@free.fr> writes:
> 
>> On 31/10/2016 17:28, Mason wrote:
>>
>>> On 31/10/2016 16:53, Andrew Lunn wrote:
>>>
>>>>> I'll add a log for the request_irq call.
>>>>
>>>> And take a look at /proc/interrupts
>>>
>>> You're right, there does seem to be something wrong with the interrupts.
>>
>> Having fixed that, I'm still unable to ping a box on the same
>> ethernet segment... Still investigating.
>>
>> I think I may have spotted a potential issue in the Atheros driver.
>>
>> 	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
>> 	    phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>> 		ret = at803x_enable_rx_delay(phydev);
>> 		if (ret < 0)
>> 			return ret;
>> 	}
>>
>> Looking at this code, one might believe that "rgmii rx clock delay"
>> is only enabled when the user requests it (through DT).
>>
>> http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
>> cf. PDF page 48
>>
>> *However* this bit is set to 1 at reset (both HW and SW resets).
>> Thus, "rgmii rx clock delay" is always enabled, whether the user
>> requests it or not.
>>
>> Could someone knowledgeable comment on the expected behavior of
>> enabling rgmii rx (and tx) clock delay?
> 
> Clock delay is sometimes (depending on PCB layout) required to achieve
> the correct timing between clock and data signals.  The delay can be
> applied at the MAC or the PHY.  I'd start by finding out what the PCB
> design expects or check the signals with a fast oscilloscope if you have
> one.

Considering the ethernet DT bindings:

https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt

Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid".

Assuming that "rxid" (rx internal delay) and "rx clock delay" are
in fact the same concept with different names, do you agree that
it would be unexpected for "rgmii rx clock delay" to be enabled
when a DTB specifies "rgmii" or "rgmii-txid" ?

Regards.

^ permalink raw reply

* Re: [PATCH net] r8152: Fix broken RX checksums.
From: Mark Lord @ 2016-11-04 13:50 UTC (permalink / raw)
  To: Hayes Wang, David Miller
  Cc: nic_swsd, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ddbc14e2-dfb5-01f2-0d5b-5b409a01160f@pobox.com>

Yeah, the device or driver is definitely getting confused with rx_desc structures.
I added code to check for unlikely rx_desc values, and it found this for starters:

rx_desc: 00480801 00480401 00480001 0048fc00 0048f800 0048f400 pkt_len=2045
rx_data: 00 f0 48 00 00 ec 48 00 00 e8 48 00 00 e4 48 00 00 e0 48 00 00 dc 48 00 00 d8 48 00 00 d4 48 00
rx_data: 00 d0 48 00 00 cc 48 00 00 c8 48 00 00 c4 48 00 00 c0 48 00 00 bc 48 00 00 b8 48 00 00 b4 48 00
rx_data: 00 b0 48 00 00 ac 48 00 00 01 00 00 81 ed 00 00 00 01 00 00 00 00 00 00 00 00 00 02 4d ac 00 00
rx_data: 10 00 ff ff ff ff 00 00 01 28 83 d6 ff 6d 00 20 25 b1 58 1b 68 ff 00 05 20 01 56 41 17 35 00 00
...

The MTU/MRU on this link is the standard 1500 bytes, so a pkt_len of 2045 isn't valid here.
And the rx_desc values look an awful lot like the rx_data values that follow it.

There's definitely more broken here than just TCP RX checksums.

-ml

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox