* Re: [PATCH RFC] solos-pci: Fix BUG() with shared skb
From: David Miller @ 2013-09-04 18:30 UTC (permalink / raw)
To: dwmw2; +Cc: netdev, simon, linux-atm-general, nathan
In-Reply-To: <1378223125.4210.11.camel@i7.infradead.org>
From: David Woodhouse <dwmw2@infradead.org>
Date: Tue, 03 Sep 2013 16:45:25 +0100
> Simon reported this BUG():
>
> kernel BUG at net/core/skbuff.c:1065!
> Call Trace:
> [<f9b7c12c>] ? pppoatm_send+0x3f/0x1a0 [pppoatm]
> [<f8751797>] psend+0xa9/0x14a [solos_pci]
> [<f9b7c248>] pppoatm_send+0x15b/0x1a0 [pppoatm]
> [<f8a2f77d>] ppp_push+0x76/0x533 [ppp_generic]
>
> (Rest of backtrace at http://s85.org/mn0aOxMN ― the skb appears to be
> IPv6, forwarded from another interface over PPPoATM.)
>
> I wasn't expecting to see shared skbs in the ATM driver's ->send()
> function. Is this the right fix?
skb_realloc_headroom() should do everything you need.
This is what ethernet drivers do to prepend custom headers
when skb_headroom() is not large enough.
For example, see drivers/net/ethernet/sun/niu.c:niu_start_xmit().
There, the driver is attempting to prepend a TX descriptor to the
SKB.
If the SKB is shared, skb_realloc_headroom() will do the clone
for you if necessary.
^ permalink raw reply
* [PATCH] icplus: Use netif_running to determine device state
From: Jon Mason @ 2013-09-04 18:26 UTC (permalink / raw)
To: netdev; +Cc: Francois Romieu, Sorbica Shieh
Remove the __LINK_STATE_START check to verify the device is running, in
favor of netif_running(). netif_running() performs the same check of
__LINK_STATE_START, so the code should behave the same.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Sorbica Shieh <sorbica@icplus.com.tw>
---
drivers/net/ethernet/icplus/ipg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/icplus/ipg.c b/drivers/net/ethernet/icplus/ipg.c
index 1fde90b..bdf5023 100644
--- a/drivers/net/ethernet/icplus/ipg.c
+++ b/drivers/net/ethernet/icplus/ipg.c
@@ -1004,7 +1004,7 @@ static struct net_device_stats *ipg_nic_get_stats(struct net_device *dev)
/* Check to see if the NIC has been initialized via nic_open,
* before trying to read statistic registers.
*/
- if (!test_bit(__LINK_STATE_START, &dev->state))
+ if (!netif_running(dev))
return &sp->stats;
sp->stats.rx_packets += ipg_r32(IPG_FRAMESRCVDOK);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] net: fec: fix the error to get the previous BD entry
From: David Miller @ 2013-09-04 18:13 UTC (permalink / raw)
To: B38611; +Cc: b20596, netdev, bhutchings, stephen
In-Reply-To: <1378176078-10380-1-git-send-email-B38611@freescale.com>
From: Fugang Duan <B38611@freescale.com>
Date: Tue, 3 Sep 2013 10:41:18 +0800
> Bug: error to get the previous BD entry. When the current BD
> is the first BD, the previous BD entry must be the last BD,
> not "bdp - 1" in current logic.
...
> Reviewed-by: Li Frank <B20596@freescale.com>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
Applied.
^ permalink raw reply
* Re: [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
From: David Miller @ 2013-09-04 18:13 UTC (permalink / raw)
To: hannes; +Cc: michele, netdev, yoshfuji, sasha.levin
In-Reply-To: <20130903001331.GA8820@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 3 Sep 2013 02:13:31 +0200
> Hi!
>
> On Mon, Sep 02, 2013 at 10:31:28PM +0100, Michele Baldessari wrote:
>> with the latest linux master git tree from Linus
>> (248d296d6d9df384996c2ed95676b367d876d48c - 2 Sep) I can reproduceably oops
>> the kernel with the following commands:
>> ip addrlabel flush
>> ip addrlabel add prefix ::1/128 label 0
>> ip addrlabel add prefix ::/0 label 1
>
> Thanks for the report! This patch should fix this issue:
>
> [PATCH] ipv6: fix null pointer dereference in __ip6addrlbl_add
>
> Commit b67bfe0d42cac56c512dd5da4b1b347a23f4b70a ("hlist: drop
> the node parameter from iterators") changed the behavior of
> hlist_for_each_entry_safe to leave the p argument NULL.
>
> Fix this up by tracking the last argument.
>
> Reported-by: Michele Baldessari <michele@acksyn.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied.
^ permalink raw reply
* Re: [PATCH] sh_eth: NAPI requires netif_receive_skb()
From: David Miller @ 2013-09-04 18:12 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, nobuhiro.iwamatsu.yj, linux-sh
In-Reply-To: <201309030303.11381.sergei.shtylyov@cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Tue, 3 Sep 2013 03:03:10 +0400
> Driver supporting NAPI should use NAPI-specific function for receiving packets,
> so netif_rx() should be changed to netif_receive_skb().
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied.
^ permalink raw reply
* Re: merge window warning
From: David Miller @ 2013-09-04 18:05 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, linux-wireless, netfilter-devel
In-Reply-To: <CAL3LdT5s=8E123aR8rwCZjjGUjv3jTzyJxX3fzu56kge6PHh2g@mail.gmail.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 4 Sep 2013 10:40:45 -0700
> Would Jesse's i40e series be acceptable if I push today, since 4
> versions have been sent previously for review? I ask, because I was
> in the process of submitting v5 and want to make sure it is ok before
> submitting.
It's a new driver, so it should be OK.
^ permalink raw reply
* Re: [-next] openvswitch BUILD_BUG_ON failed
From: Jesse Gross @ 2013-09-04 17:53 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: David Miller, Andy Zhou, dev@openvswitch.org, netdev,
Linux Kernel Mailing List, Linux-Next
In-Reply-To: <CAMuHMdVvB+GMwMrP2mieXbqAM7mn3nF_1ezbiYVBLKdsL5aCJg@mail.gmail.com>
On Tue, Sep 3, 2013 at 11:55 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, Sep 3, 2013 at 11:44 PM, Jesse Gross <jesse@nicira.com> wrote:
>> On Sat, Aug 31, 2013 at 5:11 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross <jesse@nicira.com> wrote:
>>>> On Thu, Aug 29, 2013 at 3:10 PM, David Miller <davem@davemloft.net> wrote:
>>>>> From: Jesse Gross <jesse@nicira.com>
>>>>> Date: Thu, 29 Aug 2013 14:42:22 -0700
>>>>>
>>>>>> On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven
>>>>>> <geert@linux-m68k.org> wrote:
>>>>>>> However, I have some doubts about other alignment "enforcements":
>>>>>>>
>>>>>>> "__aligned(__alignof__(long))" makes the whole struct aligned to the
>>>>>>> alignment rule for "long":
>>>>>>> 1. This is only 2 bytes on m68k, i.e. != sizeof(long).
>>>>>>> 2. This is 4 bytes on many 32-bit platforms, which may be less than the
>>>>>>> default alignment for "__be64" (cfr. some members of struct
>>>>>>> ovs_key_ipv4_tunnel), so this may make those 64-bit members unaligned.
>>>>>>
>>>>>> Do any of those 32-bit architectures actually care about alignment of
>>>>>> 64 bit values? On 32-bit x86, a long is 32 bits but the alignment
>>>>>> requirement of __be64 is also 32 bit.
>>>>>
>>>>> All except x86-32 do, it is in fact the odd man out with respect to this
>>>>> issue.
>>>>
>>>> Thanks, good to know.
>>>>
>>>> Andy, do you want to modify your patch to just drop the alignment
>>>> specification as Geert suggested (but definitely keep the new build
>>>> assert that you added)? It's probably better to just send the patch to
>>>> netdev (against net-next) as well since you'll likely get better
>>>> comments there and we can fix this faster if you cut out the
>>>> middleman.
>>>
>>> Why do you want to keep the build asserts?
>>> Is this in-memory structure also transfered as-is over the network?
>>> If yes, you definitely want the padding.
>>
>> Well they caught this bug and really don't cost anything.
>>
>>> Nevertheless, as the struct contains u32 and even __be64 members, the
>>> size of the struct will always be a multiple of the alignment unit for
>>> 64-bit quantities (and thus also for long), as per the C standard.
>>> Hence the check
>>>
>>> BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long));
>>>
>>> will only catch bad compiler bugs or people adding __packed to the struct.
>>
>> It's possible that we might want to pack the structure in the future.
>> More generally though, the contents of the struct is really
>> independent of the alignment requirements here because we're accessing
>> it as an array of bytes in long-sized chunks so implicitly depending
>> on the size of the members is not that great.
>
> So you're accessing it as an array of bytes in long-sized chunks.
> What are you doing with this accessed data?
> Transfering over the network?
> Storing on disk?
> Then it must be portable across machines and architectures, right?
It's just an in-memory hash table lookup. No one else ever sees it.
^ permalink raw reply
* Re: linux-next: Tree for Sep 4 (netfilter: xt_TPROXY)
From: Randy Dunlap @ 2013-09-04 17:46 UTC (permalink / raw)
To: Stephen Rothwell
Cc: linux-next, linux-kernel, netfilter-devel, netdev@vger.kernel.org
In-Reply-To: <20130904181300.06fbf0698863cfaacf9fad9a@canb.auug.org.au>
On 09/04/13 01:13, Stephen Rothwell wrote:
> Hi all,
>
> Please do not add any code for v3.13 to your linux-next included branches
> until after v3.12-rc1 is released.
>
> Changes since 20130902:
>
on x86_64:
when CONFIG_IPV6=m
and CONFIG_NETFILTER_XT_TARGET_TPROXY=y:
net/built-in.o: In function `tproxy_tg6_v1':
xt_TPROXY.c:(.text+0x5dc05): undefined reference to `udp6_lib_lookup'
xt_TPROXY.c:(.text+0x5e32f): undefined reference to `udp6_lib_lookup'
xt_TPROXY.c:(.text+0x5e432): undefined reference to `udp6_lib_lookup'
net/built-in.o: In function `tproxy_tg_init':
xt_TPROXY.c:(.init.text+0x1540): undefined reference to `nf_defrag_ipv6_enable'
--
~Randy
^ permalink raw reply
* Re: merge window warning
From: Jeff Kirsher @ 2013-09-04 17:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-wireless, netfilter-devel
In-Reply-To: <20130904.131001.1189384206727310769.davem@davemloft.net>
Would Jesse's i40e series be acceptable if I push today, since 4
versions have been sent previously for review? I ask, because I was
in the process of submitting v5 and want to make sure it is ok before
submitting.
On Wed, Sep 4, 2013 at 10:10 AM, David Miller <davem@davemloft.net> wrote:
>
> The merge window is now open.
>
> This means if a feature patch isn't in patchwork already, and
> completely ready to apply, you should not submit it at this time.
>
> I will just automatically reject any inappropriate patches sent from
> now until the net-next tree opens again after the merge window closes,
> in order to assist with my ability to process the current backlog
> efficiently.
>
> Thanks.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Cheers,
Jeff
^ permalink raw reply
* Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code
From: Keller, Jacob E @ 2013-09-04 17:24 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yijing Wang, Benjamin Herrenschmidt, Gavin Shan,
James E.J. Bottomley, David S. Miller,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
Hanjun Guo, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, Kirsher, Jeffrey T
In-Reply-To: <20130904162022.GE24733@google.com>
On Wed, 2013-09-04 at 10:20 -0600, Bjorn Helgaas wrote:
> [+cc Jacob, Jeff]
>
> On Tue, Sep 03, 2013 at 03:35:13PM +0800, Yijing Wang wrote:
> > use pcie_capability_read_word() to simplify code.
> >
> > Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> > Cc: e1000-devel@lists.sourceforge.net
> > Cc: netdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++----
> > 1 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index bad8f14..bfa0b06 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
> > static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> > u32 reg, u16 *value)
> > {
> > - int pos = 0;
> > struct pci_dev *parent_dev;
> > struct pci_bus *parent_bus;
> >
> > @@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> > if (!parent_dev)
> > return -1;
> >
> > - pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
> > - if (!pos)
> > + if (!pci_is_pcie(parent_dev))
> > return -1;
> >
> > - pci_read_config_word(parent_dev, pos + reg, value);
> > + pcie_capability_read_word(parent_dev, reg, value);
> > return 0;
> > }
> >
>
> Here's the caller of ixgbe_read_pci_cfg_word_parent():
>
> /* Get the negotiated link width and speed from PCI config space of the
> * parent, as this device is behind a switch
> */
> err = ixgbe_read_pci_cfg_word_parent(adapter, 18, &link_status);
>
> This should be using PCI_EXP_LNKSTA instead of "18".
Absolutely.. Not sure why I didn't do this originally.....
>
> But it would be even better if we could drop ixgbe_get_parent_bus_info()
> completely. It seems redundant after merging Jacob's new
> pcie_get_minimum_link() stuff [1].
I don't know if we can fully drop it. We need this in order to read the
parent device on some quad port Ethernet adapters which have an internal
PCIe switch to link two parts together. There are a few places we read
the parent cfg word. At least one for sure.. but maybe others.. Can't
recall. The parent bus info is still used to print out the slot width
and speed. I don't know if it would be better to just print the response
from get_minimum_link or still print the actual slot.
>
> ixgbe_disable_pcie_master() looks like it should be using
> pcie_capability_read_word() with PCI_EXP_DEVSTA instead of using
> IXGBE_PCI_DEVICE_STATUS. If fact, it looks like it could use the
> new pci_wait_for_pending_transaction() interface [2].
>
I can look at doing this. I know it was done this way for historic
reasons, (and likely for code share with non Linux drivers.. but that's
not really an excuse)
> It looks like all the #defines in the "PCI Bus Info" block
> (IXGBE_PCI_DEVICE_STATUS, IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING,
> IXGBE_PCI_LINK_STATUS, etc.) [3] are really for PCIe-generic things. If
> so, the IXGBE-specific ones should be dropped in favor of the generic
> ones.
Agreed.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] net: sctp: Fix data chunk fragmentation for MTU values which are not multiple of 4
From: David Miller @ 2013-09-04 17:20 UTC (permalink / raw)
To: alexander.sverdlin
Cc: linux-sctp, vyasevich, matija.glavinic-pecotic.ext, netdev
In-Reply-To: <52249981.8040006@nsn.com>
From: Alexander Sverdlin <alexander.sverdlin@nsn.com>
Date: Mon, 02 Sep 2013 15:58:25 +0200
> From: Alexander Sverdlin <alexander.sverdlin@nsn.com>
>
> net: sctp: Fix data chunk fragmentation for MTU values which are not multiple of 4
>
> Initially the problem was observed with ipsec, but later it became clear that
> SCTP data chunk fragmentation algorithm has problems with MTU values which are
> not multiple of 4. Test program was used which just transmits 2000 bytes long
> packets to other host. tcpdump was used to observe re-fragmentation in IP layer
> after SCTP already fragmented data chunks.
...
> Here problem in data portion limit calculation leads to re-fragmentation in IP,
> which is sub-optimal. The problem is max_data initial value, which doesn't take
> into account the fact, that data chunk must be padded to 4-bytes boundary.
> It's enough to correct max_data, because all later adjustments are correctly
> aligned to 4-bytes boundary.
>
> After the fix is applied, everything is fragmented correctly for uneven MTUs:
...
> The bug was there for years already, but
> - is a performance issue, the packets are still transmitted
> - doesn't show up with default MTU 1500, but possibly with ipsec (MTU 1438)
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] drivers:net: delete premature free_irq
From: David Miller @ 2013-09-04 17:18 UTC (permalink / raw)
To: Julia.Lawall
Cc: mcuos.com, kernel-janitors, linux-arm-kernel, netdev,
linux-kernel
In-Reply-To: <1378115661-5371-3-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Mon, 2 Sep 2013 11:54:21 +0200
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Free_irq is not needed if there has been no request_irq. Free_irq is
> removed from both the probe and remove functions. The correct request_irq
> and free_irq are found in the open and close functions.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] bnx2x: bail out if unable to acquire stats_sema
From: Neal Cardwell @ 2013-09-04 17:17 UTC (permalink / raw)
To: Dmitry Kravkov
Cc: Michal Schmidt, davem@davemloft.net, netdev@vger.kernel.org,
Ariel Elior, Eilon Greenstein
In-Reply-To: <504C9EFCA2D0054393414C9CB605C37F20D8F504@SJEXCHMB06.corp.ad.broadcom.com>
On Tue, Sep 3, 2013 at 11:51 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> -----Original Message-----
>> From: Michal Schmidt [mailto:mschmidt@redhat.com]
>> Sent: Tuesday, September 03, 2013 6:46 PM
>> To: davem@davemloft.net
>> Cc: netdev@vger.kernel.org; Dmitry Kravkov; Ariel Elior; Eilon Greenstein
>> Subject: [PATCH net] bnx2x: bail out if unable to acquire stats_sema
>>
>> If we fail to acquire stats_sema in the specified time limit, the chip is
>> probably dead. It probably does not matter whether we try to continue or
>> not, but certainly we should not up() the semaphore afterwards.
>>
>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
>> ---
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 24
>> +++++++++++++++++------
It seems like this patch has the downside that if the down_timeout()
fails, then bnx2x_stats_handle() ends up updating the stats state
machine's state without really executing the real body of the
action().
In fact it seems like there is a more general pre-existing problem of
this flavor with the bnx2x stats state machine: the
bnx2x_stats_handle() function updates the state machine
bp->stats_state while holding the spin lock, but does not execute the
action() while holding any sort of synchronization, so AFAICT there is
nothing to guarantee that the state machine actions happen in the
order the state machine wants them to happen. For example, if stats
events fire such that we want to execute actions that disable and then
enable stats, we could instead end up executing the actions in the
order that would attempt to enable and then disable them, if we get
unlucky with respect to when interrupts fire, etc.
It seems to me that instead of having all of the callees of
bnx2x_stats_handle() try to down/up the semaphore, instead
bnx2x_stats_handle() should try to down the stats_sema at the top, and
then if successful, it should change the bp->stats_state, call the
action, and up the stats_sema. Would that work?
neal
^ permalink raw reply
* Re: [Patch net-next v2] net: sync some IP headers with glibc
From: David Miller @ 2013-09-04 17:13 UTC (permalink / raw)
To: amwang; +Cc: netdev, tmb, libc-alpha, yoshfuji, carlos
In-Reply-To: <1376558891-26221-1-git-send-email-amwang@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Thu, 15 Aug 2013 17:28:10 +0800
> Solution:
> =========
>
> - Synchronize linux's `include/uapi/linux/in6.h'
> with glibc's `inet/netinet/in.h'.
> - Synchronize glibc's `inet/netinet/in.h with linux's
> `include/uapi/linux/in6.h'.
> - Allow including the headers in either other.
> - First header included defines the structures and macros.
Applied, thanks for being so patient. I want to spend the past few
weeks making sure this is the right way to handle all of this and
now I am confident that it is.
Thanks!
^ permalink raw reply
* merge window warning
From: David Miller @ 2013-09-04 17:10 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA
The merge window is now open.
This means if a feature patch isn't in patchwork already, and
completely ready to apply, you should not submit it at this time.
I will just automatically reject any inappropriate patches sent from
now until the net-next tree opens again after the merge window closes,
in order to assist with my ability to process the current backlog
efficiently.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch v2] sfc: check for allocation failure
From: David Miller @ 2013-09-04 17:08 UTC (permalink / raw)
To: dan.carpenter; +Cc: linux-net-drivers, bhutchings, netdev, kernel-janitors
In-Reply-To: <20130904150727.GA32327@elgon.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 4 Sep 2013 18:07:27 +0300
> It upsets static analyzers when we don't check for allocation failure.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: rebased on latest linux-next
Applied, thanks Dan.
^ permalink raw reply
* Re: [PATCH] net: mvneta: implement ->ndo_do_ioctl() to support PHY ioctls
From: Gregory CLEMENT @ 2013-09-04 16:45 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, netdev, Ezequiel Garcia, Lior Amsalem,
linux-arm-kernel, Jason Cooper, Andrew Lunn
In-Reply-To: <1378304812-21390-1-git-send-email-thomas.petazzoni@free-electrons.com>
On 04/09/2013 16:26, Thomas Petazzoni wrote:
> This commit implements the ->ndo_do_ioctl() operation so that the
> PHY-related ioctl() calls can work from userspace, which allows
> applications like mii-tool or mii-diag to do their job.
>
I tested it successfully with mii-diag for the Armada 370 on Mirabox
and for the Armada XP on Open Blocks AX3-4
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> drivers/net/ethernet/marvell/mvneta.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 90ab292..389a854 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -2451,6 +2451,21 @@ static int mvneta_stop(struct net_device *dev)
> return 0;
> }
>
> +static int mvneta_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> +{
> + struct mvneta_port *pp = netdev_priv(dev);
> + int ret;
> +
> + if (!pp->phy_dev)
> + return -ENOTSUPP;
> +
> + ret = phy_mii_ioctl(pp->phy_dev, ifr, cmd);
> + if (!ret)
> + mvneta_adjust_link(dev);
> +
> + return ret;
> +}
> +
> /* Ethtool methods */
>
> /* Get settings (phy address, speed) for ethtools */
> @@ -2569,6 +2584,7 @@ static const struct net_device_ops mvneta_netdev_ops = {
> .ndo_change_mtu = mvneta_change_mtu,
> .ndo_tx_timeout = mvneta_tx_timeout,
> .ndo_get_stats64 = mvneta_get_stats64,
> + .ndo_do_ioctl = mvneta_ioctl,
> };
>
> const struct ethtool_ops mvneta_eth_tool_ops = {
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [net-next 0/9][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2013-09-04 16:41 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1378300136-26003-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 4 Sep 2013 06:08:47 -0700
> This series contains updates to igb only.
>
> Todd provides a fix for igb to not look for a PBA in the iNVM on
> devices that are flashless.
>
> Akeem provides igb patches to add a new PHY id for i354, as well as
> a couple of patches to implement the new PHY id. He also provides
> several patches to correctly report the appropriate media type as
> well as correctly report advertised/supported link for i354 devices.
> Lastly Akeem implements a 1 second delay mechanism for i210 devices
> to avoid erroneous link issue with the link partner.
Pulled, thanks a lot Jeff.
^ permalink raw reply
* Re: [PATCH] net: mvneta: properly disable HW PHY polling and ensure adjust_link() works
From: Willy Tarreau @ 2013-09-04 16:32 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Lior Amsalem, Jochen De Smet, Simon Guinot, Ryan Press, netdev,
vdonnefort, Ethan Tuttle, stable, Ezequiel Garcia,
Chény Yves-Gael, Gregory Clement, Peter Sanford,
David S. Miller, linux-arm-kernel
In-Reply-To: <1378304478-21237-1-git-send-email-thomas.petazzoni@free-electrons.com>
Hi Thomas!
On Wed, Sep 04, 2013 at 04:21:18PM +0200, Thomas Petazzoni wrote:
> This commit fixes a long-standing bug that has been reported by many
> users: on some Armada 370 platforms, only the network interface that
> has been used in U-Boot to tftp the kernel works properly in
> Linux. The other network interfaces can see a 'link up', but are
> unable to transmit data. The reports were generally made on the Armada
> 370-based Mirabox, but have also been given on the Armada 370-RD
> board.
(...)
> This patch has been tested on Armada 370 Mirabox, and now both network
> interfaces are usable after boot.
Just as a complementary check, I can also confirm that the OpenBlocks
AX3 continues to work fine after this change.
Best regards!
Willy
^ permalink raw reply
* RE: [PATCH net-next 4/5] driver/net: enic: Exposing symbols for Cisco's low latency driver
From: Christian Benvenuti (benve) @ 2013-09-04 16:32 UTC (permalink / raw)
To: Ben Hutchings, Govindarajulu Varadarajan
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Sujith Sankar (ssujith),
Nishank Trivedi (nistrive), Upinder Malhi (umalhi)
In-Reply-To: <1378306376.3133.7.camel@bwh-desktop.uk.level5networks.com>
Hi Ben,
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Ben Hutchings
> Sent: Wednesday, September 04, 2013 7:53 AM
> To: Govindarajulu Varadarajan
> Cc: davem@davemloft.net; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Christian Benvenuti (benve); Sujith Sankar
> (ssujith); Nishank Trivedi (nistrive); Upinder Malhi (umalhi)
> Subject: Re: [PATCH net-next 4/5] driver/net: enic: Exposing symbols for
> Cisco's low latency driver
>
> On Wed, 2013-09-04 at 11:17 +0530, Govindarajulu Varadarajan wrote:
> > This patch exposes symbols for usnic low latency driver that can be
> > used to register and unregister vNics as well to traverse the resources on
> vNics.
> >
> > Signed-off-by: Upinder Malhi <umalhi@cisco.com>
> > Signed-off-by: Nishank Trivedi <nistrive@cisco.com>
> > Signed-off-by: Christian Benvenuti <benve@cisco.com>
>
> Will usnic, or any other user of these symbols, be submitted for inclusion in-
> tree as well? It is generally expected that exported functions do have an in-
> tree user.
The usnic driver has been posted yesterday on the rdma list.
(http://www.spinics.net/lists/linux-rdma/msg16999.html)
/Chris
> Also, header files under drivers/ generally won't be included in distribution
> -devel packages, so to support an out-of-tree module the function
> prototypes would need to be included in a header under include/ (or else
> you have to repeat them and hope the types never change).
>
> Ben.
>
> > ---
> > drivers/net/ethernet/cisco/enic/vnic_dev.c | 10 ++++++++++
> > drivers/net/ethernet/cisco/enic/vnic_dev.h | 1 +
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c
> > b/drivers/net/ethernet/cisco/enic/vnic_dev.c
> > index 97455c5..69dd925 100644
> > --- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
> > +++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
> > @@ -175,6 +175,7 @@ unsigned int vnic_dev_get_res_count(struct
> > vnic_dev *vdev, {
> > return vdev->res[type].count;
> > }
> > +EXPORT_SYMBOL(vnic_dev_get_res_count);
> >
> > void __iomem *vnic_dev_get_res(struct vnic_dev *vdev, enum
> vnic_res_type type,
> > unsigned int index)
> > @@ -193,6 +194,7 @@ void __iomem *vnic_dev_get_res(struct vnic_dev
> *vdev, enum vnic_res_type type,
> > return (char __iomem *)vdev->res[type].vaddr;
> > }
> > }
> > +EXPORT_SYMBOL(vnic_dev_get_res);
> >
> > static unsigned int vnic_dev_desc_ring_size(struct vnic_dev_ring *ring,
> > unsigned int desc_count, unsigned int desc_size) @@ -942,6 +944,7
> @@
> > void vnic_dev_unregister(struct vnic_dev *vdev)
> > kfree(vdev);
> > }
> > }
> > +EXPORT_SYMBOL(vnic_dev_unregister);
> >
> > struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
> > void *priv, struct pci_dev *pdev, struct vnic_dev_bar *bar, @@
> > -969,6 +972,13 @@ err_out:
> > vnic_dev_unregister(vdev);
> > return NULL;
> > }
> > +EXPORT_SYMBOL(vnic_dev_register);
> > +
> > +struct pci_dev *vnic_dev_get_pdev(struct vnic_dev *vdev) {
> > + return vdev->pdev;
> > +}
> > +EXPORT_SYMBOL(vnic_dev_get_pdev);
> >
> > int vnic_dev_init_prov2(struct vnic_dev *vdev, u8 *buf, u32 len) {
> > diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.h
> > b/drivers/net/ethernet/cisco/enic/vnic_dev.h
> > index f3d9b79..e670029 100644
> > --- a/drivers/net/ethernet/cisco/enic/vnic_dev.h
> > +++ b/drivers/net/ethernet/cisco/enic/vnic_dev.h
> > @@ -127,6 +127,7 @@ int vnic_dev_set_ig_vlan_rewrite_mode(struct
> > vnic_dev *vdev, struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev,
> > void *priv, struct pci_dev *pdev, struct vnic_dev_bar *bar,
> > unsigned int num_bars);
> > +struct pci_dev *vnic_dev_get_pdev(struct vnic_dev *vdev);
> > int vnic_dev_init_prov2(struct vnic_dev *vdev, u8 *buf, u32 len);
> > int vnic_dev_enable2(struct vnic_dev *vdev, int active); int
> > vnic_dev_enable2_done(struct vnic_dev *vdev, int *status);
>
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] bonding: restructure and add rcu for bond_for_each_slave_next()
From: Veaceslav Falico @ 2013-09-04 16:29 UTC (permalink / raw)
To: Ding Tianhong
Cc: Ding Tianhong, Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <52274B84.2010509@gmail.com>
On Wed, Sep 04, 2013 at 11:02:28PM +0800, Ding Tianhong wrote:
>于 2013/9/4 18:35, Veaceslav Falico 写道:
>>On Wed, Sep 04, 2013 at 05:44:15PM +0800, Ding Tianhong wrote:
>>...snip...
>>>+/* Check whether the slave is the only one in bond */
>>>+#define bond_is_only_slave(bond, pos) \
>>>+ (((pos)->list.prev == &(bond)->slave_list) && \
>>>+ ((pos)->list.next == &(bond)->slave_list))
>>
>>Could be done without pos at all -
>>
>>!list_empty(&(bond)->slave_list) && \
>>&(bond)->slave_list.next == &(bond)->slave_list.prev
>>
>>If we have only one slave and pos is NOT our slave then... well.. we have
>>big troubles.
>>
>yes, more simple more beautiful, thanks.
>
>but if the pos is not our slave, it is the mistake, not bug. :)
>
>>>+
>>>/**
>>>* bond_for_each_slave_from - iterate the slaves list from a
>>>starting point
>>>* @bond: the bond holding this list.
>>>* @pos: current slave.
>>>- * @cnt: counter for max number of moves
>>>* @start: starting point.
>>>*
>>>* Caller must hold bond->lock
>>>*/
>>>-#define bond_for_each_slave_from(bond, pos, cnt, start) \
>>>- for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
>>>- cnt++, pos = bond_next_slave(bond, pos))
>>>+#define bond_for_each_slave_from(bond, pos, start) \
>>>+ for (pos = start; pos && (bond_is_only_slave(bond, start) ? \
>>>+ &pos->list != &bond->slave_list : \
>>>+ &pos->list != &start->list); bond_is_only_slave(bond, start) ? \
>>>+ (pos = list_entry(pos->list.next, typeof(*pos), list)) : \
>>>+ (pos = bond_next_slave(bond, pos)))
>>
>>Did you check that?
>>
>>pos = slave1 (bond has more than one slave);
>>pos && &pos->list != &slave1->list - false.
>>
>>We won't ever enter this loop if we have >1 slaves.
>>
>>I don't understand this at all.
>>
>
>ok, the logic is : if slaves == 1, run once for the slave.
your code, actually, says differently:
(bond_is_only_slave(bond, start) ? &pos->list != &bond->slave_list :
which means that if bond_is_only_slave(bond, start) == true then we check
&pos->list != &bond->slave_list, so we run till the end of the list.
Ternary operator works like that
expression ? what_to_do_if_true : what_to_do_if_false
http://en.wikipedia.org/wiki/?:#C
>if slaves > 1, run loops until reach the end list.
Here we test for &pos->list != &start->list, which is false cause pos ==
start.
>I could not get a better way to simplify the function, I think it is
>a suitable scheme.
Nope. bond_for_each_slave_from() is supposed to loop through slaves from a
starting slave and till that starting slave, not included.
Your loop... I don't understand what it does. But clearly not what it was
doing (and supposed to do).
>
>by the way, I test the function and works well. :)
>
>>>+
>>>+/**
>>>+ * bond_for_each_slave_from_rcu - iterate the slaves list from a
>>>starting point
>>>+ * @bond: the bond holding this list.
>>>+ * @pos: current slave.
>>>+ * @start: starting point.
>>>+ *
>>>+ * Caller must hold rcu_read_lock
>>>+ */
>>>+#define bond_for_each_slave_from_rcu(bond, pos, start) \
>>>+ for (pos = start; pos && (bond_is_only_slave(bond, start) ? \
>>>+ &pos->list != &bond->slave_list : \
>>>+ &pos->list != &start->list); bond_is_only_slave(bond, start) ? \
>>>+ (pos = list_entry_rcu(pos->list.next, typeof(*pos), list)) : \
>>>+ (pos = bond_next_slave_rcu(bond, pos)))
>>
>>Ditto as bond_for_each_slave_from() and, also, see my comment about RCU
>>from patch 1.
>>
>>>
>>>/**
>>>* bond_for_each_slave - iterate over all slaves
>>>--
>>>1.8.2.1
>>>
>>>
>>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe netdev" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply
* Re: [PATCH 0/4] netfilter updates for net-next
From: David Miller @ 2013-09-04 16:28 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1378299625-4638-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 4 Sep 2013 15:00:21 +0200
> The following batch contains:
>
> * Three fixes for the new synproxy target available in your
> net-next tree, from Jesper D. Brouer and Patrick McHardy.
>
> * One fix for TCPMSS to correctly handling the fragmentation
> case, from Phil Oester. I'll pass this one to -stable.
Pulled, thanks Pablo.
^ permalink raw reply
* Re: [PATCH net-next v2 1/6] bonding: simplify and use RCU protection for 3ad xmit path
From: David Miller @ 2013-09-04 16:25 UTC (permalink / raw)
To: vfalico; +Cc: dingtianhong, fubar, andy, nikolay, netdev
In-Reply-To: <20130904101823.GO1992@redhat.com>
From: Veaceslav Falico <vfalico@redhat.com>
Date: Wed, 4 Sep 2013 12:18:24 +0200
> On Wed, Sep 04, 2013 at 05:43:45PM +0800, Ding Tianhong wrote:
> ...snip...
>>+/**
>>+ * IMPORTANT: bond_first/last_slave_rcu can return NULL in case of an
>>empty list
>>+ * Caller must hold rcu_read_lock
>>+ */
>>+#define bond_first_slave_rcu(bond) \
>>+ list_first_or_null_rcu(&(bond)->slave_list, struct slave, list)
>>+#define bond_last_slave_rcu(bond) \
>>+ (list_empty(&(bond)->slave_list) ? NULL : \
>>+ bond_to_slave_rcu((bond)->slave_list.prev))
>
> Here, bond_last_slave_rcu() is racy. The list can be non-empty when
> list_empty() is verified, however afterwards it might become empty,
> when
> you call bond_to_slave_rcu(), and thus you'll get
> bond_to_slave(bond->slave_list) in the result, which is not a slave.
>
> Take a look at list_first_or_null_rcu() for a reference. The main idea
> is
> that it first gets the ->next pointer, with RCU protection, and then
> verifies if it's the list head or not, and if not - it gets the
> container
> already. This way the ->next pointer won't get away.
>
> These kind of bugs are really rare, but are *EXTREMELY* hard to debug.
I agree with this analysis.
Ding, "rcu_read_lock()" doesn't "lock" anything. It's just a memory
barrier.
All the list can still change on you asynchronously to your accesses.
That's why list_first_or_null_rcu() is so carefully arranged.
Therefore, you must make similar accomodations.
^ permalink raw reply
* Re: [PATCH 5/7] ixgbe: use pcie_capability_read_word() to simplify code
From: Bjorn Helgaas @ 2013-09-04 16:20 UTC (permalink / raw)
To: Yijing Wang
Cc: Benjamin Herrenschmidt, Gavin Shan, James E.J. Bottomley,
David S. Miller, linux-kernel, linux-pci, Hanjun Guo, e1000-devel,
netdev, Jeff Kirsher, Jacob Keller
In-Reply-To: <1378193715-25328-5-git-send-email-wangyijing@huawei.com>
[+cc Jacob, Jeff]
On Tue, Sep 03, 2013 at 03:35:13PM +0800, Yijing Wang wrote:
> use pcie_capability_read_word() to simplify code.
>
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index bad8f14..bfa0b06 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -152,7 +152,6 @@ MODULE_VERSION(DRV_VERSION);
> static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> u32 reg, u16 *value)
> {
> - int pos = 0;
> struct pci_dev *parent_dev;
> struct pci_bus *parent_bus;
>
> @@ -164,11 +163,10 @@ static int ixgbe_read_pci_cfg_word_parent(struct ixgbe_adapter *adapter,
> if (!parent_dev)
> return -1;
>
> - pos = pci_find_capability(parent_dev, PCI_CAP_ID_EXP);
> - if (!pos)
> + if (!pci_is_pcie(parent_dev))
> return -1;
>
> - pci_read_config_word(parent_dev, pos + reg, value);
> + pcie_capability_read_word(parent_dev, reg, value);
> return 0;
> }
>
Here's the caller of ixgbe_read_pci_cfg_word_parent():
/* Get the negotiated link width and speed from PCI config space of the
* parent, as this device is behind a switch
*/
err = ixgbe_read_pci_cfg_word_parent(adapter, 18, &link_status);
This should be using PCI_EXP_LNKSTA instead of "18".
But it would be even better if we could drop ixgbe_get_parent_bus_info()
completely. It seems redundant after merging Jacob's new
pcie_get_minimum_link() stuff [1].
ixgbe_disable_pcie_master() looks like it should be using
pcie_capability_read_word() with PCI_EXP_DEVSTA instead of using
IXGBE_PCI_DEVICE_STATUS. If fact, it looks like it could use the
new pci_wait_for_pending_transaction() interface [2].
It looks like all the #defines in the "PCI Bus Info" block
(IXGBE_PCI_DEVICE_STATUS, IXGBE_PCI_DEVICE_STATUS_TRANSACTION_PENDING,
IXGBE_PCI_LINK_STATUS, etc.) [3] are really for PCIe-generic things. If
so, the IXGBE-specific ones should be dropped in favor of the generic
ones.
[1] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c?id=e027d1aec4bb49030646d2c186a721f94372d7f2
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/pci.c?id=3775a209d38aa3a0c7ed89a7d0f529e0230f280e
[3] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h#n1833
^ permalink raw reply
* Re: [PATCH] net: mvneta: properly disable HW PHY polling and ensure adjust_link() works
From: Gregory CLEMENT @ 2013-09-04 16:08 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, netdev, Lior Amsalem, Jochen De Smet,
Simon Guinot, Ryan Press, vdonnefort, Ethan Tuttle, stable,
Ezequiel Garcia, Chény Yves-Gael, Peter Sanford,
Willy Tarreau, linux-arm-kernel
In-Reply-To: <1378304478-21237-1-git-send-email-thomas.petazzoni@free-electrons.com>
On 04/09/2013 16:21, Thomas Petazzoni wrote:
> This commit fixes a long-standing bug that has been reported by many
> users: on some Armada 370 platforms, only the network interface that
> has been used in U-Boot to tftp the kernel works properly in
> Linux. The other network interfaces can see a 'link up', but are
> unable to transmit data. The reports were generally made on the Armada
> 370-based Mirabox, but have also been given on the Armada 370-RD
> board.
>
> The network MAC in the Armada 370/XP (supported by the mvneta driver
> in Linux) has a functionality that allows it to continuously poll the
> PHY and directly update the MAC configuration accordingly (speed,
> duplex, etc.). The very first versions of the driver submitted for
> review were using this hardware mechanism, but due to this, the driver
> was not integrated with the kernel phylib. Following reviews, the
> driver was changed to use the phylib, and therefore a software based
> polling. In software based polling, Linux regularly talks to the PHY
> over the MDIO bus, and sees if the link status has changed. If it's
> the case then the adjust_link() callback of the driver is called to
> update the MAC configuration accordingly.
>
> However, it turns out that the adjust_link() callback was not
> configuring the hardware in a completely correct way: while it was
> setting the speed and duplex bits correctly, it wasn't telling the
> hardware to actually take into account those bits rather than what the
> hardware-based PHY polling mechanism has concluded. So, in fact the
> adjust_link() callback was basically a no-op.
>
> However, the network happened to be working because on the network
> interfaces used by U-Boot for tftp on Armada 370 platforms because the
> hardware PHY polling was enabled by the bootloader, and left enabled
> by Linux. However, the second network interface not used for tftp (or
> both network interfaces if the kernel is loaded from USB, NAND or SD
> card) didn't had the hardware PHY polling enabled.
>
> This patch fixes this situation by:
>
> (1) Making sure that the hardware PHY polling is disabled by clearing
> the MVNETA_PHY_POLLING_ENABLE bit in the MVNETA_UNIT_CONTROL
> register in the driver ->probe() function.
>
> (2) Making sure that the duplex and speed selections made by the
> adjust_link() callback are taken into account by clearing the
> MVNETA_GMAC_AN_SPEED_EN and MVNETA_GMAC_AN_DUPLEX_EN bits in the
> MVNETA_GMAC_AUTONEG_CONFIG register.
>
> This patch has been tested on Armada 370 Mirabox, and now both network
> interfaces are usable after boot.
>
Well done Thomas!
I have successfully tested it on Armada 370 Mirabox:
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Thanks
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Willy Tarreau <w@1wt.eu>
> Cc: Jochen De Smet <jochen.armkernel@leahnim.org>
> Cc: Peter Sanford <psanford@nearbuy.io>
> Cc: Ethan Tuttle <ethan@ethantuttle.com>
> Cc: Chény Yves-Gael <yves@cheny.fr>
> Cc: Ryan Press <ryan@presslab.us>
> Cc: Simon Guinot <simon.guinot@sequanux.org>
> Cc: vdonnefort@lacie.com
> Cc: stable@vger.kernel.org
> ---
> David, this patch is a fix for a problem that has been here since 3.8
> (when the mvneta driver was introduced), so I've Cc'ed stable@ and if
> possible I'd like to patch to be included for 3.12.
> ---
> drivers/net/ethernet/marvell/mvneta.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index b017818..90ab292 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -138,7 +138,9 @@
> #define MVNETA_GMAC_FORCE_LINK_PASS BIT(1)
> #define MVNETA_GMAC_CONFIG_MII_SPEED BIT(5)
> #define MVNETA_GMAC_CONFIG_GMII_SPEED BIT(6)
> +#define MVNETA_GMAC_AN_SPEED_EN BIT(7)
> #define MVNETA_GMAC_CONFIG_FULL_DUPLEX BIT(12)
> +#define MVNETA_GMAC_AN_DUPLEX_EN BIT(13)
> #define MVNETA_MIB_COUNTERS_BASE 0x3080
> #define MVNETA_MIB_LATE_COLLISION 0x7c
> #define MVNETA_DA_FILT_SPEC_MCAST 0x3400
> @@ -915,6 +917,13 @@ static void mvneta_defaults_set(struct mvneta_port *pp)
> /* Assign port SDMA configuration */
> mvreg_write(pp, MVNETA_SDMA_CONFIG, val);
>
> + /* Disable PHY polling in hardware, since we're using the
> + * kernel phylib to do this.
> + */
> + val = mvreg_read(pp, MVNETA_UNIT_CONTROL);
> + val &= ~MVNETA_PHY_POLLING_ENABLE;
> + mvreg_write(pp, MVNETA_UNIT_CONTROL, val);
> +
> mvneta_set_ucast_table(pp, -1);
> mvneta_set_special_mcast_table(pp, -1);
> mvneta_set_other_mcast_table(pp, -1);
> @@ -2307,7 +2316,9 @@ static void mvneta_adjust_link(struct net_device *ndev)
> val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
> val &= ~(MVNETA_GMAC_CONFIG_MII_SPEED |
> MVNETA_GMAC_CONFIG_GMII_SPEED |
> - MVNETA_GMAC_CONFIG_FULL_DUPLEX);
> + MVNETA_GMAC_CONFIG_FULL_DUPLEX |
> + MVNETA_GMAC_AN_SPEED_EN |
> + MVNETA_GMAC_AN_DUPLEX_EN);
>
> if (phydev->duplex)
> val |= MVNETA_GMAC_CONFIG_FULL_DUPLEX;
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox