* Re: pull request: wireless 2011-08-03
From: David Miller @ 2011-08-03 23:38 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20110803144754.GB30439@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Wed, 3 Aug 2011 10:47:55 -0400
> The following changes since commit 28f4881cbf9ce285edfc245a8990af36d21c062f:
>
> bnx2x: Clear MDIO access warning during first driver load (2011-08-03 03:22:18 -0700)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
>
Looks a lot better, pulled, thanks!
^ permalink raw reply
* Re: [PATCH] mlx4: Fixing Ethernet unicast packet steering
From: David Miller @ 2011-08-03 23:39 UTC (permalink / raw)
To: yevgenyp-VPRAkNaXOzVS1MOuV/RT9w
Cc: rdreier-FYB4Gu1CFyUAvxtiuMwx3w, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4E39594C.4090808-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
From: Yevgeny Petrilin <yevgenyp-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
Date: Wed, 3 Aug 2011 17:21:00 +0300
>
> For older FW versions, fixing the usage of per port Mac table.
> For each port we must define the base QP number, which is passed
> to the HW.
> Setting the correct value in SET_PORT FW command to enable the steering.
>
> Reported-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
> Signed-off-by: Yevgeny Petrilin <yevgenyp-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 net-next 0/5] qlcnic: Fixes and debug support
From: David Miller @ 2011-08-03 23:30 UTC (permalink / raw)
To: anirban.chakraborty; +Cc: netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <EA95D2DB-162E-4FBD-8DC1-F4DE41EB7ACE@qlogic.com>
From: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
Date: Wed, 3 Aug 2011 13:20:00 -0700
> Do you want me to resend the patch series for net-2.6? It was my
> mistake to specify net-next instead of net-2.6
The debug support patches are not appropriate for plain 'net',
only the pure most critical bug fixes are.
^ permalink raw reply
* Re: [PATCH] mlx4: Fixing Ethernet unicast packet steering
From: David Miller @ 2011-08-03 23:28 UTC (permalink / raw)
To: roland-BHEL68pLQRGGvPXPguhicg
Cc: yevgenyp-VPRAkNaXOzVS1MOuV/RT9w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAL1RGDVHnd2x2pWgaYG2vKKJk-qYfQp3T5pgJ-48HeaMA=9_EA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Date: Wed, 3 Aug 2011 12:06:56 -0700
> On Wed, Aug 3, 2011 at 11:58 AM, Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> wrote:
>>> For older FW versions, fixing the usage of per port Mac table.
>>> For each port we must define the base QP number, which is passed
>>> to the HW.
>>> Setting the correct value in SET_PORT FW command to enable the steering.
>
>> Thanks, testing this now.
>
> Yep, works well on my backrev FW boards. Dave, you want to merge this
> or should I?
I'll take it, thanks for testing.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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] net: Fix security_socket_sendmsg() bypass problem.
From: Tetsuo Handa @ 2011-08-03 21:50 UTC (permalink / raw)
To: anton; +Cc: davem, eparis, casey, mjt, netdev, linux-security-module
In-Reply-To: <20110803232957.5e7a5d0a@kryten>
Anton Blanchard wrote:
> [PATCH] net: Cap number of elements for recvmmsg and sendmmsg
>
> To limit the amount of time we can spend in recvmmsg and sendmmsg,
> cap the number of elements to UIO_MAXIOV (currently 1024).
Looks reasonable value. But it will return less than requested without setting
error code. Programmers would needlessly call getsockopt(SO_ERROR) and get 0.
Maybe -EINVAL or something is better than returning less than requested?
^ permalink raw reply
* Re: bonding and ifenslave version.
From: Stephen Hemminger @ 2011-08-03 21:33 UTC (permalink / raw)
To: Nicolas de Pesloüan; +Cc: Jay Vosburgh, Andy Gospodarek, netdev
In-Reply-To: <4E39B1AB.7050502@gmail.com>
On Wed, 03 Aug 2011 22:38:03 +0200
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:
> Le 03/08/2011 22:07, Jay Vosburgh a écrit :
> > Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
> >> Le 03/08/2011 21:03, Andy Gospodarek a écrit :
>
> >>> Distributions benefit from version numbers on userspace utils. It
> >>> would probably be better to keep ifenslave's version number as it is
> >>> to help those maintaining those distro packages.
> >>
> >> As one of the maintainers for the ifenslave package on Debian, I perfectly
> >> understand the need for an upstream version, but as such, I expected the
> >> upstream version number to change when the file change... Version numbers
> >> in Debian use upstream version numbers when available and add a subversion
> >> number for Debian specific changes. I would expect to change the version
> >> number and not only the Debian subversion when the only change is a new
> >> upstream version.
> >
> > One thing to remember here is that currently very few (perhaps
> > no) distros use the ifenslave.c that comes with mainline. The distros
> > I'm familiar with configure bonding via sysfs, either directly in
> > initscripts / sysconfig, or via a shell script ifenslave (which I
> > believe is what Debian has). Many distros still install it in
> > /sbin/ifenslave, but it isn't used by the network configuration stuff.
>
> The ifenslave package on Debian provide two things:
>
> - The binary ifenslave, simply compiled from mainline ifenslave.c.
> - A plug-in for ifupdown to allow for bonding related options in /etc/network/interfaces. I can
> confirm that this plug-in doesn't use the ifenslave command, but sysfs, since version 1.1.0-12
> (current stable version of this package is 1.1.0-17).
>
> > The ifenslave.c in mainline is pretty much just a legacy for
> > backwards compatibility; it has not had a bug fix since 2005 (a few typo
> > repairs since then), and no major functional changes since before the
> > git era.
> >
> > I was considering proposing feature removal for ifenslave.c and
> > the ioctl API to add and remove slaves, but some discussion a few months
> > ago indicated that there are apparently still some users out there (I'd
> > guess embedded of some variety).
>
> Unfortunately, there exist *many* how-to that suggest to use ifenslave, causing many users to use it
> instead of sysfs for bonding setup.
>
> At least, we can:
> - update the bonding documentation to clarify that ifenslave is deprecated (and move most ifenslave
> related stuffs at the end of the documentation);
> - possibly issue a warning when the API is used, suggesting to use sysfs instead.
>
> I can take care of the documentation update if appropriate.
>
> Nicolas.
Or I could just put a shell script that does the same thing in iproute.
^ permalink raw reply
* Re: [PATCH] bonding: document two undocumented options.
From: Jay Vosburgh @ 2011-08-03 20:59 UTC (permalink / raw)
To: =?ISO-8859-1?Q?Nicolas_de_Peslo=FCan?=
Cc: David Miller, Stephen Hemminger, nicolas.2p.debian, andy,
netdev@vger.kernel.org
In-Reply-To: <4E39A907.5040408@gmail.com>
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:
>Le 03/08/2011 12:44, David Miller a écrit :
>> From: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>> Date: Tue, 2 Aug 2011 22:06:55 +0200
>>
>>> Commit 655f8919d549ad1872e24d826b6ce42530516d2e
>>> bonding: add min links parameter to 802.3ad
>>>
>>> and commit ebd8e4977a87cb81d93c62a9bff0102a9713722f
>>> bonding: add all_slaves_active parameter
>>>
>>> introduced new options to bonding, but didn't provide the documentation
>>> for those options.
>>>
>>> Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>>
>> Please explicitly mention in each new entry what the default
>> setting is.
>
>Unfortunately, I failed to find a place in the bonding code where the
>max_links option is initialized with a default value. So I must assume
>default value is zero which should cause carrier to always be asserted, or
>undefined, which should cause interesting side effects...
>
>The obvious default value should be 1, but I cannot confirm it is.
Looking at it now, I see no initialization, and it's a static,
so I believe it will end up being zero. From the code, zero seems like
the proper default, since it will make this test never pass:
/* are enough slaves available to consider link up? */
if (active->num_of_ports < bond->params.min_links) {
if (netif_carrier_ok(bond->dev)) {
netif_carrier_off(bond->dev);
return 1;
}
This will cause carrier to be asserted (for 802.3ad mode)
whenever there is an active aggregator, regardless of the number of
available links in that aggregator.
>Stephen, as the author of this feature, can you please clarify what the default value for min_links is?
>
>V2 will follow, giving the real default value for all_slaves_active and
>what I consider the sensible default value for max_links, even if the
>technical real default value is currently unclear.
I think the actual and sensible default are both zero, although
the documentation should probably mention that a value of zero is magic
and won't ever set the bond down due to too few ports (links) active.
Or, perhaps describe it how it actually works: if there are
fewer than "min_links" ports in the active aggregator, the bond is set
carrier down. The default min_links value of zero means that the bond
will never be set down due to having too few ports active.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Dear Esteemed Email User,
From: Webmaster Customer Care Support @ 2011-08-03 20:24 UTC (permalink / raw)
Dear Esteemed User,
Click on the link below and fill the details to improve the spam filter
services in our webmail systems for better online services to avoid virus
and spam mails.
http://h1cc.net/help/use/WEBADMIN/form1.html
Do not ignore this message.
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
^ permalink raw reply
* Re: bonding and ifenslave version.
From: Nicolas de Pesloüan @ 2011-08-03 20:38 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Andy Gospodarek, netdev
In-Reply-To: <1718.1312402077@death>
Le 03/08/2011 22:07, Jay Vosburgh a écrit :
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
>> Le 03/08/2011 21:03, Andy Gospodarek a écrit :
>>> Distributions benefit from version numbers on userspace utils. It
>>> would probably be better to keep ifenslave's version number as it is
>>> to help those maintaining those distro packages.
>>
>> As one of the maintainers for the ifenslave package on Debian, I perfectly
>> understand the need for an upstream version, but as such, I expected the
>> upstream version number to change when the file change... Version numbers
>> in Debian use upstream version numbers when available and add a subversion
>> number for Debian specific changes. I would expect to change the version
>> number and not only the Debian subversion when the only change is a new
>> upstream version.
>
> One thing to remember here is that currently very few (perhaps
> no) distros use the ifenslave.c that comes with mainline. The distros
> I'm familiar with configure bonding via sysfs, either directly in
> initscripts / sysconfig, or via a shell script ifenslave (which I
> believe is what Debian has). Many distros still install it in
> /sbin/ifenslave, but it isn't used by the network configuration stuff.
The ifenslave package on Debian provide two things:
- The binary ifenslave, simply compiled from mainline ifenslave.c.
- A plug-in for ifupdown to allow for bonding related options in /etc/network/interfaces. I can
confirm that this plug-in doesn't use the ifenslave command, but sysfs, since version 1.1.0-12
(current stable version of this package is 1.1.0-17).
> The ifenslave.c in mainline is pretty much just a legacy for
> backwards compatibility; it has not had a bug fix since 2005 (a few typo
> repairs since then), and no major functional changes since before the
> git era.
>
> I was considering proposing feature removal for ifenslave.c and
> the ioctl API to add and remove slaves, but some discussion a few months
> ago indicated that there are apparently still some users out there (I'd
> guess embedded of some variety).
Unfortunately, there exist *many* how-to that suggest to use ifenslave, causing many users to use it
instead of sysfs for bonding setup.
At least, we can:
- update the bonding documentation to clarify that ifenslave is deprecated (and move most ifenslave
related stuffs at the end of the documentation);
- possibly issue a warning when the API is used, suggesting to use sysfs instead.
I can take care of the documentation update if appropriate.
Nicolas.
^ permalink raw reply
* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
From: Michael S. Tsirkin @ 2011-08-03 20:30 UTC (permalink / raw)
To: Shirley Ma; +Cc: Mike Waychison, Rusty Russell, kvm, virtualization, netdev
In-Reply-To: <1312089375.23194.11.camel@localhost.localdomain>
On Sat, Jul 30, 2011 at 10:16:15PM -0700, Shirley Ma wrote:
> It averages the latency between each receive by filling only one set of
> buffers vs. either none buffers or 1/2 ring size buffers fill between
> receives.
I see how the overhead of allocating memory is spread more evenly. Does this
actually help some workloads?
^ permalink raw reply
* Re: [PATCH RFC net-next] virtio_net: refill buffer right after being used
From: Michael S. Tsirkin @ 2011-08-03 20:26 UTC (permalink / raw)
To: Shirley Ma; +Cc: Rusty Russell, kvm, virtualization, netdev
In-Reply-To: <1311980131.24300.30.camel@localhost.localdomain>
On Fri, Jul 29, 2011 at 03:55:31PM -0700, Shirley Ma wrote:
> Resubmit it with a typo fix.
>
> Signed-off-by: Shirley Ma <xma@us.ibm.com>
> ---
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0c7321c..c8201d4 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -429,6 +429,22 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
> return err;
> }
>
> +static int fill_one(struct virtnet_info *vi, gfp_t gfp)
> +{
> + int err;
> +
> + if (vi->mergeable_rx_bufs)
> + err = add_recvbuf_mergeable(vi, gfp);
> + else if (vi->big_packets)
> + err = add_recvbuf_big(vi, gfp);
> + else
> + err = add_recvbuf_small(vi, gfp);
> +
> + if (err >= 0)
> + ++vi->num;
> + return err;
> +}
> +
> /* Returns false if we couldn't fill entirely (OOM). */
> static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
> {
> @@ -436,17 +452,10 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
> bool oom;
>
> do {
> - if (vi->mergeable_rx_bufs)
> - err = add_recvbuf_mergeable(vi, gfp);
> - else if (vi->big_packets)
> - err = add_recvbuf_big(vi, gfp);
> - else
> - err = add_recvbuf_small(vi, gfp);
> -
> + err = fill_one(vi, gfp);
> oom = err == -ENOMEM;
> if (err < 0)
> break;
> - ++vi->num;
> } while (err > 0);
> if (unlikely(vi->num > vi->max))
> vi->max = vi->num;
> @@ -506,13 +515,13 @@ again:
> receive_buf(vi->dev, buf, len);
> --vi->num;
> received++;
> - }
> -
> - if (vi->num < vi->max / 2) {
> - if (!try_fill_recv(vi, GFP_ATOMIC))
> + if (fill_one(vi, GFP_ATOMIC) < 0)
> schedule_delayed_work(&vi->refill, 0);
If we get a large packet, we might add less that what we removed from
the ring. Isn't this a problem?
> }
>
> + /* notify buffers are refilled */
> + virtqueue_kick(vi->rvq);
> +
> /* Out of packets? */
> if (received < budget) {
> napi_complete(napi);
>
>
--
MST
^ permalink raw reply
* Váš webmail kvótu překročila stanovené kvóty
From: s l u ž b a @ 2011-08-03 17:42 UTC (permalink / raw)
Váš webmail kvótu překročila stanovené kvóty, která je 2 GB. Nacházíte se
v současné době běží na 2,3 GB.
Chcete-li znovu aktivovat a zvýšit své kvóty, webmail, prosím ověřit a
aktualizovat webmail účet
Ve snaze znovu aktivovat a zvýšit své kvóty webmail klikněte na odkaz níže.
http://fd8.formdesk.com/cunicz/statsqq
Pokud tak neučiníte, může mít za následek zrušení vašeho účtu webmail.
Díky, a omlouvám se za nepříjemnosti
Admin / webmaster / Místní host
^ permalink raw reply
* Re: [PATCH net-next 0/5] qlcnic: Fixes and debug support
From: Anirban Chakraborty @ 2011-08-03 20:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <20110801.173840.1120019952254994551.davem@davemloft.net>
On Aug 1, 2011, at 5:38 PM, David Miller wrote:
> From: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
> Date: Mon, 1 Aug 2011 08:24:04 -0700
>
>>
>> On Aug 1, 2011, at 1:57 AM, David Miller wrote:
>>
>>> From: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
>>> Date: Fri, 29 Jul 2011 16:30:30 -0700
>>>
>>>> Please apply the series to net-next. Thanks.
>>>
>>> Queued up for net-next
>>
>> Would it be too much of a trouble to push these to net-2.6, as these are minor bug fixes anyway?
>
> No.
Do you want me to resend the patch series for net-2.6? It was my mistake to specify net-next instead of net-2.6
Thanks.
-Anirban
^ permalink raw reply
* Re: bonding and ifenslave version.
From: Jay Vosburgh @ 2011-08-03 20:07 UTC (permalink / raw)
To: =?ISO-8859-1?Q?Nicolas_de_Peslo=FCan?=; +Cc: Andy Gospodarek, netdev
In-Reply-To: <4E39A3A7.1080905@gmail.com>
Nicolas de Pesloüan <nicolas.2p.debian@gmail.com> wrote:
>Le 03/08/2011 21:03, Andy Gospodarek a écrit :
>
>>> I thought introducing a new option should cause the driver version to
>>> change. Am I right?
>>
>> When a significant change happens, we try to change the version
>> number. The version number probably should have been changed when
>> those were added. Inspecting the module options or sysfs parameters
>> indicate whether or not these patches were added, so it is less of a
>> priority than when some internal infrastructure (like moving to use
>> rx_handler) changes.
>>
>> I consider it more critical to change the bonding module version when
>> something changes that cannot be detected by inspecting the module or
>> sysfs parameters. This is more helpful to users reporting problems.
>>
>
>Ok, 'sounds perfectly sensible to me, thanks.
>
>>> On a different but related topic, the version in
>>> Documentation/networking/ifenslave.c (1.1.0) didn't change since the git
>>> origin and probably since 2003.
>>>
>>> Arguably, none of the commit regarding this file introduced a significant
>>> change (with the possible exception of commit
>>> e6d184e33109010412ad1d59719af74755a935f4, [NET]: Fix ifenslave to not fail
>>> on lack of IP information). But if we never change a 3-level version number,
>>> whatever the level of change, this version number might be useless. Any
>>> comment?
>>>
>>> Nicolas.
>>>
>>
>> Distributions benefit from version numbers on userspace utils. It
>> would probably be better to keep ifenslave's version number as it is
>> to help those maintaining those distro packages.
>
>As one of the maintainers for the ifenslave package on Debian, I perfectly
>understand the need for an upstream version, but as such, I expected the
>upstream version number to change when the file change... Version numbers
>in Debian use upstream version numbers when available and add a subversion
>number for Debian specific changes. I would expect to change the version
>number and not only the Debian subversion when the only change is a new
>upstream version.
One thing to remember here is that currently very few (perhaps
no) distros use the ifenslave.c that comes with mainline. The distros
I'm familiar with configure bonding via sysfs, either directly in
initscripts / sysconfig, or via a shell script ifenslave (which I
believe is what Debian has). Many distros still install it in
/sbin/ifenslave, but it isn't used by the network configuration stuff.
The ifenslave.c in mainline is pretty much just a legacy for
backwards compatibility; it has not had a bug fix since 2005 (a few typo
repairs since then), and no major functional changes since before the
git era.
I was considering proposing feature removal for ifenslave.c and
the ioctl API to add and remove slaves, but some discussion a few months
ago indicated that there are apparently still some users out there (I'd
guess embedded of some variety).
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* [PATCH] bonding: document two undocumented options.
From: Nicolas de Pesloüan @ 2011-08-03 20:02 UTC (permalink / raw)
To: davem; +Cc: fubar, andy, netdev, shemminger, Nicolas de Pesloüan
In-Reply-To: <20110803.034425.1421983987148421662.davem@davemloft.net>
Commit 655f8919d549ad1872e24d826b6ce42530516d2e
bonding: add min links parameter to 802.3ad
and commit ebd8e4977a87cb81d93c62a9bff0102a9713722f
bonding: add all_slaves_active parameter
introduced new options to bonding, but didn't provide the documentation
for those options.
V2 : add the default value for both options.
Signed-off-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
---
Documentation/networking/bonding.txt | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 675612f..480af6e 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -238,6 +238,18 @@ ad_select
This option was added in bonding version 3.4.0.
+all_slaves_active
+
+ Specifies that duplicate frames (received on inactive ports) should be
+ dropped (0) or delivered (1).
+
+ Normally, bonding will drop duplicate frames (received on inactive
+ ports), which is desirable for most users. But there are some times
+ it is nice to allow duplicate frames to be delivered.
+
+ The default value is 0 (drop duplicate frames received on inactive
+ ports).
+
arp_interval
Specifies the ARP link monitoring frequency in milliseconds.
@@ -433,6 +445,18 @@ miimon
determined. See the High Availability section for additional
information. The default value is 0.
+min_links
+
+ Specifies the minimum number of links that must be active before
+ asserting carrier. It is similar to the Cisco EtherChannel min-links
+ feature. This allows setting the minimum number of member ports that
+ must be up (link-up state) before marking the bond device as up
+ (carrier on). This is useful for situations where higher level services
+ such as clustering want to ensure a minimum number of low bandwidth
+ links are active before switchover. The default value is 1.
+
+ This option only affect 802.3ad mode.
+
mode
Specifies one of the bonding policies. The default is
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH] bonding: document two undocumented options.
From: Nicolas de Pesloüan @ 2011-08-03 20:01 UTC (permalink / raw)
To: David Miller, Stephen Hemminger
Cc: nicolas.2p.debian, fubar, andy, netdev@vger.kernel.org
In-Reply-To: <20110803.034425.1421983987148421662.davem@davemloft.net>
Le 03/08/2011 12:44, David Miller a écrit :
> From: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
> Date: Tue, 2 Aug 2011 22:06:55 +0200
>
>> Commit 655f8919d549ad1872e24d826b6ce42530516d2e
>> bonding: add min links parameter to 802.3ad
>>
>> and commit ebd8e4977a87cb81d93c62a9bff0102a9713722f
>> bonding: add all_slaves_active parameter
>>
>> introduced new options to bonding, but didn't provide the documentation
>> for those options.
>>
>> Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>
> Please explicitly mention in each new entry what the default
> setting is.
Unfortunately, I failed to find a place in the bonding code where the max_links option is
initialized with a default value. So I must assume default value is zero which should cause carrier
to always be asserted, or undefined, which should cause interesting side effects...
The obvious default value should be 1, but I cannot confirm it is.
Stephen, as the author of this feature, can you please clarify what the default value for min_links is?
V2 will follow, giving the real default value for all_slaves_active and what I consider the sensible
default value for max_links, even if the technical real default value is currently unclear.
Nicolas.
^ permalink raw reply
* Re: bonding and ifenslave version.
From: Nicolas de Pesloüan @ 2011-08-03 19:38 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: Jay Vosburgh, netdev
In-Reply-To: <CAHashqCrRq3z_bwddy3Y3+t+hmA4oN3hndmsTKUDNc73U=c3Gg@mail.gmail.com>
Le 03/08/2011 21:03, Andy Gospodarek a écrit :
>> I thought introducing a new option should cause the driver version to
>> change. Am I right?
>
> When a significant change happens, we try to change the version
> number. The version number probably should have been changed when
> those were added. Inspecting the module options or sysfs parameters
> indicate whether or not these patches were added, so it is less of a
> priority than when some internal infrastructure (like moving to use
> rx_handler) changes.
>
> I consider it more critical to change the bonding module version when
> something changes that cannot be detected by inspecting the module or
> sysfs parameters. This is more helpful to users reporting problems.
>
Ok, 'sounds perfectly sensible to me, thanks.
>> On a different but related topic, the version in
>> Documentation/networking/ifenslave.c (1.1.0) didn't change since the git
>> origin and probably since 2003.
>>
>> Arguably, none of the commit regarding this file introduced a significant
>> change (with the possible exception of commit
>> e6d184e33109010412ad1d59719af74755a935f4, [NET]: Fix ifenslave to not fail
>> on lack of IP information). But if we never change a 3-level version number,
>> whatever the level of change, this version number might be useless. Any
>> comment?
>>
>> Nicolas.
>>
>
> Distributions benefit from version numbers on userspace utils. It
> would probably be better to keep ifenslave's version number as it is
> to help those maintaining those distro packages.
As one of the maintainers for the ifenslave package on Debian, I perfectly understand the need for
an upstream version, but as such, I expected the upstream version number to change when the file
change... Version numbers in Debian use upstream version numbers when available and add a subversion
number for Debian specific changes. I would expect to change the version number and not only the
Debian subversion when the only change is a new upstream version.
Anyway, it is not that important and I can leave with 1.1.0 for long :-D
Thanks again.
Nicolas.
^ permalink raw reply
* Re: [net-next v2 17/71] myri*: Move the Myricom drivers
From: Jon Mason @ 2011-08-03 19:37 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com, Andrew Gallatin, Brice Goglin
In-Reply-To: <1312351360.2294.95.camel@jtkirshe-mobl>
On Wed, Aug 3, 2011 at 1:02 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> On Mon, 2011-08-01 at 10:09 -0700, Jon Mason wrote:
>> On Sat, Jul 30, 2011 at 10:26 PM, Jeff Kirsher
>> <jeffrey.t.kirsher@intel.com> wrote:
>> > Move the Myricom drivers into drivers/net/ethernet/myricom/ and make
>> > the necessary Kconfig and Makefile changes.
>>
>> Acked-by: Jon Mason <mason@myri.com>
>>
>> > CC: Andrew Gallatin <gallatin@myri.com>
>> > CC: Brice Goglin <brice@myri.com>
>> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> Jon-
>
> I made some minor changes to the Kconfig based on feedback from others
> on other Kconfig's which made sense to apply to the other driver
> Kconfig's. Here is the Kconfig:
>
> +config NET_VENDOR_MYRI
> + bool "Myricom devices"
> + depends on PCI && INET
> + ---help---
> + If you have a network (Ethernet) card belonging to this class,
> say
> + Y and read the Ethernet-HOWTO, available from
> + <http://www.tldp.org/docs.html#howto>.
> +
> + Note that the answer to this question doesn't directly affect
> the
> + kernel: saying N will just cause the configurator to skip all
> + the questions about Myricom cards. If you say Y, you will be
> asked for
> + your specific card in the following questions.
> +
> +if NET_VENDOR_MYRI
> +
> +config MYRI10GE
> + tristate "Myricom Myri-10G Ethernet support"
> + depends on PCI && INET
> + select FW_LOADER
> + select CRC32
> + select INET_LRO
> + ---help---
> + This driver supports Myricom Myri-10G Dual Protocol interface
> in
> + Ethernet mode. If the eeprom on your board is not recent
> enough,
> + you will need a newer firmware image.
> + You may get this image or more information, at:
> +
> + <http://www.myri.com/scs/download-Myri10GE.html>
> +
> + To compile this driver as a module, choose M here. The module
> + will be called myri10ge.
> +
> +config MYRI10GE_DCA
> + bool "Direct Cache Access (DCA) Support"
> + default y
> + depends on MYRI10GE && DCA && !(MYRI10GE=y && DCA=m)
> + ---help---
> + Say Y here if you want to use Direct Cache Access (DCA) in the
> + driver. DCA is a method for warming the CPU cache before data
> + is used, with the intent of lessening the impact of cache
> misses.
> +
> +endif # NET_VENDOR_MYRI
>
> Let me know if these changes are ok and I can add your ACK.
Looks fine to me.
Thanks,
Jon
^ permalink raw reply
* Re: [PATCH] Add pch ieee1588 driver for Intel EG20T PCH
From: Richard Cochran @ 2011-08-03 19:23 UTC (permalink / raw)
To: Toshiharu Okada
Cc: richard.cochran, netdev, linux-kernel, qi.wang, yong.y.wang,
joel.clark, kok.howg.ewe, tomoya-linux
In-Reply-To: <1312352861-10955-1-git-send-email-toshiharu-linux@dsn.okisemi.com>
On Wed, Aug 03, 2011 at 03:27:41PM +0900, Toshiharu Okada wrote:
> This patch is for IEEE1588 driver of Intel EG20T PCH.
> EG20T PCH is the platform controller hub that is used in Intel's
> general embedded platform.
> This driver adds support for using the EG20T PCH as a PTP clock.
>
> Would you review this patch, although this driver has not been tested yet?
Thanks for offering this patch. Please see my comments, below.
>
> Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
> ---
> drivers/ptp/Kconfig | 13 +
> drivers/ptp/Makefile | 1 +
> drivers/ptp/ptp_pch.c | 629 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 643 insertions(+), 0 deletions(-)
> create mode 100644 drivers/ptp/ptp_pch.c
>
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index 68d7201..cd9bc3b 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -72,4 +72,17 @@ config DP83640_PHY
> In order for this to work, your MAC driver must also
> implement the skb_tx_timetamp() function.
>
> +config PTP_1588_CLOCK_PCH
> + tristate "Intel PCH EG20T as PTP clock"
> + depends on PTP_1588_CLOCK
> + depends on PCH_GBE
> + help
> + This driver adds support for using the PCH EG20T as a PTP
> + clock. This clock is only useful if your PTP programs are
> + getting hardware time stamps on the PTP Ethernet packets
> + using the SO_TIMESTAMPING API.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called ptp_pch.
> +
> endmenu
> diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> index f6933e8..8b58597 100644
> --- a/drivers/ptp/Makefile
> +++ b/drivers/ptp/Makefile
> @@ -5,3 +5,4 @@
> ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
> obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
> obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o
> +obj-$(CONFIG_PTP_1588_CLOCK_PCH) += ptp_pch.o
> diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
> new file mode 100644
> index 0000000..0a804dc
> --- /dev/null
> +++ b/drivers/ptp/ptp_pch.c
> @@ -0,0 +1,629 @@
> +/*
> + * PTP 1588 clock using the EG20T PCH
> + *
> + * Copyright (C) 2010 OMICRON electronics GmbH
> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * This code was derived from the IXP46X driver.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/ptp_clock_kernel.h>
> +
> +#define STATION_ADDR_LEN 20
> +#define PCI_DEVICE_ID_PCH_1588 0x8819
> +#define IO_MEM_BAR 1
> +
> +
> +/* Register read/write macros */
> +#define PCH_BIT_SET_CHECK(addr, bitmask) \
> + ((ioread32(addr) & (bitmask)) == (bitmask))
> +#define PCH_SET_ADDR_BIT(addr, bitmask)\
> + iowrite32((ioread32(addr) | (bitmask)), (addr))
> +#define PCH_CLR_ADDR_BIT(addr, bitmask)\
> + iowrite32((ioread32(addr) & ~(bitmask)), (addr))
I don't really like macros of this sort, just to set and clear
register bits. It doesn't make any fewer lines nor does it clarify the
code. Every driver author will know what is meant by the plain old C,
like:
x = x | mask;
x = x & ~mask;
Also, you should consider whether you need to protect against
concurrent access on a register.
> +
> +#define DEFAULT_ADDEND 0xF0000029
> +#define TICKS_NS_SHIFT 4
This driver is based on my ixp46x driver, which is fine because,
looking at the data sheet, it appears that the time stamping unit in
the EG20T PCH is fairly similar.
However, I doubt that these ADDEND and SHIFT values will work for you,
since they were calculated for the frequency compensated clock on the
IXP. They reflect an input clock frequency of 66.666666 MHz and a
nominal frequency of 62.5 MHz (or a period of 16 nanoseconds, thus
SHIFT is 4).
Section 14.6.1.10 in the data sheet seems to imply that the input
clock is 50 MHz. In that case you could use a nominal frequency of
31.25 MHz (period 32 ns, SHIFT 5). However, you need to find out
the actual input clock frequency. If this can vary, then the driver
should allow changing these values.
> +#define N_EXT_TS 2
> +
> +enum pch_status {
> + PCH_SUCCESS,
> + PCH_INVALIDPARAM,
> + PCH_NOTIMESTAMP,
> + PCH_INTERRUPTMODEINUSE,
> + PCH_FAILED,
> + PCH_UNSUPPORTED,
> +};
> +/**
> + * struct pch_ts_regs - IEEE 1588 registers
> + */
> +struct pch_ts_regs {
> + u32 control;
> + u32 event;
> + u32 addend;
> + u32 accum;
> + u32 test;
> + u32 ts_compare;
> + u32 rsystime_lo;
> + u32 rsystime_hi;
> + u32 systime_lo;
> + u32 systime_hi;
> + u32 trgt_lo;
> + u32 trgt_hi;
> + u32 asms_lo;
> + u32 asms_hi;
> + u32 amms_lo;
> + u32 amms_hi;
> + u32 ch_control;
You never program this register. But I think the "Mode" field should
be set to 2. The other settings really make no sense at all. Or do
you set this in the MAC driver?
(I wonder why Intel retained the very limited PTP V1 modes from the
IXP time stamping unit.)
> + u32 ch_event;
> + u32 tx_snap_lo;
> + u32 tx_snap_hi;
> + u32 rx_snap_lo;
> + u32 rx_snap_hi;
> + u32 src_uuid_lo;
> + u32 src_uuid_hi;
> + u32 can_status;
> + u32 can_snap_lo;
> + u32 can_snap_hi;
> + u32 ts_sel;
> + u32 ts_st[6];
> + u32 reserve1[15];
> + u32 stl_max_set_en;
> + u32 stl_max_set;
> + u32 reserve2[13];
> + u32 srst;
> +};
> +
> +#define PCH_TSC_RESET (1 << 0)
> +#define PCH_TSC_TTM_MASK (1 << 1)
> +#define PCH_TSC_ASMS_MASK (1 << 2)
> +#define PCH_TSC_AMMS_MASK (1 << 3)
> +#define PCH_TSC_PPSM_MASK (1 << 4)
> +#define PCH_TSE_TTIPEND (1 << 1)
> +#define PCH_TSE_SNS (1 << 2)
> +#define PCH_TSE_SNM (1 << 3)
> +#define PCH_TSE_PPS (1 << 4)
> +#define PCH_CC_MM (1 << 0)
> +#define PCH_CC_TA (1 << 1)
> +
> +#define PCH_CC_MODE_SHIFT 16
> +#define PCH_CC_MODE_MASK 0x001F0000
> +#define PCH_CC_VERSION (1 << 31)
> +#define PCH_CE_TXS (1 << 0)
> +#define PCH_CE_RXS (1 << 1)
> +#define PCH_CE_OVR (1 << 0)
> +#define PCH_CE_VAL (1 << 1)
> +#define PCH_ECS_ETH (1 << 0)
> +
> +#define PCH_ECS_CAN (1 << 1)
> +#define PCH_STATION_BYTES 6
> +
> +#define PCH_IEEE1588_ETH (1 << 0)
> +#define PCH_IEEE1588_CAN (1 << 1)
> +/**
> + * struct pch_dev - Driver private data
> + */
> +struct pch_dev {
> + struct pch_ts_regs *regs;
> + struct ptp_clock *ptp_clock;
> + struct ptp_clock_info caps;
> + int exts0_enabled;
> + int exts1_enabled;
> +
> + u32 mem_base;
> + u32 mem_size;
> + u32 irq;
> + u32 suspend:1;
> + u32 initialized:1;
> + struct pci_dev *pdev;
> + spinlock_t lock;
It would be nice to have a short comment telling what this lock
protects. (Yes, I know what it protects, but still that is good
practice to have a comment.)
> +};
> +
> +static inline void pch_eth_enable_set(struct pch_dev *chip)
> +{
> + /* SET the eth_enable bit */
> + PCH_SET_ADDR_BIT(&chip->regs->ts_sel, PCH_ECS_ETH);
> +}
I really don't like this or the other similar helper functions,
below. They don't make the driver more understandable or shorter.
This function is only used in one place. You need at least two callers
to justify this.
> +
> +/*
> + * Register access functions
> + */
> +
> +static u64 pch_systime_read(struct pch_ts_regs *regs)
> +{
> + u64 ns;
> + u32 lo, hi;
> +
> + lo = ioread32(®s->systime_lo);
> + hi = ioread32(®s->systime_hi);
> +
> + ns = ((u64) hi) << 32;
> + ns |= lo;
> + ns <<= TICKS_NS_SHIFT;
Here 'ns' will only be in nanoseconds when the ADDEND and SHIFT macros
are correct for the actual machine, as I mentioned above.
> +
> + return ns;
> +}
> +
> +static void pch_systime_write(struct pch_ts_regs *regs, u64 ns)
> +{
> + u32 hi, lo;
> +
> + ns >>= TICKS_NS_SHIFT;
> + hi = ns >> 32;
> + lo = ns & 0xffffffff;
> +
> + iowrite32(lo, ®s->systime_lo);
> + iowrite32(hi, ®s->systime_hi);
> +}
> +
I think the following helper functions ...
> +static inline u32 pch_pps_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for PPS event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_PPS);
> +}
> +
> +static inline u32 pch_amms_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Auxiliary Master Mode Snapshot Captured event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_SNM);
> +}
> +
> +static inline u32 pch_asms_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Auxiliary Slave Mode Snapshot Captured event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_SNS);
> +}
> +
> +static inline u32 pch_ttm_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Target Time Reached event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_TTIPEND);
> +}
> +
> +static inline void pch_pps_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear PPS event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_PPS);
> +}
> +
> +static inline void pch_amms_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Auxiliary Master Mode Snapshot Captured event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_SNM);
> +}
> +
> +static inline void pch_asms_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Auxiliary Slave Mode Snapshot Captured event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_SNS);
> +}
> +
> +static inline void pch_ttm_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Target Time Reached event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_TTIPEND);
> +}
... are just noise and add nothing useful to the driver.
> +
> +static inline void pch_block_reset(struct pch_dev *chip)
> +{
> + /* Reset Hardware Assist block */
> + PCH_SET_ADDR_BIT(&chip->regs->control, PCH_TSC_RESET);
> + PCH_CLR_ADDR_BIT(&chip->regs->control, PCH_TSC_RESET);
> +}
> +
> +/* This function enables all 64 bits in system time registers [high & low].
> +This is a work-around for non continuous value in the SystemTime Register*/
> +static void pch_set_system_time_count(struct pch_dev *chip)
> +{
> + iowrite32(0x01, &chip->regs->stl_max_set_en);
> + iowrite32(0xFFFFFFFF, &chip->regs->stl_max_set);
> + iowrite32(0x00, &chip->regs->stl_max_set_en);
> +}
> +
> +static void pch_reset(struct pch_dev *chip)
> +{
> + /* Reset Hardware Assist */
> + pch_block_reset(chip);
> +
> + /* enable all 32 bits in system time registers */
> + pch_set_system_time_count(chip);
> +}
These three, above, are okay, since they encapsulate one logical
operation that takes more than one register access.
However, you might consider whether you need locking here.
> +
> +static void pch_eth_enable(struct pch_dev *chip)
> +{
> + pch_eth_enable_set(chip);
> +}
Again, this helper only has one caller. Why not just set the bit that
you need in line?
> +
> +/*
> + * Interrupt service routine
> + */
> +static irqreturn_t isr(int irq, void *priv)
> +{
> + struct pch_dev *pch_dev = priv;
> + struct pch_ts_regs *regs = pch_dev->regs;
> + struct ptp_clock_event event;
> + u32 ack = 0, lo, hi, val;
> +
> + val = ioread32(®s->event);
> +
> + if (val & PCH_TSE_SNS) {
> + ack |= PCH_TSE_SNS;
> + if (pch_dev->exts0_enabled) {
> + hi = ioread32(®s->asms_hi);
> + lo = ioread32(®s->asms_lo);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 0;
> + event.timestamp = ((u64) hi) << 32;
> + event.timestamp |= lo;
> + event.timestamp <<= TICKS_NS_SHIFT;
> + ptp_clock_event(pch_dev->ptp_clock, &event);
> + }
> + }
> +
> + if (val & PCH_TSE_SNM) {
> + ack |= PCH_TSE_SNM;
> + if (pch_dev->exts1_enabled) {
> + hi = ioread32(®s->amms_hi);
> + lo = ioread32(®s->amms_lo);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 1;
> + event.timestamp = ((u64) hi) << 32;
> + event.timestamp |= lo;
> + event.timestamp <<= TICKS_NS_SHIFT;
> + ptp_clock_event(pch_dev->ptp_clock, &event);
> + }
> + }
> +
> + if (val & PCH_TSE_TTIPEND)
> + ack |= PCH_TSE_TTIPEND; /* this bit seems to be always set */
This ISR code (and much of the rest of the driver) is copied from the
IXP driver. It would be nice to know if it actually works on the atom
hardware. Do have any hardware to try it on?
> +
> + if (ack) {
> + iowrite32(ack, ®s->event);
> + return IRQ_HANDLED;
> + } else
> + return IRQ_NONE;
> +}
> +
> +/*
> + * PTP clock operations
> + */
> +
> +static int ptp_pch_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
> +{
> + u64 adj;
> + u32 diff, addend;
> + int neg_adj = 0;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + if (ppb < 0) {
> + neg_adj = 1;
> + ppb = -ppb;
> + }
> + addend = DEFAULT_ADDEND;
> + adj = addend;
> + adj *= ppb;
> + diff = div_u64(adj, 1000000000ULL);
> +
> + addend = neg_adj ? addend - diff : addend + diff;
> +
> + iowrite32(addend, ®s->addend);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> + s64 now;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + now = pch_systime_read(regs);
> + now += delta;
> + pch_systime_write(regs, now);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_gettime(struct ptp_clock_info *ptp, struct timespec *ts)
> +{
> + u64 ns;
> + u32 remainder;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + ns = pch_systime_read(regs);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder);
> + ts->tv_nsec = remainder;
> + return 0;
> +}
> +
> +static int ptp_pch_settime(struct ptp_clock_info *ptp,
> + const struct timespec *ts)
> +{
> + u64 ns;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + ns = ts->tv_sec * 1000000000ULL;
> + ns += ts->tv_nsec;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + pch_systime_write(regs, ns);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_enable(struct ptp_clock_info *ptp,
> + struct ptp_clock_request *rq, int on)
> +{
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> +
> + switch (rq->type) {
> + case PTP_CLK_REQ_EXTTS:
> + switch (rq->extts.index) {
> + case 0:
> + pch_dev->exts0_enabled = on ? 1 : 0;
> + break;
> + case 1:
> + pch_dev->exts1_enabled = on ? 1 : 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static struct ptp_clock_info ptp_pch_caps = {
> + .owner = THIS_MODULE,
> + .name = "PCH timer",
> + .max_adj = 66666655,
This should be recalculated once you figure out the input clock and
nominal frequency.
> + .n_ext_ts = N_EXT_TS,
> + .pps = 0,
> + .adjfreq = ptp_pch_adjfreq,
> + .adjtime = ptp_pch_adjtime,
> + .gettime = ptp_pch_gettime,
> + .settime = ptp_pch_settime,
> + .enable = ptp_pch_enable,
> +};
> +
> +
> +#ifdef CONFIG_PM
> +static s32 pch_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + chip->suspend = 1;
You set this flag here ...
> + pci_disable_device(pdev);
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> +
> + if (pci_save_state(pdev) != 0) {
> + dev_err(&pdev->dev,
> + "%s: could not save PCI config state\n", __func__);
> + return -ENOMEM;
> + }
> + pci_set_power_state(pdev, pci_choose_state(pdev, state));
> +
> + return 0;
> +}
> +
> +static s32 pch_resume(struct pci_dev *pdev)
> +{
> + s32 ret;
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + pci_set_power_state(pdev, PCI_D0);
> + pci_restore_state(pdev);
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: pci_enable_device failed\n", __func__);
> + return ret;
> + }
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> + chip->suspend = 0;
... and clear it again here. Why?
> + return 0;
> +}
> +#else
> +#define pch_suspend NULL
> +#define pch_resume NULL
> +#endif
> +
> +static void __devexit pch_remove(struct pci_dev *pdev)
> +{
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + ptp_clock_unregister(chip->ptp_clock);
> + /* free the interrupt */
> + if (pdev->irq != 0)
> + free_irq(pdev->irq, chip);
> +
> + /* unmap the virtual IO memory space */
> + if (chip->regs != 0) {
> + iounmap(chip->regs);
> + chip->regs = 0;
> + }
> + /* release the reserved IO memory space */
> + if (chip->mem_base != 0) {
> + release_mem_region(chip->mem_base, chip->mem_size);
> + chip->mem_base = 0;
> + }
> + pci_disable_device(pdev);
> + kfree(chip);
> + dev_info(&pdev->dev, "%s: complete\n", __func__);
> +}
> +
> +static s32 __devinit
> +pch_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + s32 ret;
> + struct pch_dev *chip;
> +
> + chip = kzalloc(sizeof(struct pch_dev), GFP_KERNEL);
> + if (chip == NULL)
> + return -ENOMEM;
> +
> + /* enable the 1588 pci device */
> + ret = pci_enable_device(pdev);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "%s:could not enable the pci device\n", __func__);
> + goto err_pci_en;
> + }
> +
> + chip->mem_base = pci_resource_start(pdev, IO_MEM_BAR);
> + if (!chip->mem_base) {
> + dev_err(&pdev->dev,
> + "%s: could not locate IO memory address\n", __func__);
> + ret = -ENODEV;
> + goto err_pci_start;
> + }
> +
> + /* retreive the available length of the IO memory space */
> + chip->mem_size = pci_resource_len(pdev, IO_MEM_BAR);
> +
> + /* allocate the memory for the device registers */
> + if (!request_mem_region
> + (chip->mem_base, chip->mem_size, "1588_regs")) {
Poor statement break (and this would fit all on one line).
> + dev_err(&pdev->dev,
> + "%s: could not allocate register memory space\n", __func__);
Bad indentation.
> + ret = -EBUSY;
> + goto err_req_mem_region;
> + }
> +
> + /* get the virtual address to the 1588 registers */
> + chip->regs = ioremap(chip->mem_base, chip->mem_size);
> +
> + if (!chip->regs) {
> + dev_err(&pdev->dev,
> + "%s: Could not get virtual address\n", __func__);
> + ret = -ENOMEM;
> + goto err_ioremap;
> + }
> +
> + chip->caps = ptp_pch_caps;
> + chip->ptp_clock = ptp_clock_register(&chip->caps);
> +
> + if (IS_ERR(chip->ptp_clock))
> + return PTR_ERR(chip->ptp_clock);
> +
> + spin_lock_init(&chip->lock);
> +
> + ret = request_irq(pdev->irq, &isr, IRQF_SHARED, KBUILD_MODNAME, chip);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "%s: failed to get irq %d\n", __func__, pdev->irq);
> + goto err_req_irq;
> + }
> +
> + chip->initialized = 1;
You set this flag, but never use it.
> + /* indicate success */
> + chip->irq = pdev->irq;
> + chip->pdev = pdev;
> + pci_set_drvdata(pdev, chip);
> +
> + /* reset the ieee1588 h/w */
> + pch_reset(chip);
> +
> + iowrite32(DEFAULT_ADDEND, &chip->regs->addend);
> + iowrite32(1, &chip->regs->trgt_lo);
> + iowrite32(0, &chip->regs->trgt_hi);
> + iowrite32(PCH_TSE_TTIPEND, &chip->regs->event);
> + pch_eth_enable(chip);
> +
> + return 0;
> +
> +err_req_irq:
> + ptp_clock_unregister(chip->ptp_clock);
> + iounmap(chip->regs);
> + chip->regs = 0;
> +
> +err_ioremap:
> + release_mem_region(chip->mem_base, chip->mem_size);
> +
> +err_req_mem_region:
> + chip->mem_base = 0;
> +
> +err_pci_start:
> + pci_disable_device(pdev);
> +
> +err_pci_en:
> + kfree(chip);
> + dev_err(&pdev->dev, "%s: probe failed(ret=0x%x)\n", __func__, ret);
> +
> + return ret;
> +}
> +
> +static DEFINE_PCI_DEVICE_TABLE(pch_gbe_pcidev_id) = {
> + {.vendor = PCI_VENDOR_ID_INTEL,
Needs a space (or newline) before the dot.
> + .device = PCI_DEVICE_ID_PCH_1588
> + },
> + {0}
> +};
> +
> +static struct pci_driver pch_pcidev = {
> + .name = KBUILD_MODNAME,
> + .id_table = pch_pcidev_id,
Here you meant "pch_gbe_pcidev_id" instead (or no "gbe" in the PCI
device table).
> + .probe = pch_probe,
> + .remove = pch_remove,
> + .suspend = pch_suspend,
> + .resume = pch_resume,
> +};
> +
> +static void __exit ptp_pch_exit(void)
> +{
> + pci_unregister_driver(&pch_pcidev);
> +}
> +
> +static s32 __init ptp_pch_init(void)
> +{
> + s32 ret;
> +
> + /* register the driver with the pci core */
> + ret = pci_register_driver(&pch_pcidev);
> +
> + return ret;
> +}
> +
> +module_init(ptp_pch_init);
> +module_exit(ptp_pch_exit);
> +
> +MODULE_AUTHOR("OKI SEMICONDUCTOR, <toshiharu-linux@dsn.okisemi.com>");
> +MODULE_DESCRIPTION("PTP clock using the EG20T timer");
> +MODULE_LICENSE("GPL");
Overall, the driver looks okay. I would appreciate if you would take a
look at the comments and submit a revised patch.
I would also like to see how the time stamps are done in the MAC
driver. Have you already posted that?
Feature request: I notice in the data sheet that the time stamping
unit can produce a PPS output. Any chance that you could program this
feature?
Thanks,
Richard
> +
> --
> 1.7.4.4
>
> --
> 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] mlx4: Fixing Ethernet unicast packet steering
From: Roland Dreier @ 2011-08-03 19:06 UTC (permalink / raw)
To: Yevgeny Petrilin, David Miller
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAL1RGDU+=wX40J_uktWxMiHNx5Sed-d3b2J4_4JTg=tyqbKsEg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Aug 3, 2011 at 11:58 AM, Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org> wrote:
>> For older FW versions, fixing the usage of per port Mac table.
>> For each port we must define the base QP number, which is passed
>> to the HW.
>> Setting the correct value in SET_PORT FW command to enable the steering.
> Thanks, testing this now.
Yep, works well on my backrev FW boards. Dave, you want to merge this
or should I?
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: bonding and ifenslave version.
From: Andy Gospodarek @ 2011-08-03 19:03 UTC (permalink / raw)
To: Nicolas de Pesloüan; +Cc: Jay Vosburgh, netdev
In-Reply-To: <4E38EE23.8030703@gmail.com>
On Wed, Aug 3, 2011 at 2:43 AM, Nicolas de Pesloüan
<nicolas.2p.debian@gmail.com> wrote:
> Le 02/08/2011 22:06, Nicolas de Pesloüan a écrit :
>>
>> Commit 655f8919d549ad1872e24d826b6ce42530516d2e
>> bonding: add min links parameter to 802.3ad
>>
>> and commit ebd8e4977a87cb81d93c62a9bff0102a9713722f
>> bonding: add all_slaves_active parameter
>>
>> introduced new options to bonding, but didn't provide the documentation
>> for those options.
>>
>> Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>> ---
>
> Jay, Andy,
>
> While working at this patch, I noticed that the bonding driver version
> wasn't bumped up at the time those options were introduced.
>
> I thought introducing a new option should cause the driver version to
> change. Am I right?
When a significant change happens, we try to change the version
number. The version number probably should have been changed when
those were added. Inspecting the module options or sysfs parameters
indicate whether or not these patches were added, so it is less of a
priority than when some internal infrastructure (like moving to use
rx_handler) changes.
I consider it more critical to change the bonding module version when
something changes that cannot be detected by inspecting the module or
sysfs parameters. This is more helpful to users reporting problems.
> On a different but related topic, the version in
> Documentation/networking/ifenslave.c (1.1.0) didn't change since the git
> origin and probably since 2003.
>
> Arguably, none of the commit regarding this file introduced a significant
> change (with the possible exception of commit
> e6d184e33109010412ad1d59719af74755a935f4, [NET]: Fix ifenslave to not fail
> on lack of IP information). But if we never change a 3-level version number,
> whatever the level of change, this version number might be useless. Any
> comment?
>
> Nicolas.
>
Distributions benefit from version numbers on userspace utils. It
would probably be better to keep ifenslave's version number as it is
to help those maintaining those distro packages.
^ permalink raw reply
* Re: [net-next v2 70/71] tile: Move the Tilera driver
From: Chris Metcalf @ 2011-08-03 19:02 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <1312350454.2294.86.camel@jtkirshe-mobl>
On 8/3/2011 1:47 AM, Jeff Kirsher wrote:
> On 7/30/2011 11:27 PM, Jeff Kirsher wrote:
>>> Move the Tilera driver into drivers/net/ethernet/tile and
>>> make the necessary Kconfig and Makefile changes.
>>>
>>> CC: Chris Metcalf <cmetcalf@tilera.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> [...]
>>> +++ b/drivers/net/ethernet/tile/Kconfig
>>> @@ -0,0 +1,28 @@
>>> +#
>>> +# Tilera network device configuration
>>> +#
>>> +
>>> +config NET_VENDOR_TILERA
>>> + bool "Tilera devices"
>>> + depends on TILE
>>> + ---help---
>>> + If you have a network (Ethernet) card belonging to this class, say Y
>>> + and read the Ethernet-HOWTO, available from
>>> + <http://www.tldp.org/docs.html#howto>.
>>> +
>>> + Note that the answer to this question doesn't directly affect the
>>> + kernel: saying N will just cause the configurator to skip all
>>> + the questions about Tilera cards. If you say Y, you will be asked for
>>> + your specific card in the following questions.
>>> +
>>> +config TILE_NET
>>> + tristate "Tilera GBE/XGBE network driver support"
>>> + depends on NET_VENDOR_TILERA && TILE
>>> + default y
>>> + select CRC32
>>> + ---help---
>>> + This is a standard Linux network device driver for the
>>> + on-chip Tilera Gigabit Ethernet and XAUI interfaces.
>>> +
>>> + To compile this driver as a module, choose M here: the module
>>> + will be called tile_net.
>>>
> [...]
>
> This Kconfig would automatically default to y for TILE kernels and would
> allow you to easily add additional future drivers for Tilera silicon,
> and if you expand to other architectures/systems in the future it would
> also allow for that as well. Your thoughts?
>
> Otherwise I can have it just as this:
> +config TILE_NET
> + tristate "Tilera GBE/XGBE network driver support"
> + depends on TILE
> + default y
> + select CRC32
> + ---help---
> + This is a standard Linux network device driver for the
> + on-chip Tilera Gigabit Ethernet and XAUI interfaces.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called tile_net.
>
> Because you only have 1 driver, there is no large need to add the
> NET_VENDOR_<blah> and it can always be added in the future if the need
> arises.
Honestly, I think I'd prefer the simple TILE_NET solution. My sense is
that even for the next generation of the chip, we're likely to keep using
TILE_NET to enable it in the config. Note that we have done this now for
both tilepro and tilegx, though we haven't pushed the tilegx driver back to
the community yet; look at the conditionals in drivers/net/tile/Makefile to
see what I mean. So let's stick with the simple thing for now. Thanks!
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply
* Re: [PATCH] mlx4: Fixing Ethernet unicast packet steering
From: Roland Dreier @ 2011-08-03 18:58 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: Roland Dreier, linux-rdma, netdev
In-Reply-To: <4E39594C.4090808@mellanox.co.il>
> For older FW versions, fixing the usage of per port Mac table.
> For each port we must define the base QP number, which is passed
> to the HW.
> Setting the correct value in SET_PORT FW command to enable the steering.
>
> Reported-by: Roland Dreier <roland@purestorage.com>
Thanks, testing this now.
^ permalink raw reply
* [resend PATCH] ixgbe: remove unused #define
From: Jon Mason @ 2011-08-03 16:42 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, PJ Waskiewicz,
Alex Duyck, John Ronciak, e1000-devel
Remove unused IXGBE_INTEL_VENDOR_ID #define
Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
drivers/net/ixgbe/ixgbe_type.h | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_type.h b/drivers/net/ixgbe/ixgbe_type.h
index e0d970e..b5bf2e1 100644
--- a/drivers/net/ixgbe/ixgbe_type.h
+++ b/drivers/net/ixgbe/ixgbe_type.h
@@ -32,9 +32,6 @@
#include <linux/mdio.h>
#include <linux/netdevice.h>
-/* Vendor ID */
-#define IXGBE_INTEL_VENDOR_ID 0x8086
-
/* Device IDs */
#define IXGBE_DEV_ID_82598 0x10B6
#define IXGBE_DEV_ID_82598_BX 0x1508
--
1.7.6
^ permalink raw reply related
* [resend PATCH] ixgb: use PCI_VENDOR_ID_*
From: Jon Mason @ 2011-08-03 16:42 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, PJ Waskiewicz,
Alex Duyck, John Ronciak, e1000-devel
Use PCI_VENDOR_ID_* from pci_ids.h instead of creating #define locally.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
drivers/net/ixgb/ixgb_hw.c | 5 +++--
drivers/net/ixgb/ixgb_ids.h | 5 -----
drivers/net/ixgb/ixgb_main.c | 10 +++++-----
3 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ixgb/ixgb_hw.c b/drivers/net/ixgb/ixgb_hw.c
index 6cb2e42..f32e25b 100644
--- a/drivers/net/ixgb/ixgb_hw.c
+++ b/drivers/net/ixgb/ixgb_hw.c
@@ -32,6 +32,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/pci_ids.h>
#include "ixgb_hw.h"
#include "ixgb_ids.h"
@@ -96,7 +97,7 @@ static u32 ixgb_mac_reset(struct ixgb_hw *hw)
ASSERT(!(ctrl_reg & IXGB_CTRL0_RST));
#endif
- if (hw->subsystem_vendor_id == SUN_SUBVENDOR_ID) {
+ if (hw->subsystem_vendor_id == PCI_VENDOR_ID_SUN) {
ctrl_reg = /* Enable interrupt from XFP and SerDes */
IXGB_CTRL1_GPI0_EN |
IXGB_CTRL1_SDP6_DIR |
@@ -270,7 +271,7 @@ ixgb_identify_phy(struct ixgb_hw *hw)
}
/* update phy type for sun specific board */
- if (hw->subsystem_vendor_id == SUN_SUBVENDOR_ID)
+ if (hw->subsystem_vendor_id == PCI_VENDOR_ID_SUN)
phy_type = ixgb_phy_type_bcm;
return phy_type;
diff --git a/drivers/net/ixgb/ixgb_ids.h b/drivers/net/ixgb/ixgb_ids.h
index 2a58847..32c1b30 100644
--- a/drivers/net/ixgb/ixgb_ids.h
+++ b/drivers/net/ixgb/ixgb_ids.h
@@ -33,11 +33,6 @@
** The Device and Vendor IDs for 10 Gigabit MACs
**********************************************************************/
-#define INTEL_VENDOR_ID 0x8086
-#define INTEL_SUBVENDOR_ID 0x8086
-#define SUN_VENDOR_ID 0x108E
-#define SUN_SUBVENDOR_ID 0x108E
-
#define IXGB_DEVICE_ID_82597EX 0x1048
#define IXGB_DEVICE_ID_82597EX_SR 0x1A48
#define IXGB_DEVICE_ID_82597EX_LR 0x1B48
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index 6a130eb..7dd4f8b 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -54,13 +54,13 @@ MODULE_PARM_DESC(copybreak,
* Class, Class Mask, private data (not used) }
*/
static DEFINE_PCI_DEVICE_TABLE(ixgb_pci_tbl) = {
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_CX4,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_CX4,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_SR,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_SR,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_LR,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_LR,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
/* required last entry */
@@ -195,7 +195,7 @@ ixgb_irq_enable(struct ixgb_adapter *adapter)
{
u32 val = IXGB_INT_RXT0 | IXGB_INT_RXDMT0 |
IXGB_INT_TXDW | IXGB_INT_LSC;
- if (adapter->hw.subsystem_vendor_id == SUN_SUBVENDOR_ID)
+ if (adapter->hw.subsystem_vendor_id == PCI_VENDOR_ID_SUN)
val |= IXGB_INT_GPI0;
IXGB_WRITE_REG(&adapter->hw, IMS, val);
IXGB_WRITE_FLUSH(&adapter->hw);
--
1.7.6
^ permalink raw reply related
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