Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Linus Torvalds @ 2014-11-17 20:52 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: linux-arch@vger.kernel.org, Network Development,
	Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
	Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
	Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
	matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
	Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
	Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <20141117171812.22333.90395.stgit@ahduyck-server>

On Mon, Nov 17, 2014 at 9:18 AM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
> There are a number of situations where the mandatory barriers rmb() and
> wmb() are used to order memory/memory operations in the device drivers
> and those barriers are much heavier than they actually need to be.

Ugh. I absolutely despise the name.

It's not "fast". It's just limited. It's the same as "smp_*mb()", in
that it works on cacheable memory, but it actually stays around even
for non-SMP builds.

So I think the name is actively misleading.

Naming should be about what it does, not about some kind of PR thing
that confuses people into thinking it's "better".

Maybe "dma_*mb()" would be acceptable, and ends up having the same
naming convention as "smb_*mb()", and explains what it's about.

And yes, in the same spirit, it would probably be good to try to
eventually get rid of the plain "*mb()" functions, and perhaps call
them "mmio_*mb()" to clarify that they are about ordering memory wrt
mmio.

Hmm?

                        Linus

^ permalink raw reply

* [PATCH 1/1 net-next] dccp: remove blank lines between function/EXPORT_SYMBOL
From: Fabian Frederick @ 2014-11-17 20:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev

See Documentation/CodingStyle chapter 6.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/dccp/ipv4.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 6ca645c..e45b968 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -140,7 +140,6 @@ failure:
 	inet->inet_dport = 0;
 	goto out;
 }
-
 EXPORT_SYMBOL_GPL(dccp_v4_connect);
 
 /*
@@ -376,7 +375,6 @@ void dccp_v4_send_check(struct sock *sk, struct sk_buff *skb)
 						 inet->inet_saddr,
 						 inet->inet_daddr);
 }
-
 EXPORT_SYMBOL_GPL(dccp_v4_send_check);
 
 static inline u64 dccp_v4_init_sequence(const struct sk_buff *skb)
@@ -444,7 +442,6 @@ put_and_exit:
 	dccp_done(newsk);
 	goto exit;
 }
-
 EXPORT_SYMBOL_GPL(dccp_v4_request_recv_sock);
 
 static struct sock *dccp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
@@ -670,7 +667,6 @@ drop:
 	DCCP_INC_STATS_BH(DCCP_MIB_ATTEMPTFAILS);
 	return -1;
 }
-
 EXPORT_SYMBOL_GPL(dccp_v4_conn_request);
 
 int dccp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
@@ -729,7 +725,6 @@ discard:
 	kfree_skb(skb);
 	return 0;
 }
-
 EXPORT_SYMBOL_GPL(dccp_v4_do_rcv);
 
 /**
@@ -802,7 +797,6 @@ int dccp_invalid_packet(struct sk_buff *skb)
 
 	return 0;
 }
-
 EXPORT_SYMBOL_GPL(dccp_invalid_packet);
 
 /* this is called when real data arrives */
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] dccp: replace min/casting by min_t
From: Fabian Frederick @ 2014-11-17 20:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/dccp/ackvec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index ba07824..bd9e718 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -218,7 +218,7 @@ static void dccp_ackvec_add_new(struct dccp_ackvec *av, u32 num_packets,
 		 * different underlying data structure.
 		 */
 		for (num_packets = num_cells = 1; lost_packets; ++num_cells) {
-			u8 len = min(lost_packets, (u32)DCCPAV_MAX_RUNLEN);
+			u8 len = min_t(u32, lost_packets, DCCPAV_MAX_RUNLEN);
 
 			av->av_buf_head = __ackvec_idx_sub(av->av_buf_head, 1);
 			av->av_buf[av->av_buf_head] = DCCPAV_NOT_RECEIVED | len;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] dccp: spelling s/reseting/resetting
From: Fabian Frederick @ 2014-11-17 21:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/dccp/input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dccp/input.c b/net/dccp/input.c
index 3c8ec7d..3bd14e8 100644
--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -537,7 +537,7 @@ static int dccp_rcv_respond_partopen_state_process(struct sock *sk,
 	case DCCP_PKT_DATAACK:
 	case DCCP_PKT_ACK:
 		/*
-		 * FIXME: we should be reseting the PARTOPEN (DELACK) timer
+		 * FIXME: we should be resetting the PARTOPEN (DELACK) timer
 		 * here but only if we haven't used the DELACK timer for
 		 * something else, like sending a delayed ack for a TIMESTAMP
 		 * echo, etc, for now were not clearing it, sending an extra
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] net: fix spelling for synchronized
From: Fabian Frederick @ 2014-11-17 21:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/core/dev_addr_lists.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index b6b2306..c0548d2 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -278,8 +278,8 @@ int __hw_addr_sync_dev(struct netdev_hw_addr_list *list,
 EXPORT_SYMBOL(__hw_addr_sync_dev);
 
 /**
- *  __hw_addr_unsync_dev - Remove synchonized addresses from device
- *  @list: address list to remove syncronized addresses from
+ *  __hw_addr_unsync_dev - Remove synchronized addresses from device
+ *  @list: address list to remove synchronized addresses from
  *  @dev:  device to sync
  *  @unsync: function to call if address should be removed
  *
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH net-next] i40e: Reduce stack in i40e_dbg_dump_desc
From: David Miller @ 2014-11-17 21:06 UTC (permalink / raw)
  To: joe; +Cc: jeffrey.t.kirsher, shannon.nelson, netdev
In-Reply-To: <1416175968.24600.1.camel@perches.com>

From: Joe Perches <joe@perches.com>
Date: Sun, 16 Nov 2014 14:12:48 -0800

> Reduce stack use by using kmemdup and not using a very
> large struct on stack.
> 
> In function ‘i40e_dbg_dump_desc’:
> warning: the frame size of 8192 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> 
> Signed-off-by: Joe Perches <joe@perches.com>

I'll let Jeff pick this one up, thanks.

^ permalink raw reply

* [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: Fabian Frederick @ 2014-11-17 21:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/core/link_watch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index bd0767e..49a9e3e 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -21,7 +21,7 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <linux/bitops.h>
-#include <asm/types.h>
+#include <linux/types.h>
 
 
 enum lw_bits {
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] net/core: include linux/uaccess.h instead of asm/uaccess.h
From: Fabian Frederick @ 2014-11-17 21:10 UTC (permalink / raw)
  To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/core/gen_estimator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 9dfb88a..d654d6f 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -13,7 +13,7 @@
  *              names to make it usable in general net subsystem.
  */
 
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <linux/bitops.h>
 #include <linux/module.h>
 #include <linux/types.h>
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-17 21:11 UTC (permalink / raw)
  To: paulmck
  Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
	heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
	matthew.vick, geert, jeffrey.t.kirsher, romieu, nic_swsd,
	will.deacon, michael, tony.luck, torvalds, oleg, schwidefsky,
	fweisbec, davem
In-Reply-To: <20141117201823.GD5050@linux.vnet.ibm.com>

On 11/17/2014 12:18 PM, Paul E. McKenney wrote:
> On Mon, Nov 17, 2014 at 09:18:13AM -0800, Alexander Duyck wrote:
>> There are a number of situations where the mandatory barriers rmb() and
>> wmb() are used to order memory/memory operations in the device drivers
>> and those barriers are much heavier than they actually need to be.  For
>> example in the case of PowerPC wmb() calls the heavy-weight sync
>> instruction when for memory/memory operations all that is really needed is
>> an lsync or eieio instruction.
>
> Is this still the case if one of the memory operations is MMIO?  Last
> I knew, it was not.

This barrier is not meant for use in MMIO operations, for that you still 
need a full barrier as I call out in the documentation section. What the 
barrier does is allow for a lightweight barrier for accesses to coherent 
system memory. So for example many device drivers have to perform a read 
of the descriptor to see if the device is done with it. We need an rmb() 
following that check to prevent any other accesses.

Right now on x86 that rmb() becomes an lfence instruction and is quite 
expensive, and as it turns out we don't need it since the x86 doesn't 
reorder reads. The same kind of thing applies to PowerPC, only in that 
case we use a sync when what we really need is a lwsync.

>> This commit adds a fast (and loose) version of the mandatory memory
>> barriers rmb() and wmb().  The prefix to the name is actually based on the
>> version of the functions that already exist in the mips and tile trees.
>> However I thought it applicable since it gets at what we are trying to
>> accomplish with these barriers and somethat implies their risky nature.
>>
>> These new barriers are not as safe as the standard rmb() and wmb().
>> Specifically they do not guarantee ordering between cache-enabled and
>> cache-inhibited memories.  The primary use case for these would be to
>> enforce ordering of memory reads/writes when accessing cache-enabled memory
>> that is shared between the CPU and a device.
>>
>> It may also be noted that there is no fast_mb().  This is due to the fact
>> that most architectures didn't seem to have a good way to do a full memory
>> barrier quickly and so they usually resorted to an mb() for their smp_mb
>> call.  As such there no point in adding a fast_mb() function if it is going
>> to map to mb() for all architectures anyway.
>
> I must confess that I still don't entirely understand the motivation.

The motivation is to provide finer grained barriers. So this provides an 
in-between that allows us to "choose the right hammer". In the case of 
PowerPC it is the difference between sync/lwsync, on ARM it is 
dsb()/dmb(), and on x86 it is lfence/barrier().

> Some problems in PowerPC barrier.h called out below.
>
> 							Thanx, Paul
>

<snip>

>> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
>> index 22a969c..fe55dea 100644
>> --- a/Documentation/memory-barriers.txt
>> +++ b/Documentation/memory-barriers.txt
>> @@ -1615,6 +1615,47 @@ There are some more advanced barrier functions:
>>        operations" subsection for information on where to use these.
>>
>>
>> + (*) fast_wmb();
>> + (*) fast_rmb();
>> +
>> +     These are for use with memory based device I/O to guarantee the ordering
>> +     of cache-enabled writes or reads with respect to other writes or reads
>> +     to cache-enabled memory.
>> +
>> +     For example, consider a device driver that shares memory with a device
>> +     and uses a descriptor status value to indicate if the descriptor belongs
>> +     to the device or the CPU, and a doorbell to notify it when new
>> +     descriptors are available:
>> +
>> +	if (desc->status != DEVICE_OWN) {
>> +		/* do not read data until we own descriptor */
>> +		fast_rmb();
>> +
>> +		/* read/modify data */
>> +		read_data = desc->data;
>> +		desc->data = write_data;
>> +
>> +		/* flush modifications before status update */
>> +		fast_wmb();
>> +
>> +		/* assign ownership */
>> +		desc->status = DEVICE_OWN;
>> +
>> +		/* force memory to sync before notifying device via MMIO */
>> +		wmb();
>> +
>> +		/* notify device of new descriptors */
>> +		writel(DESC_NOTIFY, doorbell);
>> +	}
>> +
>> +     The fast_rmb() allows us guarantee the device has released ownership
>> +     before we read the data from the descriptor, and he fast_wmb() allows
>> +     us to guarantee the data is written to the descriptor before the device
>> +     can see it now has ownership.  The wmb() is needed to guarantee that the
>> +     cache-enabled memory writes have completed before attempting a write to
>> +     the cache-inhibited MMIO region.
>> +
>> +
>>   MMIO WRITE BARRIER
>>   ------------------

The general idea is that the device/CPU follow acquire/release style 
semantics and we need the lightweight barriers to enforce ordering to 
prevent us from accessing the descriptors during those periods where we 
do not own them.  As the example shows we still need a full barrier when 
going between MMIO and standard memory.  Hopefully that is what is 
conveyed in the documentation bits I have above.

<snip>

>> diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
>> index cb6d66c..f480097 100644
>> --- a/arch/powerpc/include/asm/barrier.h
>> +++ b/arch/powerpc/include/asm/barrier.h
>> @@ -36,22 +36,20 @@
>>
>>   #define set_mb(var, value)	do { var = value; mb(); } while (0)
>>
>> -#ifdef CONFIG_SMP
>> -
>>   #ifdef __SUBARCH_HAS_LWSYNC
>>   #    define SMPWMB      LWSYNC
>>   #else
>>   #    define SMPWMB      eieio
>>   #endif
>>
>> -#define __lwsync()	__asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>> +#define fast_rmb()	__asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>> +#define fast_wmb()	__asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>>
>> +#ifdef CONFIG_SMP
>>   #define smp_mb()	mb()
>> -#define smp_rmb()	__lwsync()
>> -#define smp_wmb()	__asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>> +#define smp_rmb()	fast_rmb()
>> +#define smp_wmb()	fast_wmb()
>>   #else
>> -#define __lwsync()	barrier()
>> -
>>   #define smp_mb()	barrier()
>>   #define smp_rmb()	barrier()
>>   #define smp_wmb()	barrier()
>> @@ -69,10 +67,16 @@
>>   #define data_barrier(x)	\
>>   	asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
>>
>> +/*
>> + * The use of smp_rmb() in these functions are actually meant to map from
>> + * smp_rmb()->fast_rmb()->LWSYNC.  This way if smp is disabled then
>> + * smp_rmb()->barrier(), or if the platform doesn't support lwsync it will
>> + * map to the more heavy-weight sync.
>> + */
>>   #define smp_store_release(p, v)						\
>>   do {									\
>>   	compiletime_assert_atomic_type(*p);				\
>> -	__lwsync();							\
>> +	smp_rmb();							\
>
> This is not good at all.  For smp_store_release(), we absolutely
> must order prior loads and stores against the assignment on the following
> line.  This is not something that smp_rmb() does, nor is it something
> that smp_wmb() does.  Yes, it might happen to now, but this could easily
> break in the future -- plus this change is extremely misleading.
>
> The original __lwsync() is much more clear.

The problem I had with __lwsync is that it really wasn't all that clear. 
It was the lwsync instruction if SMP was enabled, otherwise it was just 
a barrier call. What I did is move the definition of __lwsync in the SMP 
case into fast_rmb, which in turn is accessed by smp_rmb. I tried to 
make this clear in the comment just above the two calls. The resultant 
assembly code should be exactly the same.

What I could do is have it added back as a smp_lwsync if that works for 
you. That way there is something there to give you a hint that it 
becomes a barrier() call as soon as SMP is disabled.

Thanks,

Alex

^ permalink raw reply

* Re: [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: Eric Dumazet @ 2014-11-17 21:22 UTC (permalink / raw)
  To: Fabian Frederick; +Cc: linux-kernel, David S. Miller, netdev
In-Reply-To: <1416258502-9001-1-git-send-email-fabf@skynet.be>

On Mon, 2014-11-17 at 22:08 +0100, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  net/core/link_watch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
> index bd0767e..49a9e3e 100644
> --- a/net/core/link_watch.c
> +++ b/net/core/link_watch.c
> @@ -21,7 +21,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
>  #include <linux/bitops.h>
> -#include <asm/types.h>
> +#include <linux/types.h>
>  

We have 99 such occurrences in net/

Are you going to send 99 patches ?

^ permalink raw reply

* [PATCH 1/1 net-next] dev_ioctl: use sizeof(x) instead of sizeof x
From: Fabian Frederick @ 2014-11-17 21:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev

Also remove spaces after cast.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/core/dev_ioctl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 72e899a..b94b1d2 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -142,10 +142,12 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
 
 	case SIOCGIFHWADDR:
 		if (!dev->addr_len)
-			memset(ifr->ifr_hwaddr.sa_data, 0, sizeof ifr->ifr_hwaddr.sa_data);
+			memset(ifr->ifr_hwaddr.sa_data, 0,
+			       sizeof(ifr->ifr_hwaddr.sa_data));
 		else
 			memcpy(ifr->ifr_hwaddr.sa_data, dev->dev_addr,
-			       min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+			       min(sizeof(ifr->ifr_hwaddr.sa_data),
+				   (size_t)dev->addr_len));
 		ifr->ifr_hwaddr.sa_family = dev->type;
 		return 0;
 
@@ -265,7 +267,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
 		if (ifr->ifr_hwaddr.sa_family != dev->type)
 			return -EINVAL;
 		memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
-		       min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+		       min(sizeof(ifr->ifr_hwaddr.sa_data),
+			   (size_t)dev->addr_len));
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 		return 0;
 
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH net-next] i40e: Reduce stack in i40e_dbg_dump_desc
From: Jeff Kirsher @ 2014-11-17 21:30 UTC (permalink / raw)
  To: David Miller; +Cc: joe, shannon.nelson, netdev
In-Reply-To: <20141117.160636.429874041980998120.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

On Mon, 2014-11-17 at 16:06 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sun, 16 Nov 2014 14:12:48 -0800
> 
> > Reduce stack use by using kmemdup and not using a very
> > large struct on stack.
> > 
> > In function ‘i40e_dbg_dump_desc’:
> > warning: the frame size of 8192 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> I'll let Jeff pick this one up, thanks.

Consider it picked up.  Sorry, trying to catch up on all the
email/patches the went out this weekend.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 4/4] fm10k/igb/ixgbe: Use fast_rmb on Rx descriptor reads
From: Jeff Kirsher @ 2014-11-17 21:32 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
	heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
	matthew.vick, geert, romieu, paulmck, nic_swsd, will.deacon,
	michael, tony.luck, torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141117171839.22333.25487.stgit@ahduyck-server>

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Mon, 2014-11-17 at 09:18 -0800, Alexander Duyck wrote:
> This change makes it so that fast_rmb is used when reading the Rx
> descriptor.  The advantage of fast_rmb is that it allows for a much
> lower cost barrier on x86, powerpc, arm, and arm64 architectures than
> a
> traditional memory barrier when dealing with reads that only have to
> synchronize to system memory.
> 
> In addition I have updated the code so that it just checks to see if
> any
> bits have been set instead of just the DD bit since the DD bit will
> always
> be set as a part of a descriptor write-back so we just need to check
> for a
> non-zero value being present at that memory location rather than just
> checking for any specific bit.  This allows the code itself to appear
> much
> cleaner and allows the compiler more room to optimize.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Matthew Vick <matthew.vick@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_main.c |    6 +++---
>  drivers/net/ethernet/intel/igb/igb_main.c     |    6 +++---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    9 ++++-----
>  3 files changed, 10 insertions(+), 11 deletions(-)

Looks like more changes will be coming, based on the feedback on earlier
patches.  So I won't be picking this up for validation purposes.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: David Miller @ 2014-11-17 21:35 UTC (permalink / raw)
  To: eric.dumazet; +Cc: fabf, linux-kernel, netdev
In-Reply-To: <1416259376.18588.8.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 17 Nov 2014 13:22:56 -0800

> On Mon, 2014-11-17 at 22:08 +0100, Fabian Frederick wrote:
>> Signed-off-by: Fabian Frederick <fabf@skynet.be>
>> ---
>>  net/core/link_watch.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
>> index bd0767e..49a9e3e 100644
>> --- a/net/core/link_watch.c
>> +++ b/net/core/link_watch.c
>> @@ -21,7 +21,7 @@
>>  #include <linux/spinlock.h>
>>  #include <linux/workqueue.h>
>>  #include <linux/bitops.h>
>> -#include <asm/types.h>
>> +#include <linux/types.h>
>>  
> 
> We have 99 such occurrences in net/
> 
> Are you going to send 99 patches ?

Yeah, probably best to consolidate into one or a few patches.

^ permalink raw reply

* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-17 21:54 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch@vger.kernel.org, Network Development,
	Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
	Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
	Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
	matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
	Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
	Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <CA+55aFxZ9dhB074p3OAQLyQ53TuRjVOo33h6OOda1soh-9xxnQ@mail.gmail.com>


On 11/17/2014 12:52 PM, Linus Torvalds wrote:
> On Mon, Nov 17, 2014 at 9:18 AM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>> There are a number of situations where the mandatory barriers rmb() and
>> wmb() are used to order memory/memory operations in the device drivers
>> and those barriers are much heavier than they actually need to be.
> Ugh. I absolutely despise the name.
>
> It's not "fast". It's just limited. It's the same as "smp_*mb()", in
> that it works on cacheable memory, but it actually stays around even
> for non-SMP builds.
>
> So I think the name is actively misleading.
>
> Naming should be about what it does, not about some kind of PR thing
> that confuses people into thinking it's "better".
>
> Maybe "dma_*mb()" would be acceptable, and ends up having the same
> naming convention as "smb_*mb()", and explains what it's about.

What would you think of the name "coherent_*mb()"?  I would prefer to 
avoid dma in the name since, at least in my mind, that implies MMIO.

It also ties in well with dma_alloc_coherent/dma_free_coherent which is 
what would typically be used to allocate the memory we would be using 
the barrier to protect anyway.

> And yes, in the same spirit, it would probably be good to try to
> eventually get rid of the plain "*mb()" functions, and perhaps call
> them "mmio_*mb()" to clarify that they are about ordering memory wrt
> mmio.
>
> Hmm?
>
>                          Linus

I will work on pulling all of the coherent barrier cases out of using 
the plain "*mb()" calls first.  We need to sort that out before we could 
look at renaming the plain barrier functions.

- Alex

^ permalink raw reply

* Re: [PATCH Iproute2 next v1] ip link: Add ipvlan support to the iproute2/ip util
From: Mahesh Bandewar @ 2014-11-17 21:54 UTC (permalink / raw)
  To: Sathya Perla
  Cc: netdev, Stephen Hemminger, Eric Dumazet, Maciej Zenczykowski,
	Laurent Chavey, Tim Hockin, David Miller, Brandon Philips,
	Pavel Emelianov
In-Reply-To: <CF9D1877D81D214CB0CA0669EFAE020C68CEC1DB@CMEXMB1.ad.emulex.com>

On Sun, Nov 16, 2014 at 10:57 PM, Sathya Perla <Sathya.Perla@emulex.com> wrote:
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> >
> > Adding basic support to create virtual devices using 'ip'
> > utility. Following is the syntax -
> >
> >       ip link add link <master> <virtual> type ipvlan mode [ l2 | l3 ]
> >       e.g. ip link add link eth0 ipvl0 type ipvlan mode l3
> >
> ..
> > ---
> >  include/linux/if_link.h | 14 ++++++++
> >  ip/Makefile             |  2 +-
> >  ip/iplink.c             |  2 +-
> >  ip/iplink_ipvlan.c      | 85
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 101 insertions(+), 2 deletions(-)
> >  create mode 100644 ip/iplink_ipvlan.c
> >
> > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > index 47320636361c..ef1e9f73fb15 100644
> > --- a/include/linux/if_link.h
> > +++ b/include/linux/if_link.h
> > @@ -325,6 +325,20 @@ enum macvlan_macaddr_mode {
> >       MACVLAN_MACADDR_SET,
> >  };
> >
> > +/* IPVLAN section */
> > +enum {
> > +     IFLA_IPVLAN_UNSPEC,
> > +     IFLA_IPVLAN_MODE,
> > +     __IFLA_IPVLAN_MAX,
> > +};
> > +
> > +#define IFLA_IPVLAN_MAX (__IFLA_IPVLAN_MAX - 1)
> > +
> > +enum ipvlan_mode {
> > +     IPVLAN_MODE_L2 = 0, /* Process packets all the way upto L2 */
> > +     IPVLAN_MODE_L3 = 1, /* Process Packets all the way upto L3 */
> > +};
> > +
> >  #define MACVLAN_FLAG_NOPROMISC       1
> >
> >... +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/socket.h>
> > +#include <linux/if_link.h>
> > +
> > +#include "rt_names.h"
> > +#include "utils.h"
> > +#include "ip_common.h"
> > +
> > +static void explain(void)
> > +{
> > +     fprintf(stderr, "Usage: ... ipvlan [ mode { l2 | l3 } ]\n");
> > +}
> > +
> > +static int mode_arg(void)
> > +{
> > +     fprintf(stderr, "Error: argument of \"mode\" must be either \"l2\", "
> > +             "or \"l3\"\n");
> > +        return -1;
> I guess you wanted to "return -1" from the caller routine ipvlan_parse_opt()
> and not from this routine.
>
Hmmm, that caught another error where I intended it to be "return
mode_arg()" instead of just "mode_arg" in the caller. This is clearly
wrong! Thanks for catching it.
> > +}
> > +
> > +static int ipvlan_parse_opt(struct link_util *lu, int argc, char **argv,
> > +                       struct nlmsghdr *n)
> > +{
> > +     while (argc > 0) {
> > +         if (matches(*argv, "mode") == 0) {
> > +                     __u16 mode = 0;
> > +                     NEXT_ARG();
> > +
> > +                     if (strcmp(*argv, "l2") == 0)
> > +                             mode = IPVLAN_MODE_L2;
> > +                     else if (strcmp(*argv, "l3") == 0)
> > +                             mode = IPVLAN_MODE_L3;
> > +                     else
> > +                             mode_arg();
> > +
> > +                     addattr16(n, 1024, IFLA_IPVLAN_MODE, mode);
> > +             } else if (matches(*argv, "help") == 0) {
> > +                     explain();
> > +                     return -1;
> > +             } else {
> > +                     fprintf(stderr, "ipvlan: unknown option \"%s\"?\n",
> > *argv);
> > +                     explain();
> > +                     return -1;
> > +             }
> > +             argc--, argv++;
> > +     }
> > +
> > +     return 0;
> > +}
> > +

^ permalink raw reply

* ipip6 - integer underrun when handlince icmpv4 unreachable messages
From: Alexander Wetzel @ 2014-11-17 21:52 UTC (permalink / raw)
  To: netdev; +Cc: roque, kuznet, r.venning, nate

Hello netdev,

the current code to translate icmpv4 "destination unreachable" packets
to icmpv6 is later generating an integer underrun when calling
icmpv6_send, by later calling skb_network_header_len and subtracting a
bigger number from a lower one.

The issue is not visible for vanilla kernels and works correctly from
a user perspective, but a kernel with the PAX patches and enabled
"size overflow protection" will panic immediately when it's getting an
icmpv4 destination unreachable packet back for an encapsulated ipv6
packet. (Remote tunnel endpoint not reachable.)

I think I've tracked the issue down and can show you the problem with
the code... as I understand it as non-programmer greping the sources
and googeling functions. I was even able to find a fix which passes
the functionality test, but I'm unqualified to rate the correctness of
it and so are reaching out to you for that.

What happens (output of printk's) with transport_header and
network_header around "skb_reset_network_header" is described below
the function.

Near the end of the mail there are two links to the gentoo bug tracker
and pax forum, suggesting to put this forward to the lkml/netdev for
review, also including more details on the panics.

So here the function "ipip6_err_gen_icmpv6_unreach" from
net/ipv6/sit.c with some remarks and one new line which seems to fix
the problem:

************************************************************************
static int ipip6_err_gen_icmpv6_unreach(struct sk_buff *skb)
{
        int ihl = ((const struct iphdr *)skb->data)->ihl*4;
        struct rt6_info *rt;
        struct sk_buff *skb2;

        if (!pskb_may_pull(skb, ihl + sizeof(struct ipv6hdr) + 8))
                return 1;

// we clone the ipv4 skb in skb2 to prepare the icmpv6 packet
        skb2 = skb_clone(skb, GFP_ATOMIC);

        if (!skb2)
                return 1;

// we clean up the cloned skb2
        skb_dst_drop(skb2);
        skb_pull(skb2, ihl);
// The network header is reset
        skb_reset_network_header(skb2);

//THE PROPOSED FIX: The following line is NOT in the current code
        skb_reset_transport_header(skb2);

        rt = rt6_lookup(dev_net(skb->dev), &ipv6_hdr(skb2)->saddr,
NULL, 0, 0);

        if (rt && rt->dst.dev)
                skb2->dev = rt->dst.dev;

        icmpv6_send(skb2, ICMPV6_DEST_UNREACH, ICMPV6_ADDR_UNREACH, 0);

        if (rt)
                ip6_rt_put(rt);

        kfree_skb(skb2);

        return 0;

************************************************************************

With debug printk's prior to "skb_reset_network_header(skb2);" I get
the following values when the code is used:
	sk2b->transport_header: 62
	skb2->network_header  : 4e

After "skb_reset_network_header(skb2);" it reads:
	sk2b->transport_header: 62
	skb2->network_header  : 7e

That are the same values which are later causing the integer underrun
(and with PAX a kernel panic) in "skb_network_header_len"

Adding "skb_reset_network_header(skb2);" prevents that, the transport
header size is extended:
	sk2b->transport_header: 7e
	skb2->network_header  : 7e


Some more details on the error, a clean diff for the proposed patch
(without the comments) and  the full debugging can be fond here:

https://forums.grsecurity.net/viewtopic.php?t=4083
https://bugs.gentoo.org/show_bug.cgi?id=529352

Can you please verify if this is the correct way to fix it and include
the correct fix - if any - in future kernel releases?
Or do you think that the PAX patch is wrong and an integer underrun is
acceptable here?

I'll check the mailing list archives from time to time for replies,
but I'm not subscribed and if you need me for more information or
tests please add me on CC.

Cheers,

Alexander Wetzel

^ permalink raw reply

* [PATCH v2 net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: Rick Jones @ 2014-11-17 22:04 UTC (permalink / raw)
  To: netdev; +Cc: davem

From: Rick Jones <rick.jones2@hp.com>

If icmp_rcv() has successfully processed the incoming ICMP datagram, we
should use consume_skb() rather than kfree_skb() because a hit on the likes
of perf -e skb:kfree_skb is not called-for.

Signed-off-by: Rick Jones <rick.jones2@hp.com>

---

A test system hit with a flood ping hits on perf top -e ksb:kfre_skb before
the change and none after for the normal/success path.  The IPv6 path would
be somewhat more ugly.  For the time being, just deal with the overlap on
ping_rcv() between the two to avoid a possible double free of an skb.


diff --git a/include/net/ping.h b/include/net/ping.h
index 026479b..f074060 100644
--- a/include/net/ping.h
+++ b/include/net/ping.h
@@ -82,7 +82,7 @@ int  ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
 int  ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		     size_t len);
 int  ping_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
-void ping_rcv(struct sk_buff *skb);
+bool ping_rcv(struct sk_buff *skb);
 
 #ifdef CONFIG_PROC_FS
 struct ping_seq_afinfo {
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 36b7bfa..36f5584 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -190,7 +190,7 @@ EXPORT_SYMBOL(icmp_err_convert);
  */
 
 struct icmp_control {
-	void (*handler)(struct sk_buff *skb);
+	bool (*handler)(struct sk_buff *skb);
 	short   error;		/* This ICMP is classed as an error message */
 };
 
@@ -746,7 +746,7 @@ static bool icmp_tag_validation(int proto)
  *	ICMP_PARAMETERPROB.
  */
 
-static void icmp_unreach(struct sk_buff *skb)
+static bool icmp_unreach(struct sk_buff *skb)
 {
 	const struct iphdr *iph;
 	struct icmphdr *icmph;
@@ -839,10 +839,10 @@ static void icmp_unreach(struct sk_buff *skb)
 	icmp_socket_deliver(skb, info);
 
 out:
-	return;
+	return true;
 out_err:
 	ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
-	goto out;
+	return false;
 }
 
 
@@ -850,17 +850,20 @@ out_err:
  *	Handle ICMP_REDIRECT.
  */
 
-static void icmp_redirect(struct sk_buff *skb)
+static bool icmp_redirect(struct sk_buff *skb)
 {
 	if (skb->len < sizeof(struct iphdr)) {
 		ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
-		return;
+		return false;
 	}
 
-	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
-		return;
+	if (!pskb_may_pull(skb, sizeof(struct iphdr))) {
+		/* there aught to be a stat */
+		return false;
+	}
 
 	icmp_socket_deliver(skb, icmp_hdr(skb)->un.gateway);
+	return true;
 }
 
 /*
@@ -875,7 +878,7 @@ static void icmp_redirect(struct sk_buff *skb)
  *	See also WRT handling of options once they are done and working.
  */
 
-static void icmp_echo(struct sk_buff *skb)
+static bool icmp_echo(struct sk_buff *skb)
 {
 	struct net *net;
 
@@ -891,6 +894,8 @@ static void icmp_echo(struct sk_buff *skb)
 		icmp_param.head_len	   = sizeof(struct icmphdr);
 		icmp_reply(&icmp_param, skb);
 	}
+	/* should there be an ICMP stat for ignored echos? */
+	return true;
 }
 
 /*
@@ -900,7 +905,7 @@ static void icmp_echo(struct sk_buff *skb)
  *		  MUST be accurate to a few minutes.
  *		  MUST be updated at least at 15Hz.
  */
-static void icmp_timestamp(struct sk_buff *skb)
+static bool icmp_timestamp(struct sk_buff *skb)
 {
 	struct timespec tv;
 	struct icmp_bxm icmp_param;
@@ -927,15 +932,17 @@ static void icmp_timestamp(struct sk_buff *skb)
 	icmp_param.data_len	   = 0;
 	icmp_param.head_len	   = sizeof(struct icmphdr) + 12;
 	icmp_reply(&icmp_param, skb);
-out:
-	return;
+	return true;
+
 out_err:
 	ICMP_INC_STATS_BH(dev_net(skb_dst(skb)->dev), ICMP_MIB_INERRORS);
-	goto out;
+	return false;
 }
 
-static void icmp_discard(struct sk_buff *skb)
+static bool icmp_discard(struct sk_buff *skb)
 {
+	/* pretend it was a success */
+	return true;
 }
 
 /*
@@ -946,6 +953,7 @@ int icmp_rcv(struct sk_buff *skb)
 	struct icmphdr *icmph;
 	struct rtable *rt = skb_rtable(skb);
 	struct net *net = dev_net(rt->dst.dev);
+	bool success;
 
 	if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
 		struct sec_path *sp = skb_sec_path(skb);
@@ -1012,7 +1020,12 @@ int icmp_rcv(struct sk_buff *skb)
 		}
 	}
 
-	icmp_pointers[icmph->type].handler(skb);
+	success = icmp_pointers[icmph->type].handler(skb);
+
+	if (success)  {
+		consume_skb(skb);
+		return 0;
+	}
 
 drop:
 	kfree_skb(skb);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 736236c..ce2920f 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -955,7 +955,7 @@ EXPORT_SYMBOL_GPL(ping_queue_rcv_skb);
  *	All we need to do is get the socket.
  */
 
-void ping_rcv(struct sk_buff *skb)
+bool ping_rcv(struct sk_buff *skb)
 {
 	struct sock *sk;
 	struct net *net = dev_net(skb->dev);
@@ -974,11 +974,11 @@ void ping_rcv(struct sk_buff *skb)
 		pr_debug("rcv on socket %p\n", sk);
 		ping_queue_rcv_skb(sk, skb_get(skb));
 		sock_put(sk);
-		return;
+		return true;
 	}
 	pr_debug("no socket, dropping\n");
 
-	/* We're called from icmp_rcv(). kfree_skb() is done there. */
+	return false;
 }
 EXPORT_SYMBOL_GPL(ping_rcv);
 
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 0929340..39b3ff9 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -679,6 +679,7 @@ static int icmpv6_rcv(struct sk_buff *skb)
 	const struct in6_addr *saddr, *daddr;
 	struct icmp6hdr *hdr;
 	u8 type;
+	bool success = false;
 
 	if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
 		struct sec_path *sp = skb_sec_path(skb);
@@ -726,7 +727,7 @@ static int icmpv6_rcv(struct sk_buff *skb)
 		break;
 
 	case ICMPV6_ECHO_REPLY:
-		ping_rcv(skb);
+		success = ping_rcv(skb);
 		break;
 
 	case ICMPV6_PKT_TOOBIG:
@@ -790,7 +791,14 @@ static int icmpv6_rcv(struct sk_buff *skb)
 		icmpv6_notify(skb, type, hdr->icmp6_code, hdr->icmp6_mtu);
 	}
 
-	kfree_skb(skb);
+	/* until the v6 path can be better sorted assume failure and
+	 * preserve the status quo behaviour for the rest of the paths to here
+	 */
+	if (success)
+		consume_skb(skb);
+	else
+		kfree_skb(skb);
+
 	return 0;
 
 csum_error:

^ permalink raw reply related

* Re: [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: Eric Dumazet @ 2014-11-17 22:05 UTC (permalink / raw)
  To: David Miller; +Cc: fabf, linux-kernel, netdev
In-Reply-To: <20141117.163532.788436432373669437.davem@davemloft.net>

On Mon, 2014-11-17 at 16:35 -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 17 Nov 2014 13:22:56 -0800
> 
> > On Mon, 2014-11-17 at 22:08 +0100, Fabian Frederick wrote:
> >> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> >> ---
> >>  net/core/link_watch.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
> >> index bd0767e..49a9e3e 100644
> >> --- a/net/core/link_watch.c
> >> +++ b/net/core/link_watch.c
> >> @@ -21,7 +21,7 @@
> >>  #include <linux/spinlock.h>
> >>  #include <linux/workqueue.h>
> >>  #include <linux/bitops.h>
> >> -#include <asm/types.h>
> >> +#include <linux/types.h>
> >>  
> > 
> > We have 99 such occurrences in net/
> > 
> > Are you going to send 99 patches ?
> 
> Yeah, probably best to consolidate into one or a few patches.

BTW the 99 occurrences are for asm/uaccess.h, it seems I gave the reply
on the wrong patch.

^ permalink raw reply

* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Maximilian Engelhardt @ 2014-11-17 22:36 UTC (permalink / raw)
  To: Rafał Miłecki, Arend van Spriel
  Cc: Michael Tokarev, Seth Forshee, brcm80211 development,
	linux-wireless@vger.kernel.org, Network Development
In-Reply-To: <CACna6rw3XgdJz0uKkAuas8RTzozTdFp2mgF5g1uo7LHJVSMt4g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]

On Thursday 09 October 2014 10:21:12 Rafał Miłecki wrote:
> On 9 October 2014 09:52, Arend van Spriel <arend@broadcom.com> wrote:
> > I have been staring at differences brcmsmac and our internal driver (which
> > wl is derived from as well). Would you be able to make an mmiotrace
> > loading
> > wl and brcmsmac.
> 
> Arend,
> 
> As you noticed earlier, this is a WiFi card with BT. I've checked
> siutils.c from SDK 7.14.43.7 (got it from Netgear R8000) and it has
> three BCM4313 workarounds:
> 
> 
> 1) si_doattach
> SI_MSG(("Applying 4313 WARs\n"));
> si_pmu_chipcontrol(sih, 0, CCTRL_4313_12MA_LED_DRIVE,
> CCTRL_4313_12MA_LED_DRIVE);
> 
> It's already implemented in bcma, see driver_chipcommon_pmu.c
> 
> 
> 2) si_epa_4313war
> /* EPA Fix */
> W_REG(sii->osh, &cc->gpiocontrol,
> R_REG(sii->osh, &cc->gpiocontrol) | GPIO_CTRL_EPA_EN_MASK);
> 
> This is what you already implement in ai_epa_4313war
> 
> 
> 3) si_pmu_synth_pwrsw_4313_war
> I don't think it's implemented.
> 
> 
> 4) si_btcombo_p250_4313_war
> I also don't think it's implemented
> 
> 
> The last one (si_btcombo_p250_4313_war) looks promising, the comment
> above the function says:
> /** WL/BT control for 4313 btcombo boards >= P250 */
> I guess you need to re-implement this function and call it somewhere
> in brcmsmac.

Hi,

I just wanted to ask if there is any progress on this issue since I haven't 
heard anything for a month. Please let me know if you need any additional 
information.

Greetings,
Maxi


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH net-next 0/4] igb: auxiliary PHC functions for the i210.
From: Richard Cochran @ 2014-11-17 23:06 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
	John Ronciak, Matthew Vick

This patch series adds three features: time stamping external events,
producing a periodic output signal, and an internal PPS event. The
i210 PCIe card has a 6 pin header with SDP0-3, making it easy to try
out this new functionality.

An earlier version of this series was posted way back in May, 2013,
but that version lacked a good way to assign functions to the pins. In
the mean time, the PHC core has a standard method to configure the
pins, and this series makes use of it.

Thanks,
Richard

Richard Cochran (4):
  igb: refactor time sync interrupt handling
  igb: do not clobber the TSAUXC bits on reset.
  igb: enable internal PPS for the i210.
  igb: enable auxiliary PHC functions for the i210.

 drivers/net/ethernet/intel/igb/igb.h      |    9 ++
 drivers/net/ethernet/intel/igb/igb_main.c |  100 ++++++++----
 drivers/net/ethernet/intel/igb/igb_ptp.c  |  251 ++++++++++++++++++++++++++++-
 3 files changed, 326 insertions(+), 34 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH net-next 1/4] igb: refactor time sync interrupt handling
