* [PATCH net-next] fib_trie: avoid a redundant bit judgement in inflate
From: baker.kernel @ 2013-09-29 13:24 UTC (permalink / raw)
To: davem; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel,
baker.zhang
From: "baker.zhang" <baker.kernel@gmail.com>
Because 'node' is the i'st child of 'oldnode',
thus, here 'i' equals
tkey_extract_bits(node->key, oldtnode->pos, oldtnode->bits)
we just get 1 more bit,
and need not care the detail value of this bits.
Signed-off-by: baker.zhang <baker.kernel@gmail.com>
---
net/ipv4/fib_trie.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 3df6d3e..92a4c7f8 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -762,12 +762,9 @@ static struct tnode *inflate(struct trie *t, struct tnode *tn)
if (IS_LEAF(node) || ((struct tnode *) node)->pos >
tn->pos + tn->bits - 1) {
- if (tkey_extract_bits(node->key,
- oldtnode->pos + oldtnode->bits,
- 1) == 0)
- put_child(tn, 2*i, node);
- else
- put_child(tn, 2*i+1, node);
+ put_child(t, tn,
+ tkey_extract_bits(node->key, oldtnode->pos, oldtnode->bits + 1),
+ node);
continue;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH V2 net-next] fib_trie: avoid a redundant bit judgement in inflate
From: baker.kernel @ 2013-09-29 13:31 UTC (permalink / raw)
To: davem; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel,
baker.zhang
From: "baker.zhang" <baker.kernel@gmail.com>
Because 'node' is the i'st child of 'oldnode',
thus, here 'i' equals
tkey_extract_bits(node->key, oldtnode->pos, oldtnode->bits)
we just get 1 more bit,
and need not care the detail value of this bits.
Signed-off-by: baker.zhang <baker.kernel@gmail.com>
---
net/ipv4/fib_trie.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 3df6d3e..45c74ba 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -762,12 +762,9 @@ static struct tnode *inflate(struct trie *t, struct tnode *tn)
if (IS_LEAF(node) || ((struct tnode *) node)->pos >
tn->pos + tn->bits - 1) {
- if (tkey_extract_bits(node->key,
- oldtnode->pos + oldtnode->bits,
- 1) == 0)
- put_child(tn, 2*i, node);
- else
- put_child(tn, 2*i+1, node);
+ put_child(tn,
+ tkey_extract_bits(node->key, oldtnode->pos, oldtnode->bits + 1),
+ node);
continue;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH] ath6kl: fix compilation warning in ath6kl_htc_pipe_conn_service
From: Vladimir Murzin @ 2013-09-29 13:54 UTC (permalink / raw)
To: netdev; +Cc: kvalo, linville, Vladimir Murzin
Fix the warning
drivers/net/wireless/ath/ath6kl/htc_pipe.c: In function
'ath6kl_htc_pipe_conn_service':
drivers/net/wireless/ath/ath6kl/htc_pipe.c:1293:26: warning: integer overflow
in expression [-Woverflow]
by giving a hint to compiler about unsigned nature of
HTC_CONN_FLGS_SET_RECV_ALLOC_MASK
Signed-off-by: Vladimir Murzin <murzin.v@gmail.com>
---
drivers/net/wireless/ath/ath6kl/htc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath6kl/htc.h b/drivers/net/wireless/ath/ath6kl/htc.h
index a2c8ff8..14cab14 100644
--- a/drivers/net/wireless/ath/ath6kl/htc.h
+++ b/drivers/net/wireless/ath/ath6kl/htc.h
@@ -60,7 +60,7 @@
/* disable credit flow control on a specific service */
#define HTC_CONN_FLGS_DISABLE_CRED_FLOW_CTRL (1 << 3)
#define HTC_CONN_FLGS_SET_RECV_ALLOC_SHIFT 8
-#define HTC_CONN_FLGS_SET_RECV_ALLOC_MASK 0xFF00
+#define HTC_CONN_FLGS_SET_RECV_ALLOC_MASK 0xFF00U
/* connect response status codes */
#define HTC_SERVICE_SUCCESS 0
--
1.8.1.5
^ permalink raw reply related
* Re: [PATCH] tcp: TSQ can use a dynamic limit
From: Cong Wang @ 2013-09-29 15:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Wei Liu, Linux Kernel Network Developers,
Yuchung Cheng, Neal Cardwell
In-Reply-To: <1380277734.30872.25.camel@edumazet-glaptop.roam.corp.google.com>
On Fri, Sep 27, 2013 at 3:28 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Additional change : skb->destructor should be set to tcp_wfree().
>
> A future patch (for linux 3.13+) might remove tcp_limit_output_bytes
Yeah, now it is only used in tcp_xmit_size_goal()...
^ permalink raw reply
* Re: [PATCH] IPv6: Allow the MTU of ipip6 tunnel to be set below 1280
From: Hannes Frederic Sowa @ 2013-09-29 15:45 UTC (permalink / raw)
To: Oussama Ghorbel
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel,
ou.ghorbel
In-Reply-To: <CA+ev270d2Ztq9i34Lv9U15y1TbH7W-fioXFn-Q_sJd5F4biHuw@mail.gmail.com>
On Sun, Sep 29, 2013 at 10:40:11AM +0100, Oussama Ghorbel wrote:
> On Fri, Sep 27, 2013 at 6:03 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > Ok, let's go with one function per protocol type. Seems easier.
> >
> > It seems to get more hairy, because it depends on the tunnel driver if the
> > prepended ip header is accounted in hard_header_len. :/
> >
> > I don't know if it works out cleanly. Otherwise I would be ok if the checks
> > just get repeated in ip6_tunnel and leave the rest as-is.
> >
> Yes, It will be the clean way to do it.
Fine. :)
> >
> > Linux currently cannot create "jumbograms" (only the receiving side
> > is supported).
> >
> I understand, but what are the benefit from this limit or the harm
> from not specifying it?
> Please check this comment from eth.c
>
> /**
> * eth_change_mtu - set new MTU size
> * @dev: network device
> * @new_mtu: new Maximum Transfer Unit
> *
> * Allow changing MTU size. Needs to be overridden for devices
> * supporting jumbo frames.
> */
> int eth_change_mtu(struct net_device *dev, int new_mtu)
Hmm, I cannot judge without the full patch. Will it be applicable
to all net_devices or just ethernet ones? The name could be a bit
misleading. Remindes me a lot of dev_set_mtu based on the signature, btw.
> So wouldn't be a good idea to let our function open for jumbo frames...?
Hm, we can document the fact that the function would needed to be updated in
that case. But we should not allow to set a mtu which would require jumbograms
currently.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH] IPv6: Allow the MTU of ipip6 tunnel to be set below 1280
From: Oussama Ghorbel @ 2013-09-29 16:33 UTC (permalink / raw)
To: Oussama Ghorbel, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel,
Oussama Ghorbel
In-Reply-To: <20130929154546.GA10771@order.stressinduktion.org>
On Sun, Sep 29, 2013 at 4:45 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Sun, Sep 29, 2013 at 10:40:11AM +0100, Oussama Ghorbel wrote:
>> On Fri, Sep 27, 2013 at 6:03 PM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>> > Ok, let's go with one function per protocol type. Seems easier.
>> >
>> > It seems to get more hairy, because it depends on the tunnel driver if the
>> > prepended ip header is accounted in hard_header_len. :/
>> >
>> > I don't know if it works out cleanly. Otherwise I would be ok if the checks
>> > just get repeated in ip6_tunnel and leave the rest as-is.
>> >
>> Yes, It will be the clean way to do it.
>
> Fine. :)
>
>> >
>> > Linux currently cannot create "jumbograms" (only the receiving side
>> > is supported).
>> >
>> I understand, but what are the benefit from this limit or the harm
>> from not specifying it?
>> Please check this comment from eth.c
>>
>> /**
>> * eth_change_mtu - set new MTU size
>> * @dev: network device
>> * @new_mtu: new Maximum Transfer Unit
>> *
>> * Allow changing MTU size. Needs to be overridden for devices
>> * supporting jumbo frames.
>> */
>> int eth_change_mtu(struct net_device *dev, int new_mtu)
>
> Hmm, I cannot judge without the full patch. Will it be applicable
> to all net_devices or just ethernet ones? The name could be a bit
> misleading. Remindes me a lot of dev_set_mtu based on the signature, btw.
Normally to all net_devices, otherwise it could get complicated to
check for every dev separately ...
But, never mind, the comment below solve the issue
>
>> So wouldn't be a good idea to let our function open for jumbo frames...?
>
> Hm, we can document the fact that the function would needed to be updated in
> that case. But we should not allow to set a mtu which would require jumbograms
> currently.
OK, sounds a good. I will check the mtu against the limit
IPV6_MAXPLEN, and document the jumbo restriction ...
>
> Greetings,
>
> Hannes
>
Regards,
Oussama
^ permalink raw reply
* Re: [PATCH v2.40 2/7] odp: Allow VLAN actions after MPLS actions
From: Ben Pfaff @ 2013-09-29 17:05 UTC (permalink / raw)
To: Joe Stringer
Cc: Simon Horman, dev@openvswitch.org, netdev, Jesse Gross,
Pravin B Shelar, Ravi K, Isaku Yamahata
In-Reply-To: <CAOftzPg_FJoyS0oS8Ua7cpMaCh=BboWwyn8kVVoXGT9X9HQ0hg@mail.gmail.com>
On Sun, Sep 29, 2013 at 02:51:19PM +1300, Joe Stringer wrote:
> On Sat, Sep 28, 2013 at 7:21 AM, Ben Pfaff <blp@nicira.com> wrote:
>
> > On Fri, Sep 27, 2013 at 09:18:31AM +0900, Simon Horman wrote:
> > > From: Joe Stringer <joe@wand.net.nz>
> > >
> > > OpenFlow 1.2 and 1.3 differ on their handling of MPLS actions in the
> > > presence of VLAN tags. To allow correct behaviour to be committed in
> > > each situation, this patch adds a second round of VLAN tag action
> > > handling to commit_odp_actions(), which occurs after MPLS actions. This
> > > is implemented with a new field in 'struct xlate_in' called 'vlan_tci'.
> > >
> > > When an push_mpls action is composed, the flow's current VLAN state is
> > > stored into xin->vlan_tci, and flow->vlan_tci is set to 0 (pop_vlan). If
> > > a VLAN tag is present, it is stripped; if not, then there is no change.
> > > Any later modifications to the VLAN state is written to xin->vlan_tci.
> > > When committing the actions, flow->vlan_tci is used before MPLS actions,
> > > and xin->vlan_tci is used afterwards. This retains the current datapath
> > > behaviour, but allows VLAN actions to be applied in a more flexible
> > > manner.
> > >
> > > Signed-off-by: Joe Stringer <joe@wand.net.nz>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > The commit message talks about handling OpenFlow 1.2 and 1.3 both
> > properly, but I don't see how the code distinguishes between the cases.
> > Can you explain? Maybe this is something added in a later patch, in
> > which case it would be nice to mention that in the commit message.
> >
>
> It is added in patch #5. IIRC this patch should cause no difference in
> behaviour, but set up infrastructure to be used later.
>
> There seems to be a typo in the comment in vlan_tci_restore() here:
> > > + /* If MPLS actions were executed after MPLS, copy the final
> > vlan_tci out
> > > + * and restore the intermediate VLAN state. */
> >
>
> Right, that should probably be "...executed after VLAN actions...".
>
> I was a little confused by the new local variable 'vlan_tci' in
> > do_xlate_actions(). Partly this was because the fact that I didn't find
> > it obvious that sometimes it points to different VLAN tags. I think
> > this would be easier to see if it were initially assigned just under the
> > big comment rather than in an initializer, so that one would know to
> > look at the comment.
> >
>
> Perhaps the big comment could be rearranged and put above the initialiser,
> something like the following:-
>
> /* VLAN actions are stored to '*vlan_tci'. This variable initially points
> to 'xin->flow->vlan_tci', so that
> * VLAN actions are applied before any MPLS actions. When an MPLS action is
> translated,
> * 'vlan_tci' is updated to point to 'xin->vlan_tci'. This causes later
> VLAN actions to be applied after MPLS actions.
> * Each time through the loop, 'xin->vlan_tci' is updated to reflect the
> final VLAN state of the flow. */
>
> Then, the place where 'xin->vlan_tci' is updated to '*vlan_tci' could have
> a simple comment to refer back:-
>
> /* Update the final vlan state to match the current state. */
All that makes sense, thanks.
^ permalink raw reply
* [PATCH net-next] include/linux/skbuff.h: move CONFIG_XFRM check inside the skb_sec_path()
From: Denis Kirjanov @ 2013-09-29 14:07 UTC (permalink / raw)
To: netdev, davem; +Cc: Denis Kirjanov
And thus we have only one function definition
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
include/linux/skbuff.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2ddb48d..7399045 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2736,17 +2736,14 @@ extern u16 __skb_tx_hash(const struct net_device *dev,
const struct sk_buff *skb,
unsigned int num_tx_queues);
-#ifdef CONFIG_XFRM
static inline struct sec_path *skb_sec_path(struct sk_buff *skb)
{
+#ifdef CONFIG_XFRM
return skb->sp;
-}
#else
-static inline struct sec_path *skb_sec_path(struct sk_buff *skb)
-{
return NULL;
-}
#endif
+}
/* Keeps track of mac header offset relative to skb->head.
* It is useful for TSO of Tunneling protocol. e.g. GRE.
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2013-09-29 19:03 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, wg, linux-can, linux-sh
In-Reply-To: <201309280211.39068.sergei.shtylyov@cogentembedded.com>
[-- Attachment #1: Type: text/plain, Size: 36209 bytes --]
On 09/28/2013 12:11 AM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs.
Is there a public available datasheet for the CAN core? What
architecture are the Renesas R-Car SoCs? They're ARM, aren't they?
What's R-Car's status on device tree conversion?
More comments inline
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'linux-can-next.git' repo.
>
> drivers/net/can/Kconfig | 9
> drivers/net/can/Makefile | 1
> drivers/net/can/rcar_can.c | 898 ++++++++++++++++++++++++++++++++++
> include/linux/can/platform/rcar_can.h | 15
> 4 files changed, 923 insertions(+)
>
> Index: linux-can-next/drivers/net/can/Kconfig
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Kconfig
> +++ linux-can-next/drivers/net/can/Kconfig
> @@ -125,6 +125,15 @@ config CAN_GRCAN
> endian syntheses of the cores would need some modifications on
> the hardware level to work.
>
> +config CAN_RCAR
> + tristate "Renesas R-Car CAN controller"
> + ---help---
> + Say Y here if you want to use CAN controller found on Renesas R-Car
> + SoCs.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called rcar_can.
> +
> source "drivers/net/can/mscan/Kconfig"
>
> source "drivers/net/can/sja1000/Kconfig"
> Index: linux-can-next/drivers/net/can/Makefile
> ===================================================================
> --- linux-can-next.orig/drivers/net/can/Makefile
> +++ linux-can-next/drivers/net/can/Makefile
> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ica
> obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
> obj-$(CONFIG_PCH_CAN) += pch_can.o
> obj-$(CONFIG_CAN_GRCAN) += grcan.o
> +obj-$(CONFIG_CAN_RCAR) += rcar_can.o
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> Index: linux-can-next/drivers/net/can/rcar_can.c
> ===================================================================
> --- /dev/null
> +++ linux-can-next/drivers/net/can/rcar_can.c
> @@ -0,0 +1,898 @@
> +/*
> + * Renesas R-Car CAN device driver
> + *
> + * Copyright (C) 2013 Cogent Embedded, Inc. <source@cogentembedded.com>
> + * Copyright (C) 2013 Renesas Solutions Corp.
> + *
> + * 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; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/errno.h>
> +#include <linux/netdevice.h>
> +#include <linux/platform_device.h>
> +#include <linux/can/led.h>
> +#include <linux/can/dev.h>
> +#include <linux/clk.h>
> +#include <linux/can/platform/rcar_can.h>
> +
> +#define DRV_NAME "rcar_can"
> +
> +#define RCAR_CAN_MIER1 0x42C /* CANi Mailbox Interrupt Enable Register 1 */
> +#define RCAR_CAN_MKR(n) ((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
> + /* CANi Mask Register */
> +#define RCAR_CAN_MKIVLR0 0x438 /* CANi Mask Invalid Register 0 */
> +#define RCAR_CAN_MIER0 0x43C /* CANi Mailbox Interrupt Enable Register 0 */
> +
> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
> +#define RCAR_CAN_CTLR 0x840 /* CANi Control Register */
> +#define RCAR_CAN_STR 0x842 /* CANi Status Register */
> +#define RCAR_CAN_BCR 0x844 /* CANi Bit Configuration Register */
> +#define RCAR_CAN_CLKR 0x847 /* CANi Clock Select Register */
> +#define RCAR_CAN_EIER 0x84C /* CANi Error Interrupt Enable Register */
> +#define RCAR_CAN_EIFR 0x84D /* CANi Err Interrupt Factor Judge Register */
> +#define RCAR_CAN_RECR 0x84E /* CANi Receive Error Count Register */
> +#define RCAR_CAN_TECR 0x84F /* CANi Transmit Error Count Register */
> +#define RCAR_CAN_ECSR 0x850 /* CANi Error Code Store Register */
> +#define RCAR_CAN_MSSR 0x852 /* CANi Mailbox Search Status Register */
> +#define RCAR_CAN_MSMR 0x853 /* CANi Mailbox Search Mode Register */
> +#define RCAR_CAN_TCR 0x858 /* CANi Test Control Register */
> +#define RCAR_CAN_IER 0x860 /* CANi Interrupt Enable Register */
> +#define RCAR_CAN_ISR 0x861 /* CANi Interrupt Status Register */
You might consider using an enum for the register offsets.
> +
> +/* Offsets of RCAR_CAN Mailbox Registers */
> +#define MBX_HDR_OFFSET 0x0
> +#define MBX_DLC_OFFSET 0x5
> +#define MBX_DATA_OFFSET 0x6
can you please add the RCAR_ prefix to all defines.
> +
> +#define RCAR_CAN_MBX_SIZE 0x10
> +
> +/* Control Register bits */
> +#define CTLR_SLPM BIT(10)
> +#define CTLR_HALT BIT(9)
> +#define CTLR_RESET BIT(8)
> +#define CTLR_FORCE_RESET (3 << 8)
> +#define CTLR_TPM BIT(4) /* Transmission Priority Mode Select Bit */
> +#define CTLR_IDFM_MIXED BIT(2) /* Mixed ID mode */
> +
> +/* Message Control Register bits */
> +#define MCTL_TRMREQ BIT(7)
> +#define MCTL_RECREQ BIT(6)
> +#define MCTL_ONESHOT BIT(4)
> +#define MCTL_SENTDATA BIT(0)
> +#define MCTL_NEWDATA BIT(0)
> +
> +#define N_RX_MKREGS 2 /* Number of mask registers */
> + /* for Rx mailboxes 0-31 */
> +
> +/* Bit Configuration Register settings */
> +#define BCR_TSEG1(x) (((x) & 0x0f) << 28)
> +#define BCR_BPR(x) (((x) & 0x3ff) << 16)
> +#define BCR_SJW(x) (((x) & 0x3) << 12)
> +#define BCR_TSEG2(x) (((x) & 0x07) << 8)
> +
> +/* Mailbox and Mask Registers bits */
> +#define RCAR_CAN_IDE BIT(31)
> +#define RCAR_CAN_RTR BIT(30)
> +#define RCAR_CAN_SID_SHIFT 18
> +
> +/* Interrupt Enable Register bits */
> +#define IER_ERSIE BIT(5) /* Error (ERS) Interrupt Enable Bit */
> +#define IER_RXM0IE BIT(2) /* Mailbox 0 Successful Reception (RXM0) */
> + /* Interrupt Enable Bit */
> +#define IER_RXM1IE BIT(1) /* Mailbox 1 Successful Reception (RXM0) */
> + /* Interrupt Enable Bit */
> +#define IER_TXMIE BIT(0) /* Mailbox 32 to 63 Successful Tx */
> + /* Interrupt Enable Bit */
> +
> +/* Interrupt Status Register bits */
> +#define ISR_ERSF BIT(5) /* Error (ERS) Interrupt Status Bit */
> +#define ISR_RXM0F BIT(2) /* Mailbox 0 Successful Reception (RXM0) */
> + /* Interrupt Status Bit */
> +#define ISR_RXM1F BIT(1) /* Mailbox 1 to 63 Successful Reception */
> + /* (RXM1) Interrupt Status Bit */
> +#define ISR_TXMF BIT(0) /* Mailbox 32 to 63 Successful Transmission */
> + /* (TXM) Interrupt Status Bit */
> +
> +/* Error Interrupt Enable Register bits */
> +#define EIER_BLIE BIT(7) /* Bus Lock Interrupt Enable */
> +#define EIER_OLIE BIT(6) /* Overload Frame Transmit Interrupt Enable */
> +#define EIER_ORIE BIT(5) /* Receive Overrun Interrupt Enable */
> +#define EIER_BORIE BIT(4) /* Bus-Off Recovery Interrupt Enable */
> +
> +#define EIER_BOEIE BIT(3) /* Bus-Off Entry Interrupt Enable */
> +#define EIER_EPIE BIT(2) /* Error Passive Interrupt Enable */
> +#define EIER_EWIE BIT(1) /* Error Warning Interrupt Enable */
> +#define EIER_BEIE BIT(0) /* Bus Error Interrupt Enable */
> +
> +/* Error Interrupt Factor Judge Register bits */
> +#define EIFR_BLIF BIT(7) /* Bus Lock Detect Flag */
> +#define EIFR_OLIF BIT(6) /* Overload Frame Transmission Detect Flag */
> +#define EIFR_ORIF BIT(5) /* Receive Overrun Detect Flag */
> +#define EIFR_BORIF BIT(4) /* Bus-Off Recovery Detect Flag */
> +#define EIFR_BOEIF BIT(3) /* Bus-Off Entry Detect Flag */
> +#define EIFR_EPIF BIT(2) /* Error Passive Detect Flag */
> +#define EIFR_EWIF BIT(1) /* Error Warning Detect Flag */
> +#define EIFR_BEIF BIT(0) /* Bus Error Detect Flag */
> +
> +/* Error Code Store Register bits */
> +#define ECSR_EDPM BIT(7) /* Error Display Mode Select Bit */
> +#define ECSR_ADEF BIT(6) /* ACK Delimiter Error Flag */
> +#define ECSR_BE0F BIT(5) /* Bit Error (dominant) Flag */
> +#define ECSR_BE1F BIT(4) /* Bit Error (recessive) Flag */
> +#define ECSR_CEF BIT(3) /* CRC Error Flag */
> +#define ECSR_AEF BIT(2) /* ACK Error Flag */
> +#define ECSR_FEF BIT(1) /* Form Error Flag */
> +#define ECSR_SEF BIT(0) /* Stuff Error Flag */
> +
> +/* Mailbox Search Status Register bits */
> +#define MSSR_SEST BIT(7) /* Search Result Status Bit */
> +#define MSSR_MBNST 0x3f /* Search Result Mailbox Number Status mask */
> +
> +/* Mailbox Search Mode Register values */
> +#define MSMR_TXMB 1 /* Transmit mailbox search mode */
> +#define MSMR_RXMB 0 /* Receive mailbox search mode */
> +
> +/* Mailbox configuration:
> + * mailbox 0 - not used
> + * mailbox 1-31 - Rx
> + * mailbox 32-63 - Tx
> + * no FIFO mailboxes
> + */
> +#define N_MBX 64
> +#define FIRST_TX_MB 32
> +#define RX_MBX_MASK 0xFFFFFFFE
> +
> +#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
> +
> +struct rcar_can_priv {
> + struct can_priv can; /* Must be the first member! */
> + struct net_device *ndev;
> + struct napi_struct napi;
> + void __iomem *reg_base;
> + struct clk *clk;
> + spinlock_t mier_lock;
> + u8 clock_select;
> +};
> +
> +static const struct can_bittiming_const rcar_can_bittiming_const = {
> + .name = DRV_NAME,
> + .tseg1_min = 4,
> + .tseg1_max = 16,
> + .tseg2_min = 2,
> + .tseg2_max = 8,
> + .sjw_max = 4,
> + .brp_min = 1,
> + .brp_max = 1024,
> + .brp_inc = 1,
> +};
> +
If you use use an enum for the register offsets you can change the int
to that enum in the accessor functions.
> +static inline u32 rcar_can_readl(struct rcar_can_priv *priv, int reg)
> +{
> + return readl(priv->reg_base + reg);
> +}
> +
> +static inline u16 rcar_can_readw(struct rcar_can_priv *priv, int reg)
> +{
> + return readw(priv->reg_base + reg);
> +}
> +
> +static inline u8 rcar_can_readb(struct rcar_can_priv *priv, int reg)
> +{
> + return readb(priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writel(struct rcar_can_priv *priv, int reg, u32 val)
> +{
> + writel(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writew(struct rcar_can_priv *priv, int reg, u16 val)
> +{
> + writew(val, priv->reg_base + reg);
> +}
> +
> +static inline void rcar_can_writeb(struct rcar_can_priv *priv, int reg, u8 val)
> +{
> + writeb(val, priv->reg_base + reg);
> +}
> +
> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
> + u32 mbxno, u8 offset)
> +{
> + return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
Can you define hide the RCAR_CAN_MBX_SIZE * mbxno into a macro?
Something like:
RCAR_CAN_MBX(mbxno)
> +}
> +
> +static inline u8 rcar_can_mbx_readb(struct rcar_can_priv *priv,
> + u32 mbxno, u8 offset)
> +{
> + return rcar_can_readb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> +}
> +
> +static inline void rcar_can_mbx_writel(struct rcar_can_priv *priv, u32 mbxno,
> + u8 offset, u32 val)
> +{
> + rcar_can_writel(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static inline void rcar_can_mbx_writeb(struct rcar_can_priv *priv, u32 mbxno,
> + u8 offset, u8 val)
> +{
> + rcar_can_writeb(priv, RCAR_CAN_MBX_SIZE * mbxno + offset, val);
> +}
> +
> +static void rcar_can_error(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &ndev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + u8 eifr;
> +
> + /* Propagate the error condition to the CAN stack */
> + skb = alloc_can_err_skb(ndev, &cf);
> + if (!skb) {
> + if (printk_ratelimit())
> + netdev_err(priv->ndev,
> + "%s: alloc_can_err_skb() failed\n",
> + __func__);
> + return;
> + }
> +
> + eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
> + if (eifr & EIFR_EWIF) {
> + netdev_dbg(priv->ndev, "Error warning interrupt\n");
> + priv->can.state = CAN_STATE_ERROR_WARNING;
> + priv->can.can_stats.error_warning++;
> + cf->can_id |= CAN_ERR_CRTL;
> + if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
> + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> + if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
> + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> + /* Clear interrupt condition */
> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);
The cast is not needed.
> + }
> + if (eifr & EIFR_EPIF) {
> + netdev_dbg(priv->ndev, "Error passive interrupt\n");
> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> + priv->can.can_stats.error_passive++;
> + cf->can_id |= CAN_ERR_CRTL;
> + if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
> + cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> + if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
> + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> + /* Clear interrupt condition */
> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
> + }
> + if (eifr & EIFR_BOEIF) {
> + netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> + priv->can.state = CAN_STATE_BUS_OFF;
> + cf->can_id |= CAN_ERR_BUSOFF;
> + /* Clear interrupt condition */
> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
> + /* Disable all interrupts in bus-off to avoid int hog */
> + rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> + rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> + can_bus_off(ndev);
How does the rcan recover from bus off?
> + }
> + if (eifr & EIFR_BEIF) {
> + int rx_errors = 0, tx_errors = 0, bus_errors = 0;
> + u8 ecsr;
> +
> + netdev_dbg(priv->ndev, "Bus error interrupt:\n");
> + cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_UNSPEC;
> +
> + ecsr = rcar_can_readb(priv, RCAR_CAN_ECSR);
> + if (ecsr & ECSR_ADEF) {
> + netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> + bus_errors++;
> + tx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);
> + }
> + if (ecsr & ECSR_BE0F) {
> + netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + bus_errors++;
> + tx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE0F);
> + }
> + if (ecsr & ECSR_BE1F) {
> + netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + bus_errors++;
> + tx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_BE1F);
> + }
> + if (ecsr & ECSR_CEF) {
> + netdev_dbg(priv->ndev, "CRC Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> + bus_errors++;
> + rx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_CEF);
> + }
> + if (ecsr & ECSR_AEF) {
> + netdev_dbg(priv->ndev, "ACK Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> + bus_errors++;
> + tx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_AEF);
> + }
> + if (ecsr & ECSR_FEF) {
> + netdev_dbg(priv->ndev, "Form Error\n");
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + bus_errors++;
> + rx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_FEF);
> + }
> + if (ecsr & ECSR_SEF) {
> + netdev_dbg(priv->ndev, "Stuff Error\n");
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + bus_errors++;
> + rx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_SEF);
> + }
> +
> + priv->can.can_stats.bus_error += bus_errors;
> + ndev->stats.rx_errors += rx_errors;
> + ndev->stats.tx_errors += tx_errors;
> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BEIF);
> + }
> + if (eifr & EIFR_ORIF) {
> + netdev_dbg(priv->ndev, "Receive overrun error interrupt\n");
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> + ndev->stats.rx_over_errors++;
> + ndev->stats.rx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_ORIF);
> + }
> + if (eifr & EIFR_OLIF) {
> + netdev_dbg(priv->ndev,
> + "Overload Frame Transmission error interrupt\n");
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> + ndev->stats.rx_over_errors++;
> + ndev->stats.rx_errors++;
> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_OLIF);
> + }
> +
> + netif_rx(skb);
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
> +{
> + struct net_device *ndev = (struct net_device *)dev_id;
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &ndev->stats;
> + u8 isr;
> +
> + isr = rcar_can_readb(priv, RCAR_CAN_ISR);
> + if (isr & ISR_ERSF)
> + rcar_can_error(ndev);
> +
> + if (isr & ISR_TXMF) {
> + u32 ie_mask = 0;
> +
> + /* Set Transmit Mailbox Search Mode */
> + rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
Can you please outline how to mailbox search mode works? What happens if
you activate tx mailbox search mode here and rx mailbox search mode below?
> + while (1) {
I presonally don't like while (1) loops in an interrupt handler, can you
rearange the code, so that you have an explizid loop termination?
> + u8 mctl, mbx;
> +
> + mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> + if (mbx & MSSR_SEST)
> + break;
> + mbx &= MSSR_MBNST;
> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> + /* Bits SENTDATA and TRMREQ cannot be
> + * set to 0 simultaneously
> + */
> + mctl &= ~MCTL_TRMREQ;
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> + mctl &= ~MCTL_SENTDATA;
> + /* Clear interrupt */
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
Can you combine both writes to RCAR_CAN_MCTL(mbx) or does the hardware
require two separate writes?
> + ie_mask |= BIT(mbx - FIRST_TX_MB);
> + stats->tx_bytes += can_get_echo_skb(ndev,
> + mbx - FIRST_TX_MB);
Can you guarantee that you call can_get_echo_skb in the same order the
frames get send?
> + stats->tx_packets++;
> + can_led_event(ndev, CAN_LED_EVENT_TX);
> + }
> + /* Set receive mailbox search mode */
> + rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_RXMB);
> + /* Disable mailbox interrupt, mark mailbox as free */
> + if (ie_mask) {
> + u32 mier1;
> +
> + spin_lock(&priv->mier_lock);
> + mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> + rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
> + spin_unlock(&priv->mier_lock);
> + if (unlikely(netif_queue_stopped(ndev)))
> + netif_wake_queue(ndev);
You can call netif_wake_queue() unconditionally, it does the check for
queue stopped anyway.
> + }
> + }
> + if (isr & ISR_RXM1F) {
> + if (napi_schedule_prep(&priv->napi)) {
> + /* Disable Rx interrupts */
> + rcar_can_writeb(priv, RCAR_CAN_IER,
> + rcar_can_readb(priv, RCAR_CAN_IER) &
> + ~IER_RXM1IE);
> + __napi_schedule(&priv->napi);
> + }
> + }
> + return IRQ_HANDLED;
Do not return IRQ_HANDLED unconditionally. You should return IRQ_HANDLED
only if there really was a interrupt for your device.
> +}
> +
> +static int rcar_can_set_bittiming(struct net_device *dev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(dev);
> + struct can_bittiming *bt = &priv->can.bittiming;
> + u32 bcr;
> + u16 ctlr;
> + u8 clkr;
> +
> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> + if (ctlr & CTLR_SLPM) {
> + /* Write to BCR in CAN reset mode or CAN halt mode */
> + return -EBUSY;
The framework guarantees that set_bittiming is only called when the
interface is down.
> + }
> + /* Don't overwrite CLKR with 32-bit BCR access */
> + /* CLKR has 8-bit access */
> + clkr = rcar_can_readb(priv, RCAR_CAN_CLKR);
> + bcr = BCR_TSEG1(bt->phase_seg1 + bt->prop_seg - 1) |
> + BCR_BPR(bt->brp - 1) | BCR_SJW(bt->sjw - 1) |
> + BCR_TSEG2(bt->phase_seg2 - 1);
> + rcar_can_writel(priv, RCAR_CAN_BCR, bcr);
> + rcar_can_writeb(priv, RCAR_CAN_CLKR, clkr);
> + return 0;
> +}
> +
> +static void rcar_can_start(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr, n;
> +
> + /* Set controller to known mode:
> + * - normal mailbox mode (no FIFO);
Does the controller support FIFO?
> + * - accept all messages (no filter).
> + * CAN is in sleep mode after MCU hardware or software reset.
> + */
> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> + ctlr &= ~CTLR_SLPM;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + /* Go to reset mode */
> + ctlr |= CTLR_FORCE_RESET;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + ctlr |= CTLR_IDFM_MIXED; /* Select mixed ID mode */
> + ctlr &= ~CTLR_TPM; /* Set ID priority transmit mode */
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> +
> + rcar_can_writeb(priv, RCAR_CAN_CLKR, priv->clock_select);
> +
> + /* Accept all SID and EID */
> + for (n = 0; n < N_RX_MKREGS; n++)
> + rcar_can_writel(priv, RCAR_CAN_MKR(n), 0);
> + rcar_can_writel(priv, RCAR_CAN_MKIVLR0, 0);
> +
> + rcar_can_set_bittiming(ndev);
> +
> + /* Initial value of MIER1 undefined. Mark all Tx mailboxes as free. */
> + rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> +
> + rcar_can_writeb(priv, RCAR_CAN_IER, IER_TXMIE | IER_ERSIE | IER_RXM1IE);
> +
> + /* Accumulate error codes */
> + rcar_can_writeb(priv, RCAR_CAN_ECSR, ECSR_EDPM);
> + /* Enable error interrupts */
> + rcar_can_writeb(priv, RCAR_CAN_EIER,
> + EIER_EWIE | EIER_EPIE | EIER_BOEIE | EIER_BEIE |
> + EIER_ORIE | EIER_OLIE);
> + /* Enable interrupts for RX mailboxes */
> + rcar_can_writel(priv, RCAR_CAN_MIER0, RX_MBX_MASK);
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + /* Write to the CiMCTLj register in CAN
> + * operation mode or CAN halt mode.
> + * Configure mailboxes 0-31 as Rx mailboxes.
> + * Configure mailboxes 32-63 as Tx mailboxes.
> + */
> + /* Go to halt mode */
> + ctlr |= CTLR_HALT;
> + ctlr &= ~CTLR_RESET;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + for (n = 0; n < FIRST_TX_MB; n++) {
> + /* According to documentation we should clear MCTL
> + * register before configuring mailbox.
> + */
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(n), 0);
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(n), MCTL_RECREQ);
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(FIRST_TX_MB + n), 0);
> + }
> + /* Go to operation mode */
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr & ~CTLR_FORCE_RESET);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + int err;
> +
> + err = open_candev(ndev);
> + if (err) {
> + netdev_err(ndev, "open_candev() failed %d\n", err);
> + goto out;
> + }
> + napi_enable(&priv->napi);
> + err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
> + if (err) {
> + netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
> + goto out_close;
> + }
> + can_led_event(ndev, CAN_LED_EVENT_OPEN);
> + rcar_can_start(ndev);
> + netif_start_queue(ndev);
> + return 0;
> +out_close:
> + napi_disable(&priv->napi);
> + close_candev(ndev);
> +out:
> + return err;
> +}
> +
> +static void rcar_can_stop(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr;
> +
> + /* Go to (force) reset mode */
> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_FORCE_RESET);
> + rcar_can_writel(priv, RCAR_CAN_MIER0, 0);
> + rcar_can_writel(priv, RCAR_CAN_MIER1, 0);
> + rcar_can_writeb(priv, RCAR_CAN_IER, 0);
> + rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
> + priv->can.state = CAN_STATE_STOPPED;
> +}
> +
> +static int rcar_can_close(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> +
> + netif_stop_queue(ndev);
> + rcar_can_stop(ndev);
> + free_irq(ndev->irq, ndev);
> + napi_disable(&priv->napi);
> + close_candev(ndev);
> + can_led_event(ndev, CAN_LED_EVENT_STOP);
> + return 0;
> +}
> +
> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
> + struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + u32 data, mier1, mbxno, i;
> + unsigned long flags;
> + u8 mctl;
> +
> + if (can_dropped_invalid_skb(ndev, skb))
> + return NETDEV_TX_OK;
> +
> + spin_lock_irqsave(&priv->mier_lock, flags);
> + mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
> + if (unlikely(mier1 == ~0U)) {
> + spin_unlock_irqrestore(&priv->mier_lock, flags);
> + netif_stop_queue(ndev);
> + return NETDEV_TX_BUSY;
> + }
> + rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
> + spin_unlock_irqrestore(&priv->mier_lock, flags);
> + mbxno = ffz(mier1) + FIRST_TX_MB;
This smells fishy, for several reasons:
The driver should guarantee that the frames are send in the same order
as this function is called. If you pick the first free mailbox I suspect
the hardware send the first mailbox ready to send. I don't know the
hardware, but several CAN controllers I've worked with, function in that
way.
Then you should rethink your flow control. You should call
netif_stop_queue if all your hardware buffers are full, then the above
NETDEV_TX_BUSY should not happen.
> +
> + if (cf->can_id & CAN_EFF_FLAG) {
> + /* Extended frame format */
> + data = (cf->can_id & CAN_EFF_MASK) | RCAR_CAN_IDE;
> + } else {
> + /* Standard frame format */
> + data = (cf->can_id & CAN_SFF_MASK) << RCAR_CAN_SID_SHIFT;
> + }
> + if (cf->can_id & CAN_RTR_FLAG) {
> + /* Remote transmission request */
> + data |= RCAR_CAN_RTR;
> + }
> + rcar_can_mbx_writel(priv, mbxno, MBX_HDR_OFFSET, data);
> +
> + rcar_can_mbx_writeb(priv, mbxno, MBX_DLC_OFFSET, cf->can_dlc);
> +
> + for (i = 0; i < cf->can_dlc; i++)
> + rcar_can_mbx_writeb(priv, mbxno,
> + MBX_DATA_OFFSET + i, cf->data[i]);
> +
> + can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
> +
> + rcar_can_writeb(priv, RCAR_CAN_IER,
> + rcar_can_readb(priv, RCAR_CAN_IER) | IER_TXMIE);
> +
> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));
Do you have to read mctl here?
> + if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> + mctl |= MCTL_ONESHOT;
> + else
> + mctl &= ~MCTL_ONESHOT;
> + /* Start TX */
> + mctl |= MCTL_TRMREQ;
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbxno), mctl);
> + return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops rcar_can_netdev_ops = {
> + .ndo_open = rcar_can_open,
> + .ndo_stop = rcar_can_close,
> + .ndo_start_xmit = rcar_can_start_xmit,
> +};
> +
> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
> +{
> + struct net_device_stats *stats = &priv->ndev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + u32 data;
> + u8 dlc;
> +
> + skb = alloc_can_skb(priv->ndev, &cf);
> + if (!skb) {
> + stats->rx_dropped++;
> + if (printk_ratelimit())
> + netdev_err(priv->ndev,
> + "%s: alloc_can_skb() failed\n", __func__);
> + return;
> + }
> +
> + data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
> + if (data & RCAR_CAN_IDE)
> + cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
> + else
> + cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
> + if (data & RCAR_CAN_RTR)
> + cf->can_id |= CAN_RTR_FLAG;
> +
> + dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
> + cf->can_dlc = get_can_dlc(dlc);
Please don't copy data on received RTR frames.
> + for (dlc = 0; dlc < cf->can_dlc; dlc++)
> + cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
> + MBX_DATA_OFFSET + dlc);
> +
> + can_led_event(priv->ndev, CAN_LED_EVENT_RX);
> +
> + netif_receive_skb(skb);
> + stats->rx_bytes += cf->can_dlc;
> + stats->rx_packets++;
> +}
> +
> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
> +{
> + struct rcar_can_priv *priv = container_of(napi,
> + struct rcar_can_priv, napi);
> + u32 num_pkts = 0;
please make it an int
> +
> + /* Find mailbox */
> + while (1) {
please put the quota check into the while()
> + u8 mctl, mbx;
> +
> + mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
> + if (mbx & MSSR_SEST || num_pkts >= quota)
> + break;
> + mbx &= MSSR_MBNST;
> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
> + /* Clear interrupt */
> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
> + mctl & ~MCTL_NEWDATA);
> + rcar_can_rx_pkt(priv, mbx);
Which instruction reenables the mailbox?
> + ++num_pkts;
> + }
> + /* All packets processed */
> + if (num_pkts < quota) {
> + u8 ier;
> +
> + napi_complete(napi);
> + ier = rcar_can_readb(priv, RCAR_CAN_IER);
> + rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);
I the hardware doesn't modify the IER register, you can work on a shadow
copy and only write, but never need to read from the hardware.
> + }
> + return num_pkts;
> +}
> +
> +static int rcar_can_do_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> + switch (mode) {
> + case CAN_MODE_START:
> + rcar_can_start(ndev);
> + netif_wake_queue(ndev);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int rcar_can_get_berr_counter(const struct net_device *dev,
> + struct can_berr_counter *bec)
> +{
> + struct rcar_can_priv *priv = netdev_priv(dev);
> +
> + bec->txerr = rcar_can_readb(priv, RCAR_CAN_TECR);
> + bec->rxerr = rcar_can_readb(priv, RCAR_CAN_RECR);
> + return 0;
> +}
> +
> +static int rcar_can_probe(struct platform_device *pdev)
> +{
> + struct rcar_can_platform_data *pdata;
> + struct rcar_can_priv *priv;
> + struct net_device *ndev;
> + struct resource *mem;
> + void __iomem *addr;
> + int err = -ENODEV;
> + int irq;
> +
> + pdata = pdev->dev.platform_data;
please use dev_get_platdata()
> + if (!pdata) {
> + dev_err(&pdev->dev, "No platform data provided!\n");
> + goto fail;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (!irq) {
> + dev_err(&pdev->dev, "No IRQ resource\n");
> + goto fail;
> + }
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + addr = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(addr)) {
> + err = PTR_ERR(addr);
> + goto fail;
> + }
> +
> + ndev = alloc_candev(sizeof(struct rcar_can_priv), N_MBX - FIRST_TX_MB);
> + if (!ndev) {
> + dev_err(&pdev->dev, "alloc_candev failed\n");
> + err = -ENOMEM;
> + goto fail;
> + }
> +
> + priv = netdev_priv(ndev);
> +
> + priv->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->clk)) {
> + err = PTR_ERR(priv->clk);
> + dev_err(&pdev->dev, "cannot get clock: %d\n", err);
> + goto fail_clk;
> + }
> + clk_enable(priv->clk);
You are missing the clock's prepare step. You should call
clk_prepare_enable(). Please move clock_prepare_enable() to the open()
(and the disable_unprepare() to the close() function) so tha the core is
not powered as long as the interface is down. You might have to enable
the clock in the rcar_can_get_berr_counter() as it may be called if the
interface is still down.
> +
> + ndev->netdev_ops = &rcar_can_netdev_ops;
> + ndev->irq = irq;
> + ndev->flags |= IFF_ECHO;
> + priv->ndev = ndev;
> + priv->reg_base = addr;
> + priv->clock_select = pdata->clock_select;
> + priv->can.clock.freq = clk_get_rate(priv->clk);
> + priv->can.bittiming_const = &rcar_can_bittiming_const;
> + priv->can.do_set_bittiming = rcar_can_set_bittiming;
No need to set this callback, as you're calling rcar_can_set_bittiming
explizidly.
> + priv->can.do_set_mode = rcar_can_do_set_mode;
> + priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
> + CAN_CTRLMODE_ONE_SHOT;
You don't handle 3_SAMPLES. Have you tested your driver in ONE_SHOT
mode? How does the controller react if a frame cannot be send? Do you
clear the skb that has previously been can_put_echo_skb()?
> + platform_set_drvdata(pdev, ndev);
> + SET_NETDEV_DEV(ndev, &pdev->dev);
> + spin_lock_init(&priv->mier_lock);
> +
> + netif_napi_add(ndev, &priv->napi, rcar_can_rx_poll,
> + RCAR_CAN_NAPI_WEIGHT);
> + err = register_candev(ndev);
> + if (err) {
> + dev_err(&pdev->dev, "register_candev() failed\n");
> + goto fail_candev;
> + }
> +
> + devm_can_led_init(ndev);
> +
> + dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
> + priv->reg_base, ndev->irq);
> +
> + return 0;
> +fail_candev:
> + netif_napi_del(&priv->napi);
> +fail_clk:
> + free_candev(ndev);
> +fail:
> + return err;
> +}
> +
> +static int rcar_can_remove(struct platform_device *pdev)
> +{
> + struct net_device *ndev = platform_get_drvdata(pdev);
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr;
> +
> + unregister_candev(ndev);
> + netif_napi_del(&priv->napi);
> + /* Go to sleep mode */
> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);
You should put the controller in to sleep mode in close()
> + clk_disable(priv->clk);
> + free_candev(ndev);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rcar_can_suspend(struct device *dev)
> +{
> + struct net_device *ndev = dev_get_drvdata(dev);
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr;
> +
> + if (netif_running(ndev)) {
> + netif_stop_queue(ndev);
> + netif_device_detach(ndev);
> + }
> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> + ctlr |= CTLR_HALT;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + ctlr |= CTLR_SLPM;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + priv->can.state = CAN_STATE_SLEEPING;
> +
> + clk_disable(priv->clk);
> + return 0;
> +}
> +
> +static int rcar_can_resume(struct device *dev)
> +{
> + struct net_device *ndev = dev_get_drvdata(dev);
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u16 ctlr;
> +
> + clk_enable(priv->clk);
> +
> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
> + ctlr &= ~CTLR_SLPM;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + ctlr &= ~CTLR_FORCE_RESET;
> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr);
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + if (netif_running(ndev)) {
> + netif_device_attach(ndev);
> + netif_start_queue(ndev);
> + }
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rcar_can_pm_ops, rcar_can_suspend, rcar_can_resume);
> +
> +static struct platform_driver rcar_can_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .pm = &rcar_can_pm_ops,
> + },
> + .probe = rcar_can_probe,
> + .remove = rcar_can_remove,
> +};
> +
> +module_platform_driver(rcar_can_driver);
> +
> +MODULE_AUTHOR("Cogent Embedded, Inc.");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("CAN driver for Renesas R-Car SoC");
> +MODULE_ALIAS("platform:" DRV_NAME);
> Index: linux-can-next/include/linux/can/platform/rcar_can.h
> ===================================================================
> --- /dev/null
> +++ linux-can-next/include/linux/can/platform/rcar_can.h
> @@ -0,0 +1,15 @@
> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
> +#define _CAN_PLATFORM_RCAR_CAN_H_
> +
> +#include <linux/types.h>
> +
> +/* Clock Select Register settings */
> +#define CLKR_CLKEXT 3 /* Externally input clock */
> +#define CLKR_CLKP2 1 /* Peripheral clock (clkp2) */
> +#define CLKR_CLKP1 0 /* Peripheral clock (clkp1) */
Can this be handled by the clock framework and or Device Tree?
> +
> +struct rcar_can_platform_data {
> + u8 clock_select; /* Clock source select */
> +};
> +
> +#endif /* !_CAN_PLATFORM_RCAR_CAN_H_ */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* [PATCH 1/2] ipv4 igmp: use in_dev_put in timer handlers instead of __in_dev_put
From: Salam Noureddine @ 2013-09-29 20:39 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
Cc: Salam Noureddine
It is possible for the timer handlers to run after the call to
ip_mc_down so use in_dev_put instead of __in_dev_put in the handler
function in order to do proper cleanup when the refcnt reaches 0.
Otherwise, the refcnt can reach zero without the in_device being
destroyed and we end up leaking a reference to the net_device and
see messages like the following,
unregister_netdevice: waiting for eth0 to become free. Usage count = 1
Tested on linux-3.4.43.
Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
---
net/ipv4/igmp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index dace87f..7defdc9 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -736,7 +736,7 @@ static void igmp_gq_timer_expire(unsigned long data)
in_dev->mr_gq_running = 0;
igmpv3_send_report(in_dev, NULL);
- __in_dev_put(in_dev);
+ in_dev_put(in_dev);
}
static void igmp_ifc_timer_expire(unsigned long data)
@@ -749,7 +749,7 @@ static void igmp_ifc_timer_expire(unsigned long data)
igmp_ifc_start_timer(in_dev,
unsolicited_report_interval(in_dev));
}
- __in_dev_put(in_dev);
+ in_dev_put(in_dev);
}
static void igmp_ifc_event(struct in_device *in_dev)
--
1.7.4.4
^ permalink raw reply related
* [PATCH 2/2] ipv6 mcast: use in6_dev_put in timer handlers instead of __in6_dev_put
From: Salam Noureddine @ 2013-09-29 20:41 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
Cc: Salam Noureddine
It is possible for the timer handlers to run after the call to
ipv6_mc_down so use in6_dev_put instead of __in6_dev_put in the
handler function in order to do proper cleanup when the refcnt
reaches 0. Otherwise, the refcnt can reach zero without the
inet6_dev being destroyed and we end up leaking a reference to
the net_device and see messages like the following,
unregister_netdevice: waiting for eth0 to become free. Usage count = 1
Tested on linux-3.4.43.
Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
---
net/ipv6/mcast.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 096cd67..d18f9f9 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -2034,7 +2034,7 @@ static void mld_dad_timer_expire(unsigned long data)
if (idev->mc_dad_count)
mld_dad_start_timer(idev, idev->mc_maxdelay);
}
- __in6_dev_put(idev);
+ in6_dev_put(idev);
}
static int ip6_mc_del1_src(struct ifmcaddr6 *pmc, int sfmode,
@@ -2379,7 +2379,7 @@ static void mld_gq_timer_expire(unsigned long data)
idev->mc_gq_running = 0;
mld_send_report(idev, NULL);
- __in6_dev_put(idev);
+ in6_dev_put(idev);
}
static void mld_ifc_timer_expire(unsigned long data)
@@ -2392,7 +2392,7 @@ static void mld_ifc_timer_expire(unsigned long data)
if (idev->mc_ifc_count)
mld_ifc_start_timer(idev, idev->mc_maxdelay);
}
- __in6_dev_put(idev);
+ in6_dev_put(idev);
}
static void mld_ifc_event(struct inet6_dev *idev)
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH V2 net-next] fib_trie: avoid a redundant bit judgement in inflate
From: David Miller @ 2013-09-29 21:31 UTC (permalink / raw)
To: baker.kernel; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <1380461518-10095-1-git-send-email-baker.kernel@gmail.com>
Based upon how quickly you sent these two patches, the first of which
wouldn't even compile, I have zero confidence that you actually
booted and tested this patch at all.
I'm not applying this until you take the time to boot and test the
change in some way, and say explicitly in your commit message how
you tested it.
^ permalink raw reply
* RE
From: amikam_t @ 2013-09-29 21:43 UTC (permalink / raw)
To: Recipients
I am christy walton i have a proposal for you
^ permalink raw reply
* Re: [ipv4:PATCH] Allow userspace to specify primary or secondary ip on interface
From: David Miller @ 2013-09-29 21:59 UTC (permalink / raw)
To: vincent.mc.li; +Cc: netdev, linux-kernel
In-Reply-To: <1380056988-6717-1-git-send-email-vincent.mc.li@gmail.com>
From: Vincent Li <vincent.mc.li@gmail.com>
Date: Tue, 24 Sep 2013 14:09:48 -0700
> the reason for this patch is that we have a multi blade cluster platform
> sharing 'floating management ip' and also that each blade has its own
> management ip on the management interface, so whichever blade in the
> cluster becomes primary blade, the 'floating mangaement ip' follows it,
> and we want any of our traffic originated from the primary blade source from
> the 'floating management ip' for consistency. but in this case, since the local
> blade management ip is always the primary ip on the mangaement interface and
> 'floating management ip' is always secondary, kernel always choose the primary
> ip as source ip address. thus we would like to add the flexibility in kernel to
> allow us to specify which ip to be primary or secondary.
You have the flexibility already.
You can specify a specific source address ot use in routes.
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: switch to use XPS to choose txq
From: Rusty Russell @ 2013-09-29 23:35 UTC (permalink / raw)
To: Jason Wang, netdev, linux-kernel, virtualization
Cc: Jason Wang, Michael S. Tsirkin
In-Reply-To: <1380261444-40918-1-git-send-email-jasowang@redhat.com>
Jason Wang <jasowang@redhat.com> writes:
> We used to use a percpu structure vq_index to record the cpu to queue
> mapping, this is suboptimal since it duplicates the work of XPS and
> loses all other XPS functionality such as allowing use to configure
> their own transmission steering strategy.
>
> So this patch switches to use XPS and suggest a default mapping when
> the number of cpus is equal to the number of queues. With XPS support,
> there's no need for keeping per-cpu vq_index and .ndo_select_queue(),
> so they were removed also.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/net/virtio_net.c | 55 +++++++--------------------------------------
> 1 files changed, 9 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index defec2b..4102c1b 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -127,9 +127,6 @@ struct virtnet_info {
> /* Does the affinity hint is set for virtqueues? */
> bool affinity_hint_set;
>
> - /* Per-cpu variable to show the mapping from CPU to virtqueue */
> - int __percpu *vq_index;
> -
> /* CPU hot plug notifier */
> struct notifier_block nb;
> };
> @@ -1063,7 +1060,6 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
> static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
> {
> int i;
> - int cpu;
>
> if (vi->affinity_hint_set) {
> for (i = 0; i < vi->max_queue_pairs; i++) {
> @@ -1073,20 +1069,11 @@ static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>
> vi->affinity_hint_set = false;
> }
> -
> - i = 0;
> - for_each_online_cpu(cpu) {
> - if (cpu == hcpu) {
> - *per_cpu_ptr(vi->vq_index, cpu) = -1;
> - } else {
> - *per_cpu_ptr(vi->vq_index, cpu) =
> - ++i % vi->curr_queue_pairs;
> - }
> - }
> }
>
> static void virtnet_set_affinity(struct virtnet_info *vi)
> {
> + cpumask_var_t cpumask;
> int i;
> int cpu;
>
> @@ -1100,15 +1087,21 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
> return;
> }
>
> + if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
> + return;
> +
> i = 0;
> for_each_online_cpu(cpu) {
> virtqueue_set_affinity(vi->rq[i].vq, cpu);
> virtqueue_set_affinity(vi->sq[i].vq, cpu);
> - *per_cpu_ptr(vi->vq_index, cpu) = i;
> + cpumask_clear(cpumask);
> + cpumask_set_cpu(cpu, cpumask);
> + netif_set_xps_queue(vi->dev, cpumask, i);
> i++;
> }
>
> vi->affinity_hint_set = true;
> + free_cpumask_var(cpumask);
> }
Um, isn't this just cpumask_of(cpu)?
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCH v2.40 2/7] odp: Allow VLAN actions after MPLS actions
From: Simon Horman @ 2013-09-30 1:10 UTC (permalink / raw)
To: Joe Stringer
Cc: Ben Pfaff, dev@openvswitch.org, netdev, Jesse Gross,
Pravin B Shelar, Ravi K, Isaku Yamahata
In-Reply-To: <CAOftzPg_FJoyS0oS8Ua7cpMaCh=BboWwyn8kVVoXGT9X9HQ0hg@mail.gmail.com>
On Sun, Sep 29, 2013 at 02:51:19PM +1300, Joe Stringer wrote:
> On Sat, Sep 28, 2013 at 7:21 AM, Ben Pfaff <blp@nicira.com> wrote:
>
> > On Fri, Sep 27, 2013 at 09:18:31AM +0900, Simon Horman wrote:
> > > From: Joe Stringer <joe@wand.net.nz>
> > >
> > > OpenFlow 1.2 and 1.3 differ on their handling of MPLS actions in the
> > > presence of VLAN tags. To allow correct behaviour to be committed in
> > > each situation, this patch adds a second round of VLAN tag action
> > > handling to commit_odp_actions(), which occurs after MPLS actions. This
> > > is implemented with a new field in 'struct xlate_in' called 'vlan_tci'.
> > >
> > > When an push_mpls action is composed, the flow's current VLAN state is
> > > stored into xin->vlan_tci, and flow->vlan_tci is set to 0 (pop_vlan). If
> > > a VLAN tag is present, it is stripped; if not, then there is no change.
> > > Any later modifications to the VLAN state is written to xin->vlan_tci.
> > > When committing the actions, flow->vlan_tci is used before MPLS actions,
> > > and xin->vlan_tci is used afterwards. This retains the current datapath
> > > behaviour, but allows VLAN actions to be applied in a more flexible
> > > manner.
> > >
> > > Signed-off-by: Joe Stringer <joe@wand.net.nz>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > The commit message talks about handling OpenFlow 1.2 and 1.3 both
> > properly, but I don't see how the code distinguishes between the cases.
> > Can you explain? Maybe this is something added in a later patch, in
> > which case it would be nice to mention that in the commit message.
> >
>
> It is added in patch #5. IIRC this patch should cause no difference in
> behaviour, but set up infrastructure to be used later.
>
> There seems to be a typo in the comment in vlan_tci_restore() here:
> > > + /* If MPLS actions were executed after MPLS, copy the final
> > vlan_tci out
> > > + * and restore the intermediate VLAN state. */
> >
>
> Right, that should probably be "...executed after VLAN actions...".
>
> I was a little confused by the new local variable 'vlan_tci' in
> > do_xlate_actions(). Partly this was because the fact that I didn't find
> > it obvious that sometimes it points to different VLAN tags. I think
> > this would be easier to see if it were initially assigned just under the
> > big comment rather than in an initializer, so that one would know to
> > look at the comment.
> >
>
> Perhaps the big comment could be rearranged and put above the initialiser,
> something like the following:-
>
> /* VLAN actions are stored to '*vlan_tci'. This variable initially points
> to 'xin->flow->vlan_tci', so that
> * VLAN actions are applied before any MPLS actions. When an MPLS action is
> translated,
> * 'vlan_tci' is updated to point to 'xin->vlan_tci'. This causes later
> VLAN actions to be applied after MPLS actions.
> * Each time through the loop, 'xin->vlan_tci' is updated to reflect the
> final VLAN state of the flow. */
>
> Then, the place where 'xin->vlan_tci' is updated to '*vlan_tci' could have
> a simple comment to refer back:-
>
> /* Update the final vlan state to match the current state. */
>
> Simon, do you mind handling the change?
Not at all. I'll include this change the next time I post the series.
^ permalink raw reply
* Re: [PATCH v2.40 3/7] ofp-actions: Add OFPUTIL_OFPAT13_PUSH_MPLS
From: Simon Horman @ 2013-09-30 1:13 UTC (permalink / raw)
To: Ben Pfaff
Cc: dev, netdev, Jesse Gross, Pravin B Shelar, Ravi K, Isaku Yamahata,
Joe Stringer
In-Reply-To: <20130927193010.GB17506@nicira.com>
On Fri, Sep 27, 2013 at 12:30:10PM -0700, Ben Pfaff wrote:
> On Fri, Sep 27, 2013 at 09:18:32AM +0900, Simon Horman wrote:
> > From: Joe Stringer <joe@wand.net.nz>
> >
> > This patch adds a new compatibility enum for use with MPLS, so that the
> > differing behaviour between OpenFlow 1.2 and 1.3 can be implemented in
> > ofproto-dpif-xlate.
>
> It seems a little awkward to me to do this via a new OFPACT_, mostly
> because there isn't currently any distinction between OF1.1 and OF1.3 in
> terms of OFPACT_ definitions. Did you consider adding a new field to
> struct ofpact_push_mpls that would say whether the label should be added
> before or after a VLAN tag?
Hi Ben,
I think that the main reason that Joe and I (probably much more I than Joe)
chose to use OFPACT_ the way that we did is that it seemed to be in keeping
with the way the code already works. Clearly you don't feel that is the
case which seems entirely reasonable to me. I'll see about re-working the
code as you have suggested.
^ permalink raw reply
* Re: [PATCH v2.40 3/7] ofp-actions: Add OFPUTIL_OFPAT13_PUSH_MPLS
From: Ben Pfaff @ 2013-09-30 2:00 UTC (permalink / raw)
To: Simon Horman
Cc: dev, netdev, Jesse Gross, Pravin B Shelar, Ravi K, Isaku Yamahata,
Joe Stringer
In-Reply-To: <20130930011344.GB4768@verge.net.au>
On Mon, Sep 30, 2013 at 10:13:44AM +0900, Simon Horman wrote:
> On Fri, Sep 27, 2013 at 12:30:10PM -0700, Ben Pfaff wrote:
> > On Fri, Sep 27, 2013 at 09:18:32AM +0900, Simon Horman wrote:
> > > From: Joe Stringer <joe@wand.net.nz>
> > >
> > > This patch adds a new compatibility enum for use with MPLS, so that the
> > > differing behaviour between OpenFlow 1.2 and 1.3 can be implemented in
> > > ofproto-dpif-xlate.
> >
> > It seems a little awkward to me to do this via a new OFPACT_, mostly
> > because there isn't currently any distinction between OF1.1 and OF1.3 in
> > terms of OFPACT_ definitions. Did you consider adding a new field to
> > struct ofpact_push_mpls that would say whether the label should be added
> > before or after a VLAN tag?
>
> I think that the main reason that Joe and I (probably much more I than Joe)
> chose to use OFPACT_ the way that we did is that it seemed to be in keeping
> with the way the code already works. Clearly you don't feel that is the
> case which seems entirely reasonable to me. I'll see about re-working the
> code as you have suggested.
Thanks. I think that it will make the code easier to understand.
^ permalink raw reply
* Re: [PATCH v2.40 4/7] ofp-actions: Add separate OpenFlow 1.3 action parser
From: Simon Horman @ 2013-09-30 2:12 UTC (permalink / raw)
To: Ben Pfaff
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Ravi K, netdev-u79uwXL29TY76Z2rM5mHXA,
Isaku Yamahata
In-Reply-To: <20130927194119.GC17506-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
On Fri, Sep 27, 2013 at 12:41:19PM -0700, Ben Pfaff wrote:
> On Fri, Sep 27, 2013 at 09:18:33AM +0900, Simon Horman wrote:
> > From: Joe Stringer <joe-Q1GJJQv1iO6lP80pJB477g@public.gmane.org>
> >
> > This patch adds new ofpact_from_openflow13() and
> > ofpacts_from_openflow13() functions parallel to the existing ofpact
> > handling code. In the OpenFlow 1.3 version, push_mpls is handled
> > differently, but all other actions are handled by the existing code.
> >
> > For push_mpls, ofpact_push_mpls.ofpact.compat is set to
> > OFPUTIL_OFPAT13_PUSH_MPLS, which allows correct VLAN+MPLS datapath
> > behaviour to be determined at odp translation time.
> >
> > Signed-off-by: Joe Stringer <joe-Q1GJJQv1iO6lP80pJB477g@public.gmane.org>
> > Signed-off-by: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
>
> I am nervous about this idea of having ofpacts_pull_openflow11_actions()
> and ofpacts_pull_openflow11_instructions() try to figure out what
> version of OpenFlow is involved. It is a new and somewhat surprising
> requirements that the callers provide not just actions or instructions
> but a complete OpenFlow message. I see that the callers in ovs-ofctl.c
> don't satisfy this requirement.
Thanks, sorry for overlooking that.
I believe that Joe was worried about this at the time he implemented
this patch but for some reason I was not.
> I think that it would be better to make these functions' callers say
> what version of OpenFlow they are working with. One could provide a
> helper function like get_version_from_ofpbuf(), which should probably go
> in ofp-util.c, but I would want it to fail (assert-fail or segfault or
> whatever) if there is no L2 header pointer, instead of returning a
> default value (that, in this case anyway, we know must be wrong because
> OF1.0 never contains OF1.1+ actions or instructions).
I have written an incremental patch to implement your suggestion
and it seems that a helper function is not necessary.
For reference the incremental change that I currently have is as follows.
It is a bit noisy but seems clean to me.
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 0c4a98b..e94f189 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1119,22 +1119,14 @@ get_actions_from_instruction(const struct ofp11_instruction *inst,
*n_actions = (ntohs(inst->len) - sizeof *inst) / OFP11_INSTRUCTION_ALIGN;
}
-static uint8_t
-get_version_from_ofpbuf(const struct ofpbuf *openflow)
-{
- if (openflow && openflow->l2) {
- struct ofp_header *oh = openflow->l2;
- return oh->version;
- }
-
- return OFP10_VERSION;
-}
-
-/* Attempts to convert 'actions_len' bytes of OpenFlow 1.1 actions from the
+/* Attempts to convert 'actions_len' bytes of OpenFlow actions from the
* front of 'openflow' into ofpacts. On success, replaces any existing content
* in 'ofpacts' by the converted ofpacts; on failure, clears 'ofpacts'.
* Returns 0 if successful, otherwise an OpenFlow error.
*
+ * Actions are processed according to their OpenFlow version which
+ * is provided in the 'version' parameter.
+ *
* In most places in OpenFlow 1.1 and 1.2, actions appear encapsulated in
* instructions, so you should call ofpacts_pull_openflow11_instructions()
* instead of this function.
@@ -1146,22 +1138,27 @@ get_version_from_ofpbuf(const struct ofpbuf *openflow)
* valid in a specific context. */
enum ofperr
ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
+ enum ofp_version version,
unsigned int actions_len,
struct ofpbuf *ofpacts)
{
- uint8_t version = get_version_from_ofpbuf(openflow);
-
- if (version < OFP13_VERSION) {
+ switch (version) {
+ case OFP10_VERSION:
+ case OFP11_VERSION:
+ case OFP12_VERSION:
return ofpacts_pull_actions(openflow, actions_len, ofpacts,
ofpacts_from_openflow11);
- } else {
+ case OFP13_VERSION:
return ofpacts_pull_actions(openflow, actions_len, ofpacts,
ofpacts_from_openflow13);
+ default:
+ NOT_REACHED();
}
}
enum ofperr
ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
+ enum ofp_version version,
unsigned int instructions_len,
struct ofpbuf *ofpacts)
{
@@ -1209,7 +1206,6 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
if (insts[OVSINST_OFPIT11_APPLY_ACTIONS]) {
const union ofp_action *actions;
size_t n_actions;
- uint8_t version = get_version_from_ofpbuf(openflow);
get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS],
&actions, &n_actions);
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 2184489..d67fc3f 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -511,9 +511,11 @@ enum ofperr ofpacts_pull_openflow10(struct ofpbuf *openflow,
unsigned int actions_len,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
+ enum ofp_version version,
unsigned int actions_len,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
+ enum ofp_version version,
unsigned int instructions_len,
struct ofpbuf *ofpacts);
enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6fe1cee..a0615b5 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2224,7 +2224,7 @@ ofp_print_group_desc(struct ds *s, const struct ofp_header *oh)
struct ofputil_group_desc gd;
int retval;
- retval = ofputil_decode_group_desc_reply(&gd, &b);
+ retval = ofputil_decode_group_desc_reply(&gd, &b, oh->version);
if (retval) {
if (retval != EOF) {
ds_put_cstr(s, " ***parse error***");
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 173b534..570447a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1504,7 +1504,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
return error;
}
- error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
+ error = ofpacts_pull_openflow11_instructions(&b, oh->version,
+ b.size, ofpacts);
if (error) {
return error;
}
@@ -2360,7 +2361,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
return EINVAL;
}
- if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
+ if (ofpacts_pull_openflow11_instructions(msg, oh->version,
+ length - sizeof *ofs -
padded_match_len, ofpacts)) {
VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
return EINVAL;
@@ -3092,7 +3094,8 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po,
return error;
}
- error = ofpacts_pull_openflow11_actions(&b, ntohs(opo->actions_len),
+ error = ofpacts_pull_openflow11_actions(&b, oh->version,
+ ntohs(opo->actions_len),
ofpacts);
if (error) {
return error;
@@ -5674,8 +5677,8 @@ ofputil_append_group_desc_reply(const struct ofputil_group_desc *gds,
}
static enum ofperr
-ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
- struct list *buckets)
+ofputil_pull_buckets(struct ofpbuf *msg, enum ofp_version version,
+ size_t buckets_length, struct list *buckets)
{
struct ofp11_bucket *ob;
@@ -5708,8 +5711,8 @@ ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
buckets_length -= ob_len;
ofpbuf_init(&ofpacts, 0);
- error = ofpacts_pull_openflow11_actions(msg, ob_len - sizeof *ob,
- &ofpacts);
+ error = ofpacts_pull_openflow11_actions(msg, version,
+ ob_len - sizeof *ob, &ofpacts);
if (error) {
ofpbuf_uninit(&ofpacts);
ofputil_bucket_list_destroy(buckets);
@@ -5745,7 +5748,7 @@ ofputil_pull_buckets(struct ofpbuf *msg, size_t buckets_length,
* otherwise a positive errno value. */
int
ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
- struct ofpbuf *msg)
+ struct ofpbuf *msg, enum ofp_version version)
{
struct ofp11_group_desc_stats *ogds;
size_t length;
@@ -5774,7 +5777,8 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
return OFPERR_OFPBRC_BAD_LEN;
}
- return ofputil_pull_buckets(msg, length - sizeof *ogds, &gd->buckets);
+ return ofputil_pull_buckets(msg, version, length - sizeof *ogds,
+ &gd->buckets);
}
/* Converts abstract group mod 'gm' into a message for OpenFlow version
@@ -5857,7 +5861,7 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
gm->type = ogm->type;
gm->group_id = ntohl(ogm->group_id);
- return ofputil_pull_buckets(&msg, msg.size, &gm->buckets);
+ return ofputil_pull_buckets(&msg, oh->version, msg.size, &gm->buckets);
}
/* Parse a queue status request message into 'oqsr'.
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index d5f34d7..5fa8fee 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -973,7 +973,7 @@ int ofputil_decode_group_stats_reply(struct ofpbuf *,
struct ofputil_group_stats *);
int ofputil_decode_group_desc_reply(struct ofputil_group_desc *,
- struct ofpbuf *);
+ struct ofpbuf *, enum ofp_version);
void ofputil_append_group_desc_reply(const struct ofputil_group_desc *,
struct list *buckets,
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index c2cc1f6..00d35aa 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2968,8 +2968,8 @@ ofctl_parse_ofp11_actions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
/* Convert to ofpacts. */
ofpbuf_init(&ofpacts, 0);
size = of11_in.size;
- error = ofpacts_pull_openflow11_actions(&of11_in, of11_in.size,
- &ofpacts);
+ error = ofpacts_pull_openflow11_actions(&of11_in, OFP11_VERSION,
+ of11_in.size, &ofpacts);
if (error) {
printf("bad OF1.1 actions: %s\n\n", ofperr_get_name(error));
ofpbuf_uninit(&ofpacts);
@@ -3036,8 +3036,8 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
/* Convert to ofpacts. */
ofpbuf_init(&ofpacts, 0);
size = of11_in.size;
- error = ofpacts_pull_openflow11_instructions(&of11_in, of11_in.size,
- &ofpacts);
+ error = ofpacts_pull_openflow11_instructions(&of11_in, OFP11_VERSION,
+ of11_in.size, &ofpacts);
if (!error) {
/* Verify actions. */
struct flow flow;
^ permalink raw reply related
* Re: [PATCH v2.40 4/7] ofp-actions: Add separate OpenFlow 1.3 action parser
From: Ben Pfaff @ 2013-09-30 2:19 UTC (permalink / raw)
To: Simon Horman
Cc: dev-yBygre7rU0TnMu66kgdUjQ, Ravi K, netdev-u79uwXL29TY76Z2rM5mHXA,
Isaku Yamahata
In-Reply-To: <20130930021246.GC4768-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
On Mon, Sep 30, 2013 at 11:12:49AM +0900, Simon Horman wrote:
> I have written an incremental patch to implement your suggestion
> and it seems that a helper function is not necessary.
>
> For reference the incremental change that I currently have is as follows.
> It is a bit noisy but seems clean to me.
A quick glance through the incremental shows that we are on the same
page. Thanks!
^ permalink raw reply
* Re: [PATCH v5] IPv6 NAT: Do not drop DNATed 6to4/6rd packets
From: catab @ 2013-09-30 3:05 UTC (permalink / raw)
To: David Miller; +Cc: hannes, netdev, yoshfuji, joe
In-Reply-To: <20130928.155750.1130089685321379918.davem@davemloft.net>
On Sat, 28 Sep 2013, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Tue, 24 Sep 2013 23:36:06 +0200
>
>> On Mon, Sep 23, 2013 at 11:04:19PM +0300, Catalin(ux) M. BOIE wrote:
>>> When a router is doing DNAT for 6to4/6rd packets the latest anti-spoofing
>>> patch (218774dc) will drop them because the IPv6 address embedded
>>> does not match the IPv4 destination. This patch will allow them to
>>> pass by testing if we have an address that matches on 6to4/6rd interface.
>>> I have been hit by this problem using Fedora and IPV6TO4_IPV4ADDR.
>>> Also, log the dropped packets (with rate limit).
>>>
>>> Signed-off-by: Catalin(ux) M. BOIE <catab@embedromix.ro>
>>
>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> Applied, but Catalin please strictly refer to changes in the following
> precise format:
>
> commit $SHA1_ID ("Commit message header line text")
>
> Because SHA1_IDs are ambiguous, especially when the change in question
> is backported into various -stable branches.
>
> The only way to resolve the ambiguity is to provide the commit message
> text (in parenthesis and double quotes).
Roger.
Should I resubmit it?
Should I send it also to 'stable'?
Thank you.
--
Catalin(ux) M. BOIE
http://kernel.embedromix.ro/
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: switch to use XPS to choose txq
From: Jason Wang @ 2013-09-30 3:20 UTC (permalink / raw)
To: Rusty Russell, netdev, linux-kernel, virtualization; +Cc: Michael S. Tsirkin
In-Reply-To: <874n932obk.fsf@rustcorp.com.au>
On 09/30/2013 07:35 AM, Rusty Russell wrote:
> Jason Wang <jasowang@redhat.com> writes:
>> We used to use a percpu structure vq_index to record the cpu to queue
>> mapping, this is suboptimal since it duplicates the work of XPS and
>> loses all other XPS functionality such as allowing use to configure
>> their own transmission steering strategy.
>>
>> So this patch switches to use XPS and suggest a default mapping when
>> the number of cpus is equal to the number of queues. With XPS support,
>> there's no need for keeping per-cpu vq_index and .ndo_select_queue(),
>> so they were removed also.
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/net/virtio_net.c | 55 +++++++--------------------------------------
>> 1 files changed, 9 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index defec2b..4102c1b 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -127,9 +127,6 @@ struct virtnet_info {
>> /* Does the affinity hint is set for virtqueues? */
>> bool affinity_hint_set;
>>
>> - /* Per-cpu variable to show the mapping from CPU to virtqueue */
>> - int __percpu *vq_index;
>> -
>> /* CPU hot plug notifier */
>> struct notifier_block nb;
>> };
>> @@ -1063,7 +1060,6 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev,
>> static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>> {
>> int i;
>> - int cpu;
>>
>> if (vi->affinity_hint_set) {
>> for (i = 0; i < vi->max_queue_pairs; i++) {
>> @@ -1073,20 +1069,11 @@ static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>>
>> vi->affinity_hint_set = false;
>> }
>> -
>> - i = 0;
>> - for_each_online_cpu(cpu) {
>> - if (cpu == hcpu) {
>> - *per_cpu_ptr(vi->vq_index, cpu) = -1;
>> - } else {
>> - *per_cpu_ptr(vi->vq_index, cpu) =
>> - ++i % vi->curr_queue_pairs;
>> - }
>> - }
>> }
>>
>> static void virtnet_set_affinity(struct virtnet_info *vi)
>> {
>> + cpumask_var_t cpumask;
>> int i;
>> int cpu;
>>
>> @@ -1100,15 +1087,21 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
>> return;
>> }
>>
>> + if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
>> + return;
>> +
>> i = 0;
>> for_each_online_cpu(cpu) {
>> virtqueue_set_affinity(vi->rq[i].vq, cpu);
>> virtqueue_set_affinity(vi->sq[i].vq, cpu);
>> - *per_cpu_ptr(vi->vq_index, cpu) = i;
>> + cpumask_clear(cpumask);
>> + cpumask_set_cpu(cpu, cpumask);
>> + netif_set_xps_queue(vi->dev, cpumask, i);
>> i++;
>> }
>>
>> vi->affinity_hint_set = true;
>> + free_cpumask_var(cpumask);
>> }
> Um, isn't this just cpumask_of(cpu)?
True, I thought it should be somewhat more easier.
Will post V2.
>
> Cheers,
> Rusty.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 1/1] linux-firmware: Add Brocade FC/FCOE Adapter firmware files
From: Ben Hutchings @ 2013-09-30 3:57 UTC (permalink / raw)
To: Rasesh Mody; +Cc: dwmw2, netdev, linux-scsi
In-Reply-To: <1378854654-11398-1-git-send-email-rmody@brocade.com>
[-- Attachment #1: Type: text/plain, Size: 708 bytes --]
On Tue, 2013-09-10 at 16:10 -0700, Rasesh Mody wrote:
> This patch adds firmware files for Brocade HBA and CNA drivers(BFA and BNA).
>
> Signed-off-by: Rasesh Mody <rmody@brocade.com>
> ---
> WHENCE | 27 +++++++++++++++++++++++++++
> cbfw-3.2.1.1.bin | Bin 0 -> 412528 bytes
> ct2fw-3.2.1.1.bin | Bin 0 -> 582440 bytes
> ctfw-3.2.1.1.bin | Bin 0 -> 537160 bytes
> 4 files changed, 27 insertions(+)
> create mode 100644 cbfw-3.2.1.1.bin
> create mode 100644 ct2fw-3.2.1.1.bin
> create mode 100644 ctfw-3.2.1.1.bin
[...]
Applied, thanks. Sorry for the delay.
Ben.
--
Ben Hutchings
Life is like a sewer:
what you get out of it depends on what you put into it.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* [PATCH] drivers: net: phy: marvell.c: removed checkpatch.pl warnings
From: Avinash kumar @ 2013-09-30 4:06 UTC (permalink / raw)
To: davem; +Cc: michal.simek, lars, RHoover, netdev, Avinash kumar
removes following warnings-
drivers/net/phy/marvell.c:37: WARNING: Use #include <linux/io.h> instead of <asm/io.h>
drivers/net/phy/marvell.c:39: WARNING: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
Signed-off-by: Avinash Kumar <avi.kp.137@gmail.com>
---
drivers/net/phy/marvell.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 2e91477..2e3c778e 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -34,9 +34,9 @@
#include <linux/marvell_phy.h>
#include <linux/of.h>
-#include <asm/io.h>
+#include <linux/io.h>
#include <asm/irq.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#define MII_MARVELL_PHY_PAGE 22
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/3] net: can: c_can_platform: Remove redundant of_match_ptr
From: Sachin Kamat @ 2013-09-30 4:25 UTC (permalink / raw)
To: netdev; +Cc: davem, sachin.kamat, Marc Kleine-Budde, linux-can
In-Reply-To: <1380515114-2823-1-git-send-email-sachin.kamat@linaro.org>
The data structure of_match_ptr() protects is always compiled in.
Hence of_match_ptr() is not needed.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can@vger.kernel.org
---
drivers/net/can/c_can/c_can_platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 294ced3..d66ac26 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -322,7 +322,7 @@ static struct platform_driver c_can_plat_driver = {
.driver = {
.name = KBUILD_MODNAME,
.owner = THIS_MODULE,
- .of_match_table = of_match_ptr(c_can_of_table),
+ .of_match_table = c_can_of_table,
},
.probe = c_can_plat_probe,
.remove = c_can_plat_remove,
--
1.7.9.5
^ 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