* [PATCH net-next V1 6/9] net/mlx4_core: Fix checking order in MR table init
From: Or Gerlitz @ 2013-11-03 8:03 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, roland, jackm, Or Gerlitz
In-Reply-To: <1383465805-28640-1-git-send-email-ogerlitz@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
In procedure mlx4_init_mr_table(), slaves should do no processing,
but should return success. This initialization is hypervisor-only.
However, the check for num_mpts being a power-of-2 was performed
before the check to return immediately if the driver is for a slave.
This resulted in spurious failures.
The order of performing the checks is reversed, so that if the
driver is for a slave, no processing is done and success is returned.
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/mr.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index f91719a..63391a1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -755,14 +755,14 @@ int mlx4_init_mr_table(struct mlx4_dev *dev)
struct mlx4_mr_table *mr_table = &priv->mr_table;
int err;
- if (!is_power_of_2(dev->caps.num_mpts))
- return -EINVAL;
-
/* Nothing to do for slaves - all MR handling is forwarded
* to the master */
if (mlx4_is_slave(dev))
return 0;
+ if (!is_power_of_2(dev->caps.num_mpts))
+ return -EINVAL;
+
err = mlx4_bitmap_init(&mr_table->mpt_bitmap, dev->caps.num_mpts,
~0, dev->caps.reserved_mrws, 0);
if (err)
--
1.7.1
^ permalink raw reply related
* [PATCH net-next V1 4/9] net/mlx4_core: Resource tracker for reg/unreg vlans
From: Or Gerlitz @ 2013-11-03 8:03 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, roland, jackm, Or Gerlitz
In-Reply-To: <1383465805-28640-1-git-send-email-ogerlitz@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
Add resource tracker support for reg/unreg vlans calls done by VFs.
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
.../net/ethernet/mellanox/mlx4/resource_tracker.c | 127 +++++++++++++++++++-
1 files changed, 121 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 993a2ef..e18bfff 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -55,6 +55,14 @@ struct mac_res {
u8 port;
};
+struct vlan_res {
+ struct list_head list;
+ u16 vlan;
+ int ref_count;
+ int vlan_index;
+ u8 port;
+};
+
struct res_common {
struct list_head list;
struct rb_node node;
@@ -266,6 +274,7 @@ static const char *ResourceType(enum mlx4_resource rt)
case RES_MPT: return "RES_MPT";
case RES_MTT: return "RES_MTT";
case RES_MAC: return "RES_MAC";
+ case RES_VLAN: return "RES_VLAN";
case RES_EQ: return "RES_EQ";
case RES_COUNTER: return "RES_COUNTER";
case RES_FS_RULE: return "RES_FS_RULE";
@@ -274,6 +283,7 @@ static const char *ResourceType(enum mlx4_resource rt)
};
}
+static void rem_slave_vlans(struct mlx4_dev *dev, int slave);
int mlx4_init_resource_tracker(struct mlx4_dev *dev)
{
struct mlx4_priv *priv = mlx4_priv(dev);
@@ -309,11 +319,18 @@ void mlx4_free_resource_tracker(struct mlx4_dev *dev,
int i;
if (priv->mfunc.master.res_tracker.slave_list) {
- if (type != RES_TR_FREE_STRUCTS_ONLY)
- for (i = 0 ; i < dev->num_slaves; i++)
+ if (type != RES_TR_FREE_STRUCTS_ONLY) {
+ for (i = 0; i < dev->num_slaves; i++) {
if (type == RES_TR_FREE_ALL ||
dev->caps.function != i)
mlx4_delete_all_resources_for_slave(dev, i);
+ }
+ /* free master's vlans */
+ i = dev->caps.function;
+ mutex_lock(&priv->mfunc.master.res_tracker.slave_list[i].mutex);
+ rem_slave_vlans(dev, i);
+ mutex_unlock(&priv->mfunc.master.res_tracker.slave_list[i].mutex);
+ }
if (type != RES_TR_FREE_SLAVES_ONLY) {
kfree(priv->mfunc.master.res_tracker.slave_list);
@@ -1469,12 +1486,96 @@ static int mac_alloc_res(struct mlx4_dev *dev, int slave, int op, int cmd,
return err;
}
-static int vlan_alloc_res(struct mlx4_dev *dev, int slave, int op, int cmd,
- u64 in_param, u64 *out_param, int port)
+static int vlan_add_to_slave(struct mlx4_dev *dev, int slave, u16 vlan,
+ int port, int vlan_index)
{
+ struct mlx4_priv *priv = mlx4_priv(dev);
+ struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
+ struct list_head *vlan_list =
+ &tracker->slave_list[slave].res_list[RES_VLAN];
+ struct vlan_res *res, *tmp;
+
+ list_for_each_entry_safe(res, tmp, vlan_list, list) {
+ if (res->vlan == vlan && res->port == (u8) port) {
+ /* vlan found. update ref count */
+ ++res->ref_count;
+ return 0;
+ }
+ }
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return -ENOMEM;
+ res->vlan = vlan;
+ res->port = (u8) port;
+ res->vlan_index = vlan_index;
+ res->ref_count = 1;
+ list_add_tail(&res->list,
+ &tracker->slave_list[slave].res_list[RES_VLAN]);
return 0;
}
+
+static void vlan_del_from_slave(struct mlx4_dev *dev, int slave, u16 vlan,
+ int port)
+{
+ struct mlx4_priv *priv = mlx4_priv(dev);
+ struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
+ struct list_head *vlan_list =
+ &tracker->slave_list[slave].res_list[RES_VLAN];
+ struct vlan_res *res, *tmp;
+
+ list_for_each_entry_safe(res, tmp, vlan_list, list) {
+ if (res->vlan == vlan && res->port == (u8) port) {
+ if (!--res->ref_count) {
+ list_del(&res->list);
+ kfree(res);
+ }
+ break;
+ }
+ }
+}
+
+static void rem_slave_vlans(struct mlx4_dev *dev, int slave)
+{
+ struct mlx4_priv *priv = mlx4_priv(dev);
+ struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
+ struct list_head *vlan_list =
+ &tracker->slave_list[slave].res_list[RES_VLAN];
+ struct vlan_res *res, *tmp;
+ int i;
+
+ list_for_each_entry_safe(res, tmp, vlan_list, list) {
+ list_del(&res->list);
+ /* dereference the vlan the num times the slave referenced it */
+ for (i = 0; i < res->ref_count; i++)
+ __mlx4_unregister_vlan(dev, res->port, res->vlan);
+ kfree(res);
+ }
+}
+
+static int vlan_alloc_res(struct mlx4_dev *dev, int slave, int op, int cmd,
+ u64 in_param, u64 *out_param, int port)
+{
+ int err;
+ u16 vlan;
+ int vlan_index;
+
+ if (!port || op != RES_OP_RESERVE_AND_MAP)
+ return -EINVAL;
+
+ vlan = (u16) in_param;
+
+ err = __mlx4_register_vlan(dev, port, vlan, &vlan_index);
+ if (!err) {
+ set_param_l(out_param, (u32) vlan_index);
+ err = vlan_add_to_slave(dev, slave, vlan, port, vlan_index);
+ if (err)
+ __mlx4_unregister_vlan(dev, port, vlan);
+ }
+ return err;
+}
+
static int counter_alloc_res(struct mlx4_dev *dev, int slave, int op, int cmd,
u64 in_param, u64 *out_param)
{
@@ -1755,7 +1856,21 @@ static int mac_free_res(struct mlx4_dev *dev, int slave, int op, int cmd,
static int vlan_free_res(struct mlx4_dev *dev, int slave, int op, int cmd,
u64 in_param, u64 *out_param, int port)
{
- return 0;
+ int err = 0;
+
+ switch (op) {
+ case RES_OP_RESERVE_AND_MAP:
+ if (!port)
+ return -EINVAL;
+ vlan_del_from_slave(dev, slave, in_param, port);
+ __mlx4_unregister_vlan(dev, port, in_param);
+ break;
+ default:
+ err = -EINVAL;
+ break;
+ }
+
+ return err;
}
static int counter_free_res(struct mlx4_dev *dev, int slave, int op, int cmd,
@@ -3968,7 +4083,7 @@ void mlx4_delete_all_resources_for_slave(struct mlx4_dev *dev, int slave)
struct mlx4_priv *priv = mlx4_priv(dev);
mutex_lock(&priv->mfunc.master.res_tracker.slave_list[slave].mutex);
- /*VLAN*/
+ rem_slave_vlans(dev, slave);
rem_slave_macs(dev, slave);
rem_slave_fs_rule(dev, slave);
rem_slave_qps(dev, slave);
--
1.7.1
^ permalink raw reply related
* [PATCH net] net/mlx4_core: Fix call to __mlx4_unregister_mac
From: Or Gerlitz @ 2013-11-03 8:04 UTC (permalink / raw)
To: davem; +Cc: netdev, amirv, Jack Morgenstein, Or Gerlitz
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
In function mlx4_master_deactivate_admin_state() __mlx4_unregister_mac was
called using the MAC index. It should be called with the value of the MAC itself.
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
Dave, this patch is against commit 6f092343 "net: flow_dissector: fail
on evil iph->ihl", since we want it to go into -stable for kernels >= 3.10,
and per your guideline, I'm submitting it against the net tree and not net-next.
drivers/net/ethernet/mellanox/mlx4/cmd.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index ea20182..bb11624 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -1691,7 +1691,7 @@ static void mlx4_master_deactivate_admin_state(struct mlx4_priv *priv, int slave
vp_oper->vlan_idx = NO_INDX;
}
if (NO_INDX != vp_oper->mac_idx) {
- __mlx4_unregister_mac(&priv->dev, port, vp_oper->mac_idx);
+ __mlx4_unregister_mac(&priv->dev, port, vp_oper->state.mac);
vp_oper->mac_idx = NO_INDX;
}
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-03 12:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383400897.4291.47.camel@edumazet-glaptop2.roam.corp.google.com>
On Sat, Nov 02, 2013 at 07:01:37AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Christoph Paasch and Jerry Chu reported crashes in skb_segment() caused
> by commit 8a29111c7ca6 ("net: gro: allow to build full sized skb")
>
> skb_segment() only deals with a frag_list chain containing MSS sized
> fragments. Even if we fix this problem, its better if GRO layer
> doesn't build skb with a frag_list in the first place, to let TSO
> packets reaching output devices.
>
> David Miller and Ben Hutchings suggested we keep track of number of
> forwarding users to be able to :
>
> - Disable LRO
> - Make sure GRO layer do not use skb frag_list to extend skb capacity
Why are we still doing this instead of fixing skb_segment to
deal with skb frag_list properly?
LRO is legacy code and we should not be adding similar cruft
to GRO.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [virtio-net] BUG: sleeping function called from invalid context at kernel/mutex.c:616
From: Fengguang Wu @ 2013-11-03 14:28 UTC (permalink / raw)
To: Jason Wang, David S. Miller; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <526638D4.1030403@redhat.com>
I noticed that this fix is still not upstreamed. Any chance to push it
to Linus before the 3.12 release?
> >From 01e6c3f71c202aa02e4feda169e7cc9fb24193f5 Mon Sep 17 00:00:00 2001
> From: Jason Wang <jasowang@redhat.com>
> Date: Mon, 21 Oct 2013 20:39:09 +0800
> Subject: [PATCH] virtio-net: fix
>
> ---
> drivers/net/virtio_net.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 9fbdfcd..bbc9cb8 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1118,11 +1118,6 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> {
> struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
>
> - mutex_lock(&vi->config_lock);
> -
> - if (!vi->config_enable)
> - goto done;
> -
> switch(action & ~CPU_TASKS_FROZEN) {
> case CPU_ONLINE:
> case CPU_DOWN_FAILED:
> @@ -1136,8 +1131,6 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> break;
> }
>
> -done:
> - mutex_unlock(&vi->config_lock);
> return NOTIFY_OK;
> }
>
> @@ -1699,6 +1692,8 @@ static int virtnet_freeze(struct virtio_device *vdev)
> struct virtnet_info *vi = vdev->priv;
> int i;
>
> + unregister_hotcpu_notifier(&vi->nb);
> +
> /* Prevent config work handler from accessing the device */
> mutex_lock(&vi->config_lock);
> vi->config_enable = false;
> @@ -1747,6 +1742,10 @@ static int virtnet_restore(struct virtio_device *vdev)
> virtnet_set_queues(vi, vi->curr_queue_pairs);
> rtnl_unlock();
>
> + err = register_hotcpu_notifier(&vi->nb);
> + if (err)
> + return err;
> +
> return 0;
> }
> #endif
> --
> 1.8.1.2
>
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-03 16:28 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131103122824.GA17394@gondor.apana.org.au>
On Sun, 2013-11-03 at 20:28 +0800, Herbert Xu wrote:
> On Sat, Nov 02, 2013 at 07:01:37AM -0700, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Christoph Paasch and Jerry Chu reported crashes in skb_segment() caused
> > by commit 8a29111c7ca6 ("net: gro: allow to build full sized skb")
> >
> > skb_segment() only deals with a frag_list chain containing MSS sized
> > fragments. Even if we fix this problem, its better if GRO layer
> > doesn't build skb with a frag_list in the first place, to let TSO
> > packets reaching output devices.
> >
> > David Miller and Ben Hutchings suggested we keep track of number of
> > forwarding users to be able to :
> >
> > - Disable LRO
> > - Make sure GRO layer do not use skb frag_list to extend skb capacity
>
> Why are we still doing this instead of fixing skb_segment to
> deal with skb frag_list properly?
>
> LRO is legacy code and we should not be adding similar cruft
> to GRO.
1) Because we should not call skb_segment() at all on a router ?
2) If you aggregate too much on a router, you increase latencies,
or if you prefer the RTT of TCP flows.
3) Because skb_segment() layer only builds MSS sized skb, so this
remove TSO ability on output path
Splitting a 45 MSS packet into 3 TSO packets (16 + 16 + X MSS) is going
to be quite complex, given the gso_segment() stuff is meant to segment
in MSS packets. Adding complexity in this already complex stuff is
simply not worth it.
For local TCP, its different, because if you receive such high
throughput, ability to build full size GRO packet helps to reduce number
of ACK segments and number of SKB in receive queue (or OFO queue),
without impacting ACK clocking and TCP dynamics.
And even if a router does not do this aggregation, the final receiver
will do.
So in conclusion, GRO is like TSO : Its not because they are able to use
64KB skbs you always _have_ to fill skb to max capacity.
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-03 16:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383496104.4291.69.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 08:28:24AM -0800, Eric Dumazet wrote:
>
> 1) Because we should not call skb_segment() at all on a router ?
>
> 2) If you aggregate too much on a router, you increase latencies,
> or if you prefer the RTT of TCP flows.
>
> 3) Because skb_segment() layer only builds MSS sized skb, so this
> remove TSO ability on output path
>
> Splitting a 45 MSS packet into 3 TSO packets (16 + 16 + X MSS) is going
> to be quite complex, given the gso_segment() stuff is meant to segment
> in MSS packets. Adding complexity in this already complex stuff is
> simply not worth it.
>
> For local TCP, its different, because if you receive such high
> throughput, ability to build full size GRO packet helps to reduce number
> of ACK segments and number of SKB in receive queue (or OFO queue),
> without impacting ACK clocking and TCP dynamics.
>
> And even if a router does not do this aggregation, the final receiver
> will do.
>
> So in conclusion, GRO is like TSO : Its not because they are able to use
> 64KB skbs you always _have_ to fill skb to max capacity.
I give up. It's as if you've ignored everything I've said before.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4 net-next] net: introduce dev_set_forwarding()
From: Christoph Paasch @ 2013-11-03 17:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, herbert, netdev, hkchu, mwdalton
In-Reply-To: <1383422330.4291.58.camel@edumazet-glaptop2.roam.corp.google.com>
On 02/11/13 - 12:58:50, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Christoph Paasch and Jerry Chu reported crashes in skb_segment() caused
> by commit 8a29111c7ca6 ("net: gro: allow to build full sized skb")
>
> skb_segment() only deals with a frag_list chain containing MSS sized
> fragments. Even if we fix this problem, its better if GRO layer
> doesn't build skb with a frag_list in the first place, to let TSO
> packets reaching output devices.
>
> David Miller and Ben Hutchings suggested we keep track of number of
> forwarding users to be able to :
>
> - Disable LRO
> - Make sure GRO layer do not use skb frag_list to extend skb capacity
>
> Note that after this patch, LRO is automatically re-enabled if
> forwarding is disabled on the device, or if a device is removed
> from a bridge.
>
> Tested:
>
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: on
> lpq84:~# echo 1 >/proc/sys/net/ipv4/conf/eth0/forwarding
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: off [requested on]
> lpq84:~# echo 0 >/proc/sys/net/ipv4/conf/eth0/forwarding
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: on
>
>
> lpq84:~# ethtool -K eth0 lro off
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: off
> lpq84:~# echo 1 >/proc/sys/net/ipv4/conf/eth0/forwarding
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: off
> lpq84:~# echo 0 >/proc/sys/net/ipv4/conf/eth0/forwarding
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: off
> lpq84:~# ethtool -K eth0 lro on
>
>
> lpq84:~# cat /proc/sys/net/ipv4/ip_forward
> 0
> lpq84:~# echo 1 >/proc/sys/net/ipv4/ip_forward
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: off [requested on]
> lpq84:~# echo 0 >/proc/sys/net/ipv4/ip_forward
> lpq84:~# ethtool -k eth0 | grep "large-receive"
> large-receive-offload: on
>
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> Reported-by: Jerry Chu <hkchu@google.com>
> Cc: Michael Dalton <mwdalton@google.com>
> Fixes: 8a29111c7ca6 ("net: gro: allow to build full sized skb")
> ---
> v4: drop LRO in netdev_fix_features(), ase Ben pointed out.
>
> include/linux/netdevice.h | 3 ++-
> net/bridge/br_if.c | 4 +++-
> net/core/dev.c | 31 ++++++++++++++++++++-----------
> net/core/skbuff.c | 11 ++++++++---
> net/ipv4/devinet.c | 14 ++++++++------
> net/ipv6/addrconf.c | 5 ++---
> net/ipv6/addrconf_core.c | 2 ++
> 7 files changed, 45 insertions(+), 25 deletions(-)
Good, this fixes the crash I experienced on my workload.
Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Thanks, Eric!
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Eric Dumazet @ 2013-11-03 17:26 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131103163103.GA18894@gondor.apana.org.au>
On Mon, 2013-11-04 at 00:31 +0800, Herbert Xu wrote:
> I give up. It's as if you've ignored everything I've said before.
Not really.
Have you took a look at the GSO path recently ?
The days it was handling only IP+TCP are gone.
If you think you can do better, please do so.
^ permalink raw reply
* Re: [PATCH v3] can: c_can: Speed up rx_poll function
From: Wolfgang Grandegger @ 2013-11-03 19:20 UTC (permalink / raw)
To: Markus Pargmann, Marc Kleine-Budde
Cc: Joe Perches, linux-can, netdev, linux-kernel, kernel
In-Reply-To: <1383298596-18385-1-git-send-email-mpa@pengutronix.de>
On 11/01/2013 10:36 AM, Markus Pargmann wrote:
> This patch speeds up the rx_poll function by reducing the number of
> register reads.
>
> Replace the 32bit register read by a 16bit register read. Currently
> the 32bit register read is implemented by using 2 16bit reads. This is
> inefficient as we only use the lower 16bit in rx_poll.
>
> The for loop reads the pending interrupts in every iteration. This
> leads up to 16 reads of pending interrupts. The patch introduces a new
> outer loop to read the pending interrupts as long as 'quota' is above 0.
> This reduces the total number of reads.
>
> The third change is to replace the for-loop by a ffs loop.
>
> Tested on AM335x. I removed all 'static' and 'inline' from c_can.c to
> see the timings for all functions. I used the function tracer with
> trace_stats.
>
> 125kbit:
> Function Hit Time Avg s^2
> -------- --- ---- --- ---
> c_can_do_rx_poll 63960 10168178 us 158.977 us 1493056 us
> With patch:
> c_can_do_rx_poll 63941 3764057 us 58.867 us 776162.2 us
>
> 1Mbit:
> Function Hit Time Avg s^2
> -------- --- ---- --- ---
> c_can_do_rx_poll 69489 30049498 us 432.435 us 9271851 us
> With patch:
> c_can_do_rx_poll 207109 24322185 us 117.436 us 171469047 us
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>
> Notes:
> Changes in v3:
> - Update commit message (measurements and ffs)
>
> Changes in v2:
> - Small changes, find_next_bit -> ffs and other
>
> drivers/net/can/c_can/c_can.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index a668cd4..428681e 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -798,17 +798,19 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
> u32 num_rx_pkts = 0;
> unsigned int msg_obj, msg_ctrl_save;
> struct c_can_priv *priv = netdev_priv(dev);
> - u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
> + u16 val;
> +
> + /*
> + * It is faster to read only one 16bit register. This is only possible
> + * for a maximum number of 16 objects.
> + */
> + BUILD_BUG_ON_MSG(C_CAN_MSG_OBJ_RX_LAST > 16,
> + "Implementation does not support more message objects than 16");
> +
> + while (quota > 0 && (val = priv->read_reg(priv, C_CAN_INTPND1_REG))) {
> + while ((msg_obj = ffs(val)) && quota > 0) {
> + val &= ~BIT(msg_obj - 1);
IIRC, we should avoid assignment in if/while statements.
Wolfgang.
^ permalink raw reply
* Re: [PATCH v2] can: add Renesas R-Car CAN driver
From: Wolfgang Grandegger @ 2013-11-03 19:38 UTC (permalink / raw)
To: Sergei Shtylyov, netdev, mkl, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <52742A0F.7040707@cogentembedded.com>
On 11/01/2013 11:24 PM, Sergei Shtylyov wrote:
> On 10/25/2013 11:28 PM, Wolfgang Grandegger wrote:
>
>>> Add support for the CAN controller found in Renesas R-Car SoCs.
>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> [...]
>
>>> Index: linux-can-next/drivers/net/can/rcar_can.c
>>> ===================================================================
>>> --- /dev/null
>>> +++ linux-can-next/drivers/net/can/rcar_can.c
>>> @@ -0,0 +1,920 @@
> [...]
>>> +static bool autorecovery;
>>> +module_param(autorecovery, bool, 0644);
>>> +MODULE_PARM_DESC(autorecovery, "Automatic hardware recovery from
>>> bus-off");
>
>> Software recovery is the preferred solution. No need to support
>> automatic recovery by the hardware.
>
> OK, removed it.
>
>>> +/* Mailbox registers structure */
>>> +struct rcar_can_mbox_regs {
>>> + u32 id; /* IDE and RTR bits, SID and EID */
>>> + u8 stub; /* Not used */
>>> + u8 dlc; /* Data Length Code - bits [0..3] */
>>> + u8 data[8]; /* Data Bytes */
>>> + u8 tsh; /* Time Stamp Higher Byte */
>>> + u8 tsl; /* Time Stamp Lower Byte */
>
>> I would add padding bytes here to ensure alignment.
>
> What padding? This is how the hardware registers are laid out. I
> think I should rather add __packed after }.
OK, I was confused that data does not start on a 4 byte boundary.
>>> + rcar_can_writeb(priv, RCAR_CAN_ECSR, (u8)~ECSR_ADEF);
>
>> Please avoid casts here and below.
>
> These casts help avoid compiler warnings.
Well instead of using casts the declaration of ECSR_ADEF and otheres
should be fixed. I think the problem is that BIT is declared as shown below:
#define BIT(nr) (1UL << (nr))
Using BIT seems not appropriate here.
Wolfgang,
^ permalink raw reply
* Re: [PATCH v3] can: add Renesas R-Car CAN driver
From: Marc Kleine-Budde @ 2013-11-03 20:11 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, wg, linux-can, linux-sh, vksavl
In-Reply-To: <201311020240.13354.sergei.shtylyov@cogentembedded.com>
[-- Attachment #1: Type: text/plain, Size: 35443 bytes --]
On 11/02/2013 12:40 AM, Sergei Shtylyov wrote:
> Add support for the CAN controller found in Renesas R-Car SoCs.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
See comment inline.
>
> ---
> The patch is against the 'linux-can-next.git' repo.
>
> Changes in version 3:
> - replaced the register #define's with 'struct rcar_can_regs' fields, replaced
> rcar_can_{read|write}[bwl]() with mere {read|write}[bwl]();
> - removed hardware bus-off recovery support which allowed to also remove
> rcar_can_start() prototype;
> - added RX/TX error count to error data frame for error warning/passive;
> - moved TX completion interrupt handling into separate function;
> - removed unneeded type cast in the probe() method.
>
> Changes in version 2:
> - added function to clean up TX mailboxes after bus error and bus-off;
> - added module parameter to enable hardware recovery from bus-off, added handler
> for the bus-off recovery interrupt, and set the control register according to
> the parameter value and the restart timer setting in rcar_can_start();
> - changed the way CAN_ERR_CRTL_[RT]X_{PASSIVE|WARNING} flags are set to a more
> realicstic one;
> - replaced MBX_* macros and rcar_can_mbx_{read|write}[bl]() functions with
> 'struct rcar_can_mbox_regs', 'struct rcar_can_regs', and {read|write}[bl](),
> replaced 'reg_base' field of 'struct rcar_can_priv' with 'struct rcar_can_regs
> __iomem *regs';
> - added 'ier' field to 'struct rcar_can_priv' to cache the current value of the
> interrupt enable register;
> - added a check for enabled interrupts on entry to rcar_can_interrupt();
> - limited transmit mailbox search loop in rcar_can_interrupt();
> - decoupled TX byte count increment from can_get_echo_skb() call;
> - removed netif_queue_stopped() call from rcar_can_interrupt();
> - added clk_prepare_enable()/clk_disable_unprepare() to ndo_{open|close}(),
> do_set_bittiming(), and do_get_berr_counter() methods, removed clk_enable()
> call from the probe() method and clk_disable() call from the remove() method;
> - allowed rcar_can_set_bittiming() to be called when the device is closed and
> remove explicit call to it from rcar_can_start();
> - switched to using mailbox number priority transmit mode, and switched to the
> sequential mailbox use in ndo_start_xmit() method;
> - stopped reading the message control registers in ndo_start_xmit() method;
> - avoided returning NETDEV_TX_BUSY from ndo_start_xmit() method;
> - stopped reading data when RTR bit is set in the CAN frame;
> - made 'num_pkts' variable *int* and moved its check to the *while* condition in
> rcar_can_rx_poll();
> - used dev_get_platdata() in the probe() method;
> - enabled bus error interrupt only if CAN_CTRLMODE_BERR_REPORTING flag is set;
> - started reporting CAN_CTRLMODE_BERR_REPORTING support and stopped reporting
> CAN_CTRLMODE_3_SAMPLES support;
> - set CAN_ERR_ACK flag on ACK error;
> - switched to incrementing bus error counter only once per bus error interrupt;
> - started switching to CAN sleep mode in rcar_can_stop() and stopped switching
> to it in the remove() method;
> - removed netdev_err() calls on allocation failure in rcar_can_error() and
> rcar_can_rx_pkt();
> - removed "CANi" from the register offset macro comments.
>
> drivers/net/can/Kconfig | 9
> drivers/net/can/Makefile | 1
> drivers/net/can/rcar_can.c | 893 ++++++++++++++++++++++++++++++++++
> include/linux/can/platform/rcar_can.h | 15
> 4 files changed, 918 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,893 @@
> +/*
> + * 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"
> +
> +/* 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 N_TX_MB (N_MBX - FIRST_TX_MB)
> +#define RX_MBX_MASK 0xFFFFFFFE
Please use a common prefix for all defines.
> +
> +/* Mailbox registers structure */
> +struct rcar_can_mbox_regs {
> + u32 id; /* IDE and RTR bits, SID and EID */
> + u8 stub; /* Not used */
> + u8 dlc; /* Data Length Code - bits [0..3] */
> + u8 data[8]; /* Data Bytes */
> + u8 tsh; /* Time Stamp Higher Byte */
> + u8 tsl; /* Time Stamp Lower Byte */
> +} __packed;
> +
> +struct rcar_can_regs {
> + struct rcar_can_mbox_regs mb[N_MBX]; /* Mailbox registers */
> + u32 mkr_2_9[8]; /* Mask Registers 2-9 */
> + u32 fidcr[2]; /* FIFO Received ID Compare Register */
> + u32 mkivlr1; /* Mask Invalid Register 1 */
> + u32 mier1; /* Mailbox Interrupt Enable Register 1 */
> + u32 mkr_0_1[2]; /* Mask Registers 0-1 */
> + u32 mkivlr0; /* Mask Invalid Register 0*/
> + u32 mier0; /* Mailbox Interrupt Enable Register 0 */
> + u8 pad_440[0x3c0];
> + u8 mctl[64]; /* Message Control Registers */
> + u16 ctlr; /* Control Register */
> + u16 str; /* Status register */
> + u8 bcr[3]; /* Bit Configuration Register */
> + u8 clkr; /* Clock Select Register */
> + u8 rfcr; /* Receive FIFO Control Register */
> + u8 rfpcr; /* Receive FIFO Pointer Control Register */
> + u8 tfcr; /* Transmit FIFO Control Register */
> + u8 tfpcr; /* Transmit FIFO Pointer Control Register */
> + u8 eier; /* Error Interrupt Enable Register */
> + u8 eifr; /* Error Interrupt Factor Judge Register */
> + u8 recr; /* Receive Error Count Register */
> + u8 tecr; /* Transmit Error Count Register */
> + u8 ecsr; /* Error Code Store Register */
> + u8 cssr; /* Channel Search Support Register */
> + u8 mssr; /* Mailbox Search Status Register */
> + u8 msmr; /* Mailbox Search Mode Register */
> + u16 tsr; /* Time Stamp Register */
> + u8 afsr; /* Acceptance Filter Support Register */
> + u8 pad_857;
> + u8 tcr; /* Test Control Register */
> + u8 pad_859[7];
> + u8 ier; /* Interrupt Enable Register */
> + u8 isr; /* Interrupt Status Register */
> + u8 pad_862;
> + u8 mbsmr; /* Mailbox Search Mask Register */
> +} __packed;
> +
> +struct rcar_can_priv {
> + struct can_priv can; /* Must be the first member! */
> + struct net_device *ndev;
> + struct napi_struct napi;
> + struct rcar_can_regs __iomem *regs;
> + struct clk *clk;
> + spinlock_t mier_lock;
> + u8 clock_select;
> + u8 ier;
> +};
> +
> +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,
> +};
> +
> +/* Control Register bits */
> +#define CTLR_BOM (3 << 11) /* Bus-Off Recovery Mode Bits */
> +#define CTLR_BOM_ENT BIT(11) /* Entry to halt mode at bus-off entry */
> +#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 */
> +
> +#define RCAR_CAN_NAPI_WEIGHT (FIRST_TX_MB - 1)
> +
> +static void tx_failure_cleanup(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + u32 mier1;
> + u8 mbx;
> +
> + spin_lock(&priv->mier_lock);
> + mier1 = readl(&priv->regs->mier1);
> + for (mbx = FIRST_TX_MB; mbx < N_MBX; mbx++) {
> + if (mier1 & BIT(mbx - FIRST_TX_MB)) {
> + writeb(0, &priv->regs->mctl[mbx]);
> + can_free_echo_skb(ndev, mbx - FIRST_TX_MB);
> + }
> + }
> + writel(0, &priv->regs->mier1);
> + spin_unlock(&priv->mier_lock);
> +}
> +
> +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, txerr = 0, rxerr = 0;
> +
> + /* Propagate the error condition to the CAN stack */
> + skb = alloc_can_err_skb(ndev, &cf);
> + if (!skb)
> + return;
> +
> + eifr = readb(&priv->regs->eifr);
> + if (eifr & (EIFR_EWIF | EIFR_EPIF)) {
> + cf->can_id |= CAN_ERR_CRTL;
> + txerr = readb(&priv->regs->tecr);
> + rxerr = readb(&priv->regs->recr);
> + cf->data[6] = txerr;
> + cf->data[7] = rxerr;
> + }
> + if (eifr & EIFR_BEIF) {
> + int rx_errors = 0, tx_errors = 0;
> + u8 ecsr;
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> + tx_failure_cleanup(ndev);
> + 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 = readb(&priv->regs->ecsr);
> + if (ecsr & ECSR_ADEF) {
> + netdev_dbg(priv->ndev, "ACK Delimiter Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK_DEL;
> + tx_errors++;
> + writeb((u8)~ECSR_ADEF, &priv->regs->ecsr);
> + }
> + if (ecsr & ECSR_BE0F) {
> + netdev_dbg(priv->ndev, "Bit Error (dominant)\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + tx_errors++;
> + writeb((u8)~ECSR_BE0F, &priv->regs->ecsr);
> + }
> + if (ecsr & ECSR_BE1F) {
> + netdev_dbg(priv->ndev, "Bit Error (recessive)\n");
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + tx_errors++;
> + writeb((u8)~ECSR_BE1F, &priv->regs->ecsr);
> + }
> + if (ecsr & ECSR_CEF) {
> + netdev_dbg(priv->ndev, "CRC Error\n");
> + cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
> + rx_errors++;
> + writeb((u8)~ECSR_CEF, &priv->regs->ecsr);
> + }
> + if (ecsr & ECSR_AEF) {
> + netdev_dbg(priv->ndev, "ACK Error\n");
> + cf->can_id |= CAN_ERR_ACK;
> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> + tx_errors++;
> + writeb((u8)~ECSR_AEF, &priv->regs->ecsr);
> + }
> + if (ecsr & ECSR_FEF) {
> + netdev_dbg(priv->ndev, "Form Error\n");
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + rx_errors++;
> + writeb((u8)~ECSR_FEF, &priv->regs->ecsr);
> + }
> + if (ecsr & ECSR_SEF) {
> + netdev_dbg(priv->ndev, "Stuff Error\n");
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + rx_errors++;
> + writeb((u8)~ECSR_SEF, &priv->regs->ecsr);
> + }
> +
> + priv->can.can_stats.bus_error++;
> + ndev->stats.rx_errors += rx_errors;
> + ndev->stats.tx_errors += tx_errors;
> + writeb((u8)~EIFR_BEIF, &priv->regs->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->data[1] |= txerr > rxerr ? CAN_ERR_CRTL_TX_WARNING :
> + CAN_ERR_CRTL_RX_WARNING;
> + /* Clear interrupt condition */
> + writeb((u8)~EIFR_EWIF, &priv->regs->eifr);
> + }
> + 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->data[1] |= txerr > rxerr ? CAN_ERR_CRTL_TX_PASSIVE :
> + CAN_ERR_CRTL_RX_PASSIVE;
> + /* Clear interrupt condition */
> + writeb((u8)~EIFR_EPIF, &priv->regs->eifr);
> + }
> + if (eifr & EIFR_BOEIF) {
> + netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
> + tx_failure_cleanup(ndev);
> + priv->ier = IER_ERSIE;
> + writeb(priv->ier, &priv->regs->ier);
> + priv->can.state = CAN_STATE_BUS_OFF;
> + cf->can_id |= CAN_ERR_BUSOFF;
> + /* Clear interrupt condition */
> + writeb((u8)~EIFR_BOEIF, &priv->regs->eifr);
> + can_bus_off(ndev);
> + }
> + 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++;
> + writeb((u8)~EIFR_ORIF, &priv->regs->eifr);
> + }
> + 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++;
> + writeb((u8)~EIFR_OLIF, &priv->regs->eifr);
> + }
> +
> + netif_rx(skb);
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void rcar_can_tx_done(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + struct net_device_stats *stats = &ndev->stats;
> + u32 ie_mask = 0;
> + int i;
> +
> + /* Set Transmit Mailbox Search Mode */
> + writeb(MSMR_TXMB, &priv->regs->msmr);
> + for (i = 0; i < N_TX_MB; i++) {
> + u8 mctl, mbx;
> +
> + mbx = readb(&priv->regs->mssr);
> + if (mbx & MSSR_SEST)
> + break;
> + mbx &= MSSR_MBNST;
> + stats->tx_bytes += readb(&priv->regs->mb[mbx].dlc);
> + stats->tx_packets++;
> + mctl = readb(&priv->regs->mctl[mbx]);
> + /* Bits SENTDATA and TRMREQ cannot be
> + * set to 0 simultaneously
> + */
> + mctl &= ~MCTL_TRMREQ;
> + writeb(mctl, &priv->regs->mctl[mbx]);
> + mctl &= ~MCTL_SENTDATA;
> + /* Clear interrupt */
> + writeb(mctl, &priv->regs->mctl[mbx]);
> + ie_mask |= BIT(mbx - FIRST_TX_MB);
> + can_get_echo_skb(ndev, mbx - FIRST_TX_MB);
> + can_led_event(ndev, CAN_LED_EVENT_TX);
> + }
> + /* Set receive mailbox search mode */
> + writeb(MSMR_RXMB, &priv->regs->msmr);
> + /* Disable mailbox interrupt, mark mailbox as free */
> + if (ie_mask) {
> + u32 mier1;
> +
> + spin_lock(&priv->mier_lock);
> + mier1 = readl(&priv->regs->mier1);
> + writel(mier1 & ~ie_mask, &priv->regs->mier1);
> + spin_unlock(&priv->mier_lock);
> + netif_wake_queue(ndev);
> + }
> +}
> +
> +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);
> + u8 isr;
> +
> + isr = readb(&priv->regs->isr);
> + if (!(isr & priv->ier))
> + return IRQ_NONE;
> +
> + if (isr & ISR_ERSF)
> + rcar_can_error(ndev);
> +
> + if (isr & ISR_TXMF)
> + rcar_can_tx_done(ndev);
> +
> + if (isr & ISR_RXM1F) {
> + if (napi_schedule_prep(&priv->napi)) {
> + /* Disable Rx interrupts */
> + priv->ier &= ~IER_RXM1IE;
> + writeb(priv->ier, &priv->regs->ier);
> + __napi_schedule(&priv->napi);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +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;
> +
> + clk_prepare_enable(priv->clk);
> + /* rcar_can_set_bittiming() is called in CAN sleep mode.
> + * Can write to BCR in CAN reset mode or CAN halt mode.
> + * Cannot write to CLKR in halt mode, so go to reset mode.
> + */
> + ctlr = readw(&priv->regs->ctlr);
> + ctlr &= ~CTLR_SLPM;
> + ctlr |= CTLR_FORCE_RESET;
> + writew(ctlr, &priv->regs->ctlr);
> + /* Don't overwrite CLKR with 32-bit BCR access */
> + /* CLKR has 8-bit access */
> + clkr = readb(&priv->regs->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);
> + writel(bcr, &priv->regs->bcr);
> + writeb(clkr, &priv->regs->clkr);
> + ctlr |= CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + clk_disable_unprepare(priv->clk);
> + 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);
> + * - accept all messages (no filter).
> + * CAN is in sleep mode after MCU hardware or software reset.
> + */
> + ctlr = readw(&priv->regs->ctlr);
> + ctlr &= ~CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + /* Go to reset mode */
> + ctlr |= CTLR_FORCE_RESET;
> + writew(ctlr, &priv->regs->ctlr);
> + ctlr |= CTLR_IDFM_MIXED; /* Select mixed ID mode */
> + ctlr |= CTLR_TPM; /* Set mailbox number priority transmit mode */
> + ctlr |= CTLR_BOM_ENT; /* Entry to halt mode automatically */
> + /* at bus-off */
> + writew(ctlr, &priv->regs->ctlr);
> +
> + writeb(priv->clock_select, &priv->regs->clkr);
> +
> + /* Accept all SID and EID */
> + for (n = 0; n < N_RX_MKREGS; n++)
> + writel(0, &priv->regs->mkr_0_1[n]);
> + writel(0, &priv->regs->mkivlr0);
> +
> + /* Initial value of MIER1 undefined. Mark all Tx mailboxes as free. */
> + writel(0, &priv->regs->mier1);
> +
> + priv->ier = IER_TXMIE | IER_ERSIE | IER_RXM1IE;
> + writeb(priv->ier, &priv->regs->ier);
> +
> + /* Accumulate error codes */
> + writeb(ECSR_EDPM, &priv->regs->ecsr);
> + /* Enable error interrupts */
> + writeb(EIER_EWIE | EIER_EPIE | EIER_BOEIE |
> + (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING ?
> + EIER_BEIE : 0) | EIER_ORIE | EIER_OLIE,
> + &priv->regs->eier);
> + /* Enable interrupts for RX mailboxes */
> + writel(RX_MBX_MASK, &priv->regs->mier0);
> + 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;
> + writew(ctlr, &priv->regs->ctlr);
> + for (n = 0; n < FIRST_TX_MB; n++) {
> + /* According to documentation we should clear MCTL
> + * register before configuring mailbox.
> + */
> + writeb(0, &priv->regs->mctl[n]);
> + writeb(MCTL_RECREQ, &priv->regs->mctl[n]);
> + writeb(0, &priv->regs->mctl[FIRST_TX_MB + n]);
> + }
> + /* Go to operation mode */
> + writew(ctlr & ~CTLR_FORCE_RESET, &priv->regs->ctlr);
> +}
> +
> +static int rcar_can_open(struct net_device *ndev)
> +{
> + struct rcar_can_priv *priv = netdev_priv(ndev);
> + int err;
> +
> + clk_prepare_enable(priv->clk);
> + 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);
> + clk_disable_unprepare(priv->clk);
> +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 = readw(&priv->regs->ctlr);
> + ctlr |= CTLR_FORCE_RESET;
> + writew(ctlr, &priv->regs->ctlr);
> + writel(0, &priv->regs->mier0);
> + writel(0, &priv->regs->mier1);
> + writeb(0, &priv->regs->ier);
> + writeb(0, &priv->regs->eier);
> + /* Go to sleep mode */
> + ctlr |= CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + 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);
> + clk_disable_unprepare(priv->clk);
> + 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 = 0;
> +
> + if (can_dropped_invalid_skb(ndev, skb))
> + return NETDEV_TX_OK;
> +
> + spin_lock_irqsave(&priv->mier_lock, flags);
> + mier1 = readl(&priv->regs->mier1);
> + if (mier1) {
> + i = __builtin_clz(mier1);
> + mbxno = i ? N_MBX - i : FIRST_TX_MB;
> + } else {
> + mbxno = FIRST_TX_MB;
> + }
Can you explain how the hardware arbitration works, and you do you
guarantee the CAN frames are send by the hardware in the same order you
put them into the hardware.
> + mier1 |= BIT(mbxno - FIRST_TX_MB);
> + writel(mier1, &priv->regs->mier1);
> + spin_unlock_irqrestore(&priv->mier_lock, flags);
> + if (unlikely(mier1 == 0xffffffff))
> + netif_stop_queue(ndev);
> +
> + 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;
> + }
> + writel(data, &priv->regs->mb[mbxno].id);
> +
> + writeb(cf->can_dlc, &priv->regs->mb[mbxno].dlc);
> +
> + for (i = 0; i < cf->can_dlc; i++)
> + writeb(cf->data[i], &priv->regs->mb[mbxno].data[i]);
> +
> + can_put_echo_skb(skb, ndev, mbxno - FIRST_TX_MB);
> +
> + priv->ier |= IER_TXMIE;
> + writeb(priv->ier, &priv->regs->ier);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
> + mctl |= MCTL_ONESHOT;
> +
> + /* Start TX */
> + mctl |= MCTL_TRMREQ;
> + writeb(mctl, &priv->regs->mctl[mbxno]);
> + 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++;
> + return;
> + }
> +
> + data = readl(&priv->regs->mb[mbx].id);
> + 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;
> +
> + dlc = readb(&priv->regs->mb[mbx].dlc);
> + cf->can_dlc = get_can_dlc(dlc);
> + if (data & RCAR_CAN_RTR) {
> + cf->can_id |= CAN_RTR_FLAG;
> + } else {
> + for (dlc = 0; dlc < cf->can_dlc; dlc++)
> + cf->data[dlc] = readb(&priv->regs->mb[mbx].data[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);
> + int num_pkts = 0;
> +
> + /* Find mailbox */
> + while (num_pkts < quota) {
> + u8 mctl, mbx;
> +
> + mbx = readb(&priv->regs->mssr);
How does the RX work? Is it a hardware FIFO?
> + if (mbx & MSSR_SEST)
> + break;
> + mbx &= MSSR_MBNST;
> + mctl = readb(&priv->regs->mctl[mbx]);
> + /* Clear interrupt */
> + writeb(mctl & ~MCTL_NEWDATA, &priv->regs->mctl[mbx]);
> + rcar_can_rx_pkt(priv, mbx);
> + ++num_pkts;
> + }
> + /* All packets processed */
> + if (num_pkts < quota) {
> + napi_complete(napi);
> + priv->ier |= IER_RXM1IE;
> + writeb(priv->ier, &priv->regs->ier);
> + }
> + 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);
> +
> + clk_prepare_enable(priv->clk);
> + bec->txerr = readb(&priv->regs->tecr);
> + bec->rxerr = readb(&priv->regs->recr);
> + clk_disable_unprepare(priv->clk);
> + 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 = dev_get_platdata(&pdev->dev);
> + 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;
> + }
> +
> + ndev->netdev_ops = &rcar_can_netdev_ops;
> + ndev->irq = irq;
> + ndev->flags |= IFF_ECHO;
> + priv->ndev = ndev;
> + priv->regs = 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;
Please call this function directly during the open() function.
> + 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_BERR_REPORTING |
> + CAN_CTRLMODE_ONE_SHOT;
> + 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->regs, 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);
> +
> + unregister_candev(ndev);
> + netif_napi_del(&priv->napi);
> + 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 = readw(&priv->regs->ctlr);
> + ctlr |= CTLR_HALT;
> + writew(ctlr, &priv->regs->ctlr);
> + ctlr |= CTLR_SLPM;
> + writew(ctlr, &priv->regs->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 = readw(&priv->regs->ctlr);
> + ctlr &= ~CTLR_SLPM;
> + writew(ctlr, &priv->regs->ctlr);
> + ctlr &= ~CTLR_FORCE_RESET;
> + writew(ctlr, &priv->regs->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) */
> +
> +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
>
regards,
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 v3] can: c_can: Speed up rx_poll function
From: Marc Kleine-Budde @ 2013-11-03 21:05 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Markus Pargmann, Joe Perches, linux-can, netdev, linux-kernel,
kernel
In-Reply-To: <5276A1FE.7040804@grandegger.com>
[-- Attachment #1: Type: text/plain, Size: 3397 bytes --]
On 11/03/2013 08:20 PM, Wolfgang Grandegger wrote:
> On 11/01/2013 10:36 AM, Markus Pargmann wrote:
>> This patch speeds up the rx_poll function by reducing the number of
>> register reads.
>>
>> Replace the 32bit register read by a 16bit register read. Currently
>> the 32bit register read is implemented by using 2 16bit reads. This is
>> inefficient as we only use the lower 16bit in rx_poll.
>>
>> The for loop reads the pending interrupts in every iteration. This
>> leads up to 16 reads of pending interrupts. The patch introduces a new
>> outer loop to read the pending interrupts as long as 'quota' is above 0.
>> This reduces the total number of reads.
>>
>> The third change is to replace the for-loop by a ffs loop.
>>
>> Tested on AM335x. I removed all 'static' and 'inline' from c_can.c to
>> see the timings for all functions. I used the function tracer with
>> trace_stats.
>>
>> 125kbit:
>> Function Hit Time Avg s^2
>> -------- --- ---- --- ---
>> c_can_do_rx_poll 63960 10168178 us 158.977 us 1493056 us
>> With patch:
>> c_can_do_rx_poll 63941 3764057 us 58.867 us 776162.2 us
>>
>> 1Mbit:
>> Function Hit Time Avg s^2
>> -------- --- ---- --- ---
>> c_can_do_rx_poll 69489 30049498 us 432.435 us 9271851 us
>> With patch:
>> c_can_do_rx_poll 207109 24322185 us 117.436 us 171469047 us
>>
>> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
>> ---
>>
>> Notes:
>> Changes in v3:
>> - Update commit message (measurements and ffs)
>>
>> Changes in v2:
>> - Small changes, find_next_bit -> ffs and other
>>
>> drivers/net/can/c_can/c_can.c | 22 ++++++++++++----------
>> 1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
>> index a668cd4..428681e 100644
>> --- a/drivers/net/can/c_can/c_can.c
>> +++ b/drivers/net/can/c_can/c_can.c
>> @@ -798,17 +798,19 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
>> u32 num_rx_pkts = 0;
>> unsigned int msg_obj, msg_ctrl_save;
>> struct c_can_priv *priv = netdev_priv(dev);
>> - u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
>> + u16 val;
>> +
>> + /*
>> + * It is faster to read only one 16bit register. This is only possible
>> + * for a maximum number of 16 objects.
>> + */
>> + BUILD_BUG_ON_MSG(C_CAN_MSG_OBJ_RX_LAST > 16,
>> + "Implementation does not support more message objects than 16");
>> +
>> + while (quota > 0 && (val = priv->read_reg(priv, C_CAN_INTPND1_REG))) {
>> + while ((msg_obj = ffs(val)) && quota > 0) {
>> + val &= ~BIT(msg_obj - 1);
>
> IIRC, we should avoid assignment in if/while statements.
Yes, but the code looks IMHO better this way.
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: [GIT PULL] Second Round of IPVS updates for v3.13
From: Pablo Neira Ayuso @ 2013-11-03 21:35 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Daniel Borkmann
In-Reply-To: <1383095486-5215-1-git-send-email-horms@verge.net.au>
On Wed, Oct 30, 2013 at 10:11:24AM +0900, Simon Horman wrote:
[...]
> The following changes since commit 6b8dbcf2c44fd7aa716560d04e9857c870bd510c:
>
> bridge: netfilter: orphan skb before invoking ip netfilter hooks (2013-10-27 21:44:33 +0100)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git tags/ipvs2-for-v3.13
Pulled, thanks Simon.
^ permalink raw reply
* Re: [PATCH net] net: flow_dissector: fail on evil iph->ihl
From: Daniel Borkmann @ 2013-11-03 22:18 UTC (permalink / raw)
To: Jason Wang
Cc: davem, edumazet, netdev, linux-kernel, Petr Matousek,
Michael S. Tsirkin
In-Reply-To: <1383289270-18952-1-git-send-email-jasowang@redhat.com>
On 11/01/2013 08:01 AM, Jason Wang wrote:
> We don't validate iph->ihl which may lead a dead loop if we meet a IPIP
> skb whose iph->ihl is zero. Fix this by failing immediately when iph->ihl
> is evil (less than 5).
>
> This issue were introduced by commit ec5efe7946280d1e84603389a1030ccec0a767ae
> (rps: support IPIP encapsulation).
>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Petr Matousek <pmatouse@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Sorry, a bit late as I was offline last 4 days, but fwiw:
Acked-by: Daniel Borkmann <dborkman@redhat.com>
^ permalink raw reply
* Re: [patch -next] mlx5_core: warn if no space left in alloc_4k()
From: Dan Carpenter @ 2013-11-03 23:16 UTC (permalink / raw)
To: Eli Cohen; +Cc: Eli Cohen, netdev, linux-rdma, kernel-janitors
In-Reply-To: <20131103070326.GA3051@mtldesk30>
On Sun, Nov 03, 2013 at 09:03:27AM +0200, Eli Cohen wrote:
> On Fri, Nov 01, 2013 at 01:20:44PM +0300, Dan Carpenter wrote:
> > The warning was unreachable. In the original code, it would print the
> > line number and the function but I have added an error message.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > I haven't tested this, hopefully the warning is not annoying.
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> > index ba816c2..cb86265 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> > @@ -193,8 +193,8 @@ static int alloc_4k(struct mlx5_core_dev *dev, u64 *addr)
> > unsigned n;
> >
> > if (list_empty(&dev->priv.free_list)) {
> > + mlx5_core_warn(dev, "no available space\n");
> > return -ENOMEM;
> > - mlx5_core_warn(dev, "\n");
> > }
>
> What we really need to do here is silently return -ENOMEM. The list
> can be found empty on a regular basis so we don't want to flood dmesg
> with annoying messages. The statement was probably left there from a
> previous debug cycle.
Ah, ok. Thanks for the review. I will remove the message.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH net-next v2] ipv6: remove old conditions on flow label sharing
From: Hannes Frederic Sowa @ 2013-11-03 23:17 UTC (permalink / raw)
To: Florent Fourcot; +Cc: netdev
In-Reply-To: <1383400507-12604-1-git-send-email-florent.fourcot@enst-bretagne.fr>
On Sat, Nov 02, 2013 at 02:55:07PM +0100, Florent Fourcot wrote:
> The code of flow label in Linux Kernel follows
> the rules of RFC 1809 (an informational one) for
> conditions on flow label sharing. There rules are
> not in the last proposed standard for flow label
> (RFC 6437), or in the previous one (RFC 3697).
>
> Since this code does not follow any current or
> old standard, we can remove it.
>
> With this removal, the ipv6_opt_cmp function is
> now a dead code and it can be removed too.
>
> Changelog to v1:
> * add justification for the change
> * remove the condition on IPv6 options
>
> Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
Yup, makes sense.
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH v2 net-next] net: introduce gro_frag_list_enable sysctl
From: David Miller @ 2013-11-03 23:19 UTC (permalink / raw)
To: sysoleg; +Cc: hkchu, herbert, eric.dumazet, christoph.paasch, netdev, mwdalton
In-Reply-To: <44571383414236@web13j.yandex.ru>
From: Oleg A. Arkhangelsky <sysoleg@yandex.ru>
Date: Sat, 02 Nov 2013 21:43:56 +0400
> 30.10.2013, 06:33, "David Miller" <davem@davemloft.net>:
>
>> GRO should always win, even on a router, because it decreases the
>> number of fundamental operations (routing lookups) that the stack
>> needs to perform.
>
> Yes, unless the case when you're using Linux as IP router which is
> forwarding 500K mixed IP (TCP and UDP) flows traffic @ 10-20 Gbit/s.
> Then GRO is unnecessarily overhead, cause there's no possibility to
> accumulate adequate GRO list in such scenario.
If it's not accumulating, then there really isn't much cost because
the bulk of the code paths are short circuited when the flow IDs do
not match.
We touch these packet headers to forward anyways, and that is the bulk
of the cost.
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: David Miller @ 2013-11-03 23:23 UTC (permalink / raw)
To: eric.dumazet
Cc: herbert, bhutchings, christoph.paasch, netdev, hkchu, mwdalton
In-Reply-To: <1383496104.4291.69.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 03 Nov 2013 08:28:24 -0800
> 1) Because we should not call skb_segment() at all on a router ?
Eric, disagreement with this very thing is the basis for the entire
thread you have engaged with Herbert Xu about. It's as if you've
ignored everything discussed there, please don't do this.
^ permalink raw reply
* Re: [PATCH RFC] ipv6: enable IPV6_FLOWLABEL_MGR for getsockopt
From: Hannes Frederic Sowa @ 2013-11-04 0:04 UTC (permalink / raw)
To: Florent Fourcot; +Cc: netdev
In-Reply-To: <1383411524-16743-1-git-send-email-florent.fourcot@enst-bretagne.fr>
On Sat, Nov 02, 2013 at 05:58:44PM +0100, Florent Fourcot wrote:
> It is already possible to set/put/renew a label
> with IPV6_FLOWLABEL_MGR and setsockopt. This patch
> add the possibility to get information about this
> label (current value, time before expiration, etc).
>
> It helps application to take decision for a renew
> or a release of the label.
>
> Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
> ---
> include/net/ipv6.h | 1 +
> net/ipv6/ip6_flowlabel.c | 23 +++++++++++++++++++++++
> net/ipv6/ipv6_sockglue.c | 23 +++++++++++++++++++++++
> 3 files changed, 47 insertions(+)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index dd96638..2a5f668 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -250,6 +250,7 @@ struct ipv6_txoptions *fl6_merge_options(struct ipv6_txoptions *opt_space,
> struct ipv6_txoptions *fopt);
> void fl6_free_socklist(struct sock *sk);
> int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen);
> +int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq);
> int ip6_flowlabel_init(void);
> void ip6_flowlabel_cleanup(void);
>
> diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
> index 819578e..34f58b7 100644
> --- a/net/ipv6/ip6_flowlabel.c
> +++ b/net/ipv6/ip6_flowlabel.c
> @@ -475,6 +475,29 @@ static inline void fl_link(struct ipv6_pinfo *np, struct ipv6_fl_socklist *sfl,
> spin_unlock_bh(&ip6_sk_fl_lock);
> }
>
> +int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq)
> +{
> + struct ipv6_pinfo *np = inet6_sk(sk);
> + struct ipv6_fl_socklist *sfl;
> +
> + rcu_read_lock_bh();
> + for_each_sk_fl_rcu(np, sfl) {
> + if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) {
We should take ip6_fl_lock here, otherwise expires extraction races with
the garbage collector (which can update it). There seem to be some other
unsafe places, e.g. fl6_renew.
> + freq->flr_label = sfl->fl->label;
> + freq->flr_dst = sfl->fl->dst;
> + freq->flr_share = sfl->fl->share;
> + freq->flr_expires = (sfl->fl->expires - jiffies) / HZ;
> + freq->flr_linger = sfl->fl->linger / HZ;
> +
> + rcu_read_unlock_bh();
> + return 0;
> + }
> + }
> + rcu_read_unlock_bh();
> +
> + return 0;
Maybe return -EINVAL for not found?
> +}
> +
> int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen)
> {
> int uninitialized_var(err);
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 4919a8e..9ca96c2 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -1212,6 +1212,29 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
> val = np->sndflow;
> break;
>
> + case IPV6_FLOWLABEL_MGR:
> + {
> + struct in6_flowlabel_req freq;
> +
Can you think about other actions one might do with this getsockopt? Maybe
we should copy_from_user freq and check if action specifies a GET
request. So a further extension to this API won't break users which did
pass uninitialized memory to this getsockopt.
> + if (len < sizeof(freq))
> + return -EINVAL;
> +
> + len = sizeof(freq);
> + memset(&freq, 0, sizeof(freq));
> +
> + val = ipv6_flowlabel_opt_get(sk, &freq);
> + if (val < 0)
> + return val;
> +
> + if (put_user(len, optlen))
> + return -EFAULT;
> + if (copy_to_user(optval, &freq, len))
> + return -EFAULT;
> +
> + return 0;
> + break;
I guess return 0 is enough here. The break is superfluous.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net-next 13/13] scripts/checkpatch.pl: Add dev_kfree_skb*(NULL) check to checkpatch
From: Joe Perches @ 2013-11-04 0:37 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: davem, gregkh, linux-usb, linux-kernel, schwidefsky, linville,
linux-wireless, netdev, IvDoorn, sbhatewara, samuel, chas, roland,
isdn, jcliburn, benve, ssujith, jeffrey.t.kirsher,
jesse.brandeburg, shahed.shaikh, apw
In-Reply-To: <1383400074-30555-14-git-send-email-govindarajulu90@gmail.com>
On Sat, 2013-11-02 at 19:17 +0530, Govindarajulu Varadarajan wrote:
> Signed-off-by: Govindarajulu Varadarajan <govindarajulu90@gmail.com>
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3719,7 +3719,7 @@ sub process {
> # check for needless "if (<foo>) fn(<foo>)" uses
> if ($prevline =~ /\bif\s*\(\s*($Lval)\s*\)/) {
> my $expr = '\s*\(\s*' . quotemeta($1) . '\s*\)\s*;';
> - if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) {
> + if ($line =~ /\b(kfree|dev_kfree_skb|dev_kfree_skb_any|dev_kfree_skb_irq|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) {
> WARN('NEEDLESS_IF',
> "$1(NULL) is safe this check is probably not required\n" . $hereprev);
> }
OK, but I think this is easier to read as
dev_kfree_skb(?:_skb|_any|_irq)?
^ permalink raw reply
* Re: [PATCH v2.47 0/4] MPLS actions and matches
From: Simon Horman @ 2013-11-04 0:53 UTC (permalink / raw)
To: Ben Pfaff
Cc: dev, netdev, Jesse Gross, Pravin B Shelar, Ravi K, Isaku Yamahata,
Joe Stringer
In-Reply-To: <20131102053916.GE26756@nicira.com>
On Fri, Nov 01, 2013 at 10:39:16PM -0700, Ben Pfaff wrote:
> On Thu, Oct 31, 2013 at 04:16:28PM +0900, Simon Horman wrote:
> > This series implements MPLS actions and matches based on work by
> > Ravi K, Leo Alterman, Yamahata-san and Joe Stringer.
>
> Would you mind rebasing this one more time?
Sure, will do.
^ permalink raw reply
* Re: [PATCH net-next 0/5] SCTP fix/updates
From: David Miller @ 2013-11-04 4:07 UTC (permalink / raw)
To: dborkman; +Cc: netdev, linux-sctp
In-Reply-To: <1383130252-1515-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann <dborkman@redhat.com>
Date: Wed, 30 Oct 2013 11:50:47 +0100
> Please see patch 5 for the main description/motivation, the rest just
> brings in the needed functionality for that. Although this is actually
> a fix, I've based it against net-next as some additional work for
> fixing it was needed.
Great work Daniel, and a long overdue simplification.
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH v3 net-next] net: introduce dev_set_forwarding()
From: Herbert Xu @ 2013-11-04 4:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383499603.4291.71.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Nov 03, 2013 at 09:26:43AM -0800, Eric Dumazet wrote:
>
> Not really.
>
> Have you took a look at the GSO path recently ?
>
> The days it was handling only IP+TCP are gone.
>
> If you think you can do better, please do so.
OK maybe I overreacted.
With regards to your point 2), GRO does not introduce any latencies
because it simply relies on NAPI to do the aggregation. IOW it is
no better or worse latency-wise compared to NAPI. If you need to
tune it, just use the usual NAPI toggles.
With repsect to point 3), sure we can allow the generation of TSO
segments in skb_segment.
You may be right that this is all too hard, since I haven't actually
sat down and tried to do it yet.
But please give me chance to have a look first before we give up and
install a permanent user-space toggle.
Since this is a crashing bug, I'm OK with adding the linearising
patch to the current tree so that we don't end up shipping with a
known crash in the network stack.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH ] net_sched: actions - Add default lookup
From: David Miller @ 2013-11-04 4:12 UTC (permalink / raw)
To: eric.dumazet; +Cc: jhs, netdev, ebiederm, alexander.h.duyck
In-Reply-To: <1383141605.4857.38.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 30 Oct 2013 07:00:05 -0700
> On Wed, 2013-10-30 at 07:25 -0400, Jamal Hadi Salim wrote:
>> Attached. Tested with simple action.
>>
>> cheers,
>> jamal
>
> Why not setting .lookup to tcf_hash_search
> in the few actions not already doing that ?
>
> This would be more consistent.
> # git grep -n tcf_hash_search
> include/net/act_api.h:92:int tcf_hash_search(struct tc_action *a, u32 index);
> net/sched/act_api.c:198:int tcf_hash_search(struct tc_action *a, u32 index)
> net/sched/act_api.c:209:EXPORT_SYMBOL(tcf_hash_search);
> net/sched/act_csum.c:588: .lookup = tcf_hash_search,
> net/sched/act_gact.c:209: .lookup = tcf_hash_search,
> net/sched/act_ipt.c:301: .lookup = tcf_hash_search,
> net/sched/act_ipt.c:315: .lookup = tcf_hash_search,
> net/sched/act_mirred.c:274: .lookup = tcf_hash_search,
> net/sched/act_nat.c:311: .lookup = tcf_hash_search,
> net/sched/act_pedit.c:246: .lookup = tcf_hash_search,
> net/sched/act_police.c:410: .lookup = tcf_hash_search,
Right, and BUG() if we try to register and action with a NULL .lookup
member.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox