* Re: [PATCH 10/16] c_can: add 16bit align 32bit access functions
From: Marc Kleine-Budde @ 2013-09-09 9:57 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-11-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 3941 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> The FlexCard only allows 32bit access to DCAN registers, otherwise the
> register value can be wraped up. Add a new helper function to access
> registers 16bit aligned but 32bit access.
You are modifying code you have previously added. You should shuffle
your patches that this is not necessary.
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> drivers/net/can/c_can/c_can.c | 1 +
> drivers/net/can/c_can/c_can.h | 1 +
> drivers/net/can/c_can/c_can_platform.c | 38 ++++++++++++++++++++++++++++++++--
> 3 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index c573399..46d741d 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -1305,6 +1305,7 @@ struct net_device *alloc_c_can_dev(void)
> priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> CAN_CTRLMODE_LISTENONLY |
> CAN_CTRLMODE_BERR_REPORTING;
> + spin_lock_init(&priv->lock);
>
> return dev;
> }
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 9f0eda8..beea437 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -175,6 +175,7 @@ struct c_can_priv {
> u32 __iomem *raminit_ctrlreg;
> unsigned int instance;
> void (*raminit) (const struct c_can_priv *priv, bool enable);
> + spinlock_t lock;
> };
>
> struct net_device *alloc_c_can_dev(void);
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 43a3e3f..c6c1eb4 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -58,6 +58,40 @@ static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv *priv,
> writew(val, priv->base + priv->regs[index]);
> }
>
> +#define ALIGN_DOWN(p, a) ((typeof(p))round_down((unsigned long)(p), (a)))
> +
> +static u16 c_can_plat_read_16bit_align_32bit_access(struct c_can_priv *priv,
> + enum reg index)
> +{
> + void *addr = priv->base + priv->regs[index];
Please compile with C=2. Should be void __iomem *.
> + u32 reg;
> + reg = readl(ALIGN_DOWN(addr, 4));
> +
> + return IS_ALIGNED((unsigned long)addr, 4) ?
> + reg & 0xffff :
> + reg >> 16;
> +}
> +
> +static void c_can_plat_write_16bit_align_32bit_access(struct c_can_priv *priv,
> + enum reg index, u16 val)
> +{
> + unsigned long flags;
> + void *addr = priv->base + priv->regs[index];
dito
> + u32 reg;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> + reg = readl(ALIGN_DOWN(addr, 4));
> + if (IS_ALIGNED((unsigned long)addr, 4)) {
> + reg &= 0xffff0000;
> + reg |= val;
> + } else {
> + reg &= 0xffff;
> + reg |= val << 16;
> + }
> + writel(reg, ALIGN_DOWN(addr, 4));
> + spin_unlock_irqrestore(&priv->lock, flags);
> +}
> +
> static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv *priv,
> enum reg index)
> {
> @@ -317,8 +351,8 @@ static int c_can_plat_probe(struct platform_device *pdev)
> case BOSCH_D_CAN_FLEXCARD:
> priv->regs = reg_map_d_can;
> priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
> - priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> - priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> + priv->read_reg = c_can_plat_read_16bit_align_32bit_access;
> + priv->write_reg = c_can_plat_write_16bit_align_32bit_access;
> priv->instance = pdev->id;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>
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
* Re: [PATCH net-next v4 1/6] bonding: simplify and use RCU protection for 3ad xmit path
From: Veaceslav Falico @ 2013-09-09 9:57 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Nikolay Aleksandrov, David S. Miller, Netdev
In-Reply-To: <522D8DB5.1030302@huawei.com>
On Mon, Sep 09, 2013 at 04:58:29PM +0800, Ding Tianhong wrote:
>On 2013/9/8 14:05, Ding Tianhong wrote:
>
>Hi Veaceslav and Nik:
>
>please take a moment to reveiw the function just modify for bond_XXX_rcu,
>and give me some advice. thanks for the help again.:)
>
>+#define bond_first_slave_rcu(bond) \
>+ list_first_or_null_rcu(&(bond)->slave_list, struct slave, list);
>+#define bond_last_slave_rcu(bond) \
>+ ({struct list_head *__slave_list = &(bond)->slave_list; \
>+ struct list_head __rcu *__prev = \
>+ (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
>+ likely(__slave_list != __prev) ? \
>+ container_of(__prev, struct slave, list) : NULL;})
Please take a look at Nikolay's reply to my RCU email -
http://www.spinics.net/lists/netdev/msg249805.html . And mine also, to his
email. In short - RCU doesn't guarantee ->prev, so better take the approach
of eliminating bond_last/prev_slave completely.
>+
> #define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list)
> #define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list)
>
>@@ -93,6 +117,29 @@
> (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
> bond_to_slave((pos)->list.prev))
>
>+/* Since bond_first/last_slave_rcu can return NULL, these can return NULL too */
>+#define bond_next_slave_rcu(bond, pos) \
>+ ({struct list_head *__slave_list = &(bond)->slave_list; \
>+ struct list_head __rcu *__next = list_next_rcu(__slave_list); \
>+ struct list_head *__pos_list = &(pos)->list; \
>+ struct list_head __rcu *__pos_next = list_next_rcu(__pos_list); \
>+ likely(__pos_next != __slave_list) ? \
>+ container_of(__pos_next, struct slave, list) : \
>+ container_of(__next, struct slave, list); \
>+ })
Nice, but can be shortened - we know that pos won't go away.
>+
>+#define bond_prev_slave_rcu(bond, pos) \
>+ ({struct list_head *__slave_list = &(bond)->slave_list; \
>+ struct list_head __rcu *__prev = \
>+ (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
>+ struct list_head *__pos_list = &(pos)->list; \
>+ struct list_head __rcu *__pos_prev = (__pos_list->prev != LIST_POISON2) ? \
>+ (*((struct list_head __rcu **)(&(__pos_list)->prev))) : NULL; \
>+ likely(__pos_prev != __slave_list) ? \
>+ ((__pos_prev) ? list_entry_rcu(__pos_prev, struct slave, list) : NULL;) : \
>+ (list_entry_rcu(__prev, struct slave, list)); \
>+ })
Same remark as above about prev.
>+
>
>
>-#define bond_for_each_slave_from(bond, pos, cnt, start) \
>- for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
>- cnt++, pos = bond_next_slave(bond, pos))
>-
>+#define bond_for_each_slave_from(bond, pos, start) \
>+ for (pos = start; pos; (pos = bond_next_slave(bond, pos)) != start ? \
>+ (pos) : (pos = NULL))
>+
>+#define bond_for_each_slave_from_rcu(bond, pos, start) \
>+ for ({struct list_head *__start = &(start)->list; \
>+ struct list_head *__slave_list = &(bond)->slave_list; \
>+ pos = list_entry_rcu(__start, struct slave, list);}; \
>+ pos; \
>+ {struct list_head __rcu *__next = list_next_rcu(pos->next); \
>+ __next != __slave_list ? \
>+ __next : __next = list_next_rcu(__next->next); \
>+ __next != __start ? \
>+ pos = list_entry_rcu(__next, struct slave, list) : \
>+ pos = NULL; \
>+ })
Jeez, I don't even want to review it. It's too complex and too hard to
maintain, even if it works. Can you please make something shorter/easier to
understand?
>+
>
>Best regards
>Ding
>
>
>>> --
>>> To unsubscribe from this list: send the line "unsubsc ribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> .
>>
>
>
^ permalink raw reply
* RFC: crash in fib6_clean_all() while loading ipv6 module
From: Michal Kubecek @ 2013-09-09 10:05 UTC (permalink / raw)
To: netdev
Hello,
Two of our customers encountered a crash in fib6_clean_all() when
booting their system:
[ 12.408421] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 12.408424] IP: [<ffffffffa02566b4>] fib6_clean_all+0x34/0xd0 [ipv6_lib]
[ 12.408434] PGD c3590f067 PUD c29073067 PMD 0
[ 12.408436] Oops: 0000 [#1] SMP
[ 12.408439] CPU 1
[ 12.408440] Modules linked in: processor(+) ipv6(+) thermal_sys ipv6_lib
ahci(+) ixgbe(+) ehci_hcd libahci hwmon igb(+) libata usbcore i2c_i801 dca
iTCO_wdt pcspkr scsi_mod e1000e i2c_core usb_common iTCO_vendor_support
rtc_cmos container ptp pps_core button mdio
[ 12.408449] Supported: Yes
[ 12.408449]
[ 12.408451] Pid: 211, comm: modprobe Not tainted 3.0.76-0.11-default #1 ...
[ 12.408453] RIP: 0010:[<ffffffffa02566b4>] [<ffffffffa02566b4>] fib6_clean_all+0x34/0xd0 [ipv6_lib]
[ 12.408460] RSP: 0018:ffff880c1e983b98 EFLAGS: 00010246
[ 12.408462] RAX: 0000000000000000 RBX: ffffffffffffffff RCX: 0000000000000000
[ 12.408463] RDX: 0000000000000000 RSI: ffffffffa0254f90 RDI: ffffffff81a72280
[ 12.408464] RBP: ffffffff81a72280 R08: 0000000000000000 R09: 000000003520aec8
[ 12.408466] R10: 000000005f000c28 R11: 000000003520aec8 R12: 0000000000000000
[ 12.408467] R13: ffffffff81a72280 R14: 0000000000000000 R15: ffffffffa0254f90
[ 12.408469] FS: 00007fb2039f1700(0000) GS:ffff880c7f040000(0000) knlGS:0000000000000000
[ 12.408470] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 12.408471] CR2: 0000000000000000 CR3: 0000000c2904e000 CR4: 00000000001407e0
[ 12.408473] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 12.408474] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 12.408476] Process modprobe (pid: 211, threadinfo ffff880c1e982000, task ffff880c3242c540)
[ 12.408477] Stack:
[ 12.408478] 0000000000000000 0000000000000000 0000000000000000 ffffffff8146544e
[ 12.408480] 000000000006a9d0 0000000000000000 000000003520aec8 000000005f000c28
[ 12.408483] 000000003520aec8 0000000000000000 0000000000000000 0000000000000000
[ 12.408485] Call Trace:
[ 12.408507] [<ffffffffa02567a3>] fib6_run_gc+0x53/0xf0 [ipv6_lib]
[ 12.408524] [<ffffffffa025c5c6>] ndisc_netdev_event+0x186/0x190 [ipv6_lib]
[ 12.408541] [<ffffffff81460437>] notifier_call_chain+0x37/0x70
[ 12.408547] [<ffffffff813a6e52>] dev_addr_add+0x52/0x90
[ 12.408556] [<ffffffffa032e3e9>] ixgbe_probe+0x8c9/0xd10 [ixgbe]
[ 12.408565] [<ffffffff8127d5d4>] local_pci_probe+0x54/0xe0
[ 12.408570] [<ffffffff8127d740>] __pci_device_probe+0xe0/0xf0
[ 12.408573] [<ffffffff8127e953>] pci_device_probe+0x33/0x60
[ 12.408578] [<ffffffff8132ddda>] really_probe+0x7a/0x260
[ 12.408581] [<ffffffff8132e023>] driver_probe_device+0x63/0xc0
[ 12.408585] [<ffffffff8132e113>] __driver_attach+0x93/0xa0
[ 12.408588] [<ffffffff8132d468>] bus_for_each_dev+0x88/0xb0
[ 12.408591] [<ffffffff8132cbb5>] bus_add_driver+0x155/0x2a0
[ 12.408595] [<ffffffff8132e769>] driver_register+0x79/0x170
[ 12.408599] [<ffffffff8127ebe8>] __pci_register_driver+0x58/0xe0
[ 12.408604] [<ffffffff810001cb>] do_one_initcall+0x3b/0x180
[ 12.408610] [<ffffffff810a02df>] sys_init_module+0xcf/0x240
[ 12.408615] [<ffffffff81464592>] system_call_fastpath+0x16/0x1b
The crash is caused by fib6_clean_all() dereferencing a null pointer in
init_net.ipv6.fib_table_hash because ndisc_netdev_event() handler which
calls fib6_run_gc() is registered by ndisc_init() but the relevant data
structures aren't initialized until ip6_route_init() is called. If a
device intialization falls into this window, it emits NETDEV_CHANGEADDR,
leading to a crash.
This could be prevented by setting a flag when ip6_route_init() is
complete and not calling fib6_run_gc() from ndisc_netdev_event() until
the flag is set. However, I don't like the idea of adding a test which
will be useful only in a short window while loading ipv6 module.
The only other actions ndisc_netdev_event() does are flushing the neigh
table (which should be empty at the moment) and calling
ndisc_send_unsol_na() (which should do nothing as there is no IPv6
address assigned yet). Thus I believe taking the netdev event handler
registration call out of ndisc_init() into a separate function, say
ndisc_late_init(), which would be called after ip6_route_init() should
be a safe and efficient way to avoid the race condition.
Before I submit the patch, I wanted to ask if someone can see a problem
with this solution (or a better solution).
Michal Kubecek
^ permalink raw reply
* Re: [PATCH 11/16] c_can: stop netqueue if hardware is busy
From: Marc Kleine-Budde @ 2013-09-09 10:05 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-12-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 4985 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> Unlike other network devices the FlexCard do not support interrupts on TX.
> Emulate the TX interrupt by polling the MOTRX registers and restart the
> netqueue if one or more message object is available for transtition of CAN
> frames.
>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> drivers/net/can/c_can/c_can.c | 61 ++++++++++++++++++++++++++++++++++++++++---
> drivers/net/can/c_can/c_can.h | 4 +++
> 2 files changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 46d741d..02f7c89 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -550,18 +550,66 @@ static void c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
>
> static inline int c_can_is_next_tx_obj_busy(struct c_can_priv *priv, int objno)
> {
> - int val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
> + int val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG + (objno / 32) * 4);
>
> /*
> * as transmission request register's bit n-1 corresponds to
> * message object n, we need to handle the same properly.
> */
> - if (val & (1 << (objno - 1)))
> + if (val & (1 << ((objno % 32) - 1)))
> return 1;
>
> return 0;
> }
>
> +static void c_can_motrx_add_timer(struct net_device *dev)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> +
> + priv->timer.expires = round_jiffies_relative(1);
> + add_timer(&priv->timer);
> +}
> +
> +static void c_can_motrx_poll(unsigned long data)
> +{
> + struct net_device *dev = (struct net_device *)data;
> + struct c_can_priv *priv = netdev_priv(dev);
> + u32 val, empty = 0;
> + int i;
> +
> + for (i = 0; i < C_CAN_MOTRX_NR; i++) {
> + val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG + i * 4);
> +
> + if (val < priv->motrx[i]) {
> + netif_wake_queue(dev);
> + return;
> + }
> +
> + empty |= val;
> + }
> +
> + if (!empty) {
> + netif_wake_queue(dev);
> + return;
> + }
> +
> + c_can_motrx_add_timer(dev);
> +}
> +
> +static void c_can_motrx_monitor(struct net_device *dev)
> +{
> + struct c_can_priv *priv = netdev_priv(dev);
> + int i;
> +
> + if (priv->type != BOSCH_D_CAN_FLEXCARD)
> + return;
> +
> + for (i = 0; i < C_CAN_MOTRX_NR; i++)
> + priv->motrx[i] = c_can_read_reg32(priv,
> + C_CAN_TXRQST1_REG + i * 4);
> + c_can_motrx_add_timer(dev);
> +}
> +
> static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> @@ -582,6 +630,7 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> if (readl(priv->base + FC_TXFIFO_STAT) &
> FC_TXFIFO_STAT_FULL) {
> netif_stop_queue(dev);
> + c_can_motrx_monitor(dev);
> return NETDEV_TX_BUSY;
> }
>
> @@ -619,8 +668,10 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> * if the next TX message object is still in use
> */
> if (c_can_is_next_tx_obj_busy(priv, get_tx_next_msg_obj(priv))
> - || ((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) == 0))
> + || ((priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) == 0)) {
> netif_stop_queue(dev);
> + c_can_motrx_monitor(dev);
> + }
> }
>
> return NETDEV_TX_OK;
> @@ -1272,6 +1323,7 @@ static int c_can_close(struct net_device *dev)
> {
> struct c_can_priv *priv = netdev_priv(dev);
>
> + del_timer_sync(&priv->timer);
> netif_stop_queue(dev);
> napi_disable(&priv->napi);
> c_can_stop(dev);
> @@ -1306,6 +1358,9 @@ struct net_device *alloc_c_can_dev(void)
> CAN_CTRLMODE_LISTENONLY |
> CAN_CTRLMODE_BERR_REPORTING;
> spin_lock_init(&priv->lock);
> + priv->timer.function = c_can_motrx_poll;
> + priv->timer.data = (unsigned long) dev;
> + init_timer_deferrable(&priv->timer);
>
> return dev;
> }
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index beea437..6d0f805 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -153,6 +153,8 @@ enum c_can_dev_id {
> BOSCH_D_CAN_FLEXCARD,
> };
>
> +#define C_CAN_MOTRX_NR 1
Are there reasons to have C_CAN_MOTRX_NR != 1? If not, please remove the
array and make code more clean.
> /* c_can private data structure */
> struct c_can_priv {
> struct can_priv can; /* must be the first member */
> @@ -176,6 +178,8 @@ struct c_can_priv {
> unsigned int instance;
> void (*raminit) (const struct c_can_priv *priv, bool enable);
> spinlock_t lock;
> + struct timer_list timer;
> + u32 motrx[C_CAN_MOTRX_NR];
> };
>
> struct net_device *alloc_c_can_dev(void);
>
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
* Re: [PATCH 11/16] c_can: stop netqueue if hardware is busy
From: Marc Kleine-Budde @ 2013-09-09 10:21 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-12-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 796 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> Unlike other network devices the FlexCard do not support interrupts on TX.
> Emulate the TX interrupt by polling the MOTRX registers and restart the
> netqueue if one or more message object is available for transtition of CAN
> frames.
>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
It's not onlynetif_wake_queue() that you have to call on TX-complete.
Have a look at c_can_do_tx(), better call it, or trigger napi from your
timer.
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
* Re: [PATCH 12/16] c_can: Add flag to disable automatic retransmission of CAN frames
From: Marc Kleine-Budde @ 2013-09-09 10:21 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-13-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 3408 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> The C_CAN/D_CAN controler can automatic retransmit CAN after arbitration loss.
> Add a flag to CAN ctrlmode set the appropiate behaivior of this feature.
We already have this flag, it's called CAN_CTRLMODE_ONE_SHOT.
Marc
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> drivers/net/can/c_can/c_can.c | 16 +++++++++-------
> include/uapi/linux/can/netlink.h | 2 ++
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 02f7c89..51ca2a6 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -785,33 +785,35 @@ static void c_can_configure_msg_objects(struct net_device *dev, int invalidate)
> static int c_can_chip_config(struct net_device *dev)
> {
> struct c_can_priv *priv = netdev_priv(dev);
> + u16 reg;
> int ret;
>
> - /* enable automatic retransmission */
> - priv->write_reg(priv, C_CAN_CTRL_REG,
> - CONTROL_ENABLE_AR);
> + if (priv->can.ctrlmode & CAN_CTRLMODE_DAR)
> + reg = CONTROL_DISABLE_AR;
> + else
> + reg = CONTROL_ENABLE_AR;
>
> if ((priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) &&
> (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)) {
> /* loopback + silent mode : useful for hot self-test */
> priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> - CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> + reg | CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> priv->write_reg(priv, C_CAN_TEST_REG,
> TEST_LBACK | TEST_SILENT);
> } else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> /* loopback mode : useful for self-test function */
> priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> - CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> + reg | CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
> } else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> /* silent mode : bus-monitoring mode */
> priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> - CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> + reg | CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
> } else
> /* normal mode*/
> priv->write_reg(priv, C_CAN_CTRL_REG,
> - CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
> + reg | CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
>
> /* configure message objects */
> c_can_configure_msg_objects(dev, 1);
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index 14966dd..fdce5af 100644
> --- a/include/uapi/linux/can/netlink.h
> +++ b/include/uapi/linux/can/netlink.h
> @@ -88,6 +88,8 @@ struct can_ctrlmode {
> #define CAN_CTRLMODE_3_SAMPLES 0x04 /* Triple sampling mode */
> #define CAN_CTRLMODE_ONE_SHOT 0x08 /* One-Shot mode */
> #define CAN_CTRLMODE_BERR_REPORTING 0x10 /* Bus-error reporting */
> +#define CAN_CTRLMODE_DAR 0x20 /* Disable Automatic
> + * Retransmission */
>
> /*
> * CAN device statistics
>
--
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
* Re: [PATCH 13/16] c_can: flexcard: add ioctl to reset FIFO message object
From: Marc Kleine-Budde @ 2013-09-09 10:24 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-14-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> The FlexCard FIFO reset implementation did not reset the FIFO message object.
> Since the FIFO configuration is done from userspace a function to reset the
> FIFO message object is needed. Add an ioctl to reset the FIFO message object
> from userspace.
Can you elaborate this a bit.
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
>
> ---
> drivers/net/can/c_can/c_can.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 51ca2a6..670a6b1 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -1447,7 +1447,21 @@ void free_c_can_dev(struct net_device *dev)
> }
> EXPORT_SYMBOL_GPL(free_c_can_dev);
>
> +static int c_can_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> +{
> + switch (cmd) {
> + case SIOCDEVPRIVATE:
> + c_can_inval_msg_object(dev, 0, FC_TXFIFO_MO);
> + break;
What happens if you call this ioctl on non flexcard HW?
> + default:
> + return -ENOIOCTLCMD;
> + }
> +
> + return 0;
> +}
> +
> static const struct net_device_ops c_can_netdev_ops = {
> + .ndo_do_ioctl = c_can_ioctl,
> .ndo_open = c_can_open,
> .ndo_stop = c_can_close,
> .ndo_start_xmit = c_can_start_xmit,
>
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
* Re: [PATCH 15/16] flexcard: can: Configure CAN loopback packages (TXACK)
From: Marc Kleine-Budde @ 2013-09-09 10:29 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-16-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 3416 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> Signed-off-by: Holger Dengler <dengler@linutronix.de>
I'm missing your S-o-b and a patch description. I'm finding some
uncommon use of frame->can_dlc.
Marc
> ---
> drivers/net/can/c_can/c_can.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 670a6b1..92c1444 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -358,11 +358,13 @@ static inline void c_can_object_put(struct net_device *dev,
> }
>
> static void c_can_write_msg_object(struct net_device *dev,
> - int iface, struct can_frame *frame, int objno)
> + int iface, struct can_frame *frame, int objno,
> + int txie_off)
> {
> u32 flags = IF_ARB_MSGVAL;
> unsigned int id;
> struct c_can_priv *priv = netdev_priv(dev);
> + u32 int_en;
>
> if (!(frame->can_id & CAN_RTR_FLAG))
> flags |= IF_ARB_TRANSMIT;
> @@ -388,9 +390,10 @@ static void c_can_write_msg_object(struct net_device *dev,
> frame->data[4] | frame->data[5] << 8);
>
> /* enable interrupt for this message object */
> - priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface),
> - IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> - frame->can_dlc);
> + int_en = frame->can_dlc;
> + if (!txie_off)
> + int_en |= IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB;
> + priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface), int_en);
> c_can_object_put(dev, iface, objno, IF_COMM_ALL);
> }
>
> @@ -616,16 +619,17 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> u32 msg_obj_no;
> struct c_can_priv *priv = netdev_priv(dev);
> struct can_frame *frame = (struct can_frame *)skb->data;
> - int tx_fifo;
> + int tx_fifo, txie_off;
>
> if (can_dropped_invalid_skb(dev, skb))
> return NETDEV_TX_OK;
>
> tx_fifo = frame->can_dlc & FC_TXFIFO_FLAG;
> + txie_off = frame->can_dlc & FC_TXACKOFF_FLAG;
> frame->can_dlc &= FC_TXFIFO_DLC_MASK;
>
> if (tx_fifo) {
> - u32 id, *data, ctrl;
> + u32 id, *data, ctrl, conf;
>
> if (readl(priv->base + FC_TXFIFO_STAT) &
> FC_TXFIFO_STAT_FULL) {
> @@ -652,6 +656,11 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> writel(data[1], priv->base + FC_TXFIFO_MSGDB);
> }
>
> + conf = readl(priv->base + FC_TXFIFO_CONF);
> + conf &= ~FC_TXFIFO_CONF_TXACK;
> + conf |= (txie_off ? 0 : FC_TXFIFO_CONF_TXACK);
> + writel(conf, priv->base + FC_TXFIFO_CONF);
> +
> ctrl = readl(priv->base + FC_TXFIFO_CTRL);
> ctrl |= FC_TXFIFO_CTRL_REQ;
> writel(ctrl, priv->base + FC_TXFIFO_CTRL);
> @@ -660,7 +669,7 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> msg_obj_no = get_tx_next_msg_obj(priv);
>
> /* prepare message object for transmission */
> - c_can_write_msg_object(dev, 0, frame, msg_obj_no);
> + c_can_write_msg_object(dev, 0, frame, msg_obj_no, txie_off);
> priv->tx_next++;
>
> can_put_echo_skb(skb, dev, msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
>
--
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
* Re: [PATCH 16/16] c_can: fix TX packet accounting
From: Marc Kleine-Budde @ 2013-09-09 10:31 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-17-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> Due to the integration of the FlexCard support the TX stats accounting got
> lost. Readd the accounting.
Please squash into the appropriate patch.
Marc
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> drivers/net/can/c_can/c_can.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 92c1444..b409cf2 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -619,6 +619,7 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> u32 msg_obj_no;
> struct c_can_priv *priv = netdev_priv(dev);
> struct can_frame *frame = (struct can_frame *)skb->data;
> + struct net_device_stats *stats = &dev->stats;
> int tx_fifo, txie_off;
>
> if (can_dropped_invalid_skb(dev, skb))
> @@ -664,6 +665,9 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> ctrl = readl(priv->base + FC_TXFIFO_CTRL);
> ctrl |= FC_TXFIFO_CTRL_REQ;
> writel(ctrl, priv->base + FC_TXFIFO_CTRL);
> +
> + stats->tx_bytes += frame->can_dlc;
> + stats->tx_packets++;
> kfree_skb(skb);
> } else {
> msg_obj_no = get_tx_next_msg_obj(priv);
> @@ -672,6 +676,8 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> c_can_write_msg_object(dev, 0, frame, msg_obj_no, txie_off);
> priv->tx_next++;
>
> + stats->tx_bytes += frame->can_dlc;
> + stats->tx_packets++;
> can_put_echo_skb(skb, dev, msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> /* we have to stop the queue in case of a wrap around or
> * if the next TX message object is still in use
>
--
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
* Re: [PATCH 12/16] c_can: Add flag to disable automatic retransmission of CAN frames
From: Marc Kleine-Budde @ 2013-09-09 10:34 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <522DA145.9090507@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 718 bytes --]
On 09/09/2013 12:21 PM, Marc Kleine-Budde wrote:
> On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
>> The C_CAN/D_CAN controler can automatic retransmit CAN after arbitration loss.
>> Add a flag to CAN ctrlmode set the appropiate behaivior of this feature.
>
> We already have this flag, it's called CAN_CTRLMODE_ONE_SHOT.
BTW: Have a look at commit:
ee6f098 can: c_can: disable one shot mode until driver is fixed
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
* Re: [PATCH 04/16] c_can: fix receive buffer configuration
From: Marc Kleine-Budde @ 2013-09-09 10:51 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-5-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> The former implementation lacks some initialization of receive buffer.
> First the maximal buffer length was 0. The Buffer direction was not set
> and the mask register a reserverd flag was cleared.
>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> drivers/net/can/c_can/c_can.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index bdb7bcd..886163f 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -116,6 +116,11 @@
> IF_COMM_CONTROL | IF_COMM_TXRQST | \
> IF_COMM_DATAA | IF_COMM_DATAB)
>
> +/* IFx mask */
> +#define IF_MASK_MXTD BIT(31)
> +#define IF_MASK_MDIR BIT(30)
> +#define IF_MASK_RES BIT(29)
> +
> /* IFx arbitration */
> #define IF_ARB_MSGVAL BIT(31)
> #define IF_ARB_MSGXTD BIT(30)
> @@ -653,11 +658,15 @@ static void c_can_configure_msg_objects(struct net_device *dev)
>
> /* setup receive message objects */
> for (i = C_CAN_MSG_OBJ_RX_FIRST; i < C_CAN_MSG_OBJ_RX_LAST; i++)
> - c_can_setup_receive_object(dev, 0, i, 0, 0,
> - (IF_MCONT_RXIE | IF_MCONT_UMASK) & ~IF_MCONT_EOB);
> -
> - c_can_setup_receive_object(dev, 0, C_CAN_MSG_OBJ_RX_LAST, 0, 0,
> - IF_MCONT_EOB | IF_MCONT_RXIE | IF_MCONT_UMASK);
> + c_can_setup_receive_object(dev, 0, i,
> + IF_MASK_MDIR | IF_MASK_RES, 0,
> + IF_MCONT_UMASK | IF_MCONT_RXIE |
> + IF_MCONT_DLC_MAX);
DLC_MAX should be added in this patch. not in 3.
> +
> + c_can_setup_receive_object(dev, 0, C_CAN_MSG_OBJ_RX_LAST,
> + IF_MASK_MDIR | IF_MASK_RES, 0,
> + IF_MCONT_UMASK | IF_MCONT_EOB |
> + IF_MCONT_RXIE | IF_MCONT_DLC_MAX);
> }
>
> /*
>
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
* Re: [PATCH 09/16] c_can: expicit 32bit access on D_CAN to message buffer data register
From: Marc Kleine-Budde @ 2013-09-09 11:20 UTC (permalink / raw)
To: Benedikt Spranger
Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior,
Holger Dengler, linux-can@vger.kernel.org
In-Reply-To: <1378711513-2548-10-git-send-email-b.spranger@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]
On 09/09/2013 09:25 AM, Benedikt Spranger wrote:
> change the 16bit access of ARB1_REG and DATA1/2_REG to a 32bit access.
>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
> drivers/net/can/c_can/c_can.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 4b94f2d..c573399 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -360,7 +360,6 @@ static inline void c_can_object_put(struct net_device *dev,
> static void c_can_write_msg_object(struct net_device *dev,
> int iface, struct can_frame *frame, int objno)
> {
> - int i;
> u32 flags = IF_ARB_MSGVAL;
> unsigned int id;
> struct c_can_priv *priv = netdev_priv(dev);
> @@ -376,15 +375,17 @@ static void c_can_write_msg_object(struct net_device *dev,
>
> id |= flags;
>
> - priv->write_reg(priv, C_CAN_IFACE(ARB1_REG, iface),
> - IFX_WRITE_LOW_16BIT(id));
> - priv->write_reg(priv, C_CAN_IFACE(ARB2_REG, iface),
> - IFX_WRITE_HIGH_16BIT(id));
> + c_can_writereg32(priv, C_CAN_IFACE(ARB1_REG, iface),
> + IFX_WRITE_HIGH_16BIT(id),
> + IFX_WRITE_LOW_16BIT(id));
> +
> + c_can_writereg32(priv, C_CAN_IFACE(DATA1_REG, iface),
> + frame->data[2] | frame->data[3] << 8,
> + frame->data[0] | frame->data[1] << 8);
You can use something like beXX_to_cpup((__be32 *)&cf->data[0]) here.
>
> - for (i = 0; i < frame->can_dlc; i += 2) {
> - priv->write_reg(priv, C_CAN_IFACE(DATA1_REG, iface) + i / 2,
> - frame->data[i] | (frame->data[i + 1] << 8));
> - }
> + c_can_writereg32(priv, C_CAN_IFACE(DATA3_REG, iface),
> + frame->data[6] | frame->data[7] << 8,
> + frame->data[4] | frame->data[5] << 8);
You write here the upper 32 bit unconditionally, the original code doesn't.
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 net] bnx2x: Fix configuration of doorbell block
From: Ariel Elior @ 2013-09-09 11:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Eilon Greenstein, Eric Dumazet, Ariel Elior
As part of VF RSS feature doorbell block was configured not to use dpm, but
a small part of configuration was left out, preventing the driver from sending
tx messages to the device. This patch adds the missing configuration.
Reported-by: Eric Dumazet <eric.dumazet@gmil.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 1 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 3 ---
2 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 634a793..2f8dbbb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -7645,6 +7645,7 @@ static int bnx2x_init_hw_func(struct bnx2x *bp)
bnx2x_init_block(bp, BLOCK_TM, init_phase);
bnx2x_init_block(bp, BLOCK_DORQ, init_phase);
+ REG_WR(bp, DORQ_REG_MODE_ACT, 1); /* no dpm */
bnx2x_iov_init_dq(bp);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index b26eb83..2604b62 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -1756,9 +1756,6 @@ void bnx2x_iov_init_dq(struct bnx2x *bp)
REG_WR(bp, DORQ_REG_VF_TYPE_MIN_MCID_0, 0);
REG_WR(bp, DORQ_REG_VF_TYPE_MAX_MCID_0, 0x1ffff);
- /* set the number of VF allowed doorbells to the full DQ range */
- REG_WR(bp, DORQ_REG_VF_NORM_MAX_CID_COUNT, 0x20000);
-
/* set the VF doorbell threshold */
REG_WR(bp, DORQ_REG_VF_USAGE_CT_LIMIT, 4);
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH stable] ipv6: restrict neighbor entry creation to output flow
From: Jiri Pirko @ 2013-09-09 12:17 UTC (permalink / raw)
To: David Miller; +Cc: hannes, mleitner, netdev, dbanerje, yoshfuji
In-Reply-To: <20130815.155454.417440388188230172.davem@davemloft.net>
Fri, Aug 16, 2013 at 12:54:54AM CEST, davem@davemloft.net wrote:
>From: Hannes Frederic Sowa <hannes@stressinduktion.org>
>Date: Wed, 14 Aug 2013 17:00:54 +0200
>
>> On Wed, Aug 14, 2013 at 10:53:27AM -0300, Marcelo Ricardo Leitner wrote:
>>> This patch is based on 3.2.y branch, the one used by reported. Please let me
>>> know if it should be different. Thanks.
>>>
>>> ---8<---
>>>
>>> Commit 0d6a77079c475033cb622c07c5a880b392ef664e introduced a regression on
>>> which routes to local delivery would not work anymore. Like this:
>>>
>>> $ ip -6 route add local 2001::/64 dev lo
>>> $ ping6 -c1 2001::9
>>> PING 2001::9(2001::9) 56 data bytes
>>> ping: sendmsg: Invalid argument
>>>
>>> As this is a local delivery, that commit would not allow the creation of a
>>> neighbor entry and thus the packet cannot be sent.
>>>
>>> But as TPROXY scenario actually needs to avoid the neighbor entry creation only
>>> for input flow, this patch now limits previous patch to input flow, keeping
>>> output as before that patch.
>>>
>>> Reported-by: Debabrata Banerjee <dbavatar@gmail.com>
>>> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
>>> CC: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>
>> Looks good, thanks Marcelo!
>>
>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>
>> David, this patch is for all stable kernels except the 3.10 series.
>> It does not apply cleanly throughout the whole longterm kernels but the
>> changes should not be too difficult to adapt. Do you take care of this
>> or can we do something to ease this process?
>
>I've queued it up for -stable, thanks.
Hi Dave.
When do you plan to push this to stable maintainers?
Thanks!
Jiri
^ permalink raw reply
* Re: [PATCH] bnx2x: avoid atomic allocations during initialization
From: Michal Schmidt @ 2013-09-09 12:20 UTC (permalink / raw)
To: Dmitry Kravkov
Cc: David Miller, netdev@vger.kernel.org, Ariel Elior,
Eilon Greenstein
In-Reply-To: <504C9EFCA2D0054393414C9CB605C37F20D90E3D@SJEXCHMB06.corp.ad.broadcom.com>
On 09/07/2013 03:45 AM, Dmitry Kravkov wrote:
> Once you allocated the memory during initialization , you will most
> probably fail to allocate its replacement during RX handling (on this
> machine).
Why do you think this would happen "most probably"? I would instead
expect the VM subsystem to respond to the memory pressure created by the
initial GFP_KERNEL allocations by freeing some memory to allow the
future atomic allocations to succeed.
Michal
^ permalink raw reply
* Re: TSQ accounting skb->truesize degrades throughput for large packets
From: Eric Dumazet @ 2013-09-09 13:47 UTC (permalink / raw)
To: Jason Wang
Cc: Zoltan Kiss, Wei Liu, Jonathan Davies, Ian Campbell, netdev,
xen-devel, Michael S. Tsirkin
In-Reply-To: <522D9466.6020205@redhat.com>
On Mon, 2013-09-09 at 17:27 +0800, Jason Wang wrote:
> Virtio-net orphan the skb in .ndo_start_xmit() so TSQ can not throttle
> packets in device accurately, and it also can't do BQL. Does this means
> TSQ should be disabled for virtio-net?
>
If skb are orphaned, there is no way TSQ can work at all.
It is already disabled, so why do you want to disable it ?
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix bug in sctp_poll for SOCK_SELECT_ERR_QUEUE
From: Vlad Yasevich @ 2013-09-09 13:56 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp, Jacob Keller
In-Reply-To: <1378565099-20987-1-git-send-email-dborkman@redhat.com>
On 09/07/2013 10:44 AM, Daniel Borkmann wrote:
> If we do not add braces around ...
>
> mask |= POLLERR |
> sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0;
>
> ... then this condition always evaluates to true as POLLERR is
> defined as 8 and binary or'd with whatever result comes out of
> sock_flag(). Hence instead of (X | Y) ? A : B, transform it into
> X | (Y ? A : B). Unfortunatelty, commit 8facd5fb73 ("net: fix
> smatch warnings inside datagram_poll") forgot about SCTP. :-(
>
> Introduced by 7d4c04fc170 ("net: add option to enable error queue
> packets waking select").
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc Jacob Keller <jacob.e.keller@intel.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/socket.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d5d5882..5462bbb 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -6176,7 +6176,7 @@ unsigned int sctp_poll(struct file *file, struct socket *sock, poll_table *wait)
> /* Is there any exceptional events? */
> if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
> mask |= POLLERR |
> - sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0;
> + (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? POLLPRI : 0);
> if (sk->sk_shutdown & RCV_SHUTDOWN)
> mask |= POLLRDHUP | POLLIN | POLLRDNORM;
> if (sk->sk_shutdown == SHUTDOWN_MASK)
>
^ permalink raw reply
* Re: [PATCH net] bnx2x: Fix configuration of doorbell block
From: Eric Dumazet @ 2013-09-09 14:17 UTC (permalink / raw)
To: Ariel Elior; +Cc: David Miller, netdev, Eilon Greenstein
In-Reply-To: <1378727487-6921-1-git-send-email-ariele@broadcom.com>
On Mon, 2013-09-09 at 14:51 +0300, Ariel Elior wrote:
> As part of VF RSS feature doorbell block was configured not to use dpm, but
> a small part of configuration was left out, preventing the driver from sending
> tx messages to the device. This patch adds the missing configuration.
>
> Reported-by: Eric Dumazet <eric.dumazet@gmil.com>
> Signed-off-by: Ariel Elior <ariele@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 1 +
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 3 ---
> 2 files changed, 1 insertions(+), 3 deletions(-)
Thanks for fixing this.
Tested-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next v4 1/6] bonding: simplify and use RCU protection for 3ad xmit path
From: Ding Tianhong @ 2013-09-09 14:53 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Ding Tianhong, Nikolay Aleksandrov, David S. Miller, Netdev
In-Reply-To: <20130909095752.GC2048@redhat.com>
于 2013/9/9 17:57, Veaceslav Falico 写道:
> On Mon, Sep 09, 2013 at 04:58:29PM +0800, Ding Tianhong wrote:
>> On 2013/9/8 14:05, Ding Tianhong wrote:
>>
>> Hi Veaceslav and Nik:
>>
>> please take a moment to reveiw the function just modify for
>> bond_XXX_rcu,
>> and give me some advice. thanks for the help again.:)
>>
>> +#define bond_first_slave_rcu(bond) \
>> + list_first_or_null_rcu(&(bond)->slave_list, struct slave, list);
>> +#define bond_last_slave_rcu(bond) \
>> + ({struct list_head *__slave_list = &(bond)->slave_list; \
>> + struct list_head __rcu *__prev = \
>> + (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
>> + likely(__slave_list != __prev) ? \
>> + container_of(__prev, struct slave, list) : NULL;})
>
> Please take a look at Nikolay's reply to my RCU email -
> http://www.spinics.net/lists/netdev/msg249805.html . And mine also, to
> his
> email. In short - RCU doesn't guarantee ->prev, so better take the
> approach
> of eliminating bond_last/prev_slave completely.
>
yes, I see the message, the list_del_rcu will make the slave->list
->prev = LIST_POISON2,
the bond->slave_list will not be set to the messae, the prev will point
a slave->list or itself,
so I think it will be ok here, please correct me if I miss something.
Best Regards
Ding
>> +
>> #define bond_is_first_slave(bond, pos) ((pos)->list.prev ==
>> &(bond)->slave_list)
>> #define bond_is_last_slave(bond, pos) ((pos)->list.next ==
>> &(bond)->slave_list)
>>
>> @@ -93,6 +117,29 @@
>> (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
>> bond_to_slave((pos)->list.prev))
>>
>> +/* Since bond_first/last_slave_rcu can return NULL, these can return
>> NULL too */
>> +#define bond_next_slave_rcu(bond, pos) \
>> + ({struct list_head *__slave_list = &(bond)->slave_list; \
>> + struct list_head __rcu *__next = list_next_rcu(__slave_list); \
>> + struct list_head *__pos_list = &(pos)->list; \
>> + struct list_head __rcu *__pos_next = list_next_rcu(__pos_list); \
>> + likely(__pos_next != __slave_list) ? \
>> + container_of(__pos_next, struct slave, list) : \
>> + container_of(__next, struct slave, list); \
>> + })
>
> Nice, but can be shortened - we know that pos won't go away.
OK, clean it soon.
>
>> +
>> +#define bond_prev_slave_rcu(bond, pos) \
>> + ({struct list_head *__slave_list = &(bond)->slave_list; \
>> + struct list_head __rcu *__prev = \
>> + (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
>> + struct list_head *__pos_list = &(pos)->list; \
>> + struct list_head __rcu *__pos_prev = (__pos_list->prev
>> !=LIST_POISON2) ? \
yes, the pos->list will be set to LIST_POISON2 by list_del_rcu, so I add
a check for it, But
take the approach of eliminating bond_last/prev_slave completely is a
wise decision, I agree.
>> + (*((struct list_head __rcu **)(&(__pos_list)->prev))) : NULL; \
>> + likely(__pos_prev != __slave_list) ? \
>> + ((__pos_prev) ? list_entry_rcu(__pos_prev, struct slave, list) :
>> NULL;) : \
>> + (list_entry_rcu(__prev, struct slave, list)); \
>> + })
>
> Same remark as above about prev.
>
>> +
>>
>>
>> -#define bond_for_each_slave_from(bond, pos, cnt, start) \
>> - for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
>> - cnt++, pos = bond_next_slave(bond, pos))
>> -
>> +#define bond_for_each_slave_from(bond, pos, start) \
>> + for (pos = start; pos; (pos = bond_next_slave(bond, pos)) != start ? \
>> + (pos) : (pos = NULL))
>> +
>> +#define bond_for_each_slave_from_rcu(bond, pos, start) \
yes, it is a little tedious. I think it could be more easier and shorter.
>> + for ({struct list_head *__start = &(start)->list; \
>> + struct list_head *__slave_list = &(bond)->slave_list; \
>> + pos = list_entry_rcu(__start, struct slave, list);}; \
>> + pos; \
the only way to get out of the loop is that pos is NULL.
>> + {struct list_head __rcu *__next = list_next_rcu(pos->next); \
>> + __next != __slave_list ? \
>> + __next : __next = list_next_rcu(__next->next); \
first, check whether the pos->next is the last one in the slave_list, if
it does, get the
first slave of the bond->slave_list.
>>
>> + __next != __start ? \
>> + pos = list_entry_rcu(__next, struct slave, list) : \
>> + pos = NULL; \
second, check whether the pos is reach the start, if not, continue,
otherwise, the pos
will be set to NULL, so break the loop.
>> + })
>
> Jeez, I don't even want to review it. It's too complex and too hard to
> maintain, even if it works. Can you please make something
> shorter/easier to
> understand?
>
Best Regards.
Ding
>> +
>>
>> Best regards
>> Ding
>>
>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubsc ribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>>
>>> .
>>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: rtnl_lock deadlock on 3.10
From: Steve Wise @ 2013-09-09 16:48 UTC (permalink / raw)
To: Shawn Bohrer, roland-BHEL68pLQRGGvPXPguhicg
Cc: Bart Van Assche, Shawn Bohrer, Or Gerlitz, Cong Wang,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
swise-ut6Up61K2wZBDgjK7y7TUQ
In-Reply-To: <20130906231901.GB10419-/vebjAlq/uFE7V8Yqttd03bhEEblAqRIDbRjUBewulXQT0dZR+AlfA@public.gmane.org>
On 9/6/2013 6:19 PM, Shawn Bohrer wrote:
> On Thu, Sep 05, 2013 at 10:14:51AM -0500, Steve Wise wrote:
>> Roland, what do you think?
>>
>> As I've said, I think we should go ahead with using the rtnl lock in
>> the core. Is there a complete patch available for review? looks
>> like the original was a partial fix.
> I guess I should realize that when no one jumps at fixing my issues
> for me that they probably aren't simple to fix. The solution that
> Cong proposed was to acquire rtnl_lock() before acquiring the
> infiniband device_mutex, and his partial patch did that in
> ib_register_client(). The problem is that you would also need to do
> that in ib_unregister_client(), ib_register_device(), and
> ib_unregister_device(), and that brings us back to the original
> problem which was that cxgb3 was holding the rtnl_lock() when it
> called ib_register_device(). Thus with the proposed fix I believe
> cxgb3 would already be holding the rtnl_lock() and then call
> ib_register_device() which would try to acquire the rtnl_lock() again
> and deadlock for a different reason.
>
> Actually how does this currently work? ib_register_device() calls
> client->add() for each client in the list which should call
> ipoib_add_one() which calls register_netdev(). Shouldn't that also
> deadlock in the cxgb3 case?
cxgb3 is an iWARP device and doesn't support IPoIB.
>
> Also while digging through this I think I see another bug which is
> that ipoib_dev_cleanup() can be called from ipoib_add_port() but in
> the current code ipoib_add_port() is not holding the rtnl_lock() which
> appears to be a requirement of ipoib_dev_cleanup().
>
> Sigh... I'm going to stop looking at this for now and hopefully
> someone can propose a better solution to this issue.
I can help with this, but I'm waiting for Roland to chime in.
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/1] bridge: fix message_age_timer calculation
From: Chris Healy @ 2013-09-09 16:56 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller; +Cc: netdev, bridge, buytenh
This changes the message_age_timer calculation to use the BPDU's max age as opposed to the local bridge's max age. This is in accordance with section 8.6.2.3.2 Step 2 of the 802.1D-1998 sprecification.
With the current implementation, when running with very large bridge diameters, convergance will not always occur even if a root bridge is configured to have a longer max age.
Tested successfully on bridge diameters of ~200.
Signed-off-by: Chris Healy <cphealy@gmail.com>
---
net/bridge/br_stp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 1c0a50f..f1887ba 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -209,7 +209,7 @@ static void br_record_config_information(struct net_bridge_port *p,
p->designated_age = jiffies - bpdu->message_age;
mod_timer(&p->message_age_timer, jiffies
- + (p->br->max_age - bpdu->message_age));
+ + (bpdu->max_age - bpdu->message_age));
}
/* called under bridge lock */
--
1.8.1.2
^ permalink raw reply related
* Re: RFC: crash in fib6_clean_all() while loading ipv6 module
From: David Miller @ 2013-09-09 17:27 UTC (permalink / raw)
To: mkubecek; +Cc: netdev
In-Reply-To: <20130909100515.GA16056@unicorn.suse.cz>
From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 9 Sep 2013 12:05:15 +0200
> This could be prevented by setting a flag when ip6_route_init() is
> complete and not calling fib6_run_gc() from ndisc_netdev_event() until
> the flag is set. However, I don't like the idea of adding a test which
> will be useful only in a short window while loading ipv6 module.
Please just initialize the parts of ipv6 in the correct order necessary
to prevent this problem.
I stronly dislike using flags when it's simply an initialization
ordering problem. It's just like registering a device interrupt
handle before the driver's data structures are properly setup.
^ permalink raw reply
* Re: [PATCH stable] ipv6: restrict neighbor entry creation to output flow
From: David Miller @ 2013-09-09 17:29 UTC (permalink / raw)
To: jiri; +Cc: hannes, mleitner, netdev, dbanerje, yoshfuji
In-Reply-To: <20130909121719.GA3269@minipsycho.brq.redhat.com>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 9 Sep 2013 14:17:19 +0200
> When do you plan to push this to stable maintainers?
Jiri, it's in the -stable queue and that means I will send it at an
appropriate time.
I'm working on a submission at the moment, but every extra query like
your's that I have to answer takes up time I could be spending on it.
^ permalink raw reply
* bnx2x fix for stable
From: Ben Hutchings @ 2013-09-09 17:50 UTC (permalink / raw)
To: David Miller; +Cc: Eilon Greenstein, Dave Jones, netdev
I think you need to either drop the bnx2x non-fix currently in your
stable queue (commit c0a77ec74f29 'bnx2x: Add missing braces in
bnx2x:bnx2x_link_initialize', or add the follow-up (commit 937e5c3d63a1
'bnx2x: Restore a call to config_init') with the net result of fixing
indentation.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH] net: tilegx driver: avoid compiler warning
From: Chris Metcalf @ 2013-09-09 18:11 UTC (permalink / raw)
To: netdev, linux-kernel
The "id" variable was being incremented in common code, but only
initialized and used in IPv4 code. We move the increment to the IPv4
code too, and then legitimately use the uninitialized_var() macro to
avoid the gcc 4.6 warning that 'id' may be used uninitialized.
Note that gcc 4.7 does not warn.
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
drivers/net/ethernet/tile/tilegx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index 949076f..13e6fff 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -1734,7 +1734,8 @@ static void tso_headers_prepare(struct sk_buff *skb, unsigned char *headers,
unsigned int data_len = skb->len - sh_len;
unsigned char *data = skb->data;
unsigned int ih_off, th_off, p_len;
- unsigned int isum_seed, tsum_seed, id, seq;
+ unsigned int isum_seed, tsum_seed, seq;
+ unsigned int uninitialized_var(id);
int is_ipv6;
long f_id = -1; /* id of the current fragment */
long f_size = skb_headlen(skb) - sh_len; /* current fragment size */
@@ -1781,7 +1782,7 @@ static void tso_headers_prepare(struct sk_buff *skb, unsigned char *headers,
} else {
ih = (struct iphdr *)(buf + ih_off);
ih->tot_len = htons(sh_len + p_len - ih_off);
- ih->id = htons(id);
+ ih->id = htons(id++);
ih->check = csum_long(isum_seed + ih->tot_len +
ih->id) ^ 0xffff;
}
@@ -1818,7 +1819,6 @@ static void tso_headers_prepare(struct sk_buff *skb, unsigned char *headers,
slot++;
}
- id++;
seq += p_len;
/* The last segment may be less than gso_size. */
--
1.8.3.1
^ 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