From: Richard Cochran @ 2014-11-17 23:06 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
	John Ronciak, Matthew Vick
In-Reply-To: <cover.1416265321.git.richardcochran@gmail.com>

The code that handles the time sync interrupt is repeated in three
different places. This patch refactors the identical code blocks into
a single helper function.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c |   49 +++++++++++------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b0e12e7..7183a56 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5383,6 +5383,19 @@ void igb_update_stats(struct igb_adapter *adapter,
 	}
 }
 
+static void igb_tsync_interrupt(struct igb_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	u32 tsicr = rd32(E1000_TSICR);
+
+	if (tsicr & E1000_TSICR_TXTS) {
+		/* acknowledge the interrupt */
+		wr32(E1000_TSICR, E1000_TSICR_TXTS);
+		/* retrieve hardware timestamp */
+		schedule_work(&adapter->ptp_tx_work);
+	}
+}
+
 static irqreturn_t igb_msix_other(int irq, void *data)
 {
 	struct igb_adapter *adapter = data;
@@ -5414,16 +5427,8 @@ static irqreturn_t igb_msix_other(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
-	if (icr & E1000_ICR_TS) {
-		u32 tsicr = rd32(E1000_TSICR);
-
-		if (tsicr & E1000_TSICR_TXTS) {
-			/* acknowledge the interrupt */
-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
-			/* retrieve hardware timestamp */
-			schedule_work(&adapter->ptp_tx_work);
-		}
-	}
+	if (icr & E1000_ICR_TS)
+		igb_tsync_interrupt(adapter);
 
 	wr32(E1000_EIMS, adapter->eims_other);
 
@@ -6202,16 +6207,8 @@ static irqreturn_t igb_intr_msi(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
-	if (icr & E1000_ICR_TS) {
-		u32 tsicr = rd32(E1000_TSICR);
-
-		if (tsicr & E1000_TSICR_TXTS) {
-			/* acknowledge the interrupt */
-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
-			/* retrieve hardware timestamp */
-			schedule_work(&adapter->ptp_tx_work);
-		}
-	}
+	if (icr & E1000_ICR_TS)
+		igb_tsync_interrupt(adapter);
 
 	napi_schedule(&q_vector->napi);
 
@@ -6256,16 +6253,8 @@ static irqreturn_t igb_intr(int irq, void *data)
 			mod_timer(&adapter->watchdog_timer, jiffies + 1);
 	}
 
-	if (icr & E1000_ICR_TS) {
-		u32 tsicr = rd32(E1000_TSICR);
-
-		if (tsicr & E1000_TSICR_TXTS) {
-			/* acknowledge the interrupt */
-			wr32(E1000_TSICR, E1000_TSICR_TXTS);
-			/* retrieve hardware timestamp */
-			schedule_work(&adapter->ptp_tx_work);
-		}
-	}
+	if (icr & E1000_ICR_TS)
+		igb_tsync_interrupt(adapter);
 
 	napi_schedule(&q_vector->napi);
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 2/4] igb: do not clobber the TSAUXC bits on reset.
From: Richard Cochran @ 2014-11-17 23:06 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
	John Ronciak, Matthew Vick
In-Reply-To: <cover.1416265321.git.richardcochran@gmail.com>

The TSAUXC register has a number of different bits, one of which disables
the main clock function. Previously, the clock was re-enabled by clearing
the entire register. This patch changes the code to preserve the values
of the other bits in that register.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_ptp.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 794c139..ce57128 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -905,6 +905,8 @@ void igb_ptp_stop(struct igb_adapter *adapter)
 void igb_ptp_reset(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	unsigned long flags;
+	u32 tsauxc;
 
 	if (!(adapter->flags & IGB_FLAG_PTP))
 		return;
@@ -923,7 +925,11 @@ void igb_ptp_reset(struct igb_adapter *adapter)
 	case e1000_i210:
 	case e1000_i211:
 		/* Enable the timer functions and interrupts. */
-		wr32(E1000_TSAUXC, 0x0);
+		spin_lock_irqsave(&adapter->tmreg_lock, flags);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsauxc &= ~TSAUXC_DISABLE;
+		wr32(E1000_TSAUXC, tsauxc);
+		spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
 		wr32(E1000_TSIM, TSYNC_INTERRUPTS);
 		wr32(E1000_IMS, E1000_IMS_TS);
 		break;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 3/4] igb: enable internal PPS for the i210.
From: Richard Cochran @ 2014-11-17 23:06 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
	John Ronciak, Matthew Vick
In-Reply-To: <cover.1416265321.git.richardcochran@gmail.com>

The i210 device can produce an interrupt on the full second. This
patch allows using this interrupt to generate an internal PPS event
for adjusting the kernel system time.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c |    6 ++++++
 drivers/net/ethernet/intel/igb/igb_ptp.c  |   32 +++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 7183a56..9412661 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5386,8 +5386,14 @@ void igb_update_stats(struct igb_adapter *adapter,
 static void igb_tsync_interrupt(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	struct ptp_clock_event event;
 	u32 tsicr = rd32(E1000_TSICR);
 
+	if (tsicr & TSINTR_SYS_WRAP) {
+		event.type = PTP_CLOCK_PPS;
+		ptp_clock_event(adapter->ptp_clock, &event);
+	}
+
 	if (tsicr & E1000_TSICR_TXTS) {
 		/* acknowledge the interrupt */
 		wr32(E1000_TSICR, E1000_TSICR_TXTS);
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index ce57128..70d3933 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -360,6 +360,34 @@ static int igb_ptp_settime_i210(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
+				       struct ptp_clock_request *rq, int on)
+{
+	struct igb_adapter *igb =
+		container_of(ptp, struct igb_adapter, ptp_caps);
+	struct e1000_hw *hw = &igb->hw;
+	unsigned long flags;
+	u32 tsim;
+
+	switch (rq->type) {
+	case PTP_CLK_REQ_PPS:
+		spin_lock_irqsave(&igb->tmreg_lock, flags);
+		tsim = rd32(E1000_TSIM);
+		if (on)
+			tsim |= TSINTR_SYS_WRAP;
+		else
+			tsim &= ~TSINTR_SYS_WRAP;
+		wr32(E1000_TSIM, tsim);
+		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+		return 0;
+
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
 static int igb_ptp_feature_enable(struct ptp_clock_info *ptp,
 				  struct ptp_clock_request *rq, int on)
 {
@@ -802,12 +830,12 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
 		adapter->ptp_caps.n_ext_ts = 0;
-		adapter->ptp_caps.pps = 0;
+		adapter->ptp_caps.pps = 1;
 		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
 		adapter->ptp_caps.gettime = igb_ptp_gettime_i210;
 		adapter->ptp_caps.settime = igb_ptp_settime_i210;
-		adapter->ptp_caps.enable = igb_ptp_feature_enable;
+		adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 4/4] igb: enable auxiliary PHC functions for the i210.
From: Richard Cochran @ 2014-11-17 23:06 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
	John Ronciak, Matthew Vick
In-Reply-To: <cover.1416265321.git.richardcochran@gmail.com>

The i210 device offers a number of special PTP Hardware Clock features on
the Software Defined Pins (SDPs). This patch adds support for two of the
possible functions, namely time stamping external events, and periodic
output signals.

The assignment of PHC functions to the four SDP can be freely chosen by
the user.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/igb/igb.h      |    9 ++
 drivers/net/ethernet/intel/igb/igb_main.c |   47 ++++++-
 drivers/net/ethernet/intel/igb/igb_ptp.c  |  219 ++++++++++++++++++++++++++++-
 3 files changed, 269 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 82d891e..f6aca7c 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -343,6 +343,9 @@ struct hwmon_buff {
 	};
 #endif
 
+#define IGB_N_EXTTS	2
+#define IGB_N_PEROUT	2
+#define IGB_N_SDP	4
 #define IGB_RETA_SIZE	128
 
 /* board specific private data structure */
@@ -439,6 +442,12 @@ struct igb_adapter {
 	u32 tx_hwtstamp_timeouts;
 	u32 rx_hwtstamp_cleared;
 
+	struct ptp_pin_desc sdp_config[IGB_N_SDP];
+	struct {
+		struct timespec start;
+		struct timespec period;
+	} perout[IGB_N_PEROUT];
+
 	char fw_version[32];
 #ifdef CONFIG_IGB_HWMON
 	struct hwmon_buff *igb_hwmon_buff;
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 9412661..3a25661 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5387,7 +5387,8 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	struct ptp_clock_event event;
-	u32 tsicr = rd32(E1000_TSICR);
+	struct timespec ts;
+	u32 tsauxc, sec, nsec, tsicr = rd32(E1000_TSICR);
 
 	if (tsicr & TSINTR_SYS_WRAP) {
 		event.type = PTP_CLOCK_PPS;
@@ -5400,6 +5401,50 @@ static void igb_tsync_interrupt(struct igb_adapter *adapter)
 		/* retrieve hardware timestamp */
 		schedule_work(&adapter->ptp_tx_work);
 	}
+
+	if (tsicr & TSINTR_TT0) {
+		spin_lock(&adapter->tmreg_lock);
+		ts = timespec_add(adapter->perout[0].start,
+				  adapter->perout[0].period);
+		wr32(E1000_TRGTTIML0, ts.tv_nsec);
+		wr32(E1000_TRGTTIMH0, ts.tv_sec);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsauxc |= TSAUXC_EN_TT0;
+		wr32(E1000_TSAUXC, tsauxc);
+		adapter->perout[0].start = ts;
+		spin_unlock(&adapter->tmreg_lock);
+	}
+
+	if (tsicr & TSINTR_TT1) {
+		spin_lock(&adapter->tmreg_lock);
+		ts = timespec_add(adapter->perout[1].start,
+				  adapter->perout[1].period);
+		wr32(E1000_TRGTTIML1, ts.tv_nsec);
+		wr32(E1000_TRGTTIMH1, ts.tv_sec);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsauxc |= TSAUXC_EN_TT1;
+		wr32(E1000_TSAUXC, tsauxc);
+		adapter->perout[1].start = ts;
+		spin_unlock(&adapter->tmreg_lock);
+	}
+
+	if (tsicr & TSINTR_AUTT0) {
+		nsec = rd32(E1000_AUXSTMPL0);
+		sec  = rd32(E1000_AUXSTMPH0);
+		event.type = PTP_CLOCK_EXTTS;
+		event.index = 0;
+		event.timestamp = sec * 1000000000ULL + nsec;
+		ptp_clock_event(adapter->ptp_clock, &event);
+	}
+
+	if (tsicr & TSINTR_AUTT1) {
+		nsec = rd32(E1000_AUXSTMPL1);
+		sec  = rd32(E1000_AUXSTMPH1);
+		event.type = PTP_CLOCK_EXTTS;
+		event.index = 1;
+		event.timestamp = sec * 1000000000ULL + nsec;
+		ptp_clock_event(adapter->ptp_clock, &event);
+	}
 }
 
 static irqreturn_t igb_msix_other(int irq, void *data)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 70d3933..37b9fe6 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -360,16 +360,203 @@ static int igb_ptp_settime_i210(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static void igb_pin_direction(int pin, int input, u32 *ctrl, u32 *ctrl_ext)
+{
+	u32 *ptr = pin < 2 ? ctrl : ctrl_ext;
+	u32 mask[IGB_N_SDP] = {
+		E1000_CTRL_SDP0_DIR,
+		E1000_CTRL_SDP1_DIR,
+		E1000_CTRL_EXT_SDP2_DIR,
+		E1000_CTRL_EXT_SDP3_DIR,
+	};
+
+	if (input)
+		*ptr &= ~mask[pin];
+	else
+		*ptr |= mask[pin];
+}
+
+static void igb_pin_extts(struct igb_adapter *igb, int chan, int pin)
+{
+	struct e1000_hw *hw = &igb->hw;
+	u32 aux0_sel_sdp[IGB_N_SDP] = {
+		AUX0_SEL_SDP0, AUX0_SEL_SDP1, AUX0_SEL_SDP2, AUX0_SEL_SDP3,
+	};
+	u32 aux1_sel_sdp[IGB_N_SDP] = {
+		AUX1_SEL_SDP0, AUX1_SEL_SDP1, AUX1_SEL_SDP2, AUX1_SEL_SDP3,
+	};
+	u32 ts_sdp_en[IGB_N_SDP] = {
+		TS_SDP0_EN, TS_SDP1_EN, TS_SDP2_EN, TS_SDP3_EN,
+	};
+	u32 ctrl, ctrl_ext, tssdp = 0;
+
+	ctrl = rd32(E1000_CTRL);
+	ctrl_ext = rd32(E1000_CTRL_EXT);
+	tssdp = rd32(E1000_TSSDP);
+
+	igb_pin_direction(pin, 1, &ctrl, &ctrl_ext);
+
+	/* Make sure this pin is not enabled as an ouput. */
+	tssdp &= ~ts_sdp_en[pin];
+
+	if (chan == 1) {
+		tssdp &= ~AUX1_SEL_SDP3;
+		tssdp |= aux1_sel_sdp[pin] | AUX1_TS_SDP_EN;
+	} else {
+		tssdp &= ~AUX0_SEL_SDP3;
+		tssdp |= aux0_sel_sdp[pin] | AUX0_TS_SDP_EN;
+	}
+
+	wr32(E1000_TSSDP, tssdp);
+	wr32(E1000_CTRL, ctrl);
+	wr32(E1000_CTRL_EXT, ctrl_ext);
+}
+
+static void igb_pin_perout(struct igb_adapter *igb, int chan, int pin)
+{
+	struct e1000_hw *hw = &igb->hw;
+	u32 aux0_sel_sdp[IGB_N_SDP] = {
+		AUX0_SEL_SDP0, AUX0_SEL_SDP1, AUX0_SEL_SDP2, AUX0_SEL_SDP3,
+	};
+	u32 aux1_sel_sdp[IGB_N_SDP] = {
+		AUX1_SEL_SDP0, AUX1_SEL_SDP1, AUX1_SEL_SDP2, AUX1_SEL_SDP3,
+	};
+	u32 ts_sdp_en[IGB_N_SDP] = {
+		TS_SDP0_EN, TS_SDP1_EN, TS_SDP2_EN, TS_SDP3_EN,
+	};
+	u32 ts_sdp_sel_tt0[IGB_N_SDP] = {
+		TS_SDP0_SEL_TT0, TS_SDP1_SEL_TT0,
+		TS_SDP2_SEL_TT0, TS_SDP3_SEL_TT0,
+	};
+	u32 ts_sdp_sel_tt1[IGB_N_SDP] = {
+		TS_SDP0_SEL_TT1, TS_SDP1_SEL_TT1,
+		TS_SDP2_SEL_TT1, TS_SDP3_SEL_TT1,
+	};
+	u32 ts_sdp_sel_clr[IGB_N_SDP] = {
+		TS_SDP0_SEL_FC1, TS_SDP1_SEL_FC1,
+		TS_SDP2_SEL_FC1, TS_SDP3_SEL_FC1,
+	};
+	u32 ctrl, ctrl_ext, tssdp = 0;
+
+	ctrl = rd32(E1000_CTRL);
+	ctrl_ext = rd32(E1000_CTRL_EXT);
+	tssdp = rd32(E1000_TSSDP);
+
+	igb_pin_direction(pin, 0, &ctrl, &ctrl_ext);
+
+	/* Make sure this pin is not enabled as an input. */
+	if ((tssdp & AUX0_SEL_SDP3) == aux0_sel_sdp[pin])
+		tssdp &= ~AUX0_TS_SDP_EN;
+
+	if ((tssdp & AUX1_SEL_SDP3) == aux1_sel_sdp[pin])
+		tssdp &= ~AUX1_TS_SDP_EN;
+
+	tssdp &= ~ts_sdp_sel_clr[pin];
+	if (chan == 1)
+		tssdp |= ts_sdp_sel_tt1[pin];
+	else
+		tssdp |= ts_sdp_sel_tt0[pin];
+
+	tssdp |= ts_sdp_en[pin];
+
+	wr32(E1000_TSSDP, tssdp);
+	wr32(E1000_CTRL, ctrl);
+	wr32(E1000_CTRL_EXT, ctrl_ext);
+}
+
 static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
 				       struct ptp_clock_request *rq, int on)
 {
 	struct igb_adapter *igb =
 		container_of(ptp, struct igb_adapter, ptp_caps);
 	struct e1000_hw *hw = &igb->hw;
+	u32 tsauxc, tsim, tsauxc_mask, tsim_mask, trgttiml, trgttimh;
 	unsigned long flags;
-	u32 tsim;
+	struct timespec ts;
+	int pin;
+	s64 ns;
 
 	switch (rq->type) {
+	case PTP_CLK_REQ_EXTTS:
+		if (on) {
+			pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
+					   rq->extts.index);
+			if (pin < 0)
+				return -EBUSY;
+			igb_pin_extts(igb, rq->extts.index, pin);
+		}
+		if (rq->extts.index == 1) {
+			tsauxc_mask = TSAUXC_EN_TS1;
+			tsim_mask = TSINTR_AUTT1;
+		} else {
+			tsauxc_mask = TSAUXC_EN_TS0;
+			tsim_mask = TSINTR_AUTT0;
+		}
+		spin_lock_irqsave(&igb->tmreg_lock, flags);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsim = rd32(E1000_TSIM);
+		if (on) {
+			tsauxc |= tsauxc_mask;
+			tsim |= tsim_mask;
+		} else {
+			tsauxc &= ~tsauxc_mask;
+			tsim &= ~tsim_mask;
+		}
+		wr32(E1000_TSAUXC, tsauxc);
+		wr32(E1000_TSIM, tsim);
+		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+		return 0;
+
+	case PTP_CLK_REQ_PEROUT:
+		if (on) {
+			pin = ptp_find_pin(igb->ptp_clock, PTP_PF_PEROUT,
+					   rq->perout.index);
+			if (pin < 0)
+				return -EBUSY;
+			igb_pin_perout(igb, rq->perout.index, pin);
+		}
+		ts.tv_sec = rq->perout.period.sec;
+		ts.tv_nsec = rq->perout.period.nsec;
+		ns = timespec_to_ns(&ts);
+		ns = ns >> 1;
+		if (on && ns < 500000LL) {
+			/* 2k interrupts per second is an awful lot. */
+			return -EINVAL;
+		}
+		ts = ns_to_timespec(ns);
+		if (rq->perout.index == 1) {
+			tsauxc_mask = TSAUXC_EN_TT1;
+			tsim_mask = TSINTR_TT1;
+			trgttiml = E1000_TRGTTIML1;
+			trgttimh = E1000_TRGTTIMH1;
+		} else {
+			tsauxc_mask = TSAUXC_EN_TT0;
+			tsim_mask = TSINTR_TT0;
+			trgttiml = E1000_TRGTTIML0;
+			trgttimh = E1000_TRGTTIMH0;
+		}
+		spin_lock_irqsave(&igb->tmreg_lock, flags);
+		tsauxc = rd32(E1000_TSAUXC);
+		tsim = rd32(E1000_TSIM);
+		if (on) {
+			int i = rq->perout.index;
+			igb->perout[i].start.tv_sec = rq->perout.start.sec;
+			igb->perout[i].start.tv_nsec = rq->perout.start.nsec;
+			igb->perout[i].period.tv_sec = ts.tv_sec;
+			igb->perout[i].period.tv_nsec = ts.tv_nsec;
+			wr32(trgttiml, rq->perout.start.sec);
+			wr32(trgttimh, rq->perout.start.nsec);
+			tsauxc |= tsauxc_mask;
+			tsim |= tsim_mask;
+		} else {
+			tsauxc &= ~tsauxc_mask;
+			tsim &= ~tsim_mask;
+		}
+		wr32(E1000_TSAUXC, tsauxc);
+		wr32(E1000_TSIM, tsim);
+		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
+		return 0;
+
 	case PTP_CLK_REQ_PPS:
 		spin_lock_irqsave(&igb->tmreg_lock, flags);
 		tsim = rd32(E1000_TSIM);
@@ -380,9 +567,6 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
 		wr32(E1000_TSIM, tsim);
 		spin_unlock_irqrestore(&igb->tmreg_lock, flags);
 		return 0;
-
-	default:
-		break;
 	}
 
 	return -EOPNOTSUPP;
@@ -394,6 +578,20 @@ static int igb_ptp_feature_enable(struct ptp_clock_info *ptp,
 	return -EOPNOTSUPP;
 }
 
+static int igb_ptp_verify_pin(struct ptp_clock_info *ptp, unsigned int pin,
+			      enum ptp_pin_function func, unsigned int chan)
+{
+	switch (func) {
+	case PTP_PF_NONE:
+	case PTP_PF_EXTTS:
+	case PTP_PF_PEROUT:
+		break;
+	case PTP_PF_PHYSYNC:
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * igb_ptp_tx_work
  * @work: pointer to work struct
@@ -784,6 +982,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	struct net_device *netdev = adapter->netdev;
+	int i;
 
 	switch (hw->mac.type) {
 	case e1000_82576:
@@ -826,16 +1025,26 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		break;
 	case e1000_i210:
 	case e1000_i211:
+		for (i = 0; i < IGB_N_SDP; i++) {
+			struct ptp_pin_desc *ppd = &adapter->sdp_config[i];
+			snprintf(ppd->name, sizeof(ppd->name), "SDP%d", i);
+			ppd->index = i;
+			ppd->func = PTP_PF_NONE;
+		}
 		snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr);
 		adapter->ptp_caps.owner = THIS_MODULE;
 		adapter->ptp_caps.max_adj = 62499999;
-		adapter->ptp_caps.n_ext_ts = 0;
+		adapter->ptp_caps.n_ext_ts = IGB_N_EXTTS;
+		adapter->ptp_caps.n_per_out = IGB_N_PEROUT;
+		adapter->ptp_caps.n_pins = IGB_N_SDP;
 		adapter->ptp_caps.pps = 1;
+		adapter->ptp_caps.pin_config = adapter->sdp_config;
 		adapter->ptp_caps.adjfreq = igb_ptp_adjfreq_82580;
 		adapter->ptp_caps.adjtime = igb_ptp_adjtime_i210;
 		adapter->ptp_caps.gettime = igb_ptp_gettime_i210;
 		adapter->ptp_caps.settime = igb_ptp_settime_i210;
 		adapter->ptp_caps.enable = igb_ptp_feature_enable_i210;
+		adapter->ptp_caps.verify = igb_ptp_verify_pin;
 		/* Enable the timer functions by clearing bit 31. */
 		wr32(E1000_TSAUXC, 0x0);
 		break;
-- 
1.7.10.4

^ permalink raw reply related


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